Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix CONFIG_DB_INITIALIZED flag check logic and set/reset flag for warmboot #15685

Merged
merged 9 commits into from
Aug 4, 2023
Merged

Fix CONFIG_DB_INITIALIZED flag check logic and set/reset flag for warmboot #15685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e6e34db
Fix CONFIG_DB_INITIALIZED flag check logic and set/reset flag for war…
vaibhavhd Jun 30, 2023
c05af46
Fix db-cli usage
vaibhavhd Jun 30, 2023
e0d3f0a
Merge remote-tracking branch 'upstream/master' into config-init-flag
vaibhavhd Jul 7, 2023
65c65e1
Handle same image warm-reboot and generalize handling of INIT flag
vaibhavhd Jul 7, 2023
678e33e
Cover boot from ONIE case: set config init flag when minigraph, confi…
vaibhavhd Jul 7, 2023
27baf15
Handle case: first boot of SONiC
vaibhavhd Jul 7, 2023
835c923
Merge remote-tracking branch 'upstream/master' into config-init-flag
vaibhavhd Jul 13, 2023
5779bfc
Check for config init flag
vaibhavhd Jul 18, 2023
fdecad1
Simplify logic, and do not call db_migrator for same image reboot
vaibhavhd Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 9 additions & 18 deletions files/build_templates/docker_image_ctl.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ function postStartAction()
($(docker exec -i database$DEV sonic-db-cli PING | grep -c PONG) -gt 0) ]]; do
sleep 1;
done

if [[ ("$BOOT_TYPE" == "warm" || "$BOOT_TYPE" == "fastfast" || "$BOOT_TYPE" == "fast") && -f $WARM_DIR/dump.rdb ]]; then
# retain the dump file from last boot for debugging purposes
mv $WARM_DIR/dump.rdb $WARM_DIR/dump.rdb.old
Expand All @@ -248,28 +249,18 @@ function postStartAction()
$SONIC_CFGGEN -j /etc/sonic/config_db$DEV.json --write-to-db
fi
fi

if [[ "$BOOT_TYPE" == "fast" ]]; then
Copy link
Contributor Author

@vaibhavhd vaibhavhd Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviewer:
This code section is being removed as it will never get called - the check for fast reboot will enter the prev parent if block and not this.

Moreoever, w/ new changes in fast-boot, we have a dedicated flag for fastboot that gets cleared as part of finalizer. The timed expiry mechanism is not needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw this, this is actually crucial for upgrading from older versions prior fast-reboot enhancements.
For example, upgrading from 202012 images, in case it exceeded 180 seconds no indication of fast-reboot is found in STATE-DB and db-migrator will fail to set fast-reboot new notation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Thanks for reporting this. The PR was merged when you added comment, and I did not check it.
Fixed this here: #16733

# set the key to expire in 3 minutes
$SONIC_DB_CLI STATE_DB SET "FAST_REBOOT|system" "1" "EX" "180"
fi

$SONIC_DB_CLI CONFIG_DB SET "CONFIG_DB_INITIALIZED" "1"
fi

if [ -e /tmp/pending_config_migration ]; then
if [ -e /tmp/pending_config_migration ] || [ -e /tmp/pending_config_initialization ]; then
Copy link
Contributor Author

@vaibhavhd vaibhavhd Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviewer:

pending_config_migration is used for sonic to sonic upgrades.
pending_config_initialization is used for sonic conversions / first boot cases.

Both of these flags are set by rc.local script.
The flags are unset (files removed) in config-setup.service.

# this is first boot to a new image, config-setup execution is pending.
# For fast/cold reboot case, DB contains nothing at this point
# Call db_migrator after config-setup loads the config (from old config or minigraph)
echo "Delaying db_migrator until config migration is over"
# for warmboot case, DB is loaded but migration is still pending
# For firstbboot/fast/cold reboot case, DB contains nothing at this point
# unset CONFIG_DB_INITIALIZED to indicate pending config load and migration
# This flag will be set to "1" after DB migration/initialization is completed as part of config-setup
$SONIC_DB_CLI CONFIG_DB SET "CONFIG_DB_INITIALIZED" "0"
else
# this is not a first time boot to a new image. Datbase container starts w/ old pre-existing config
if [[ -x /usr/local/bin/db_migrator.py ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviwer:

I don't think migration is needed for same-image reboot.

For case when device reboots to a different image, but this is not first-time boot to this image:
Cold boot: Migration is not needed for cold reboot, as config_db file in target image already has correct version.

Warm/fast boot: Migration is needed, and old config dump will have old version. But, do we want to support this path, as ideally we expect that the upgrades are always first-time-boot. If this expectation is not true then I can enable the call here for warmboot case.

# Migrate the DB to the latest schema version if needed
if [ -z "$DEV" ]; then
/usr/local/bin/db_migrator.py -o migrate
fi
fi
# set CONFIG_DB_INITIALIZED to indicate end of config load and migration
$SONIC_DB_CLI CONFIG_DB SET "CONFIG_DB_INITIALIZED" "1"
fi
# Add redis UDS to the redis group and give read/write access to the group
REDIS_SOCK="/var/run/redis${DEV}/redis.sock"
Expand Down
4 changes: 3 additions & 1 deletion files/image_config/config-setup/config-setup
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ do_config_initialization()
fi

rm -f /tmp/pending_config_initialization
sonic-db-cli CONFIG_DB SET "CONFIG_DB_INITIALIZED" "1"
}

# Restore config-setup post migration hooks from a backup copy
Expand Down Expand Up @@ -344,13 +345,14 @@ check_all_config_db_present()
}

# DB schema is subject to change between two images
# Perform DB schema migration after loading backup config from previous image
# Perform DB schema migration after loading backup config/minigraph from previous image
do_db_migration()
{
if [[ -x /usr/local/bin/db_migrator.py ]]; then
# Migrate the DB to the latest schema version if needed
/usr/local/bin/db_migrator.py -o migrate
fi
sonic-db-cli CONFIG_DB SET "CONFIG_DB_INITIALIZED" "1"
}

# Perform configuration migration from backup copy.
Expand Down
2 changes: 1 addition & 1 deletion files/image_config/warmboot-finalizer/finalize-warmboot.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function wait_for_database_service()
done

# Wait for configDB initialization
until [[ $(sonic-db-cli CONFIG_DB GET "CONFIG_DB_INITIALIZED") ]];
until [[ $(sonic-db-cli CONFIG_DB GET "CONFIG_DB_INITIALIZED") -eq 1 ]];
do sleep 1;
done

Expand Down
2 changes: 1 addition & 1 deletion files/scripts/swss.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function wait_for_database_service()
done

# Wait for configDB initialization
until [[ $($SONIC_DB_CLI CONFIG_DB GET "CONFIG_DB_INITIALIZED") ]];
until [[ $($SONIC_DB_CLI CONFIG_DB GET "CONFIG_DB_INITIALIZED") -eq 1 ]];
do sleep 1;
done
}
Expand Down
2 changes: 1 addition & 1 deletion files/scripts/syncd_common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function wait_for_database_service()
done

# Wait for configDB initialization
until [[ $($SONIC_DB_CLI CONFIG_DB GET "CONFIG_DB_INITIALIZED") ]];
until [[ $($SONIC_DB_CLI CONFIG_DB GET "CONFIG_DB_INITIALIZED") -eq 1 ]];
do sleep 1;
done
}
Expand Down