Warm reboot: Add support for orchagent pre-shutdown warm-restart state check by jipanyang · Pull Request #562 · sonic-net/sonic-swss

jipanyang · 2018-08-03T04:58:20Z

Signed-off-by: Jipan Yang [email protected]

What I did
Before stopping orchagent for warm restart, basic state check is preferred to ensure orchagent is not in transient state, so a deterministic state may be restored after restart.

Here is to implement orchagent_restart_check binary which may talk to orchagent and ask it to do self-check, return "READY " signal and freeze if everything is ok, otherwise "NOT_READY" signal should be returned.

The exact condition that is treated as restart ready need further discussion. For now, checking no un-met dependency in orchagent is used as example to verify the end to end flow. The other possible check is to make sure all config data to ASIC DB has been flushed.

Why I did it

How I verified it
VS test:

jipan@sonic-build-2:~/warm_reboot/sonic-buildimage/src/sonic-swss/tests$ sudo pytest -v --dvsname=vs  test_warm_reboot.py
[sudo] password for jipan: 
======================================================================= test session starts =======================================================================
platform linux2 -- Python 2.7.12, pytest-3.3.0, py-1.5.4, pluggy-0.6.0 -- /usr/bin/python
cachedir: .cache
rootdir: /home/jipan/warm_reboot/sonic-buildimage/src/sonic-swss/tests, inifile:
collected 1 item                                                                                                                                                  

test_warm_reboot.py::test_OrchagentWarmRestartReadyCheck PASSED                                                                                             [100%]

Details if related

lguohan · 2018-08-16T08:51:39Z

can you resolve conflict?

Signed-off-by: Jipan Yang <[email protected]>

…heck Signed-off-by: Jipan Yang <[email protected]>

Signed-off-by: Jipan Yang <[email protected]>

qiluo-msft · 2018-08-30T03:37:17Z

I believe current implementation and roadmap will allow unplanned orchagent warm start, so this PR may be not necessary. I agree it is conservative and super safe condition for a warm start. #Pending

jipanyang · 2018-08-30T04:30:07Z

@qiluo-msft With unplanned restart, the program shutdown could happen at any point of orchagent execution, if you are confident that current implementation is able to handle all the scenarios, it is great. Supporting unplanned warm restart is the ultimate goal for each component of SONiC as we have discussed at the very beginning of warm reboot development.

I'll be happy to see everything works without the planned shutdown, or at least all the potential problems could be exposed and fixed accordingly :)

orchagent/orchdaemon.cpp

qiluo-msft

As discussed, my review focuses on harmful or not, even I think it is not necessary.

lguohan · 2018-09-12T08:19:39Z

code looks good to me. but i would like to model where the tool send request, if orchagent not ready, it will wait till orchagent is ready and then the tool is unblock.

…b_7_pre_warm_restart_check

Signed-off-by: Jipan Yang <[email protected]>

qiluo-msft · 2018-09-14T21:02:49Z

tests/test_warm_reboot.py

+    assert result == "RESTARTCHECK failed\n"
+
+    # recover for test cases after this one.
+    stop_swss(dvs)


stop_swss [](start = 4, length = 9)

Not defined. It will break vs test.

qiluo-msft

Please help fix vs test.

jipanyang · 2018-09-14T22:18:14Z

@qiluo-msft if possible, could #557 be merged first. The vs test for this PR uses start_swss(dvs) & stop_swss(dvs) defined there.

I could replicate start_swss(dvs) & stop_swss(dvs) to this PR too if this is preferred.

qiluo-msft · 2018-09-14T22:57:06Z

Sure. Check #557 first

qiluo-msft · 2018-09-14T23:12:24Z

Help solve conflict?

…b_7_pre_warm_restart_check

jipanyang · 2018-09-14T23:54:46Z

done. It looks VS test env has run mad.

qiluo-msft · 2019-06-03T21:57:11Z

orchagent/orchagent_restart_check.cpp

+    swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_INFO);
+    SWSS_LOG_ENTER();
+
+    std::string skipPendingTaskCheck = "fasle";


Could you please fix the typo? @jipanyang

Ok. Fortunately it is not causing problem due to value "true" is checked
https://github.com/Azure/sonic-swss/blob/master/orchagent/switchorch.cpp#L179

