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.

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

[warm boot] move warm reboot/fast reboot knowledge to syncd service script #372

Merged
merged 5 commits into from
Nov 15, 2018

Conversation

yxieca
Copy link
Contributor

@yxieca yxieca commented Nov 8, 2018

- What I did
This PR is enabled by sonic-buildimage PR 2238

In test, we found that when issue-ing "docker kill swss", it causes swss service to become FAILED. As result, systemctrl will execute swss stop script. Which will also stop syncd service if the warmboot flag is not set in warm boot case. Before sonic-buildimage PR 2238, this will cause syncd process being killed instead of shutting down properly in both warm/fast reboot cases. Most importantly switch_remove() is not called. For both warm boot and fast reboot, properly shutting down syncd process is very crucial to the operations.

  • Setting warm boot flag before kill swss docker, so that it won't stop syncd at the same time. (syncd will be stopped explicitly later)
  • In fast reboot case, syncd script has knowledge to stop syncd process before killing the docker, which satisfies the fast reboot requirement.

- How to verify it
Please see verification steps in sonic-buildimage PR 2238

@qiluo-msft
Copy link
Contributor

qiluo-msft commented Nov 9, 2018

Let's wait sonic-net/sonic-buildimage#2242 merged or abandoned #Closed

@yxieca
Copy link
Contributor Author

yxieca commented Nov 13, 2018

  1. Move setting warm boot flag to top of the file
  2. add '-e' flag
  3. install onexit to cleanup on failure: revert the warmboot flag.
  4. stop syncd before saving database. #Resolved

@@ -109,15 +122,15 @@ docker kill teamd > /dev/null
# Kill swss dockers
docker kill swss

# syncd service stop is capable of handling both warm/fast/cold shutdown
systemctl stop syncd
Copy link
Contributor

@qiluo-msft qiluo-msft Nov 14, 2018

Choose a reason for hiding this comment

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

Possible risk with these 2 lines.

docker kill swss will trigger swss service stop(), and will docker stop syncd. Then it will trigger syncd service stop().

But you manually systemctl stop syncd again. #Closed

Copy link
Contributor Author

@yxieca yxieca Nov 15, 2018

Choose a reason for hiding this comment

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

I tested with following code:

#!/bin/bash

docker kill swss
echo $?

sudo systemctl stop syncd
echo $?

sonic prompt # ./test.sh
swss
0
0

Stopping a service after it has been stopped is no-op with success result. Does this address your concern? or are you thinking about something else?

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comment

Kill swss docker will cause service swss to fail, systemctrl will then
stop swss as result. If the flag is not set, swss will also stop syncd
without letting syncd to warm shutdown.

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Signed-off-by: Ying Xie <ying.xie@microsoft.com>
@yxieca
Copy link
Contributor Author

yxieca commented Nov 15, 2018

Rebased to latest master

@lguohan lguohan merged commit 45d85c9 into sonic-net:master Nov 15, 2018
@yxieca yxieca deleted the wb-syncd branch November 15, 2018 23:45
mihirpat1 added a commit to mihirpat1/sonic-utilities that referenced this pull request Sep 15, 2023
…single bank FW versioning (sonic-net#372)

* Display Inactive FW version from EEPROM for single bank CMIS transceivers

Signed-off-by: Mihir Patel <patelmi@microsoft.com>

* Increased code coverage

Signed-off-by: Mihir Patel <patelmi@microsoft.com>

---------

Signed-off-by: Mihir Patel <patelmi@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants