Merge pull request #4753 from jestabro/config-save-sync-atomic

T7709: Add file sync and atomic write to config save script

3 files changed, 94 insertions, 7 deletions

diff --git a/python/vyos/component_version.py b/python/vyos/component_version.py
index 136bd36e8..13fb8333f 100644
--- a/python/vyos/component_version.py
+++ b/python/vyos/component_version.py
@@ -209,6 +209,16 @@ def version_info_prune_component(x: VersionInfo, y: VersionInfo) -> VersionInfo:
     x.component = {k: v for k, v in x.component.items() if k in y.component}
 
 
+def add_system_version_string(config_str: str = None) -> str:
+    """Wrap config string with system version and return string."""
+    version_info = version_info_from_system()
+    if config_str is not None:
+        version_info.update_config_body(config_str)
+    version_info.update_footer()
+
+    return version_info.write_string()
+
+
 def add_system_version(config_str: str = None, out_file: str = None):
     """Wrap config string with system version and write to out_file.
 
diff --git a/python/vyos/configsession.py b/python/vyos/configsession.py
index 462a028bb..f0a22d930 100644
--- a/python/vyos/configsession.py
+++ b/python/vyos/configsession.py
@@ -392,13 +392,7 @@ class ConfigSession(object):
         return out
 
     def save_config(self, file_path):
-        if self._vyconf_session is None:
-            out = self.__run_command(SAVE_CONFIG + [file_path])
-        else:
-            out, _ = self._vyconf_session.save_config(
-                file=file_path, append_version=True
-            )
-
+        out = self.__run_command(SAVE_CONFIG + [file_path])
         return out
 
     def install_image(self, url):
diff --git a/python/vyos/utils/file.py b/python/vyos/utils/file.py
index 1e2de2b39..491cc6547 100644
--- a/python/vyos/utils/file.py
+++ b/python/vyos/utils/file.py
@@ -14,6 +14,8 @@
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import tempfile
+
 from vyos.utils.permission import chown
 
 def makedir(path, user=None, group=None):
@@ -185,3 +187,84 @@ def wait_for_file_write_complete(file_path, pre_hook=None, timeout=None, sleep_i
     """ Waits for a process to close a file after opening it in write mode. """
     wait_for_inotify(file_path,
       event_type='IN_CLOSE_WRITE', pre_hook=pre_hook, timeout=timeout, sleep_interval=sleep_interval)
+
+
+def copy_chown(source, target):
+    # pylint: disable=import-outside-toplevel
+    import shutil
+    import stat
+
+    shutil.copy2(source, target)
+    st = os.stat(source)
+    os.chown(target, st[stat.ST_UID], st[stat.ST_GID])
+
+
+def write_file_sync(file_path, data: str, mode='w'):
+    """Write file with explicit sync of file and directory"""
+    # pylint: disable=consider-using-with
+    file_dir = os.path.dirname(file_path)
+
+    # write and sync file
+    try:
+        file = open(file_path, mode)
+        file.write(data)
+        file.flush()
+        os.fsync(file.fileno())
+        file.close()
+    except OSError as e:
+        try:
+            file.close()
+        except OSError:
+            pass
+        raise e
+
+    # sync directory entry
+    try:
+        fd = os.open(file_dir, os.O_DIRECTORY | os.O_RDONLY)
+        os.fsync(fd)
+        os.close(fd)
+    except OSError as e:
+        try:
+            os.close(fd)
+        except OSError:
+            pass
+        raise e
+
+
+def write_file_atomic(file_path, data: str, mode='w'):
+    """Use os.rename for 'atomic' write.
+
+    Note that this requires an euid/egid of that of the source file for the
+    chown operation.
+
+    Note that this calls write_file_sync, above.
+    """
+    # pylint: disable=consider-using-with,raise-missing-from
+    file_dir = os.path.dirname(file_path)
+    temp_file = tempfile.NamedTemporaryFile(delete=False, dir=file_dir).name
+
+    def cleanup():
+        if os.path.exists(temp_file):
+            try:
+                os.unlink(temp_file)
+            except OSError:
+                pass
+
+    if os.path.exists(file_path):
+        try:
+            copy_chown(file_path, temp_file)
+        except OSError as e:
+            cleanup()
+            raise OSError(f'copy_chown {e}')
+
+    try:
+        write_file_sync(temp_file, data, mode=mode)
+    except OSError as e:
+        cleanup()
+        raise OSError(f'write_file_sync {e}')
+
+    try:
+        os.rename(temp_file, file_path)
+    except OSError as e:
+        cleanup()
+        raise OSError(f'rename {e}')