-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Move teamd warm reboot code to service script #5163
Conversation
…ice /usr/local/bin" This reverts commit d16d163.
please re-test |
please retest |
…t reboot based on latest community code
… fast reboot based on latest community code
… 2) apply multi-asic support to script
1) Remove space in variable assignement 2) Apply multi-asic support
1) Remove "systemctl stop ${SERVICE}$DEV" for fast-reboot 2) Call /user/bin/teamd.sh to stop teamd for all cases
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.
Minor comments added. It would be also better if we have output logs for start,stop,wait
.
start() { | ||
debug "Starting ${SERVICE}$DEV service..." | ||
|
||
check_warm_boot |
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.
Should we also wait for database initialization before accessing it? I see that some of the service scripts use it, and some are missing it already.
https://github.com/Azure/sonic-buildimage/blob/master/files/scripts/swss.sh#L60
https://github.com/Azure/sonic-buildimage/blob/master/files/scripts/swss.sh#L137
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.
I saw this wait_for_database_service() is called from swss and syncd. Teamd already has dependancy on swss.
retest broadcom please |
retest vsimage please |
1 similar comment
retest vsimage please |
retest vsimage please |
…les and unnecessary space
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.
LGTM, thanks Heidi.
Retest vsimage please |
Retest mellanox please |
Retest vsimage please |
3 similar comments
Retest vsimage please |
Retest vsimage please |
Retest vsimage please |
retest vsimage please |
The vsimage tests are failing due to incorrect permissions set on a new teamd script being added by this PR. Below is the fix:
This would prevent the below error seen in syslog:
@heidinet2007 I am unable to make this change on the fork. Can you make this change, or give me permission to do so? |
Signed-off-by: Ying Xie <ying.xie@microsoft.com>
@heidinet2007 I've pushed a change to your branch to change file permission. |
retest vsimage please |
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.
LGTM, thanks!
A new teamd service script has been added in sonic-buildimage as part of the PR - sonic-net/sonic-buildimage#5163 With this change, fast-reboot script will now use the new teamd script to handle killing teamd docker in both fast-boot and warm-boot mode. Replaced the existing teamd docker handling in fast-reboot script with the systemctl stop teamd. Tested the locally modified script in a DUT for warm-reboot and fast-reboot.
Summary: Move teamd functions to a new service script Motivation: To segregate teamd functions in one common place. fast-reboot script calls teamd functions that should ideally be replaced by a simple call to a service script. Changes: New teamd service script and path modification from /usr/bin/teamd.sh to /usr/local/bin/teamd.sh fast-reboot script (in sonic-utilities) modification (to use new teamd.sh to stop teamd) should follow soon after this change. Verification: VS image tests. Signed-off-by: Vaibhav Hemant Dixit <vaibhav.dixit@microsoft.com> Co-authored-by: heidi.ou@alibaba-inc.com <heidi.ou@alibaba-inc.com> Co-authored-by: Ying Xie <ying.xie@microsoft.com>
A new teamd service script has been added in sonic-buildimage as part of the PR - sonic-net/sonic-buildimage#5163 With this change, fast-reboot script will now use the new teamd script to handle killing teamd docker in both fast-boot and warm-boot mode. Replaced the existing teamd docker handling in fast-reboot script with the systemctl stop teamd. Tested the locally modified script in a DUT for warm-reboot and fast-reboot.
Move teamd warm reboot code to teamd service at /usr/local/bin