Support golden config in db migrator (#3076)

ganglyu · web-flow · commit bcb10f18ffc1 · 2023-12-23T07:52:03.000+08:00
What I did
Need to support golden config in db migrator.

How I did it
If there's golden config json, read from golden config instead of minigraph.
And db migrator will use golden config data to generate new configuration.

How to verify it
Run unit test.
diff --git a/config/main.py b/config/main.py
@@ -34,7 +34,7 @@
 from utilities_common.intf_filter import parse_interface_in_filter
 from utilities_common import bgp_util
 import utilities_common.cli as clicommon
-from utilities_common.helper import get_port_pbh_binding, get_port_acl_binding
+from utilities_common.helper import get_port_pbh_binding, get_port_acl_binding, update_config
 from utilities_common.general import load_db_config, load_module_from_source
 from .validated_config_db_connector import ValidatedConfigDBConnector
 import utilities_common.multi_asic as multi_asic_util
@@ -1950,6 +1950,7 @@ def override_config_table(db, input_config_db, dry_run):
             ns_config_input = config_input
         # Generate sysinfo if missing in ns_config_input
         generate_sysinfo(current_config, ns_config_input, ns)
+        # Use deepcopy by default to avoid modifying input config
         updated_config = update_config(current_config, ns_config_input)
 
         yang_enabled = device_info.is_yang_config_validation_enabled(config_db)
@@ -1985,14 +1986,6 @@ def validate_config_by_cm(cm, config_json, jname):
         sys.exit(1)
 
 
-def update_config(current_config, config_input):
-    updated_config = copy.deepcopy(current_config)
-    # Override current config with golden config
-    for table in config_input:
-        updated_config[table] = config_input[table]
-    return updated_config
-
-
 def override_config_db(config_db, config_input):
     # Deserialized golden config to DB recognized format
     sonic_cfggen.FormatConverter.to_deserialized(config_input)
diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py
@@ -10,9 +10,11 @@
 from sonic_py_common import device_info, logger
 from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, SonicDBConfig
 from minigraph import parse_xml
+from utilities_common.helper import update_config
 
 INIT_CFG_FILE = '/etc/sonic/init_cfg.json'
 MINIGRAPH_FILE = '/etc/sonic/minigraph.xml'
+GOLDEN_CFG_FILE = '/etc/sonic/golden_config_db.json'
 
 # mock the redis for unit test purposes #
 try:
@@ -24,10 +26,12 @@
         sys.path.insert(0, tests_path)
         INIT_CFG_FILE = os.path.join(mocked_db_path, "init_cfg.json")
         MINIGRAPH_FILE = os.path.join(mocked_db_path, "minigraph.xml")
+        GOLDEN_CFG_FILE = os.path.join(mocked_db_path, "golden_config_db.json")
 except KeyError:
     pass
 
 SYSLOG_IDENTIFIER = 'db_migrator'
+DEFAULT_NAMESPACE = ''
 
 
 # Global logger instance
@@ -60,15 +64,8 @@ def __init__(self, namespace, socket=None):
         self.TABLE_KEY       = 'DATABASE'
         self.TABLE_FIELD     = 'VERSION'
 
-        # load config data from minigraph to get the default/hardcoded values from minigraph.py
-        # this is to avoid duplicating the hardcoded these values in db_migrator
-        self.minigraph_data = None
-        try:
-            if os.path.isfile(MINIGRAPH_FILE):
-                self.minigraph_data = parse_xml(MINIGRAPH_FILE)
-        except Exception as e:
-            log.log_error('Caught exception while trying to parse minigraph: ' + str(e))
-            pass
+        # Generate config_src_data from minigraph and golden config
+        self.generate_config_src(namespace)
 
         db_kwargs = {}
         if socket:
@@ -107,6 +104,55 @@ def __init__(self, namespace, socket=None):
             from mellanox_buffer_migrator import MellanoxBufferMigrator
             self.mellanox_buffer_migrator = MellanoxBufferMigrator(self.configDB, self.appDB, self.stateDB)
 
+    def generate_config_src(self, ns):
+        '''
+        Generate config_src_data from minigraph and golden config
+        This method uses golden_config_data and minigraph_data as local variables,
+        which means they are not accessible or modifiable from outside this method.
+        This way, this method ensures that these variables are not changed unintentionally.
+        Args:
+            ns: namespace
+        Returns:
+        '''
+        # load config data from golden_config_db.json
+        golden_config_data = None
+        try:
+            if os.path.isfile(GOLDEN_CFG_FILE):
+                with open(GOLDEN_CFG_FILE) as f:
+                    golden_data = json.load(f)
+                    if ns is None:
+                        golden_config_data = golden_data
+                    else:
+                        if ns == DEFAULT_NAMESPACE:
+                            config_namespace = "localhost"
+                        else:
+                            config_namespace = ns
+                        golden_config_data = golden_data[config_namespace]
+        except Exception as e:
+            log.log_error('Caught exception while trying to load golden config: ' + str(e))
+            pass
+        # load config data from minigraph to get the default/hardcoded values from minigraph.py
+        minigraph_data = None
+        try:
+            if os.path.isfile(MINIGRAPH_FILE):
+                minigraph_data = parse_xml(MINIGRAPH_FILE)
+        except Exception as e:
+            log.log_error('Caught exception while trying to parse minigraph: ' + str(e))
+            pass
+        # When both golden config and minigraph exists, override minigraph config with golden config
+        # config_src_data is the source of truth for config data
+        # this is to avoid duplicating the hardcoded these values in db_migrator
+        self.config_src_data = None
+        if minigraph_data:
+            # Shallow copy for better performance
+            self.config_src_data = minigraph_data
+            if golden_config_data:
+                # Shallow copy for better performance
+                self.config_src_data = update_config(minigraph_data, golden_config_data, False)
+        elif golden_config_data:
+            # Shallow copy for better performance
+            self.config_src_data = golden_config_data
+
     def migrate_pfc_wd_table(self):
         '''
         Migrate all data entries from table PFC_WD_TABLE to PFC_WD
@@ -547,9 +593,9 @@ def migrate_vxlan_config(self):
 
     def migrate_restapi(self):
         # RESTAPI - add missing key
-        if not self.minigraph_data or 'RESTAPI' not in self.minigraph_data:
+        if not self.config_src_data or 'RESTAPI' not in self.config_src_data:
             return
-        restapi_data = self.minigraph_data['RESTAPI']
+        restapi_data = self.config_src_data['RESTAPI']
         log.log_notice('Migrate RESTAPI configuration')
         config = self.configDB.get_entry('RESTAPI', 'config')
         if not config:
@@ -560,9 +606,9 @@ def migrate_restapi(self):
 
     def migrate_telemetry(self):
         # TELEMETRY - add missing key
-        if not self.minigraph_data or 'TELEMETRY' not in self.minigraph_data:
+        if not self.config_src_data or 'TELEMETRY' not in self.config_src_data:
             return
-        telemetry_data = self.minigraph_data['TELEMETRY']
+        telemetry_data = self.config_src_data['TELEMETRY']
         log.log_notice('Migrate TELEMETRY configuration')
         gnmi = self.configDB.get_entry('TELEMETRY', 'gnmi')
         if not gnmi:
@@ -573,9 +619,9 @@ def migrate_telemetry(self):
 
     def migrate_console_switch(self):
         # CONSOLE_SWITCH - add missing key
-        if not self.minigraph_data or 'CONSOLE_SWITCH' not in self.minigraph_data:
+        if not self.config_src_data or 'CONSOLE_SWITCH' not in self.config_src_data:
             return
-        console_switch_data = self.minigraph_data['CONSOLE_SWITCH']
+        console_switch_data = self.config_src_data['CONSOLE_SWITCH']
         log.log_notice('Migrate CONSOLE_SWITCH configuration')
         console_mgmt = self.configDB.get_entry('CONSOLE_SWITCH', 'console_mgmt')
         if not console_mgmt:
@@ -584,11 +630,11 @@ def migrate_console_switch(self):
 
     def migrate_device_metadata(self):
         # DEVICE_METADATA - synchronous_mode entry
-        if not self.minigraph_data or 'DEVICE_METADATA' not in self.minigraph_data:
+        if not self.config_src_data or 'DEVICE_METADATA' not in self.config_src_data:
             return
         log.log_notice('Migrate DEVICE_METADATA missing configuration')
         metadata = self.configDB.get_entry('DEVICE_METADATA', 'localhost')
-        device_metadata_data = self.minigraph_data["DEVICE_METADATA"]["localhost"]
+        device_metadata_data = self.config_src_data["DEVICE_METADATA"]["localhost"]
         if 'synchronous_mode' not in metadata:
             metadata['synchronous_mode'] = device_metadata_data.get("synchronous_mode")
             self.configDB.set_entry('DEVICE_METADATA', 'localhost', metadata)
@@ -628,19 +674,19 @@ def migrate_dns_nameserver(self):
         Handle DNS_NAMESERVER table migration. Migrations handled:
         If there's no DNS_NAMESERVER in config_DB, load DNS_NAMESERVER from minigraph
         """
-        if not self.minigraph_data or 'DNS_NAMESERVER' not in self.minigraph_data:
+        if not self.config_src_data or 'DNS_NAMESERVER' not in self.config_src_data:
             return
         dns_table = self.configDB.get_table('DNS_NAMESERVER')
         if not dns_table:
-            for addr, config in self.minigraph_data['DNS_NAMESERVER'].items():
+            for addr, config in self.config_src_data['DNS_NAMESERVER'].items():
                 self.configDB.set_entry('DNS_NAMESERVER', addr, config)
 
     def migrate_routing_config_mode(self):
         # DEVICE_METADATA - synchronous_mode entry
-        if not self.minigraph_data or 'DEVICE_METADATA' not in self.minigraph_data:
+        if not self.config_src_data or 'DEVICE_METADATA' not in self.config_src_data:
             return
         device_metadata_old = self.configDB.get_entry('DEVICE_METADATA', 'localhost')
-        device_metadata_new = self.minigraph_data['DEVICE_METADATA']['localhost']
+        device_metadata_new = self.config_src_data['DEVICE_METADATA']['localhost']
         # overwrite the routing-config-mode as per minigraph parser
         # Criteria for update:
         # if config mode is missing in base OS or if base and target modes are not same
diff --git a/tests/db_migrator_input/golden_config_db.json.invalid b/tests/db_migrator_input/golden_config_db.json.invalid
@@ -0,0 +1,7 @@
+{
+    "DEVICE_METADATA": {
+        "localhost": {
+            "hostname": "SONiC-Golden-Config"
+        }}
+    }
+}
diff --git a/tests/db_migrator_input/golden_config_db.json.test b/tests/db_migrator_input/golden_config_db.json.test
@@ -0,0 +1,7 @@
+{
+    "DEVICE_METADATA": {
+        "localhost": {
+            "hostname": "SONiC-Golden-Config"
+        }
+    }
+}
diff --git a/tests/db_migrator_test.py b/tests/db_migrator_test.py
@@ -368,8 +368,8 @@ def test_dns_nameserver_migrator(self):
         dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'dns-nameserver-input')
         import db_migrator
         dbmgtr = db_migrator.DBMigrator(None)
-        # Set minigraph_data to DNS_NAMESERVERS
-        dbmgtr.minigraph_data = {
+        # Set config_src_data to DNS_NAMESERVERS
+        dbmgtr.config_src_data = {
             'DNS_NAMESERVER': {
                 '1.1.1.1': {},
                 '2001:1001:110:1001::1': {}
@@ -635,8 +635,8 @@ def test_warm_upgrade__without_mg_to_2_0_2(self):
         dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'cross_branch_upgrade_to_version_2_0_2_input')
         import db_migrator
         dbmgtr = db_migrator.DBMigrator(None)
-        # set minigraph_data to None to mimic the missing minigraph.xml scenario
-        dbmgtr.minigraph_data = None
+        # set config_src_data to None to mimic the missing minigraph.xml scenario
+        dbmgtr.config_src_data = None
         dbmgtr.migrate()
         dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'cross_branch_upgrade_without_mg_2_0_2_expected.json')
         expected_db = Db()
@@ -823,3 +823,49 @@ def test_sflow_migrator(self):
             expected_keys = expected_appl_db.get_all(expected_appl_db.APPL_DB, key)
             diff = DeepDiff(resulting_keys, expected_keys, ignore_order=True)
             assert not diff
+
+class TestGoldenConfig(object):
+    @classmethod
+    def setup_class(cls):
+        os.system("cp %s %s" % (mock_db_path + '/golden_config_db.json.test', mock_db_path + '/golden_config_db.json'))
+        os.environ['UTILITIES_UNIT_TESTING'] = "2"
+
+    @classmethod
+    def teardown_class(cls):
+        os.environ['UTILITIES_UNIT_TESTING'] = "0"
+        os.system("rm %s" % (mock_db_path + '/golden_config_db.json'))
+
+    def test_golden_config_hostname(self):
+        import db_migrator
+        dbmgtr = db_migrator.DBMigrator(None)
+        config = dbmgtr.config_src_data
+        device_metadata = config.get('DEVICE_METADATA', {})
+        assert device_metadata != {}
+        host = device_metadata.get('localhost', {})
+        assert host != {}
+        hostname = host.get('hostname', '')
+        # hostname is from golden_config_db.json
+        assert hostname == 'SONiC-Golden-Config'
+
+class TestGoldenConfigInvalid(object):
+    @classmethod
+    def setup_class(cls):
+        os.system("cp %s %s" % (mock_db_path + '/golden_config_db.json.invalid', mock_db_path + '/golden_config_db.json'))
+        os.environ['UTILITIES_UNIT_TESTING'] = "2"
+
+    @classmethod
+    def teardown_class(cls):
+        os.environ['UTILITIES_UNIT_TESTING'] = "0"
+        os.system("rm %s" % (mock_db_path + '/golden_config_db.json'))
+
+    def test_golden_config_hostname(self):
+        import db_migrator
+        dbmgtr = db_migrator.DBMigrator(None)
+        config = dbmgtr.config_src_data
+        device_metadata = config.get('DEVICE_METADATA', {})
+        assert device_metadata != {}
+        host = device_metadata.get('localhost', {})
+        assert host != {}
+        hostname = host.get('hostname', '')
+        # hostname is from minigraph.xml
+        assert hostname == 'SONiC-Dummy'
diff --git a/utilities_common/helper.py b/utilities_common/helper.py
@@ -1,6 +1,7 @@
 from dump.match_infra import MatchEngine, MatchRequest, ConnectionPool
 from dump.match_helper import get_matched_keys
 from .db import Db
+import copy
 
 def get_port_acl_binding(db_wrap, port, ns):
     """
@@ -64,3 +65,30 @@ def get_port_pbh_binding(db_wrap, port, ns):
     ret = m_engine.fetch(req)
     pbh_tables, _ = get_matched_keys(ret)
     return pbh_tables
+
+
+def update_config(current_config, config_input, deepcopy=True):
+    """
+    Override current config with golden config
+    Shallow copy only copies the references to the original object,
+    so any changes to one object will also change the other.
+    Therefore, we should be careful when using shallow copy to avoid unwanted modifications.
+
+    Args:
+        current_config: current config
+        config_input: input golden config
+        deepcopy: True for deep copy, False for shallow copy
+
+    Returns:
+        Final config after overriding
+    """
+    if deepcopy:
+        # Deep copy for safety
+        updated_config = copy.deepcopy(current_config)
+    else:
+        # Shallow copy for better performance
+        updated_config = current_config
+    # Override current config with golden config
+    for table in config_input:
+        updated_config[table] = config_input[table]
+    return updated_config
