Skip to content

Fix hostcfgd does not react to SIGINT issue. #10977

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix hostcfgd does not react to SIGINT issue. #10977

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4851e4
Initialize swsscommon signal helper
May 27, 2022
9f64bec
Improve code
Jun 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions src/sonic-host-services/scripts/hostcfgd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import signal

import jinja2
from sonic_py_common import device_info
from swsscommon.swsscommon import ConfigDBConnector, DBConnector, Table
from swsscommon.swsscommon import ConfigDBConnector, DBConnector, Table, SignalHandlerHelper, SIGNAL_INT, SIGNAL_TERM

# FILE
PAM_AUTH_CONF = "/etc/pam.d/common-auth-sonic"
Expand Down Expand Up @@ -61,12 +61,6 @@ def safe_eval(val, default_value=False):
def signal_handler(sig, frame):
if sig == signal.SIGHUP:
syslog.syslog(syslog.LOG_INFO, "HostCfgd: signal 'SIGHUP' is caught and ignoring..")
elif sig == signal.SIGINT:
syslog.syslog(syslog.LOG_INFO, "HostCfgd: signal 'SIGINT' is caught and exiting...")
sys.exit(128 + sig)
elif sig == signal.SIGTERM:
syslog.syslog(syslog.LOG_INFO, "HostCfgd: signal 'SIGTERM' is caught and exiting...")
sys.exit(128 + sig)
else:
syslog.syslog(syslog.LOG_INFO, "HostCfgd: invalid signal - ignoring..")

Expand Down Expand Up @@ -1242,9 +1236,13 @@ class HostConfigDaemon:


def main():
signal.signal(signal.SIGTERM, signal_handler)
signal.signal(signal.SIGINT, signal_handler)
signal.signal(signal.SIGHUP, signal_handler)

# Initialize swsscommon signal helper, because python signal handler will be blocked by long running c++ code.
# For more information please check: https://docs.python.org/3/library/signal.html
SignalHandlerHelper.registerSignalHandler(SIGNAL_INT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this overrides signal.signal(signal.SIGINT, signal_handler), we should probably remove signal.SIGINT processing from hostcfgd.

Copy link
Contributor

@akokhan akokhan May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Also, on reboot, by default systemd sends SIGTERM but not SIGINT. So, this will not handle delay on reboot. In addition to this, we should probably extend hostcfgd.service with KillSignal=SIGINT or ExecStop= that pkill hostcfgd with SIGTERM and then with SIGINT.

Copy link
Contributor

Choose a reason for hiding this comment

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

either

[Service]
Type=simple
ExecStart=/usr/local/bin/hostcfgd
KillSignal=SIGINT

or

[Service]
Type=simple
ExecStart=/usr/local/bin/hostcfgd
ExecStop=/bin/bash -c '/usr/bin/killall -15 hostcfgd && /usr/bin/killall -2 hostcfgd'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, currently swsscommon only handle SIGINT, so will add SIGTERM support with this PR: sonic-net/sonic-swss-common#626

I will update this PR after swsscommon updated.

SignalHandlerHelper.registerSignalHandler(SIGNAL_TERM)

daemon = HostConfigDaemon()
daemon.register_callbacks()
daemon.start()
Expand Down