From f993cf17e1800766e65c4d6a5fc07bbafbb0a711 Mon Sep 17 00:00:00 2001 From: Vaibhav Hemant Dixit Date: Wed, 26 Jul 2023 12:15:18 -0700 Subject: [PATCH] [db_migrator] Remove hardcoded config and migrate config from minigraph (#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 | 40 +++++++++++++++---- scripts/db_migrator_constants.py | 32 --------------- setup.py | 1 - ...nch_upgrade_to_version_2_0_2_expected.json | 6 +-- tests/db_migrator_input/minigraph.xml | 29 ++++++++++++++ tests/db_migrator_test.py | 19 ++++++++- 6 files changed, 82 insertions(+), 45 deletions(-) delete mode 100644 scripts/db_migrator_constants.py create mode 100644 tests/db_migrator_input/minigraph.xml diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index 9fc4b61910..af89885b6f 100755 --- a/scripts/db_migrator.py +++ b/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): diff --git a/scripts/db_migrator_constants.py b/scripts/db_migrator_constants.py deleted file mode 100644 index 71237a5655..0000000000 --- a/scripts/db_migrator_constants.py +++ /dev/null @@ -1,32 +0,0 @@ -RESTAPI = { - "config": { - "client_auth": "true", - "log_level": "info", - "allow_insecure": "false" - }, - "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" - } - } - -TELEMETRY = { - "gnmi": { - "client_auth": "true", - "log_level": "2", - "port": "50051" - }, - "certs": { - "server_key": "/etc/sonic/telemetry/streamingtelemetryserver.key", - "ca_crt": "/etc/sonic/telemetry/dsmsroot.cer", - "server_crt": "/etc/sonic/telemetry/streamingtelemetryserver.cer" - } -} - -CONSOLE_SWITCH = { - "console_mgmt": { - "enabled": "no" - } -} diff --git a/setup.py b/setup.py index 27e2ddc8d8..7b54094f21 100644 --- a/setup.py +++ b/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', diff --git a/tests/db_migrator_input/config_db/cross_branch_upgrade_to_version_2_0_2_expected.json b/tests/db_migrator_input/config_db/cross_branch_upgrade_to_version_2_0_2_expected.json index c975229287..946a9812b7 100644 --- a/tests/db_migrator_input/config_db/cross_branch_upgrade_to_version_2_0_2_expected.json +++ b/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", diff --git a/tests/db_migrator_input/minigraph.xml b/tests/db_migrator_input/minigraph.xml new file mode 100644 index 0000000000..dcadac306e --- /dev/null +++ b/tests/db_migrator_input/minigraph.xml @@ -0,0 +1,29 @@ + + + + + + + SONiC-Dummy + + + + + + + + + + SONiC-Dummy + + + + + + + + + + + SONiC-Dummy + \ No newline at end of file diff --git a/tests/db_migrator_test.py b/tests/db_migrator_test.py index 1a8fe5cf5a..eeab297398 100644 --- a/tests/db_migrator_test.py +++ b/tests/db_migrator_test.py @@ -327,7 +327,6 @@ 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) @@ -335,6 +334,24 @@ def test_warm_upgrade_to_2_0_2(self): 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):