Leave the details of service management to the distro.

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

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

cloudinit/config/cc_rsyslog.py

@@ -207,16 +207,13 @@
     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']
+        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)

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))

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()
 

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):
     """

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