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