Leave the details of service management to the distro.

[citrus-it]

I allowed the previous PR at #1014 to become stale. Re-opening as requested.

Proposed Commit Message

Leave the details of service management to the distro.

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.

Checklist:

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

[TheRealFalcon]

@citrus-it , Thanks for the update. Between the first PR being opened and this current one, we added a TestingDistro so we don't need to construct our own stub distros in testing. I updated your new tests to use it. Would you mind applying the patch and pushing it to this branch? After that, I think we should be good to merge.

diff --git a/tests/unittests/test_distros/test_manage_service.py b/tests/unittests/test_distros/test_manage_service.py
index 04b3ec6d2..47e7cfb03 100644
--- a/tests/unittests/test_distros/test_manage_service.py
+++ b/tests/unittests/test_distros/test_manage_service.py
@@ -1,45 +1,7 @@
 # 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()
+from tests.unittests.util import TestingDistro
 
 
 class TestManageService(CiTestCase):
@@ -48,30 +10,27 @@ class TestManageService(CiTestCase):
 
     def setUp(self):
         super(TestManageService, self).setUp()
-        self.dist = MyBaseDistro()
+        self.dist = TestingDistro()
 
-    @mock.patch("cloudinit.distros.uses_systemd")
+    @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']
-        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.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']
-        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.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']
-        m_sysd.return_value = True
         self.dist.manage_service('start', 'myssh')
         m_subp.assert_called_with(['systemctl', 'start', 'myssh'],
                                   capture=True)

[TheRealFalcon]

Thanks!

[citrus-it]

Thanks for your help.