From e9ae14d2e80deb6b3d7417f3a12ce61ba433becd Mon Sep 17 00:00:00 2001 From: ganglv <88995770+ganglyu@users.noreply.github.com> Date: Sat, 23 Dec 2023 07:52:03 +0800 Subject: [PATCH] Support golden config in db migrator (#3076) 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. --- config/main.py | 11 +-- scripts/db_migrator.py | 88 ++++++++++++++----- .../golden_config_db.json.invalid | 7 ++ .../golden_config_db.json.test | 7 ++ tests/db_migrator_test.py | 54 +++++++++++- utilities_common/helper.py | 28 ++++++ 6 files changed, 161 insertions(+), 34 deletions(-) create mode 100644 tests/db_migrator_input/golden_config_db.json.invalid create mode 100644 tests/db_migrator_input/golden_config_db.json.test diff --git a/config/main.py b/config/main.py index 3bdb31a10b..5eb2634614 100644 --- 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 index 0cea7c8f17..f3ffe0197d 100755 --- 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) @@ -650,19 +696,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 new file mode 100644 index 0000000000..d9ba49c5bd --- /dev/null +++ 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 new file mode 100644 index 0000000000..66fbc197b6 --- /dev/null +++ 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 index dfecccd546..6ee4589a98 100644 --- 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() @@ -858,3 +858,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 index f7a71cec36..c9f0c3a956 100644 --- 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