From 8c89009e75c7cf6c2f87635b82656f07f58095e1 Mon Sep 17 00:00:00 2001 From: Andy Fiddaman Date: Wed, 20 Oct 2021 20:58:27 +0000 Subject: Leave the details of service management to the distro (#1074) Various modules restart services and they all have logic to try and detect if they are running on a system that needs 'systemctl' or 'service', and then have code to decide which order the arguments need to be etc. On top of that, not all modules do this in the same way. The duplication and different approaches are not ideal but this also makes it hard to add support for a new distribution that does not use either 'systemctl' or 'service'. This change adds a new manage_service() method to the distro class and updates several modules to use it. --- .../unittests/test_distros/test_manage_service.py | 38 ++++++++++++++++++++++ tests/unittests/test_handler/test_handler_ntp.py | 29 ++++------------- 2 files changed, 44 insertions(+), 23 deletions(-) create mode 100644 tests/unittests/test_distros/test_manage_service.py (limited to 'tests') diff --git a/tests/unittests/test_distros/test_manage_service.py b/tests/unittests/test_distros/test_manage_service.py new file mode 100644 index 00000000..47e7cfb0 --- /dev/null +++ b/tests/unittests/test_distros/test_manage_service.py @@ -0,0 +1,38 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +from cloudinit.tests.helpers import (CiTestCase, mock) +from tests.unittests.util import TestingDistro + + +class TestManageService(CiTestCase): + + with_logs = True + + def setUp(self): + super(TestManageService, self).setUp() + self.dist = TestingDistro() + + @mock.patch.object(TestingDistro, 'uses_systemd', return_value=False) + @mock.patch("cloudinit.distros.subp.subp") + def test_manage_service_systemctl_initcmd(self, m_subp, m_sysd): + self.dist.init_cmd = ['systemctl'] + self.dist.manage_service('start', 'myssh') + m_subp.assert_called_with(['systemctl', 'start', 'myssh'], + capture=True) + + @mock.patch.object(TestingDistro, 'uses_systemd', return_value=False) + @mock.patch("cloudinit.distros.subp.subp") + def test_manage_service_service_initcmd(self, m_subp, m_sysd): + self.dist.init_cmd = ['service'] + self.dist.manage_service('start', 'myssh') + m_subp.assert_called_with(['service', 'myssh', 'start'], capture=True) + + @mock.patch.object(TestingDistro, 'uses_systemd', return_value=True) + @mock.patch("cloudinit.distros.subp.subp") + def test_manage_service_systemctl(self, m_subp, m_sysd): + self.dist.init_cmd = ['ignore'] + self.dist.manage_service('start', 'myssh') + m_subp.assert_called_with(['systemctl', 'start', 'myssh'], + capture=True) + +# vi: ts=4 sw=4 expandtab diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py index 5dfa564f..b9e2ba57 100644 --- a/tests/unittests/test_handler/test_handler_ntp.py +++ b/tests/unittests/test_handler/test_handler_ntp.py @@ -107,22 +107,6 @@ class TestNtp(FilesystemMockingTestCase): check_exe='timesyncd') install_func.assert_called_once_with([]) - @mock.patch("cloudinit.config.cc_ntp.subp") - def test_reload_ntp_defaults(self, mock_subp): - """Test service is restarted/reloaded (defaults)""" - service = 'ntp_service_name' - cmd = ['service', service, 'restart'] - cc_ntp.reload_ntp(service) - mock_subp.subp.assert_called_with(cmd, capture=True) - - @mock.patch("cloudinit.config.cc_ntp.subp") - def test_reload_ntp_systemd(self, mock_subp): - """Test service is restarted/reloaded (systemd)""" - service = 'ntp_service_name' - cc_ntp.reload_ntp(service, systemd=True) - cmd = ['systemctl', 'reload-or-restart', service] - mock_subp.subp.assert_called_with(cmd, capture=True) - def test_ntp_rename_ntp_conf(self): """When NTP_CONF exists, rename_ntp moves it.""" ntpconf = self.tmp_path("ntp.conf", self.new_root) @@ -483,10 +467,11 @@ class TestNtp(FilesystemMockingTestCase): cc_ntp.handle('notimportant', cfg, mycloud, None, None) self.assertEqual(0, m_select.call_count) + @mock.patch("cloudinit.distros.subp") @mock.patch("cloudinit.config.cc_ntp.subp") @mock.patch('cloudinit.config.cc_ntp.select_ntp_client') @mock.patch("cloudinit.distros.Distro.uses_systemd") - def test_ntp_the_whole_package(self, m_sysd, m_select, m_subp): + def test_ntp_the_whole_package(self, m_sysd, m_select, m_subp, m_dsubp): """Test enabled config renders template, and restarts service """ cfg = {'ntp': {'enabled': True}} for distro in cc_ntp.distros: @@ -504,7 +489,7 @@ class TestNtp(FilesystemMockingTestCase): if distro == 'alpine': uses_systemd = False - expected_service_call = ['service', service_name, 'restart'] + expected_service_call = ['rc-service', service_name, 'restart'] # _mock_ntp_client_config call above did not specify a client # value and so it defaults to "ntp" which on Alpine Linux only # supports servers and not pools. @@ -520,7 +505,7 @@ class TestNtp(FilesystemMockingTestCase): m_util.is_false.return_value = util.is_false( cfg['ntp']['enabled']) cc_ntp.handle('notimportant', cfg, mycloud, None, None) - m_subp.subp.assert_called_with( + m_dsubp.subp.assert_called_with( expected_service_call, capture=True) self.assertEqual(expected_content, util.load_file(confpath)) @@ -668,9 +653,8 @@ class TestNtp(FilesystemMockingTestCase): self.assertEqual(sorted(expected_cfg), sorted(result)) m_which.assert_has_calls([]) - @mock.patch('cloudinit.config.cc_ntp.reload_ntp') @mock.patch('cloudinit.config.cc_ntp.install_ntp_client') - def test_ntp_user_provided_config_with_template(self, m_install, m_reload): + def test_ntp_user_provided_config_with_template(self, m_install): custom = r'\n#MyCustomTemplate' user_template = NTP_TEMPLATE + custom confpath = os.path.join(self.new_root, 'etc/myntp/myntp.conf') @@ -697,11 +681,10 @@ class TestNtp(FilesystemMockingTestCase): util.load_file(confpath)) @mock.patch('cloudinit.config.cc_ntp.supplemental_schema_validation') - @mock.patch('cloudinit.config.cc_ntp.reload_ntp') @mock.patch('cloudinit.config.cc_ntp.install_ntp_client') @mock.patch('cloudinit.config.cc_ntp.select_ntp_client') def test_ntp_user_provided_config_template_only(self, m_select, m_install, - m_reload, m_schema): + m_schema): """Test custom template for default client""" custom = r'\n#MyCustomTemplate' user_template = NTP_TEMPLATE + custom -- cgit v1.2.3