diff options
| -rw-r--r-- | cloudinit/config/cc_fan.py | 42 | ||||
| -rw-r--r-- | cloudinit/config/cc_ntp.py | 20 | ||||
| -rw-r--r-- | cloudinit/config/cc_rsyslog.py | 17 | ||||
| -rwxr-xr-x | cloudinit/config/cc_set_passwords.py | 17 | ||||
| -rw-r--r-- | cloudinit/config/tests/test_set_passwords.py | 41 | ||||
| -rwxr-xr-x | cloudinit/distros/__init__.py | 28 | ||||
| -rw-r--r-- | tests/unittests/test_distros/test_manage_service.py | 38 | ||||
| -rw-r--r-- | tests/unittests/test_handler/test_handler_ntp.py | 29 |
8 files changed, 119 insertions, 113 deletions
diff --git a/cloudinit/config/cc_fan.py b/cloudinit/config/cc_fan.py index 77984bca..91f50e22 100644 --- a/cloudinit/config/cc_fan.py +++ b/cloudinit/config/cc_fan.py @@ -52,35 +52,26 @@ BUILTIN_CFG = { } -def stop_update_start(service, config_file, content, systemd=False): - if systemd: - cmds = {'stop': ['systemctl', 'stop', service], - 'start': ['systemctl', 'start', service], - 'enable': ['systemctl', 'enable', service]} - else: - cmds = {'stop': ['service', 'stop'], - 'start': ['service', 'start']} - - def run(cmd, msg): - try: - return subp.subp(cmd, capture=True) - except subp.ProcessExecutionError as e: - LOG.warning("failed: %s (%s): %s", service, cmd, e) - return False - - stop_failed = not run(cmds['stop'], msg='stop %s' % service) +def stop_update_start(distro, service, config_file, content): + try: + distro.manage_service('stop', service) + stop_failed = False + except subp.ProcessExecutionError as e: + stop_failed = True + LOG.warning("failed to stop %s: %s", service, e) + if not content.endswith('\n'): content += '\n' util.write_file(config_file, content, omode="w") - ret = run(cmds['start'], msg='start %s' % service) - if ret and stop_failed: - LOG.warning("success: %s started", service) - - if 'enable' in cmds: - ret = run(cmds['enable'], msg='enable %s' % service) + try: + distro.manage_service('start', service) + if stop_failed: + LOG.warning("success: %s started", service) + except subp.ProcessExecutionError as e: + LOG.warning("failed to start %s: %s", service, e) - return ret + distro.manage_service('enable', service) def handle(name, cfg, cloud, log, args): @@ -99,7 +90,8 @@ def handle(name, cfg, cloud, log, args): distro.install_packages(['ubuntu-fan']) stop_update_start( + distro, service='ubuntu-fan', config_file=mycfg.get('config_path'), - content=mycfg.get('config'), systemd=distro.uses_systemd()) + content=mycfg.get('config')) # vi: ts=4 expandtab diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index f4468c9d..c3aee798 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -473,21 +473,6 @@ def write_ntp_config_template(distro_name, service_name=None, servers=None, util.del_file(template_fn) -def reload_ntp(service, systemd=False): - """Restart or reload an ntp system service. - - @param service: A string specifying the name of the service to be affected. - @param systemd: A boolean indicating if the distro uses systemd, defaults - to False. - @returns: A tuple of stdout, stderr results from executing the action. - """ - if systemd: - cmd = ['systemctl', 'reload-or-restart', service] - else: - cmd = ['service', service, 'restart'] - subp.subp(cmd, capture=True) - - def supplemental_schema_validation(ntp_config): """Validate user-provided ntp:config option values. @@ -596,10 +581,11 @@ def handle(name, cfg, cloud, log, _args): packages=ntp_client_config['packages'], check_exe=ntp_client_config['check_exe']) try: - reload_ntp(ntp_client_config['service_name'], - systemd=cloud.distro.uses_systemd()) + cloud.distro.manage_service('reload', + ntp_client_config.get('service_name')) except subp.ProcessExecutionError as e: LOG.exception("Failed to reload/start ntp service: %s", e) raise + # vi: ts=4 expandtab diff --git a/cloudinit/config/cc_rsyslog.py b/cloudinit/config/cc_rsyslog.py index 2a2bc931..dd2bbd00 100644 --- a/cloudinit/config/cc_rsyslog.py +++ b/cloudinit/config/cc_rsyslog.py @@ -207,16 +207,11 @@ HOST_PORT_RE = re.compile( r'([:](?P<port>[0-9]+))?$') -def reload_syslog(command=DEF_RELOAD, systemd=False): - service = 'rsyslog' +def reload_syslog(distro, command=DEF_RELOAD): if command == DEF_RELOAD: - if systemd: - cmd = ['systemctl', 'reload-or-try-restart', service] - else: - cmd = ['service', service, 'restart'] - else: - cmd = command - subp.subp(cmd, capture=True) + service = distro.get_option('rsyslog_svcname', 'rsyslog') + return distro.manage_service('try-reload', service) + return subp.subp(command, capture=True) def load_config(cfg): @@ -429,9 +424,7 @@ def handle(name, cfg, cloud, log, _args): return try: - restarted = reload_syslog( - command=mycfg[KEYNAME_RELOAD], - systemd=cloud.distro.uses_systemd()), + restarted = reload_syslog(cloud.distro, command=mycfg[KEYNAME_RELOAD]) except subp.ProcessExecutionError as e: restarted = False log.warning("Failed to reload syslog", e) diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py index 803a3aa9..3843aaf7 100755 --- a/cloudinit/config/cc_set_passwords.py +++ b/cloudinit/config/cc_set_passwords.py @@ -94,18 +94,15 @@ PW_SET = (''.join([x for x in ascii_letters + digits if x not in 'loLOI01'])) -def handle_ssh_pwauth(pw_auth, service_cmd=None, service_name="ssh"): +def handle_ssh_pwauth(pw_auth, distro): """Apply sshd PasswordAuthentication changes. @param pw_auth: config setting from 'pw_auth'. Best given as True, False, or "unchanged". - @param service_cmd: The service command list (['service']) - @param service_name: The name of the sshd service for the system. + @param distro: an instance of the distro class for the target distribution @return: None""" cfg_name = "PasswordAuthentication" - if service_cmd is None: - service_cmd = ["service"] if util.is_true(pw_auth): cfg_val = 'yes' @@ -124,11 +121,7 @@ def handle_ssh_pwauth(pw_auth, service_cmd=None, service_name="ssh"): LOG.debug("No need to restart SSH service, %s not updated.", cfg_name) return - if 'systemctl' in service_cmd: - cmd = list(service_cmd) + ["restart", service_name] - else: - cmd = list(service_cmd) + [service_name, "restart"] - subp.subp(cmd) + distro.manage_service('restart', distro.get_option('ssh_svcname', 'ssh')) LOG.debug("Restarted the SSH daemon.") @@ -229,9 +222,7 @@ def handle(_name, cfg, cloud, log, args): if expired_users: log.debug("Expired passwords for: %s users", expired_users) - handle_ssh_pwauth( - cfg.get('ssh_pwauth'), service_cmd=cloud.distro.init_cmd, - service_name=cloud.distro.get_option('ssh_svcname', 'ssh')) + handle_ssh_pwauth(cfg.get('ssh_pwauth'), cloud.distro) if len(errors): log.debug("%s errors occured, re-raising the last one", len(errors)) diff --git a/cloudinit/config/tests/test_set_passwords.py b/cloudinit/config/tests/test_set_passwords.py index bbe2ee8f..79118a12 100644 --- a/cloudinit/config/tests/test_set_passwords.py +++ b/cloudinit/config/tests/test_set_passwords.py @@ -14,57 +14,52 @@ class TestHandleSshPwauth(CiTestCase): with_logs = True - @mock.patch(MODPATH + "subp.subp") + @mock.patch("cloudinit.distros.subp.subp") def test_unknown_value_logs_warning(self, m_subp): - setpass.handle_ssh_pwauth("floo") + cloud = self.tmp_cloud(distro='ubuntu') + setpass.handle_ssh_pwauth("floo", cloud.distro) self.assertIn("Unrecognized value: ssh_pwauth=floo", self.logs.getvalue()) m_subp.assert_not_called() @mock.patch(MODPATH + "update_ssh_config", return_value=True) - @mock.patch(MODPATH + "subp.subp") + @mock.patch("cloudinit.distros.subp.subp") def test_systemctl_as_service_cmd(self, m_subp, m_update_ssh_config): """If systemctl in service cmd: systemctl restart name.""" - setpass.handle_ssh_pwauth( - True, service_cmd=["systemctl"], service_name="myssh") - self.assertEqual(mock.call(["systemctl", "restart", "myssh"]), - m_subp.call_args) - - @mock.patch(MODPATH + "update_ssh_config", return_value=True) - @mock.patch(MODPATH + "subp.subp") - def test_service_as_service_cmd(self, m_subp, m_update_ssh_config): - """If systemctl in service cmd: systemctl restart name.""" - setpass.handle_ssh_pwauth( - True, service_cmd=["service"], service_name="myssh") - self.assertEqual(mock.call(["service", "myssh", "restart"]), - m_subp.call_args) + cloud = self.tmp_cloud(distro='ubuntu') + cloud.distro.init_cmd = ['systemctl'] + setpass.handle_ssh_pwauth(True, cloud.distro) + m_subp.assert_called_with( + ["systemctl", "restart", "ssh"], capture=True) @mock.patch(MODPATH + "update_ssh_config", return_value=False) - @mock.patch(MODPATH + "subp.subp") + @mock.patch("cloudinit.distros.subp.subp") def test_not_restarted_if_not_updated(self, m_subp, m_update_ssh_config): """If config is not updated, then no system restart should be done.""" - setpass.handle_ssh_pwauth(True) + cloud = self.tmp_cloud(distro='ubuntu') + setpass.handle_ssh_pwauth(True, cloud.distro) m_subp.assert_not_called() self.assertIn("No need to restart SSH", self.logs.getvalue()) @mock.patch(MODPATH + "update_ssh_config", return_value=True) - @mock.patch(MODPATH + "subp.subp") + @mock.patch("cloudinit.distros.subp.subp") def test_unchanged_does_nothing(self, m_subp, m_update_ssh_config): """If 'unchanged', then no updates to config and no restart.""" - setpass.handle_ssh_pwauth( - "unchanged", service_cmd=["systemctl"], service_name="myssh") + cloud = self.tmp_cloud(distro='ubuntu') + setpass.handle_ssh_pwauth("unchanged", cloud.distro) m_update_ssh_config.assert_not_called() m_subp.assert_not_called() - @mock.patch(MODPATH + "subp.subp") + @mock.patch("cloudinit.distros.subp.subp") def test_valid_change_values(self, m_subp): """If value is a valid changen value, then update should be called.""" + cloud = self.tmp_cloud(distro='ubuntu') upname = MODPATH + "update_ssh_config" optname = "PasswordAuthentication" for value in util.FALSE_STRINGS + util.TRUE_STRINGS: optval = "yes" if value in util.TRUE_STRINGS else "no" with mock.patch(upname, return_value=False) as m_update: - setpass.handle_ssh_pwauth(value) + setpass.handle_ssh_pwauth(value, cloud.distro) m_update.assert_called_with({optname: optval}) m_subp.assert_not_called() diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 426a2cf4..fe0015c6 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -798,6 +798,34 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta): args.append(message) return args + def manage_service(self, action, service): + """ + Perform the requested action on a service. This handles the common + 'systemctl' and 'service' cases and may be overridden in subclasses + as necessary. + May raise ProcessExecutionError + """ + init_cmd = self.init_cmd + if self.uses_systemd() or 'systemctl' in init_cmd: + init_cmd = ['systemctl'] + cmds = {'stop': ['stop', service], + 'start': ['start', service], + 'enable': ['enable', service], + 'restart': ['restart', service], + 'reload': ['reload-or-restart', service], + 'try-reload': ['reload-or-try-restart', service], + } + else: + cmds = {'stop': [service, 'stop'], + 'start': [service, 'start'], + 'enable': [service, 'start'], + 'restart': [service, 'restart'], + 'reload': [service, 'restart'], + 'try-reload': [service, 'restart'], + } + cmd = list(init_cmd) + list(cmds[action]) + return subp.subp(cmd, capture=True) + def _apply_hostname_transformations_to_url(url: str, transformations: list): """ 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 |