diff --git a/cloudinit/config/cc_fan.py b/cloudinit/config/cc_fan.py index 77984bca0e6..6e3ac80b104 100644 --- a/cloudinit/config/cc_fan.py +++ b/cloudinit/config/cc_fan.py @@ -52,33 +52,17 @@ } -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): + stop_failed = not distro.manage_service('stop', service) if not content.endswith('\n'): content += '\n' util.write_file(config_file, content, omode="w") - ret = run(cmds['start'], msg='start %s' % service) + ret = distro.manage_service('start', service) if ret and stop_failed: LOG.warning("success: %s started", service) - if 'enable' in cmds: - ret = run(cmds['enable'], msg='enable %s' % service) + ret = distro.manage_service('enable', service) return ret @@ -99,7 +83,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 e2231cbbf48..4f7f18ca6c5 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. @@ -595,11 +580,8 @@ def handle(name, cfg, cloud, log, _args): install_ntp_client(cloud.distro.install_packages, 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()) - except subp.ProcessExecutionError as e: - LOG.exception("Failed to reload/start ntp service: %s", e) - raise + cloud.distro.manage_service('reload', + ntp_client_config.get('service_name')) + # vi: ts=4 expandtab diff --git a/cloudinit/config/cc_rsyslog.py b/cloudinit/config/cc_rsyslog.py index 2a2bc931e72..c0805b25369 100644 --- a/cloudinit/config/cc_rsyslog.py +++ b/cloudinit/config/cc_rsyslog.py @@ -207,16 +207,13 @@ r'([:](?P[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'] + service = distro.get_option('rsyslog_svcname', 'rsyslog') + return distro.manage_service('try-reload', service) else: cmd = command - subp.subp(cmd, capture=True) + return subp.subp(cmd, capture=True) def load_config(cfg): @@ -429,9 +426,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 433de751fa1..9efbf61f6c0 100755 --- a/cloudinit/config/cc_set_passwords.py +++ b/cloudinit/config/cc_set_passwords.py @@ -94,18 +94,15 @@ 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 bbe2ee8faad..79118a12398 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 a0526948e1f..fff21c3792c 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -795,6 +795,32 @@ def shutdown_command(self, *, mode, delay, message): args.append(message) return args + def manage_service(self, action, service): + 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]) + try: + return subp.subp(cmd, capture=True) + except subp.ProcessExecutionError as e: + LOG.warning("failed: %s (%s): %s", service, cmd, e) + return False + 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 00000000000..67033077205 --- /dev/null +++ b/tests/unittests/test_distros/test_manage_service.py @@ -0,0 +1,87 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +from cloudinit import distros +from cloudinit.tests.helpers import (CiTestCase, mock) + + +class MyBaseDistro(distros.Distro): + # MyBaseDistro is here to test base Distro class implementations + + def __init__(self, name="basedistro", cfg=None, paths=None): + if not cfg: + cfg = {} + if not paths: + paths = {} + super(MyBaseDistro, self).__init__(name, cfg, paths) + + def install_packages(self, pkglist): + raise NotImplementedError() + + def _write_network(self, settings): + raise NotImplementedError() + + def package_command(self, command, args=None, pkgs=None): + raise NotImplementedError() + + def update_package_sources(self): + raise NotImplementedError() + + def apply_locale(self, locale, out_fn=None): + raise NotImplementedError() + + def set_timezone(self, tz): + raise NotImplementedError() + + def _read_hostname(self, filename, default=None): + raise NotImplementedError() + + def _write_hostname(self, hostname, filename): + raise NotImplementedError() + + def _read_system_hostname(self): + raise NotImplementedError() + + +class TestManageService(CiTestCase): + + with_logs = True + + def setUp(self): + super(TestManageService, self).setUp() + self.dist = MyBaseDistro() + + @mock.patch("cloudinit.distros.uses_systemd") + @mock.patch("cloudinit.distros.subp.subp") + def test_manage_service_systemctl_initcmd(self, m_subp, m_sysd): + self.dist.init_cmd = ['systemctl'] + m_sysd.return_value = False + self.dist.manage_service('start', 'myssh') + m_subp.assert_called_with(['systemctl', 'start', 'myssh'], + capture=True) + + @mock.patch("cloudinit.distros.uses_systemd") + @mock.patch("cloudinit.distros.subp.subp") + def test_manage_service_service_initcmd(self, m_subp, m_sysd): + self.dist.init_cmd = ['service'] + m_sysd.return_value = False + self.dist.manage_service('start', 'myssh') + m_subp.assert_called_with(['service', 'myssh', 'start'], capture=True) + + @mock.patch("cloudinit.distros.uses_systemd") + @mock.patch("cloudinit.distros.subp.subp") + def test_manage_service_service(self, m_subp, m_sysd): + self.dist.init_cmd = ['service'] + m_sysd.return_value = False + self.dist.manage_service('start', 'myssh') + m_subp.assert_called_with(['service', 'myssh', 'start'], capture=True) + + @mock.patch("cloudinit.distros.uses_systemd") + @mock.patch("cloudinit.distros.subp.subp") + def test_manage_service_systemctl(self, m_subp, m_sysd): + self.dist.init_cmd = ['ignore'] + m_sysd.return_value = True + 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 6b9c8377bf0..c059e2e615a 100644 --- a/tests/unittests/test_handler/test_handler_ntp.py +++ b/tests/unittests/test_handler/test_handler_ntp.py @@ -112,22 +112,6 @@ def test_ntp_install_no_op_with_empty_pkg_list(self, mock_subp): 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) @@ -488,10 +472,11 @@ def test_ntp_handler_enabled_false(self, m_select): 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: @@ -509,7 +494,7 @@ def test_ntp_the_whole_package(self, m_sysd, m_select, m_subp): 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. @@ -525,7 +510,7 @@ def test_ntp_the_whole_package(self, m_sysd, m_select, m_subp): 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)) @@ -673,9 +658,8 @@ def test_ntp_user_config_overrides_system_cfg(self, m_which): 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') @@ -702,11 +686,10 @@ def test_ntp_user_provided_config_with_template(self, m_install, m_reload): 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