According to [HLD](https://github.com/Azure/SONiC/blob/master/doc/rates-and-utilization/Rates_and_utilization_HLD.md) Signed-off-by: Mykola Faryma <[email protected]>

…e check (sonic-net#562) * Add orchagent pre-warm-restart check mechanism * Add orchagent_restart_check options: --noFreeze & --skipPendingTaskCheck * Add waitTime option for response from orchagent * Fix build issue with latest master * adapt to new dvs.runcmd() signature * Move standard header before local headers

Updating the README to reflect the new dependency. Also, removing duplicate libgtest-dev from the azure-pipeline script.

jipanyang changed the title ~~Add orchagent pre-warm-restart check mechanism~~ Warm reboot: Add orchagent pre-warm-restart check mechanism Aug 3, 2018

jipanyang mentioned this pull request Aug 10, 2018

Warm reboot: Orchagent state restore #554

Merged

jipanyang force-pushed the warm_reboot_collab_7_pre_warm_restart_check branch from ab6ff12 to 10c5bd1 Compare August 13, 2018 18:53

jipanyang changed the title ~~Warm reboot: Add orchagent pre-warm-restart check mechanism~~ Warm reboot: Add support for orchagent pre-shutdown warm-restart state check Aug 17, 2018

stcheng added the Enhancement ➕ label Aug 21, 2018

jipanyang force-pushed the warm_reboot_collab_7_pre_warm_restart_check branch 3 times, most recently from edbd459 to 8896686 Compare August 22, 2018 04:12

jipanyang added 5 commits August 22, 2018 10:47

Add orchagent pre-warm-restart check mechanism

3366c61

Signed-off-by: Jipan Yang <[email protected]>

Add orchagent_restart_check options: --noFreeze & --skipPendingTaskC…

d53130b

…heck Signed-off-by: Jipan Yang <[email protected]>

Add waitTime option for response from orchagent

038b38e

Signed-off-by: Jipan Yang <[email protected]>

Fix build issue with latest master

34db540

Signed-off-by: Jipan Yang <[email protected]>

adapt to new dvs.runcmd() signature

53d8b25

Signed-off-by: Jipan Yang <[email protected]>

jipanyang force-pushed the warm_reboot_collab_7_pre_warm_restart_check branch from 8896686 to 53d8b25 Compare August 22, 2018 17:50

jipanyang mentioned this pull request Aug 27, 2018

Support warm-restart for neighsyncd #599

Merged

qiluo-msft reviewed Sep 7, 2018

View reviewed changes

orchagent/orchdaemon.cpp Outdated Show resolved Hide resolved

qiluo-msft reviewed Sep 7, 2018

View reviewed changes

orchagent/orchdaemon.cpp Show resolved Hide resolved

qiluo-msft reviewed Sep 7, 2018

View reviewed changes

orchagent/orchdaemon.cpp Show resolved Hide resolved

qiluo-msft reviewed Sep 7, 2018

View reviewed changes

zhenggen-xu mentioned this pull request Sep 7, 2018

Warm reboot: Add support for docker upgrade sonic-net/sonic-utilities#292

Merged

lguohan approved these changes Sep 12, 2018

View reviewed changes

jipanyang added 2 commits September 12, 2018 13:45

Merge remote-tracking branch 'upstream/master' into warm_reboot_colla…

c8d46b3

…b_7_pre_warm_restart_check

Move standard header before local headers

c627391

Signed-off-by: Jipan Yang <[email protected]>

qiluo-msft approved these changes Sep 13, 2018

View reviewed changes

qiluo-msft reviewed Sep 14, 2018

View reviewed changes

qiluo-msft requested changes Sep 14, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into warm_reboot_colla…

fa4d389

…b_7_pre_warm_restart_check

qiluo-msft approved these changes Sep 14, 2018

View reviewed changes

qiluo-msft merged commit 9fda944 into sonic-net:master Sep 15, 2018

jipanyang deleted the warm_reboot_collab_7_pre_warm_restart_check branch February 9, 2019 02:33

qiluo-msft reviewed Jun 3, 2019

View reviewed changes

jianyuewu pushed a commit to jianyuewu/sonic-swss that referenced this pull request Dec 24, 2025

Update README for gmock (sonic-net#562)

6c007c0

Updating the README to reflect the new dependency. Also, removing duplicate libgtest-dev from the azure-pipeline script.

Conversation

jipanyang commented Aug 3, 2018

Uh oh!

lguohan commented Aug 16, 2018

Uh oh!

qiluo-msft commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jipanyang commented Aug 30, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

lguohan commented Sep 12, 2018

Uh oh!

qiluo-msft Sep 14, 2018

Choose a reason for hiding this comment

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

jipanyang commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft commented Sep 14, 2018

Uh oh!

qiluo-msft commented Sep 14, 2018

Uh oh!

jipanyang commented Sep 14, 2018

Uh oh!

qiluo-msft Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jipanyang Jun 3, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qiluo-msft commented Aug 30, 2018 •

edited

Loading

jipanyang commented Sep 14, 2018 •

edited

Loading

qiluo-msft Jun 3, 2019 •

edited

Loading