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

[docker-teamd]: Refactor cleanup procedure. #441

Closed
wants to merge 3 commits into from

Conversation

marian-pritsak
Copy link
Collaborator

Signed-off-by: marian-pritsak [email protected]

@marian-pritsak
Copy link
Collaborator Author

@lguohan
Copy link
Collaborator

lguohan commented Mar 27, 2017

Please update this PR when both PR are merged into sonic-swss

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

please update sonic-swss submodule

@qiluo-msft
Copy link
Collaborator

@stcheng Please help review

@@ -26,5 +26,4 @@ ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get clean -y; apt-get autoclean -y; apt-get autoremove -y
RUN rm -rf /debs

ENTRYPOINT ["/bin/bash", "-c"]
CMD ["/usr/bin/config.sh && /usr/bin/start.sh"]
ENTRYPOINT ["/usr/bin/start.sh"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could use /bin/bash as ENTRYPOINT and /usr/bin/start.sh as CMD. Thus even when the start.sh fails at some unknown point, the docker won't exit and dev could attach to the docker and debug. The Dockerfile could be similar to docker-orchagent/Dockerfile.j2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

pkill -9 teamd
if [ -d $TEAMD_CONF_PATH ]; then
for f in $TEAMD_CONF_PATH/*; do
teamd -f $f -k
Copy link
Contributor

Choose a reason for hiding this comment

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

So I believe this will successfully stop all the team instances without leaving anything that needs to be manually cleaned up after the next docker starts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

-k option kills corresponding daemon and deletes LAG interface

@@ -26,5 +26,5 @@ ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get clean -y; apt-get autoclean -y; apt-get autoremove -y
RUN rm -rf /debs

ENTRYPOINT ["/bin/bash", "-c"]
CMD ["/usr/bin/config.sh && /usr/bin/start.sh"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any special reason that we need to merge config.sh and start.sh back into one file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reason for that is to correctly handle signals sent to container.
See #435

Copy link
Contributor

@stcheng stcheng left a comment

Choose a reason for hiding this comment

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

one issue needs to be addressed.

teamd -f $f -d
done
fi
teamsyncd &
}

function clean_up {
pkill -9 teamd
if [ -d $TEAMD_CONF_PATH ]; then
for f in $TEAMD_CONF_PATH/*; do
Copy link
Contributor

Choose a reason for hiding this comment

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

check if the folder is empty or not. if the folder is empty, the variable f will become /etc/teamd/*.

@marian-pritsak
Copy link
Collaborator Author

Rebased and fixed merge conflicts

@marian-pritsak
Copy link
Collaborator Author

Still pending issue #497

@stcheng
Copy link
Contributor

stcheng commented Apr 28, 2017

@marian-pritsak I have tried the patch. I notice that even with read, the docker fails to capture the signal 15 (SIGTERM) so that the clean_up function is not executed and it will affect the next round start_app function logics.

@marian-pritsak
Copy link
Collaborator Author

@stcheng which command did you execute?

I did the following test:

# show port channels
$ ip link show | grep PortChannel
14: Ethernet4: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel0 state DOWN mode DEFAULT group default qlen 1000
15: Ethernet16: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel16 state DOWN mode DEFAULT group default qlen 1000
16: Ethernet56: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel56 state DOWN mode DEFAULT group default qlen 1000
18: Ethernet8: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel8 state DOWN mode DEFAULT group default qlen 1000
19: Ethernet44: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel40 state DOWN mode DEFAULT group default qlen 1000
20: Ethernet12: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel8 state DOWN mode DEFAULT group default qlen 1000
21: Ethernet32: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel32 state DOWN mode DEFAULT group default qlen 1000
23: Ethernet0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel0 state DOWN mode DEFAULT group default qlen 1000
24: Ethernet60: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel56 state DOWN mode DEFAULT group default qlen 1000
25: Ethernet20: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel16 state DOWN mode DEFAULT group default qlen 1000
28: Ethernet40: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel40 state DOWN mode DEFAULT group default qlen 1000
29: Ethernet52: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel48 state DOWN mode DEFAULT group default qlen 1000
35: Ethernet28: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel24 state DOWN mode DEFAULT group default qlen 1000
38: Ethernet48: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel48 state DOWN mode DEFAULT group default qlen 1000
39: Ethernet24: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel24 state DOWN mode DEFAULT group default qlen 1000
43: Ethernet36: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast master PortChannel32 state DOWN mode DEFAULT group default qlen 1000
70: PortChannel0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 
71: PortChannel16: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 
72: PortChannel24: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 
73: PortChannel32: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 
74: PortChannel40: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 
75: PortChannel48: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 
76: PortChannel56: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 
77: PortChannel8: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default 

# stop docker-teamd
$ systemctl stop teamd
$
# show port channels returns nothing
$ ip link show | grep PortChannel
$ 

I added echo messages into each function for debug, and I can see cleanup is called:

start_app
[ ok ] Starting enhanced syslogd: rsyslogd.
This program is not intended to be run as root.
This program is not intended to be run as root.
This program is not intended to be run as root.
This program is not intended to be run as root.
This program is not intended to be run as root.
This program is not intended to be run as root.
This program is not intended to be run as root.
This program is not intended to be run as root.
Listens to teamd events...
clean_up
[ ok ] Stopping enhanced syslogd: rsyslogd.

@jleveque
Copy link
Contributor

jleveque commented May 5, 2017

@marian-pritsak: I am currently working on a PR that will change all docker ENTRYPOINTs to run supervisord. This will fix issue #435. I say we hold off on merging this PR until after I submit my PR.

@marian-pritsak
Copy link
Collaborator Author

@jleveque thanks.
Let's wait till your changes will be available.

@msftclas
Copy link

@marian-pritsak, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

qiluo-msft added a commit to qiluo-msft/sonic-buildimage that referenced this pull request Jan 7, 2021
Includes sonic-swss-common commits:
```
71dc350 2021-01-07 | Lower the log level for outdated key for SubscriberStateTable notification (sonic-net#441) [Qi Luo]
7e40582 2021-01-08 | Add boost dependencies (sonic-net#442) [Ze Gan]
30a8ddf 2021-01-05 | Change DBConnector::hgetall return type from map to unordered_map (sonic-net#440) [Qi Luo]
021108d 2021-01-02 | MCLAG Enhancements per HLD sonic-net/SONiC#596 (sonic-net#405) [Praveen-Brcm]
54996fc 2021-01-02 | Implement ConfigDBConnector and ConfigDBPipeConnector in C++ (sonic-net#437) [Qi Luo]
8286525 2020-12-27 | Simply refactor DBConnector hgetall() [Qi Luo]
6d1d33b 2020-12-27 | Fix RedisTransactioner: handle empty deque [Qi Luo]
624e0b8 2020-12-26 | Move complex class constructor as explicit, and fix several mistaken copy constructor usage [Qi Luo]
3b983f9 2020-12-30 | [ci]: add timeout to 180 minutes for arm build (sonic-net#439) [lguohan]
f2e4210 2020-12-29 | Add utility for string and redis (sonic-net#434) [Ze Gan]
7a885fd 2020-12-29 | [build]: add build check for arm64 and armhf (sonic-net#436) [lguohan]
47bccc4 2020-12-24 | Add missed vector header to rediscommand.h (sonic-net#435) [Ze Gan]
```
qiluo-msft added a commit that referenced this pull request Jan 8, 2021
Includes sonic-swss-common commits:
```
71dc350 2021-01-07 | Lower the log level for outdated key for SubscriberStateTable notification (#441) [Qi Luo]
7e40582 2021-01-08 | Add boost dependencies (#442) [Ze Gan]
30a8ddf 2021-01-05 | Change DBConnector::hgetall return type from map to unordered_map (#440) [Qi Luo]
021108d 2021-01-02 | MCLAG Enhancements per HLD sonic-net/SONiC#596 (#405) [Praveen-Brcm]
54996fc 2021-01-02 | Implement ConfigDBConnector and ConfigDBPipeConnector in C++ (#437) [Qi Luo]
8286525 2020-12-27 | Simply refactor DBConnector hgetall() [Qi Luo]
6d1d33b 2020-12-27 | Fix RedisTransactioner: handle empty deque [Qi Luo]
624e0b8 2020-12-26 | Move complex class constructor as explicit, and fix several mistaken copy constructor usage [Qi Luo]
3b983f9 2020-12-30 | [ci]: add timeout to 180 minutes for arm build (#439) [lguohan]
f2e4210 2020-12-29 | Add utility for string and redis (#434) [Ze Gan]
7a885fd 2020-12-29 | [build]: add build check for arm64 and armhf (#436) [lguohan]
47bccc4 2020-12-24 | Add missed vector header to rediscommand.h (#435) [Ze Gan]
```
lguohan pushed a commit that referenced this pull request Jan 9, 2021
Includes sonic-swss-common commits:
```
71dc350 2021-01-07 | Lower the log level for outdated key for SubscriberStateTable notification (#441) [Qi Luo]
7e40582 2021-01-08 | Add boost dependencies (#442) [Ze Gan]
30a8ddf 2021-01-05 | Change DBConnector::hgetall return type from map to unordered_map (#440) [Qi Luo]
021108d 2021-01-02 | MCLAG Enhancements per HLD sonic-net/SONiC#596 (#405) [Praveen-Brcm]
54996fc 2021-01-02 | Implement ConfigDBConnector and ConfigDBPipeConnector in C++ (#437) [Qi Luo]
8286525 2020-12-27 | Simply refactor DBConnector hgetall() [Qi Luo]
6d1d33b 2020-12-27 | Fix RedisTransactioner: handle empty deque [Qi Luo]
624e0b8 2020-12-26 | Move complex class constructor as explicit, and fix several mistaken copy constructor usage [Qi Luo]
3b983f9 2020-12-30 | [ci]: add timeout to 180 minutes for arm build (#439) [lguohan]
f2e4210 2020-12-29 | Add utility for string and redis (#434) [Ze Gan]
7a885fd 2020-12-29 | [build]: add build check for arm64 and armhf (#436) [lguohan]
47bccc4 2020-12-24 | Add missed vector header to rediscommand.h (#435) [Ze Gan]
```
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.

7 participants