From 0c027a305063cee2fb4f0b1475a3df1928afc0f9 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 3 Apr 2022 13:12:48 -0700 Subject: [PATCH] Fix handling of TERM signal to exit borgmatic, not just forward the signal to Borg (#516). --- NEWS | 1 + borgmatic/signals.py | 19 ++++++++++++++---- tests/unit/test_signals.py | 40 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 tests/unit/test_signals.py diff --git a/NEWS b/NEWS index 55dedbd..8ffa41d 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ 1.5.25.dev0 + * #516: Fix handling of TERM signal to exit borgmatic, not just forward the signal to Borg. * #517: Fix borgmatic exit code (so it's zero) when initial Borg calls fail but later retries succeed. diff --git a/borgmatic/signals.py b/borgmatic/signals.py index a8b4299..3811f8c 100644 --- a/borgmatic/signals.py +++ b/borgmatic/signals.py @@ -1,23 +1,34 @@ +import logging import os import signal +import sys + +logger = logging.getLogger(__name__) -def _handle_signal(signal_number, frame): # pragma: no cover +EXIT_CODE_FROM_SIGNAL = 128 + + +def handle_signal(signal_number, frame): ''' Send the signal to all processes in borgmatic's process group, which includes child processes. ''' # Prevent infinite signal handler recursion. If the parent frame is this very same handler # function, we know we're recursing. - if frame.f_back.f_code.co_name == _handle_signal.__name__: + if frame.f_back.f_code.co_name == handle_signal.__name__: return os.killpg(os.getpgrp(), signal_number) + if signal_number == signal.SIGTERM: + logger.critical('Exiting due to TERM signal') + sys.exit(EXIT_CODE_FROM_SIGNAL + signal.SIGTERM) -def configure_signals(): # pragma: no cover + +def configure_signals(): ''' Configure borgmatic's signal handlers to pass relevant signals through to any child processes like Borg. Note that SIGINT gets passed through even without these changes. ''' for signal_number in (signal.SIGHUP, signal.SIGTERM, signal.SIGUSR1, signal.SIGUSR2): - signal.signal(signal_number, _handle_signal) + signal.signal(signal_number, handle_signal) diff --git a/tests/unit/test_signals.py b/tests/unit/test_signals.py new file mode 100644 index 0000000..83be32c --- /dev/null +++ b/tests/unit/test_signals.py @@ -0,0 +1,40 @@ +from flexmock import flexmock + +from borgmatic import signals as module + + +def test_handle_signal_forwards_to_subprocesses(): + signal_number = 100 + frame = flexmock(f_back=flexmock(f_code=flexmock(co_name='something'))) + process_group = flexmock() + flexmock(module.os).should_receive('getpgrp').and_return(process_group) + flexmock(module.os).should_receive('killpg').with_args(process_group, signal_number).once() + + module.handle_signal(signal_number, frame) + + +def test_handle_signal_bails_on_recursion(): + signal_number = 100 + frame = flexmock(f_back=flexmock(f_code=flexmock(co_name='handle_signal'))) + flexmock(module.os).should_receive('getpgrp').never() + flexmock(module.os).should_receive('killpg').never() + + module.handle_signal(signal_number, frame) + + +def test_handle_signal_exits_on_sigterm(): + signal_number = module.signal.SIGTERM + frame = flexmock(f_back=flexmock(f_code=flexmock(co_name='something'))) + flexmock(module.os).should_receive('getpgrp').and_return(flexmock) + flexmock(module.os).should_receive('killpg') + flexmock(module.sys).should_receive('exit').with_args( + module.EXIT_CODE_FROM_SIGNAL + signal_number + ).once() + + module.handle_signal(signal_number, frame) + + +def test_configure_signals_installs_signal_handlers(): + flexmock(module.signal).should_receive('signal').at_least().once() + + module.configure_signals()