Merge pull request #1653 from jestabro/trace-migration

migration: T4808: add details of configtree operations to migration log

2 files changed, 45 insertions, 38 deletions

diff --git a/python/vyos/configtree.py b/python/vyos/configtree.py
index e9cdb69e4..b88615513 100644
--- a/python/vyos/configtree.py
+++ b/python/vyos/configtree.py
@@ -1,5 +1,5 @@
 # configtree -- a standalone VyOS config file manipulation library (Python bindings)
-# Copyright (C) 2018 VyOS maintainers and contributors
+# Copyright (C) 2018-2022 VyOS maintainers and contributors
 #
 # This library is free software; you can redistribute it and/or modify it under the terms of
 # the GNU Lesser General Public License as published by the Free Software Foundation;
@@ -12,6 +12,7 @@
 # You should have received a copy of the GNU Lesser General Public License along with this library;
 # if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 
+import os
 import re
 import json
 
@@ -147,6 +148,8 @@ class ConfigTree(object):
             self.__config = address
             self.__version = ''
 
+        self.__migration = os.environ.get('VYOS_MIGRATION')
+
     def __del__(self):
         if self.__config is not None:
             self.__destroy(self.__config)
@@ -191,18 +194,27 @@ class ConfigTree(object):
             else:
                 self.__set_add_value(self.__config, path_str, str(value).encode())
 
+        if self.__migration:
+            print(f"- op: set path: {path} value: {value} replace: {replace}")
+
     def delete(self, path):
         check_path(path)
         path_str = " ".join(map(str, path)).encode()
 
         self.__delete(self.__config, path_str)
 
+        if self.__migration:
+            print(f"- op: delete path: {path}")
+
     def delete_value(self, path, value):
         check_path(path)
         path_str = " ".join(map(str, path)).encode()
 
         self.__delete_value(self.__config, path_str, value.encode())
 
+        if self.__migration:
+            print(f"- op: delete_value path: {path} value: {value}")
+
     def rename(self, path, new_name):
         check_path(path)
         path_str = " ".join(map(str, path)).encode()
@@ -216,6 +228,9 @@ class ConfigTree(object):
         if (res != 0):
             raise ConfigTreeError("Path [{}] doesn't exist".format(path))
 
+        if self.__migration:
+            print(f"- op: rename old_path: {path} new_path: {new_path}")
+
     def copy(self, old_path, new_path):
         check_path(old_path)
         check_path(new_path)
@@ -229,6 +244,9 @@ class ConfigTree(object):
         if (res != 0):
             raise ConfigTreeError("Path [{}] doesn't exist".format(old_path))
 
+        if self.__migration:
+            print(f"- op: copy old_path: {old_path} new_path: {new_path}")
+
     def exists(self, path):
         check_path(path)
         path_str = " ".join(map(str, path)).encode()
diff --git a/python/vyos/migrator.py b/python/vyos/migrator.py
index 45ea8b0eb..87c74e1ea 100644
--- a/python/vyos/migrator.py
+++ b/python/vyos/migrator.py
@@ -16,9 +16,13 @@
 import sys
 import os
 import json
-import subprocess
+import logging
+
 import vyos.defaults
 import vyos.component_version as component_version
+from vyos.util import cmd
+
+log_file = os.path.join(vyos.defaults.directories['config'], 'vyos-migrate.log')
 
 class MigratorError(Exception):
     pass
@@ -29,9 +33,21 @@ class Migrator(object):
         self._force = force
         self._set_vintage = set_vintage
         self._config_file_vintage = None
-        self._log_file = None
         self._changed = False
 
+    def init_logger(self):
+        self.logger = logging.getLogger(__name__)
+        self.logger.setLevel(logging.DEBUG)
+
+        # on adding the file handler, allow write permission for cfg_group;
+        # restore original umask on exit
+        mask = os.umask(0o113)
+        fh = logging.FileHandler(log_file)
+        formatter = logging.Formatter('%(message)s')
+        fh.setFormatter(formatter)
+        self.logger.addHandler(fh)
+        os.umask(mask)
+
     def read_config_file_versions(self):
         """
         Get component versions from config file footer and set vintage;
@@ -68,34 +84,15 @@ class Migrator(object):
         else:
             return True
 
-    def open_log_file(self):
-        """
-        Open log file for migration, catching any error.
-        Note that, on boot, migration takes place before the canonical log
-        directory is created, hence write to the config file directory.
-        """
-        self._log_file = os.path.join(vyos.defaults.directories['config'],
-                                      'vyos-migrate.log')
-        # on creation, allow write permission for cfg_group;
-        # restore original umask on exit
-        mask = os.umask(0o113)
-        try:
-            log = open('{0}'.format(self._log_file), 'w')
-            log.write("List of executed migration scripts:\n")
-        except Exception as e:
-            os.umask(mask)
-            print("Logging error: {0}".format(e))
-            return None
-
-        os.umask(mask)
-        return log
-
     def run_migration_scripts(self, config_file_versions, system_versions):
         """
         Run migration scripts iteratively, until config file version equals
         system component version.
         """
-        log = self.open_log_file()
+        os.environ['VYOS_MIGRATION'] = '1'
+        self.init_logger()
+
+        self.logger.info("List of executed migration scripts:")
 
         cfg_versions = config_file_versions
         sys_versions = system_versions
@@ -127,8 +124,9 @@ class Migrator(object):
                         '{}-to-{}'.format(cfg_ver, next_ver))
 
                 try:
-                    subprocess.check_call([migrate_script,
-                        self._config_file])
+                    out = cmd([migrate_script, self._config_file])
+                    self.logger.info(f'{migrate_script}')
+                    if out: self.logger.info(out)
                 except FileNotFoundError:
                     pass
                 except Exception as err:
@@ -136,19 +134,10 @@ class Migrator(object):
                           "".format(migrate_script, err))
                     sys.exit(1)
 
-                if log:
-                    try:
-                        log.write('{0}\n'.format(migrate_script))
-                    except Exception as e:
-                        print("Error writing log: {0}".format(e))
-
                 cfg_ver = next_ver
-
             rev_versions[key] = cfg_ver
 
-        if log:
-            log.close()
-
+        del os.environ['VYOS_MIGRATION']
         return rev_versions
 
     def write_config_file_versions(self, cfg_versions):