Run db_migrator for non first-time reboots by vivekrnv · Pull Request #16116 · sonic-net/sonic-buildimage

vivekrnv · 2023-08-11T19:55:55Z

Why I did it

The recent change #15685 (comment) removed the db migration for non first reboots.

This is problematic for many deployments which doesn't rely on ZTP and push a custom config_db.json

Port to older branches after #15685 is ported back

Work item tracking

Microsoft ADO (number only):

How I did it

Re-introduce the logic to run the db_migrator on non-first boots

How to verify it

Verified reboot and warm-reboot cases

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Vivek Reddy Karri <[email protected]>

vaibhavhd · 2023-08-14T17:41:19Z

files/build_templates/docker_image_ctl.j2

            # 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
+            if [[ "$BOOT_TYPE" != "warm" ]]; then


Why is this setting needed now, when it was not needed earlier? And, why is warm-reboot an exception?

Because db_migrator busy waits when the CONFIG_DB_INITIALIZED flag is not set. Probably because of ConfigDBConnector https://github.com/sonic-net/sonic-swss-common/blob/master/common/configdb.cpp#L29.

I've only checked for warm-reboot since warm-reboot might already contain the CONFIG_DB_INITIALIZED from the previous db.

I feel its better to update db_migrator to not depend on CONFIG_DB_INIT, what do you think?

vaibhavhd · 2023-08-14T17:52:57Z

files/build_templates/docker_image_ctl.j2

+                $SONIC_DB_CLI CONFIG_DB SET "CONFIG_DB_INITIALIZED" "0"
+            fi
+            # 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


I am thinking if we not follow the same flow as we had earlier. Rather than that, why not confine all the migration logic in one place - config-setup.service?

This would ensure that migration also happens after the DB is loaded w/ relevant config - from minigraph, old config, new pushed config, etc.

Specifically, I am asking that this db_migrator call be moved here as part of new else block:

sonic-buildimage/files/image_config/config-setup/config-setup

Line 370 in c05af46

if [ -e /tmp/pending_config_migration ] || [ -e ${CONFIG_SETUP_POST_MIGRATION_FLAG} ]; then

and to

sonic-buildimage/files/image_config/config-setup/config-setup

Line 382 in c05af46

if [ -e /tmp/pending_config_initialization ] || [ -e ${CONFIG_SETUP_INITIALIZATION_FLAG} ]; then

We have to run it here since the config_db.json has to be loaded first. If we move this to config-setup, this can't be guaranteed.

https://github.com/sonic-net/sonic-buildimage/pull/16116/files#diff-130e5ab75471398db06b73a4c6e4b56517f42e3c1f17870b5309620fc0e2fafbR247

…to db_mig_succesive_boot

Signed-off-by: Vivek Reddy Karri <[email protected]>

vivekrnv · 2023-08-14T23:49:43Z

@vaibhavhd Handled the comments. the db_migrator fix is actually not required given we are setting the INIT flag to 0 in db. service. I will fix the db_migrator script later but since that is not required here i don't want to tie that optional fix with this PR.

vivekrnv · 2023-08-21T14:34:23Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2023-08-21T14:34:27Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2023-08-21T14:34:39Z

Azure Pipelines successfully started running 1 pipeline(s).

vivekrnv · 2023-08-22T02:01:42Z

Failing in kvmtest-multi-asic-t1-lag. i.e. on masic platforms. Will move the migration back to database.service.

Signed-off-by: Vivek Reddy Karri <[email protected]>

- Why I did it The recent change #15685 (comment) removed the db migration for non first reboots. This is problematic for many deployments which doesn't rely on ZTP and push a custom config_db.json Port to older branches after #15685 is ported back - How I did it Re-introduce the logic to run the db_migrator on non-first boots - How to verify it Verified reboot and warm-reboot cases Signed-off-by: Vivek Reddy Karri <[email protected]>

- Why I did it The recent change sonic-net#15685 (comment) removed the db migration for non first reboots. This is problematic for many deployments which doesn't rely on ZTP and push a custom config_db.json Port to older branches after sonic-net#15685 is ported back - How I did it Re-introduce the logic to run the db_migrator on non-first boots - How to verify it Verified reboot and warm-reboot cases Signed-off-by: Vivek Reddy Karri <[email protected]>

mssonicbld · 2023-09-06T19:00:39Z

Cherry-pick PR to 202012: #16477

- Why I did it The recent change #15685 (comment) removed the db migration for non first reboots. This is problematic for many deployments which doesn't rely on ZTP and push a custom config_db.json Port to older branches after #15685 is ported back - How I did it Re-introduce the logic to run the db_migrator on non-first boots - How to verify it Verified reboot and warm-reboot cases Signed-off-by: Vivek Reddy Karri <[email protected]>

dgsudharsan · 2023-09-11T15:55:06Z

@yxieca Can you please help with the cherry-pick to 202205?

- Why I did it The recent change sonic-net#15685 (comment) removed the db migration for non first reboots. This is problematic for many deployments which doesn't rely on ZTP and push a custom config_db.json Port to older branches after sonic-net#15685 is ported back - How I did it Re-introduce the logic to run the db_migrator on non-first boots - How to verify it Verified reboot and warm-reboot cases Signed-off-by: Vivek Reddy Karri <[email protected]>

mssonicbld · 2023-09-12T05:27:41Z

Cherry-pick PR to 202205: #16520

- Why I did it The recent change sonic-net#15685 (comment) removed the db migration for non first reboots. This is problematic for many deployments which doesn't rely on ZTP and push a custom config_db.json Port to older branches after sonic-net#15685 is ported back - How I did it Re-introduce the logic to run the db_migrator on non-first boots - How to verify it Verified reboot and warm-reboot cases Signed-off-by: Vivek Reddy Karri <[email protected]>

vivekrnv and others added 4 commits August 11, 2023 00:40

Run db_migrator for non first-time reboots

fcc52d5

Signed-off-by: Vivek Reddy Karri <[email protected]>

Add the INIT flag before running db-migrator

861a3db

Signed-off-by: Vivek Reddy Karri <[email protected]>

Minor mistake

7cfa2f3

Signed-off-by: Vivek Reddy Karri <[email protected]>

Merge branch 'sonic-net:master' into db_mig_succesive_boot

a225182

vivekrnv requested a review from lguohan as a code owner August 11, 2023 19:55

vivekrnv assigned vaibhavhd and dgsudharsan and unassigned dgsudharsan and vaibhavhd Aug 11, 2023

vivekrnv requested review from dgsudharsan and vaibhavhd August 11, 2023 19:59

vivekrnv added Request for 202205 Branch Request for 202211 Branch Request for 202305 Branch labels Aug 12, 2023

dgsudharsan approved these changes Aug 12, 2023

View reviewed changes

vaibhavhd reviewed Aug 14, 2023

View reviewed changes

vivekrnv added 2 commits August 14, 2023 20:45

Merge branch 'master' of https://github.com/Azure/sonic-buildimage in…

fb542cb

…to db_mig_succesive_boot

Move the db migration in all cases to config-setup

0301063

Signed-off-by: Vivek Reddy Karri <[email protected]>

vivekrnv added 2 commits August 14, 2023 18:58

Merge branch 'sonic-net:master' into db_mig_succesive_boot

e7c8d00

Merge branch 'sonic-net:master' into db_mig_succesive_boot

12bb1bb

vaibhavhd approved these changes Aug 15, 2023

View reviewed changes

Merge branch 'sonic-net:master' into db_mig_succesive_boot

ddc222a

Merge branch 'sonic-net:master' into db_mig_succesive_boot

5aabb84

vivekrnv added the Request for 202012 Branch label Aug 22, 2023

Move the migration back to db.service

e3804ac

Signed-off-by: Vivek Reddy Karri <[email protected]>

mssonicbld mentioned this pull request Aug 28, 2023

[action] [PR:16116] Run db_migrator for non first-time reboots #16307

Merged

11 tasks

mssonicbld added a commit that referenced this pull request Aug 28, 2023

Run db_migrator for non first-time reboots (#16116) (#16306)

adfc486

mssonicbld added Included in 202305 Branch and removed Created PR to 202305 Branch labels Aug 28, 2023

mssonicbld added Included in 202211 Branch and removed Approved for 202211 Branch Created PR to 202211 Branch labels Aug 31, 2023

qiluo-msft added the Approved for 202012 Branch label Sep 6, 2023

mssonicbld added the Created PR to 202012 Branch label Sep 6, 2023

mssonicbld mentioned this pull request Sep 6, 2023

[action] [PR:16116] Run db_migrator for non first-time reboots #16477

Merged

11 tasks

mssonicbld added Included in 202012 Branch and removed Approved for 202012 Branch Created PR to 202012 Branch labels Sep 7, 2023

yxieca added the Approved for 202205 Branch label Sep 12, 2023

mssonicbld added the Created PR to 202205 Branch label Sep 12, 2023

mssonicbld mentioned this pull request Sep 12, 2023

[action] [PR:16116] Run db_migrator for non first-time reboots #16520

Merged

11 tasks

mssonicbld added a commit that referenced this pull request Sep 12, 2023

Run db_migrator for non first-time reboots (#16116) (#16520)

b4ab3e0

mssonicbld added Included in 202205 Branch and removed Approved for 202205 Branch Created PR to 202205 Branch labels Sep 12, 2023

vivekrnv mentioned this pull request Oct 9, 2023

Remove the CONFIG_DB_INIT flag dependency on db_migrator sonic-net/sonic-utilities#2959

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run db_migrator for non first-time reboots#16116

Run db_migrator for non first-time reboots#16116
liat-grozovik merged 12 commits intosonic-net:masterfrom
vivekrnv:db_mig_succesive_boot

vivekrnv commented Aug 11, 2023 •

edited

Loading

Uh oh!

vaibhavhd Aug 14, 2023

Uh oh!

vivekrnv Aug 14, 2023

Uh oh!

vaibhavhd Aug 14, 2023

Uh oh!

vivekrnv Aug 14, 2023

Uh oh!

vivekrnv commented Aug 14, 2023

Uh oh!

vivekrnv commented Aug 21, 2023

Uh oh!

mssonicbld commented Aug 21, 2023

Uh oh!

azure-pipelines bot commented Aug 21, 2023

Uh oh!

vivekrnv commented Aug 22, 2023

Uh oh!

mssonicbld commented Sep 6, 2023

Uh oh!

dgsudharsan commented Sep 11, 2023

Uh oh!

mssonicbld commented Sep 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

vivekrnv commented Aug 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I did it

Work item tracking

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Uh oh!

vaibhavhd Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

vivekrnv Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

vaibhavhd Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

vivekrnv Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

vivekrnv commented Aug 14, 2023

Uh oh!

vivekrnv commented Aug 21, 2023

Uh oh!

mssonicbld commented Aug 21, 2023

Uh oh!

azure-pipelines bot commented Aug 21, 2023

Uh oh!

vivekrnv commented Aug 22, 2023

Uh oh!

mssonicbld commented Sep 6, 2023

Uh oh!

dgsudharsan commented Sep 11, 2023

Uh oh!

mssonicbld commented Sep 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

vivekrnv commented Aug 11, 2023 •

edited

Loading