From 2dbcd06dc8f0316dee44a113199aed5bcc885ebe Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Thu, 5 Dec 2024 17:19:48 -0600 Subject: [PATCH] fix: uninstall custom signal handlers before shutdown --- .../cc_package_update_upgrade_install.py | 10 ++- cloudinit/config/cc_power_state_change.py | 5 +- cloudinit/signal_handler.py | 63 ++++++++++++++++--- .../test_cc_package_update_upgrade_install.py | 2 +- 4 files changed, 68 insertions(+), 12 deletions(-) diff --git a/cloudinit/config/cc_package_update_upgrade_install.py b/cloudinit/config/cc_package_update_upgrade_install.py index 3aa94e0a917..7009b852e80 100644 --- a/cloudinit/config/cc_package_update_upgrade_install.py +++ b/cloudinit/config/cc_package_update_upgrade_install.py @@ -10,7 +10,7 @@ import os import time -from cloudinit import subp, util +from cloudinit import signal_handler, subp, util from cloudinit.cloud import Cloud from cloudinit.config import Config from cloudinit.config.schema import MetaSchema @@ -48,6 +48,9 @@ def _fire_reboot( wait_attempts: int = 6, initial_sleep: int = 1, backoff: int = 2 ): """Run a reboot command and panic if it doesn't happen fast enough.""" + # systemd will kill cloud-init with a signal + # this is expected so don't behave as if this is a failure state + signal_handler.detach_handlers() subp.subp(REBOOT_CMD) start = time.monotonic() wait_time = initial_sleep @@ -106,8 +109,9 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None: break if (upgrade or pkglist) and reboot_if_required and reboot_fn_exists: try: - LOG.warning( - "Rebooting after upgrade or install per %s", reboot_marker + LOG.info( + "***WARNING*** Rebooting after upgrade or install per %s", + reboot_marker, ) # Flush the above warning + anything else out... flush_loggers(LOG) diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py index 1e3dda1d666..116c5dc157b 100644 --- a/cloudinit/config/cc_power_state_change.py +++ b/cloudinit/config/cc_power_state_change.py @@ -13,7 +13,7 @@ import subprocess import time -from cloudinit import subp, util +from cloudinit import signal_handler, subp, util from cloudinit.cloud import Cloud from cloudinit.config import Config from cloudinit.config.schema import MetaSchema @@ -217,4 +217,7 @@ def fatal(msg): except Exception as e: fatal("Unexpected Exception when checking condition: %s" % e) + # systemd could kill this process with a signal before it exits + # this is expected, so remove the signal handlers + signal_handler.detach_handlers() func(*args) diff --git a/cloudinit/signal_handler.py b/cloudinit/signal_handler.py index c5e490bb6b0..07d2d159425 100644 --- a/cloudinit/signal_handler.py +++ b/cloudinit/signal_handler.py @@ -10,6 +10,7 @@ import signal import sys from io import StringIO +from typing import Callable, Dict, NamedTuple, Union from cloudinit import version as vr from cloudinit.log import log_util @@ -17,13 +18,51 @@ LOG = logging.getLogger(__name__) +class SignalType(NamedTuple): + message: str + exit_code: int + default_signal: Union[Callable, int, None] + + +def default_handler(_num, _stack) -> None: + """an empty handler""" + return None + + +def get_handler(sig: Union[int, Callable, None]) -> Callable: + """get_handler gets a callable from signal.signal() output.""" + if callable(sig): + return sig + elif sig is None: + LOG.warning("Signal handler was not installed from Python!") + elif sig == signal.SIG_DFL: + LOG.warning("Signal was in unexpected state: SIG_DFL") + elif sig == signal.SIG_IGN: + LOG.warning("Signal was in unexpected state: SIG_IGN") + else: + LOG.warning( + "Process signal is in an unknown state: %s(%s)", type(sig), sig + ) + return default_handler + + BACK_FRAME_TRACE_DEPTH = 3 -EXIT_FOR = { - signal.SIGINT: ("Cloud-init %(version)s received SIGINT, exiting...", 1), - signal.SIGTERM: ("Cloud-init %(version)s received SIGTERM, exiting...", 1), - # Can't be caught... - # signal.SIGKILL: ('Cloud-init killed, exiting...', 1), - signal.SIGABRT: ("Cloud-init %(version)s received SIGABRT, exiting...", 1), +EXIT_FOR: Dict[int, SignalType] = { + signal.SIGINT: SignalType( + "Cloud-init %(version)s received SIGINT, exiting...", + 1, + signal.getsignal(signal.SIGINT), + ), + signal.SIGTERM: SignalType( + "Cloud-init %(version)s received SIGTERM, exiting...", + 1, + signal.getsignal(signal.SIGTERM), + ), + signal.SIGABRT: SignalType( + "Cloud-init %(version)s received SIGABRT, exiting...", + 1, + signal.getsignal(signal.SIGABRT), + ), } @@ -39,7 +78,7 @@ def _pprint_frame(frame, depth, max_depth, contents): def _handle_exit(signum, frame): - (msg, rc) = EXIT_FOR[signum] + msg, rc, _ = EXIT_FOR[signum] msg = msg % ({"version": vr.version_string()}) contents = StringIO() contents.write("%s\n" % (msg)) @@ -49,8 +88,18 @@ def _handle_exit(signum, frame): def attach_handlers(): + """attach cloud-init's handlers""" sigs_attached = 0 for signum in EXIT_FOR.keys(): signal.signal(signum, _handle_exit) sigs_attached += len(EXIT_FOR) return sigs_attached + + +def detach_handlers(): + """dettach cloud-init's handlers""" + sigs_attached = 0 + for number, sig_type in EXIT_FOR.items(): + signal.signal(number, get_handler(sig_type.default_signal)) + sigs_attached += len(EXIT_FOR) + return sigs_attached diff --git a/tests/unittests/config/test_cc_package_update_upgrade_install.py b/tests/unittests/config/test_cc_package_update_upgrade_install.py index 1cc82d71fa7..64f96d6705d 100644 --- a/tests/unittests/config/test_cc_package_update_upgrade_install.py +++ b/tests/unittests/config/test_cc_package_update_upgrade_install.py @@ -121,7 +121,7 @@ def _isfile(filename: str): # subp and not really rebooting the system subp_call = ["/sbin/reboot"] - caplog.set_level(logging.WARNING) + caplog.set_level(logging.INFO) with mock.patch( "cloudinit.subp.subp", return_value=SubpResult("{}", "fakeerr") ) as m_subp: