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

[config] Eliminate race condition between reloading Monit config and monitoring container_checker #1543

Merged
merged 1 commit into from
Apr 4, 2021

Conversation

yozhao101
Copy link
Contributor

@yozhao101 yozhao101 commented Apr 2, 2021

Signed-off-by: Yong Zhao [email protected]

What I did

Nightly test found a failure when we ran the command sudo config reload/load_minigraph, The error message is:

admin@str-a7050-acs-1:~$ sudo config reload -y
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Resetting failed status on bgp.service
Resetting failed status on caclmgrd.service
Resetting failed status on dhcp_relay.service
Resetting failed status on hostcfgd.service
Resetting failed status on hostname-config.service
Resetting failed status on interfaces-config.service
Resetting failed status on lldp.service
Resetting failed status on ntp-config.service
Resetting failed status on pmon.service
Resetting failed status on procdockerstatsd.service
Resetting failed status on radv.service
Resetting failed status on rsyslog-config.service
Resetting failed status on swss.service
Resetting failed status on syncd.service
Resetting failed status on teamd.service
Resetting failed status on telemetry.timer
Restarting SONiC target ...
Reloading Monit configuration ...
Reinitializing monit daemon
Enabling container monitoring ...
Unix socket /var/run/monit.sock connection error -- No such file or directory

The root reason is that there exists an implicit race condition between the command sudo monit reload at line 701 and
the command sudo monit monitor container_checker at line 706. Both commands need access the monit.sock socket file
under the directory /var/run/.

Specifically the sudo monit reload at line 701 will re-initialize the Monit daemon, delete old monit.sock file and then create a new one. During this re-initializing process, the command sudo monit status can always execute successfully at line 704 before the old monit.sock file was deleted, but the command sudo monit monitor container_checker at line 706 will only succeed if the new monit.sock was created, otherwise it will fail and raise this error message.

How I did it

I changed the sequence between the operation to reload Monit configuration and the operation to enable monitoring container_checker.

How to verify it

I verified this change on DuT str-a7050-acs-1 by running the command sudo config reload/load_minigraph -y to make sure the error was not raised again.

Previous command output (if the output of a command-line utility has changed)

admin@str-a7050-acs-1:~$ sudo config reload -y
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Resetting failed status on bgp.service
Resetting failed status on caclmgrd.service
Resetting failed status on dhcp_relay.service
Resetting failed status on hostcfgd.service
Resetting failed status on hostname-config.service
Resetting failed status on interfaces-config.service
Resetting failed status on lldp.service
Resetting failed status on ntp-config.service
Resetting failed status on pmon.service
Resetting failed status on procdockerstatsd.service
Resetting failed status on radv.service
Resetting failed status on rsyslog-config.service
Resetting failed status on swss.service
Resetting failed status on syncd.service
Resetting failed status on teamd.service
Resetting failed status on telemetry.timer
Restarting SONiC target ...
Reloading Monit configuration ...
Reinitializing monit daemon
Enabling container monitoring ...

New command output (if the output of a command-line utility has changed)

admin@str-a7050-acs-1:~$ sudo config reload -y
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Resetting failed status on bgp.service
Resetting failed status on caclmgrd.service
Resetting failed status on dhcp_relay.service
Resetting failed status on hostcfgd.service
Resetting failed status on hostname-config.service
Resetting failed status on interfaces-config.service
Resetting failed status on lldp.service
Resetting failed status on ntp-config.service
Resetting failed status on pmon.service
Resetting failed status on procdockerstatsd.service
Resetting failed status on radv.service
Resetting failed status on rsyslog-config.service
Resetting failed status on swss.service
Resetting failed status on syncd.service
Resetting failed status on teamd.service
Resetting failed status on telemetry.timer
Restarting SONiC target ...
Enabling container monitoring ...
Reloading Monit configuration ...
Reinitializing monit daemon

@yozhao101 yozhao101 changed the title [config] Eliminate race condition between reloading Monit config and enabling container_checker [config] Eliminate race condition between reloading Monit config and monitoring container_checker Apr 2, 2021
@yozhao101 yozhao101 requested a review from jleveque April 2, 2021 21:56
@jleveque
Copy link
Contributor

jleveque commented Apr 2, 2021

@yozhao101: Please add the "Request for " label if requesting a cherry-pick. the "Included in label is used to indicate cherry-pick has been done, or if a PR is targeted at a release branch.

@lguohan
Copy link
Contributor

lguohan commented Apr 4, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yozhao101
Copy link
Contributor Author

@yozhao101: Please add the "Request for " label if requesting a cherry-pick. the "Included in label is used to indicate cherry-pick has been done, or if a PR is targeted at a release branch.

Thank Joe! I will pay attention to such issue in future.

@yozhao101 yozhao101 merged commit 9bbc25f into sonic-net:master Apr 4, 2021
@yozhao101 yozhao101 deleted the eliminate_race_condition branch April 4, 2021 21:39
yxieca pushed a commit that referenced this pull request Apr 8, 2021
…1543)

Signed-off-by: Yong Zhao [email protected]

What I did
Nightly test found a failure when we ran the command sudo config reload/load_minigraph, The error message is:

admin@str-a7050-acs-1:~$ sudo config reload -y
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Resetting failed status on bgp.service
Resetting failed status on caclmgrd.service
Resetting failed status on dhcp_relay.service
Resetting failed status on hostcfgd.service
Resetting failed status on hostname-config.service
Resetting failed status on interfaces-config.service
Resetting failed status on lldp.service
Resetting failed status on ntp-config.service
Resetting failed status on pmon.service
Resetting failed status on procdockerstatsd.service
Resetting failed status on radv.service
Resetting failed status on rsyslog-config.service
Resetting failed status on swss.service
Resetting failed status on syncd.service
Resetting failed status on teamd.service
Resetting failed status on telemetry.timer
Restarting SONiC target ...
Reloading Monit configuration ...
Reinitializing monit daemon
Enabling container monitoring ...
Unix socket /var/run/monit.sock connection error -- No such file or directory
The root reason is that there exists an implicit race condition between the command sudo monit reload at line 701 and
the command sudo monit monitor container_checker at line 706. Both commands need access the monit.sock socket file
under the directory /var/run/.

Specifically the sudo monit reload at line 701 will re-initialize the Monit daemon, delete old monit.sock file and then create a new one. During this re-initializing process, the command sudo monit status can always execute successfully at line 704 before the old monit.sock file was deleted, but the command sudo monit monitor container_checker at line 706 will only succeed if the new monit.sock was created, otherwise it will fail and raise this error message.

How I did it
I changed the sequence between the operation to reload Monit configuration and the operation to enable monitoring container_checker.

How to verify it
I verified this change on DuT str-a7050-acs-1 by running the command sudo config reload/load_minigraph -y to make sure the error was not raised again.

Previous command output (if the output of a command-line utility has changed)
admin@str-a7050-acs-1:~$ sudo config reload -y
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Resetting failed status on bgp.service
Resetting failed status on caclmgrd.service
Resetting failed status on dhcp_relay.service
Resetting failed status on hostcfgd.service
Resetting failed status on hostname-config.service
Resetting failed status on interfaces-config.service
Resetting failed status on lldp.service
Resetting failed status on ntp-config.service
Resetting failed status on pmon.service
Resetting failed status on procdockerstatsd.service
Resetting failed status on radv.service
Resetting failed status on rsyslog-config.service
Resetting failed status on swss.service
Resetting failed status on syncd.service
Resetting failed status on teamd.service
Resetting failed status on telemetry.timer
Restarting SONiC target ...
Reloading Monit configuration ...
Reinitializing monit daemon
Enabling container monitoring ...
New command output (if the output of a command-line utility has changed)
admin@str-a7050-acs-1:~$ sudo config reload -y
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Resetting failed status on bgp.service
Resetting failed status on caclmgrd.service
Resetting failed status on dhcp_relay.service
Resetting failed status on hostcfgd.service
Resetting failed status on hostname-config.service
Resetting failed status on interfaces-config.service
Resetting failed status on lldp.service
Resetting failed status on ntp-config.service
Resetting failed status on pmon.service
Resetting failed status on procdockerstatsd.service
Resetting failed status on radv.service
Resetting failed status on rsyslog-config.service
Resetting failed status on swss.service
Resetting failed status on syncd.service
Resetting failed status on teamd.service
Resetting failed status on telemetry.timer
Restarting SONiC target ...
Enabling container monitoring ...
Reloading Monit configuration ...
Reinitializing monit daemon
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