Merge lp:~adam-collard/charm-helpers/sysv-service-pause into lp:charm-helpers

[Adam Collard]

Adds support for SysV init scripts to service_pause() and service_resume().

Detection of Upstart vs. SysV init matches that in contrib/charmsupport/nrpe.py. SystemD is left as an exercise for the future, but we'll at least note its absence by raising an exception when we fail to identify either Upstart or SysV.

[Alberto Donato (ack)]

+1, nice

review: Approve

[Chris Glass (tribaal)]

Looks great! Thanks! +1

review: Approve

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2015-08-18 21:06:39 +0000
+++ charmhelpers/core/host.py	2015-09-08 09:56:04 +0000
@@ -63,32 +63,48 @@
     return service_result
 
 
-def service_pause(service_name, init_dir=None):
+def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d"):
     """Pause a system service.
 
     Stop it, and prevent it from starting again at boot."""
-    if init_dir is None:
-        init_dir = "/etc/init"
     stopped = service_stop(service_name)
-    # XXX: Support systemd too
-    override_path = os.path.join(
-        init_dir, '{}.override'.format(service_name))
-    with open(override_path, 'w') as fh:
-        fh.write("manual\n")
+    upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
+    sysv_file = os.path.join(initd_dir, service_name)
+    if os.path.exists(upstart_file):
+        override_path = os.path.join(
+            init_dir, '{}.override'.format(service_name))
+        with open(override_path, 'w') as fh:
+            fh.write("manual\n")
+    elif os.path.exists(sysv_file):
+        subprocess.check_call(["update-rc.d", service_name, "disable"])
+    else:
+        # XXX: Support SystemD too
+        raise ValueError(
+            "Unable to detect {0} as either Upstart {1} or SysV {2}".format(
+                service_name, upstart_file, sysv_file))
     return stopped
 
 
-def service_resume(service_name, init_dir=None):
+def service_resume(service_name, init_dir="/etc/init",
+                   initd_dir="/etc/init.d"):
     """Resume a system service.
 
     Reenable starting again at boot. Start the service"""
-    # XXX: Support systemd too
-    if init_dir is None:
-        init_dir = "/etc/init"
-    override_path = os.path.join(
-        init_dir, '{}.override'.format(service_name))
-    if os.path.exists(override_path):
-        os.unlink(override_path)
+    upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
+    sysv_file = os.path.join(initd_dir, service_name)
+    if os.path.exists(upstart_file):
+        override_path = os.path.join(
+            init_dir, '{}.override'.format(service_name))
+        if os.path.exists(override_path):
+            os.unlink(override_path)
+    elif os.path.exists(sysv_file):
+        subprocess.check_call(["update-rc.d", service_name, "enable"])
+    else:
+        # XXX: Support SystemD too
+        raise ValueError(
+            "Unable to detect {0} as either Upstart {1} or SysV {2}".format(
+                service_name, upstart_file, sysv_file))
+
     started = service_start(service_name)
     return started
 

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2015-08-18 20:52:11 +0000
+++ tests/core/test_host.py	2015-09-08 09:56:04 +0000
@@ -108,10 +108,14 @@
         service.assert_called_with('restart', service_name)
 
     @patch.object(host, 'service')
-    def test_pauses_a_service(self, service):
+    def test_pauses_an_upstart_service(self, service):
         service_name = 'foo-service'
         service.side_effect = [True]
-        tempdir = mkdtemp(prefix="test_pauses_a_service")
+        tempdir = mkdtemp(prefix="test_pauses_an_upstart_service")
+        conf_path = os.path.join(tempdir, "{}.conf".format(service_name))
+        # Just needs to exist
+        with open(conf_path, "w") as fh:
+            fh.write("")
         self.addCleanup(rmtree, tempdir)
         self.assertTrue(host.service_pause(service_name, init_dir=tempdir))
 
@@ -122,11 +126,44 @@
             override_contents = fh.read()
         self.assertEqual("manual\n", override_contents)
 
-    @patch.object(host, 'service')
-    def test_resumes_a_service(self, service):
-        service_name = 'foo-service'
-        service.side_effect = [True]
-        tempdir = mkdtemp(prefix="test_resumes_a_service")
+    @patch('subprocess.check_call')
+    @patch.object(host, 'service')
+    def test_pauses_a_sysv_service(self, service, check_call):
+        service_name = 'foo-service'
+        service.side_effect = [True]
+        tempdir = mkdtemp(prefix="test_pauses_a_sysv_service")
+        sysv_path = os.path.join(tempdir, service_name)
+        # Just needs to exist
+        with open(sysv_path, "w") as fh:
+            fh.write("")
+        self.addCleanup(rmtree, tempdir)
+        self.assertTrue(host.service_pause(
+            service_name, init_dir=tempdir, initd_dir=tempdir))
+
+        service.assert_called_with('stop', service_name)
+        check_call.assert_called_with(["update-rc.d", service_name, "disable"])
+
+    @patch.object(host, 'service')
+    def test_pause_with_unknown_service(self, service):
+        service_name = 'foo-service'
+        service.side_effect = [True]
+        tempdir = mkdtemp(prefix="test_pauses_with_unknown_service")
+        self.addCleanup(rmtree, tempdir)
+        exception = self.assertRaises(
+            ValueError, host.service_pause,
+            service_name, init_dir=tempdir, initd_dir=tempdir)
+        self.assertIn(
+            "Unable to detect {0}".format(service_name), str(exception))
+        self.assertIn(tempdir, str(exception))
+
+    @patch.object(host, 'service')
+    def test_resumes_an_upstart_service(self, service):
+        service_name = 'foo-service'
+        service.side_effect = [True]
+        tempdir = mkdtemp(prefix="test_resumes_an_upstart_service")
+        conf_path = os.path.join(tempdir, "{}.conf".format(service_name))
+        with open(conf_path, "w") as fh:
+            fh.write("")
         self.addCleanup(rmtree, tempdir)
         self.assertTrue(host.service_resume(service_name, init_dir=tempdir))
 
@@ -135,6 +172,36 @@
             tempdir, "{}.override".format(service_name))
         self.assertFalse(os.path.exists(override_path))
 
+    @patch('subprocess.check_call')
+    @patch.object(host, 'service')
+    def test_resumes_a_sysv_service(self, service, check_call):
+        service_name = 'foo-service'
+        service.side_effect = [True]
+        tempdir = mkdtemp(prefix="test_resumes_a_sysv_service")
+        sysv_path = os.path.join(tempdir, service_name)
+        # Just needs to exist
+        with open(sysv_path, "w") as fh:
+            fh.write("")
+        self.addCleanup(rmtree, tempdir)
+        self.assertTrue(host.service_resume(
+            service_name, init_dir=tempdir, initd_dir=tempdir))
+
+        service.assert_called_with('start', service_name)
+        check_call.assert_called_with(["update-rc.d", service_name, "enable"])
+
+    @patch.object(host, 'service')
+    def test_resume_with_unknown_service(self, service):
+        service_name = 'foo-service'
+        service.side_effect = [True]
+        tempdir = mkdtemp(prefix="test_resumes_with_unknown_service")
+        self.addCleanup(rmtree, tempdir)
+        exception = self.assertRaises(
+            ValueError, host.service_resume,
+            service_name, init_dir=tempdir, initd_dir=tempdir)
+        self.assertIn(
+            "Unable to detect {0}".format(service_name), str(exception))
+        self.assertIn(tempdir, str(exception))
+
     @patch.object(host, 'service')
     def test_reloads_a_service(self, service):
         service_name = 'foo-service'