[db_migrator] Remove hardcoded config and migrate config from minigra…

…ph (#2887) (#2924)

Microsoft ADO: 18217044

This PR is follow up to an old PR: #2515

This PR addresses two issues:

Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py.
Not migrating these entries if minigraph.xml file is missing.
Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own.
How I did it
This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator.

How to verify it
Tested on a physical device.

scripts/db_migrator.py

@@ -10,9 +10,10 @@
 from sonic_py_common import device_info, logger
 from swsssdk import ConfigDBConnector, SonicDBConfig
 from swsscommon.swsscommon import SonicV2Connector
-from db_migrator_constants import RESTAPI, TELEMETRY, CONSOLE_SWITCH
+from minigraph import parse_xml
 
 INIT_CFG_FILE = '/etc/sonic/init_cfg.json'
+MINIGRAPH_FILE = '/etc/sonic/minigraph.xml'
 
 # mock the redis for unit test purposes #
 try:
@@ -23,6 +24,7 @@
         sys.path.insert(0, modules_path)
         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")
 except KeyError:
     pass
 
@@ -52,6 +54,16 @@ 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
+
         db_kwargs = {}
         if socket:
             db_kwargs['unix_socket_path'] = socket
@@ -461,38 +473,50 @@ 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:
+            return
+        restapi_data = self.minigraph_data['RESTAPI']
         log.log_notice('Migrate RESTAPI configuration')
         config = self.configDB.get_entry('RESTAPI', 'config')
         if not config:
-            self.configDB.set_entry("RESTAPI", "config", RESTAPI.get("config"))
+            self.configDB.set_entry("RESTAPI", "config", restapi_data.get("config"))
         certs = self.configDB.get_entry('RESTAPI', 'certs')
         if not certs:
-            self.configDB.set_entry("RESTAPI", "certs", RESTAPI.get("certs"))
+            self.configDB.set_entry("RESTAPI", "certs", restapi_data.get("certs"))
 
     def migrate_telemetry(self):
         # TELEMETRY - add missing key
+        if not self.minigraph_data or 'TELEMETRY' not in self.minigraph_data:
+            return
+        telemetry_data = self.minigraph_data['TELEMETRY']
         log.log_notice('Migrate TELEMETRY configuration')
         gnmi = self.configDB.get_entry('TELEMETRY', 'gnmi')
         if not gnmi:
-            self.configDB.set_entry("TELEMETRY", "gnmi", TELEMETRY.get("gnmi"))
+            self.configDB.set_entry("TELEMETRY", "gnmi", telemetry_data.get("gnmi"))
         certs = self.configDB.get_entry('TELEMETRY', 'certs')
         if not certs:
-            self.configDB.set_entry("TELEMETRY", "certs", TELEMETRY.get("certs"))
+            self.configDB.set_entry("TELEMETRY", "certs", telemetry_data.get("certs"))
 
     def migrate_console_switch(self):
         # CONSOLE_SWITCH - add missing key
+        if not self.minigraph_data or 'CONSOLE_SWITCH' not in self.minigraph_data:
+            return
+        console_switch_data = self.minigraph_data['CONSOLE_SWITCH']
         log.log_notice('Migrate CONSOLE_SWITCH configuration')
         console_mgmt = self.configDB.get_entry('CONSOLE_SWITCH', 'console_mgmt')
         if not console_mgmt:
             self.configDB.set_entry("CONSOLE_SWITCH", "console_mgmt",
-                CONSOLE_SWITCH.get("console_mgmt"))
+                console_switch_data.get("console_mgmt"))
 
     def migrate_device_metadata(self):
         # DEVICE_METADATA - synchronous_mode entry
-        log.log_notice('Migrate DEVICE_METADATA missing configuration (synchronous_mode=enable)')
+        if not self.minigraph_data or 'DEVICE_METADATA' not in self.minigraph_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"]
         if 'synchronous_mode' not in metadata:
-            metadata['synchronous_mode'] = 'enable'
+            metadata['synchronous_mode'] = device_metadata_data.get("synchronous_mode")
             self.configDB.set_entry('DEVICE_METADATA', 'localhost', metadata)
 
     def migrate_port_qos_map_global(self):

setup.py

@@ -75,7 +75,6 @@
         'scripts/coredump-compress',
         'scripts/configlet',
         'scripts/db_migrator.py',
-        'scripts/db_migrator_constants.py',
         'scripts/decode-syseeprom',
         'scripts/dropcheck',
         'scripts/disk_check.py',

tests/db_migrator_input/config_db/cross_branch_upgrade_to_version_2_0_2_expected.json

@@ -6,9 +6,9 @@
         },
     "RESTAPI|certs": {
             "server_key": "/etc/sonic/credentials/restapiserver.key", 
-            "ca_crt": "/etc/sonic/credentials/AME_ROOT_CERTIFICATE.pem", 
-            "server_crt": "/etc/sonic/credentials/restapiserver.crt", 
-            "client_crt_cname": "client.restapi.sonic.gbl"
+            "ca_crt": "/etc/sonic/credentials/restapica.crt",
+            "server_crt": "/etc/sonic/credentials/restapiserver.crt",
+            "client_crt_cname": "client.restapi.sonic"
         },
     "TELEMETRY|gnmi": {
             "client_auth": "true", 

tests/db_migrator_input/minigraph.xml

@@ -0,0 +1,29 @@
+<DeviceMiniGraph xmlns="Microsoft.Search.Autopilot.Evolution" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
+  <CpgDec/>
+  <DpgDec>
+    <DeviceDataPlaneInfo>
+      <LoopbackIPInterfaces/>
+      <ManagementIPInterfaces/>
+      <Hostname>SONiC-Dummy</Hostname>
+      <PortChannelInterfaces/>
+      <VlanInterfaces/>
+      <IPInterfaces/>
+      <AclInterfaces/>
+    </DeviceDataPlaneInfo>
+  </DpgDec>
+  <PngDec>
+   <Devices>
+      <Device i:type="TorRouter">
+        <Hostname>SONiC-Dummy</Hostname>
+      </Device>
+    </Devices>
+   </PngDec>
+  <MetadataDeclaration>
+    <Devices xmlns:a="http://schemas">
+      <a:DeviceMetadata>
+        <a:Properties/>
+      </a:DeviceMetadata>
+    </Devices>
+  </MetadataDeclaration>
+  <Hostname>SONiC-Dummy</Hostname>
+</DeviceMiniGraph>

tests/db_migrator_test.py

@@ -327,14 +327,31 @@ def test_warm_upgrade_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_expected')
         expected_db = Db()
 
-        expected_db
         new_tables = ["RESTAPI", "TELEMETRY", "CONSOLE_SWITCH"]
         for table in new_tables:
             resulting_table = dbmgtr.configDB.get_table(table)
             expected_table = expected_db.cfgdb.get_table(table)
             diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
             assert not diff
 
+    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
+        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()
+
+        new_tables = ["RESTAPI", "TELEMETRY", "CONSOLE_SWITCH"]
+        for table in new_tables:
+            resulting_table = dbmgtr.configDB.get_table(table)
+            expected_table = expected_db.cfgdb.get_table(table)
+            print(resulting_table)
+            diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
+            assert not diff
+
 class Test_Migrate_Loopback(object):
     @classmethod
     def setup_class(cls):