Skip to content

Commit

Permalink
Remove timer from FAST_REBOOT STATE_DB entry and use finalizer (sonic…
Browse files Browse the repository at this point in the history
…-net#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.
  • Loading branch information
arfeigin authored and isabelmsft committed Mar 23, 2023
1 parent 9693c99 commit 81b4fca
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 6 deletions.
23 changes: 21 additions & 2 deletions scripts/db_migrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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):
Expand Down
6 changes: 3 additions & 3 deletions scripts/fast-reboot
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 2 additions & 1 deletion sonic-utilities-data/templates/service_mgmt.sh.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions tests/db_migrator_input/state_db/fast_reboot_expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"FAST_RESTART_ENABLE_TABLE|system": {
"enable": "false"
}
}
2 changes: 2 additions & 0 deletions tests/db_migrator_input/state_db/fast_reboot_input.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
32 changes: 32 additions & 0 deletions tests/db_migrator_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 81b4fca

Please sign in to comment.