From d6da011fd59001f39a5c7fdf4172d13d8870a200 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 17 May 2021 11:32:30 +0300 Subject: [PATCH 1/2] [db_migrator] fix old 1911 feature config migration to a new one. This change is in addition to https://github.com/Azure/sonic-utilities/pull/1522. The init_cfg.json may have important fields added to configuration, while in previous fix these entries will not be added when table already exists. This change fixes this behaviour. Also, in order to preserve users auto_restart configuration a special logic for migrating CONTAINER_FEATURE table has been implemented. A test to cover this scenario is added. Signed-off-by: Stepan Blyschak --- scripts/db_migrator.py | 36 ++++++++++++++----- .../config_db/feature-expected.json | 18 ++++++++++ .../config_db/feature-input.json | 13 +++++++ tests/db_migrator_input/init_cfg.json | 18 ++++++++++ tests/db_migrator_test.py | 29 +++++++++++++++ 5 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/db_migrator_input/config_db/feature-expected.json create mode 100644 tests/db_migrator_input/config_db/feature-input.json diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index 79313fcc35..6b1158e996 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -175,6 +175,22 @@ def migrate_copp_table(self): for copp_key in keys: self.appDB.delete(self.appDB.APPL_DB, copp_key) + def migrate_feature_table(self): + ''' + Combine CONTAINER_FEATURE and FEATURE tables into FEATURE table. + ''' + + feature_table = self.configDB.get_table('FEATURE') + for feature, config in feature_table.items(): + state = config.pop('status', 'disabled') + config['state'] = state + self.configDB.set_entry('FEATURE', feature, config) + + container_feature_table = self.configDB.get_table('CONTAINER_FEATURE') + for feature, config in container_feature_table.items(): + self.configDB.mod_entry('FEATURE', feature, config) + self.configDB.set_entry('CONTAINER_FEATURE', feature, None) + def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, cable_len_list, default_dynamic_th, abandon_method, append_item_method): ''' Migrate buffer tables to dynamic calculation mode @@ -408,6 +424,8 @@ def version_1_0_3(self): """ log.log_info('Handling version_1_0_3') + self.migrate_feature_table() + # Check ASIC type, if Mellanox platform then need DB migration if self.asic_type == "mellanox": if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_3', 'version_1_0_4') \ @@ -484,7 +502,6 @@ def get_version(self): return 'version_unknown' - def set_version(self, version=None): if not version: version = self.CURRENT_VERSION @@ -492,7 +509,6 @@ def set_version(self, version=None): entry = { self.TABLE_FIELD : version } self.configDB.set_entry(self.TABLE_NAME, self.TABLE_KEY, entry) - def common_migration_ops(self): try: with open(INIT_CFG_FILE) as f: @@ -501,14 +517,16 @@ def common_migration_ops(self): raise Exception(str(e)) for init_cfg_table, table_val in init_db.items(): - data = self.configDB.get_table(init_cfg_table) - if data: - # Ignore overriding the values that pre-exist in configDB - continue log.log_info("Migrating table {} from INIT_CFG to config_db".format(init_cfg_table)) - # Update all tables that do not exist in configDB but are present in INIT_CFG - for init_table_key, init_table_val in table_val.items(): - self.configDB.set_entry(init_cfg_table, init_table_key, init_table_val) + for key in table_val: + curr_cfg = self.configDB.get_entry(init_cfg_table, key) + init_cfg = table_val[key] + + # Override init config with current config. + # This will leave new fields from init_config + # in new_config, but not override existing configuration. + new_cfg = {**init_cfg, **curr_cfg} + self.configDB.set_entry(init_cfg_table, key, new_cfg) self.migrate_copp_table() diff --git a/tests/db_migrator_input/config_db/feature-expected.json b/tests/db_migrator_input/config_db/feature-expected.json new file mode 100644 index 0000000000..301cb915c2 --- /dev/null +++ b/tests/db_migrator_input/config_db/feature-expected.json @@ -0,0 +1,18 @@ +{ + "FEATURE|swss": { + "auto_restart": "disabled", + "has_global_scope": "False", + "has_per_asic_scope": "True", + "has_timer": "False", + "high_mem_alert": "disabled", + "state": "enabled" + }, + "FEATURE|telemetry": { + "auto_restart": "enabled", + "has_global_scope": "False", + "has_per_asic_scope": "True", + "has_timer": "False", + "high_mem_alert": "disabled", + "state": "enabled" + } +} diff --git a/tests/db_migrator_input/config_db/feature-input.json b/tests/db_migrator_input/config_db/feature-input.json new file mode 100644 index 0000000000..31bb7895e8 --- /dev/null +++ b/tests/db_migrator_input/config_db/feature-input.json @@ -0,0 +1,13 @@ +{ + "CONTAINER_FEATURE|swss": { + "auto_restart": "disabled", + "high_mem_alert": "disabled" + }, + "CONTAINER_FEATURE|telemetry": { + "auto_restart": "enabled", + "high_mem_alert": "disabled" + }, + "FEATURE|telemetry": { + "status": "enabled" + } +} diff --git a/tests/db_migrator_input/init_cfg.json b/tests/db_migrator_input/init_cfg.json index 2c63c08510..0b8f5a213c 100644 --- a/tests/db_migrator_input/init_cfg.json +++ b/tests/db_migrator_input/init_cfg.json @@ -1,2 +1,20 @@ { + "FEATURE": { + "swss": { + "auto_restart": "enabled", + "has_global_scope": "False", + "has_per_asic_scope": "True", + "has_timer": "False", + "high_mem_alert": "disabled", + "state": "enabled" + }, + "telemetry": { + "auto_restart": "disabled", + "has_global_scope": "False", + "has_per_asic_scope": "True", + "has_timer": "False", + "high_mem_alert": "disabled", + "state": "disabled" + } + } } diff --git a/tests/db_migrator_test.py b/tests/db_migrator_test.py index bbff2a7666..18a466b662 100644 --- a/tests/db_migrator_test.py +++ b/tests/db_migrator_test.py @@ -2,6 +2,8 @@ import pytest import sys +from deepdiff import DeepDiff + from swsssdk import SonicV2Connector from sonic_py_common import device_info @@ -192,3 +194,30 @@ def test_mellanox_buffer_migrator_negative_nondefault_for_warm_reboot(self): input_config_db = 'non-default-config-input' input_appl_db = 'non-default-input' self.mellanox_buffer_migrator_warm_reboot_runner(input_config_db, input_appl_db, expected_config_db, expected_appl_db, False) + + +class TestInitConfigMigrator(object): + @classmethod + def setup_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "2" + + @classmethod + def teardown_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "0" + dbconnector.dedicated_dbs['CONFIG_DB'] = None + + def test_init_config_feature_migration(self): + dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'feature-input') + import db_migrator + dbmgtr = db_migrator.DBMigrator(None) + dbmgtr.migrate() + dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'feature-expected') + expected_db = Db() + + resulting_table = dbmgtr.configDB.get_table('FEATURE') + expected_table = expected_db.cfgdb.get_table('FEATURE') + + diff = DeepDiff(resulting_table, expected_table, ignore_order=True) + assert not diff + + assert not expected_db.cfgdb.get_table('CONTAINER_FEATURE') From 57c97d488a081e4a2dc6afea1356ab67b29bc4c7 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 2 Jun 2021 19:47:29 +0300 Subject: [PATCH 2/2] remove extra line Signed-off-by: Stepan Blyschak --- scripts/db_migrator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index 6b1158e996..5d84fc9daf 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -179,7 +179,6 @@ def migrate_feature_table(self): ''' Combine CONTAINER_FEATURE and FEATURE tables into FEATURE table. ''' - feature_table = self.configDB.get_table('FEATURE') for feature, config in feature_table.items(): state = config.pop('status', 'disabled')