From e6179afa8771bfa1643243f7ef166dd1dc256b24 Mon Sep 17 00:00:00 2001 From: Aryeh Feigin <101218333+arfeigin@users.noreply.github.com> Date: Fri, 10 Mar 2023 18:41:30 +0200 Subject: [PATCH] Remove timer from FAST_REBOOT STATE_DB entry and use finalizer (#2621) This should come along with sonic-buildimage PR (sonic-net/sonic-buildimage#13484) implementing fast-reboot finalizing logic in finalize-warmboot script and other submodules PRs utilizing the change. This PR should come along with the following PRs as well: sonic-net/sonic-swss-common#742 sonic-net/sonic-platform-daemons#335 sonic-net/sonic-sairedis#1196 This set of PRs solves the issue sonic-net/sonic-buildimage#13251 What I did Remove the timer used to clear fast-reboot entry from state-db, instead it will be cleared by fast-reboot finalize function implemented inside finalize-warmboot script (which will be invoked since fast-reboot is using warm-reboot infrastructure). As well instead of having "1" as the value for fast-reboot entry in state-db and deleting it when done it is now modified to set enable/disable according to the context. As well all scripts reading this entry should be modified to the new value options. How I did it Removed the timer usage in the fast-reboot script and adding fast-reboot finalize logic to warm-reboot in the linked PR. Use "enable/disable" instead of "1" as the entry value. How to verify it Run fast-reboot and check that the state-db entry for fast-reboot is being deleted after finalizing fast-reboot and not by an expiring timer. --- scripts/db_migrator.py | 23 +++++++++++-- scripts/fast-reboot | 6 ++-- .../templates/service_mgmt.sh.j2 | 3 +- .../state_db/fast_reboot_expected.json | 5 +++ .../state_db/fast_reboot_input.json | 2 ++ tests/db_migrator_test.py | 32 +++++++++++++++++++ 6 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 tests/db_migrator_input/state_db/fast_reboot_expected.json create mode 100644 tests/db_migrator_input/state_db/fast_reboot_input.json diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index 5c946bbb9f..b08c397971 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -45,7 +45,7 @@ def __init__(self, namespace, socket=None): none-zero values. build: sequentially increase within a minor version domain. """ - self.CURRENT_VERSION = 'version_4_0_0' + self.CURRENT_VERSION = 'version_4_0_1' self.TABLE_NAME = 'VERSIONS' self.TABLE_KEY = 'DATABASE' @@ -867,9 +867,28 @@ def version_3_0_6(self): def version_4_0_0(self): """ Version 4_0_0. - This is the latest version for master branch """ log.log_info('Handling version_4_0_0') + # Update state-db fast-reboot entry to enable if set to enable fast-reboot finalizer when using upgrade with fast-reboot + # since upgrading from previous version FAST_REBOOT table will be deleted when the timer will expire. + # reading FAST_REBOOT table can't be done with stateDB.get as it uses hget behind the scenes and the table structure is + # not using hash and won't work. + # FAST_REBOOT table exists only if fast-reboot was triggered. + keys = self.stateDB.keys(self.stateDB.STATE_DB, "FAST_REBOOT") + if keys is not None: + enable_state = 'true' + else: + enable_state = 'false' + self.stateDB.set(self.stateDB.STATE_DB, 'FAST_RESTART_ENABLE_TABLE|system', 'enable', enable_state) + self.set_version('version_4_0_1') + return 'version_4_0_1' + + def version_4_0_1(self): + """ + Version 4_0_1. + This is the latest version for master branch + """ + log.log_info('Handling version_4_0_1') return None def get_version(self): diff --git a/scripts/fast-reboot b/scripts/fast-reboot index 426c7b2727..fb162ae180 100755 --- a/scripts/fast-reboot +++ b/scripts/fast-reboot @@ -149,7 +149,7 @@ function clear_boot() #clear_fast_boot if [[ "$REBOOT_TYPE" = "fast-reboot" ]]; then - sonic-db-cli STATE_DB DEL "FAST_REBOOT|system" &>/dev/null || /bin/true + sonic-db-cli STATE_DB HSET "FAST_RESTART_ENABLE_TABLE|system" "enable" "false" &>/dev/null || /bin/true fi } @@ -270,7 +270,7 @@ function backup_database() and not string.match(k, 'WARM_RESTART_ENABLE_TABLE|') \ and not string.match(k, 'VXLAN_TUNNEL_TABLE|') \ and not string.match(k, 'BUFFER_MAX_PARAM_TABLE|') \ - and not string.match(k, 'FAST_REBOOT|') then + and not string.match(k, 'FAST_RESTART_ENABLE_TABLE|') then redis.call('del', k) end end @@ -549,7 +549,7 @@ case "$REBOOT_TYPE" in check_warm_restart_in_progress BOOT_TYPE_ARG=$REBOOT_TYPE trap clear_boot EXIT HUP INT QUIT TERM KILL ABRT ALRM - sonic-db-cli STATE_DB SET "FAST_REBOOT|system" "1" "EX" "210" &>/dev/null + sonic-db-cli STATE_DB HSET "FAST_RESTART_ENABLE_TABLE|system" "enable" "true" &>/dev/null config warm_restart enable system ;; "warm-reboot") diff --git a/sonic-utilities-data/templates/service_mgmt.sh.j2 b/sonic-utilities-data/templates/service_mgmt.sh.j2 index d206049015..5c8f4e4974 100644 --- a/sonic-utilities-data/templates/service_mgmt.sh.j2 +++ b/sonic-utilities-data/templates/service_mgmt.sh.j2 @@ -51,7 +51,8 @@ function check_warm_boot() function check_fast_boot() { - if [[ $($SONIC_DB_CLI STATE_DB GET "FAST_REBOOT|system") == "1" ]]; then + SYSTEM_FAST_REBOOT=`$SONIC_DB_CLI STATE_DB hget "FAST_RESTART_ENABLE_TABLE|system" enable` + if [[ x"${SYSTEM_FAST_REBOOT}" == x"true" ]]; then FAST_BOOT="true" else FAST_BOOT="false" diff --git a/tests/db_migrator_input/state_db/fast_reboot_expected.json b/tests/db_migrator_input/state_db/fast_reboot_expected.json new file mode 100644 index 0000000000..e3a7a5fa14 --- /dev/null +++ b/tests/db_migrator_input/state_db/fast_reboot_expected.json @@ -0,0 +1,5 @@ +{ + "FAST_RESTART_ENABLE_TABLE|system": { + "enable": "false" + } +} \ No newline at end of file diff --git a/tests/db_migrator_input/state_db/fast_reboot_input.json b/tests/db_migrator_input/state_db/fast_reboot_input.json new file mode 100644 index 0000000000..7a73a41bfd --- /dev/null +++ b/tests/db_migrator_input/state_db/fast_reboot_input.json @@ -0,0 +1,2 @@ +{ +} \ No newline at end of file diff --git a/tests/db_migrator_test.py b/tests/db_migrator_test.py index b5c70fce8e..e9c184d160 100644 --- a/tests/db_migrator_test.py +++ b/tests/db_migrator_test.py @@ -451,6 +451,38 @@ def test_move_logger_tables_in_warm_upgrade(self): diff = DeepDiff(resulting_table, expected_table, ignore_order=True) assert not diff +class TestFastRebootTableModification(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['STATE_DB'] = None + + def mock_dedicated_state_db(self): + dbconnector.dedicated_dbs['STATE_DB'] = os.path.join(mock_db_path, 'state_db') + + def test_rename_fast_reboot_table_check_enable(self): + device_info.get_sonic_version_info = get_sonic_version_info_mlnx + dbconnector.dedicated_dbs['STATE_DB'] = os.path.join(mock_db_path, 'state_db', 'fast_reboot_input') + dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'empty-config-input') + + import db_migrator + dbmgtr = db_migrator.DBMigrator(None) + dbmgtr.migrate() + + dbconnector.dedicated_dbs['STATE_DB'] = os.path.join(mock_db_path, 'state_db', 'fast_reboot_expected') + expected_db = SonicV2Connector(host='127.0.0.1') + expected_db.connect(expected_db.STATE_DB) + + resulting_table = dbmgtr.stateDB.get_all(dbmgtr.stateDB.STATE_DB, 'FAST_RESTART_ENABLE_TABLE|system') + expected_table = expected_db.get_all(expected_db.STATE_DB, 'FAST_RESTART_ENABLE_TABLE|system') + + diff = DeepDiff(resulting_table, expected_table, ignore_order=True) + assert not diff + class TestWarmUpgrade_to_2_0_2(object): @classmethod def setup_class(cls):