-
Notifications
You must be signed in to change notification settings - Fork 681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[cisco|express-boot]: Add support for cisco express boot in sonic-utilities #3056
Conversation
) Add support of express boot for cisco 8000 platform. There are 4 PRs for this feature: sonic-net/sonic-buildimage#17369 #1329 sonic-net/sonic-utilities#3056 sonic-net/sonic-host-services#90 --------- Co-authored-by: Ze Gan <[email protected]>
@vaibhavhd @saiarcot895 a kindly reminder for the review request |
@saiarcot895 updated per your comments and all checks have passed. Please review again. Thank you |
@@ -171,7 +171,7 @@ function init_warm_reboot_states() | |||
# If the current running instance was booted up with warm reboot. Then | |||
# the current DB contents will likely mark warm reboot is done. | |||
# Clear these states so that the next boot up image won't get confused. | |||
if [[ "$REBOOT_TYPE" = "warm-reboot" || "$REBOOT_TYPE" = "fastfast-reboot" || "$REBOOT_TYPE" = "fast-reboot" ]]; then | |||
if [[ "$REBOOT_TYPE" = "warm-reboot" || "$REBOOT_TYPE" = "fastfast-reboot" || "$REBOOT_TYPE" = "express-reboot" || "$REBOOT_TYPE" = "fast-reboot" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhli-cisco Could you add 'express-reboot' details in the PR description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhli-cisco could you add all related PRs to HLD - sonic-net/SONiC#1570?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will open a PR to update HLD with code PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a little more details of express-boot handling in express-boot script
@@ -171,7 +171,7 @@ function init_warm_reboot_states() | |||
# If the current running instance was booted up with warm reboot. Then | |||
# the current DB contents will likely mark warm reboot is done. | |||
# Clear these states so that the next boot up image won't get confused. | |||
if [[ "$REBOOT_TYPE" = "warm-reboot" || "$REBOOT_TYPE" = "fastfast-reboot" || "$REBOOT_TYPE" = "fast-reboot" ]]; then | |||
if [[ "$REBOOT_TYPE" = "warm-reboot" || "$REBOOT_TYPE" = "fastfast-reboot" || "$REBOOT_TYPE" = "express-reboot" || "$REBOOT_TYPE" = "fast-reboot" ]]; then | |||
sonic-db-cli STATE_DB eval " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have sonic-mgmt tests for Cisco express-reboot feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, will send out a PR for express boot sonic-management tests
@@ -245,7 +250,7 @@ function backup_database() | |||
{ | |||
debug "Backing up database ..." | |||
|
|||
if [[ "$REBOOT_TYPE" = "fastfast-reboot" || "$REBOOT_TYPE" = "fast-reboot" ]]; then | |||
if [[ "$REBOOT_TYPE" = "fastfast-reboot" || "$REBOOT_TYPE" = "express-reboot" || "$REBOOT_TYPE" = "fast-reboot" ]]; then | |||
# Advanced reboot: dump state to host disk | |||
sonic-db-cli ASIC_DB FLUSHDB > /dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the ASIC_DB be cleared during express-reboot? There might be 2 issues if it isn't cleared:
- It might have stale entries after SWSS has replayed CONFIG/APPL DB entries.
- HIDDEN keys would be present which will set m_veryFirstRun = False in Syncd.cpp. As a result, onApplyViewInFastFastBoot() won't be called during APPLY_VIEW notification processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes asic_db is cleared during express-boot:
sonic-db-cli ASIC_DB FLUSHDB > /dev/null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the clarification.
Hi @kperumalbfn @akarshgupta25 @saiarcot895 Do you still have comments? Can you please approve and help merge it if you have no further comments. Thanks! |
What I did
Add support of express boot for cisco 8000 platform. It essentiall is warm-reboot to sonic except to syncd, which handles express boot differently from warm boot in both pre-reboot and post reboot handling.
How I did it
Add support of express boot for cisco 8000 platform. There are 4 PRs for this feature:
sonic-net/sonic-buildimage#17369
sonic-net/sonic-sairedis#1329
#3056
sonic-net/sonic-host-services#90
How to verify it
execute express-reboot script
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)