From 19615e3c0c45bba1c8c8b19c02276732b70da9bc Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Thu, 17 Jun 2021 04:42:00 -0700 Subject: [PATCH] Fixing db_migrator for Feature table (#1674) #### What I did Fixing db_migrator.py for FEATURE table. In case of version_unknown, all versions' migrator APIs would be invoked which will also invoke the feature_table migration. Without the field check for 'status' the current logic will set it 'disabled' and will set 'state' field too to be 'disabled' which will in turn disable all the features bringing down the system. #### How I did it Added check to migrate only if 'status' field is present. #### How to verify it Install the image through onie and confirm if services are up --- scripts/db_migrator.py | 8 +++++--- tests/db_migrator_input/config_db/feature-expected.json | 8 ++++++++ tests/db_migrator_input/config_db/feature-input.json | 3 +++ tests/db_migrator_input/init_cfg.json | 8 ++++++++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index 41ef8ee516df..7a20982b79c7 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -180,9 +180,11 @@ def migrate_feature_table(self): ''' 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) + state = config.get('status') + if state is not None: + config['state'] = state + config.pop('status') + self.configDB.set_entry('FEATURE', feature, config) container_feature_table = self.configDB.get_table('CONTAINER_FEATURE') for feature, config in container_feature_table.items(): diff --git a/tests/db_migrator_input/config_db/feature-expected.json b/tests/db_migrator_input/config_db/feature-expected.json index 301cb915c2ad..92653771fc23 100644 --- a/tests/db_migrator_input/config_db/feature-expected.json +++ b/tests/db_migrator_input/config_db/feature-expected.json @@ -7,6 +7,14 @@ "high_mem_alert": "disabled", "state": "enabled" }, + "FEATURE|syncd": { + "auto_restart": "enabled", + "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", diff --git a/tests/db_migrator_input/config_db/feature-input.json b/tests/db_migrator_input/config_db/feature-input.json index 31bb7895e828..c6d512dad156 100644 --- a/tests/db_migrator_input/config_db/feature-input.json +++ b/tests/db_migrator_input/config_db/feature-input.json @@ -9,5 +9,8 @@ }, "FEATURE|telemetry": { "status": "enabled" + }, + "FEATURE|syncd": { + "state": "enabled" } } diff --git a/tests/db_migrator_input/init_cfg.json b/tests/db_migrator_input/init_cfg.json index 0b8f5a213ce1..634477a4f93c 100644 --- a/tests/db_migrator_input/init_cfg.json +++ b/tests/db_migrator_input/init_cfg.json @@ -8,6 +8,14 @@ "high_mem_alert": "disabled", "state": "enabled" }, + "syncd": { + "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",