From 44ebb18e87f598cada7d077ffc14693593e7da3d Mon Sep 17 00:00:00 2001 From: Sergii Golovatiuk Date: Thu, 6 Dec 2018 19:20:48 +0100 Subject: [PATCH] Raise exception on systemd failures Currently paunch doesn't check exit codes on systemd operations. This patch catch exceptions so errors are visible in the log. Change-Id: I43c4291caf3c8ec07529ee264a5960d84854d648 --- paunch/tests/test_utils_systemd.py | 20 ++++++++++--------- paunch/utils/systemd.py | 31 +++++++++++++++++++++++------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/paunch/tests/test_utils_systemd.py b/paunch/tests/test_utils_systemd.py index b83599a..4a46b61 100644 --- a/paunch/tests/test_utils_systemd.py +++ b/paunch/tests/test_utils_systemd.py @@ -23,9 +23,9 @@ from paunch.utils import systemd class TestUtilsSystemd(base.TestCase): - @mock.patch('subprocess.call', autospec=True) + @mock.patch('subprocess.check_call', autospec=True) @mock.patch('os.chmod') - def test_service_create(self, mock_chmod, mock_subprocess_call): + def test_service_create(self, mock_chmod, mock_subprocess_check_call): container = 'my_app' service = 'tripleo_' + container cconfig = {'depends_on': ['something'], 'restart': 'unless-stopped', @@ -40,7 +40,7 @@ class TestUtilsSystemd(base.TestCase): self.assertIn('ExecStop=/usr/bin/podman stop -t 15 my_app', unit) mock_chmod.assert_has_calls([mock.call(sysd_unit_f, 420)]) - mock_subprocess_call.assert_has_calls([ + mock_subprocess_check_call.assert_has_calls([ mock.call(['systemctl', 'daemon-reload']), mock.call(['systemctl', 'enable', '--now', service]), ]) @@ -49,8 +49,9 @@ class TestUtilsSystemd(base.TestCase): @mock.patch('os.remove', autospec=True) @mock.patch('os.path.isfile', autospec=True) - @mock.patch('subprocess.call', autospec=True) - def test_service_delete(self, mock_subprocess_call, mock_isfile, mock_rm): + @mock.patch('subprocess.check_call', autospec=True) + def test_service_delete(self, mock_subprocess_check_call, mock_isfile, + mock_rm): mock_isfile.return_value = True container = 'my_app' service = 'tripleo_' + container @@ -62,7 +63,7 @@ class TestUtilsSystemd(base.TestCase): mock.call(tempdir + service + '_healthcheck.service'), mock.call(tempdir + service + '_healthcheck.timer'), ]) - mock_subprocess_call.assert_has_calls([ + mock_subprocess_check_call.assert_has_calls([ mock.call(['systemctl', 'stop', service + '.service']), mock.call(['systemctl', 'disable', service + '.service']), mock.call(['systemctl', 'daemon-reload']), @@ -92,9 +93,10 @@ class TestUtilsSystemd(base.TestCase): '/openstack/healthcheck', unit) mock_chmod.assert_has_calls([mock.call(sysd_unit_f, 420)]) - @mock.patch('subprocess.call', autospec=True) + @mock.patch('subprocess.check_call', autospec=True) @mock.patch('os.chmod') - def test_healthcheck_timer_create(self, mock_chmod, mock_subprocess_call): + def test_healthcheck_timer_create(self, mock_chmod, + mock_subprocess_check_call): container = 'my_app' service = 'tripleo_' + container cconfig = {'check_interval': '15'} @@ -108,7 +110,7 @@ class TestUtilsSystemd(base.TestCase): self.assertIn('OnActiveSec=120', unit) self.assertIn('OnUnitActiveSec=15', unit) mock_chmod.assert_has_calls([mock.call(sysd_unit_f, 420)]) - mock_subprocess_call.assert_has_calls([ + mock_subprocess_check_call.assert_has_calls([ mock.call(['systemctl', 'enable', '--now', healthcheck_timer]), mock.call(['systemctl', 'daemon-reload']), ]) diff --git a/paunch/utils/systemd.py b/paunch/utils/systemd.py index cc30508..f76e7ff 100644 --- a/paunch/utils/systemd.py +++ b/paunch/utils/systemd.py @@ -79,8 +79,12 @@ ExecStop=/usr/bin/podman stop -t %(stop_grace_period)s %(name)s KillMode=process [Install] WantedBy=multi-user.target""" % s_config) - subprocess.call(['systemctl', 'daemon-reload']) - subprocess.call(['systemctl', 'enable', '--now', service]) + try: + subprocess.check_call(['systemctl', 'daemon-reload']) + subprocess.check_call(['systemctl', 'enable', '--now', service]) + except subprocess.CalledProcessError: + log.exception("systemctl failed") + raise def service_delete(container, sysdir=constants.SYSTEMD_DIR, log=None): @@ -106,11 +110,19 @@ def service_delete(container, sysdir=constants.SYSTEMD_DIR, log=None): if os.path.isfile(sysdir + sysd_f): log.debug('Stopping and disabling systemd service for %s' % service) - subprocess.call(['systemctl', 'stop', sysd_f]) - subprocess.call(['systemctl', 'disable', sysd_f]) + try: + subprocess.check_call(['systemctl', 'stop', sysd_f]) + subprocess.check_call(['systemctl', 'disable', sysd_f]) + except subprocess.CalledProcessError: + log.exception("systemctl failed") + raise log.debug('Removing systemd unit file %s' % sysd_f) os.remove(sysdir + sysd_f) - subprocess.call(['systemctl', 'daemon-reload']) + try: + subprocess.check_call(['systemctl', 'daemon-reload']) + except subprocess.CalledProcessError: + log.exception("systemctl failed") + raise else: log.warning('No systemd unit file was found for %s' % sysd_f) @@ -191,5 +203,10 @@ OnActiveSec=120 OnUnitActiveSec=%(interval)s [Install] WantedBy=timers.target""" % s_config) - subprocess.call(['systemctl', 'enable', '--now', healthcheck_timer]) - subprocess.call(['systemctl', 'daemon-reload']) + try: + subprocess.check_call(['systemctl', 'enable', '--now', + healthcheck_timer]) + subprocess.check_call(['systemctl', 'daemon-reload']) + except subprocess.CalledProcessError: + log.exception("systemctl failed") + raise