Skip service_enable if already enabled (#886)

Repeatedly calling service_enable for a service that
is already enabled can lead to unintended consequences.
Some charms frequently call servie_resume which will
call service('enable') and this adds a check to only
do so of the service is not enabled.

Related-Bug: #2058505
(cherry picked from commit a334500)

Co-authored-by: Edward Hope-Morley <[email protected]>

charmhelpers/core/host.py

@@ -256,8 +256,11 @@ def service_resume(service_name, init_dir="/etc/init",
     upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
     sysv_file = os.path.join(initd_dir, service_name)
     if init_is_systemd(service_name=service_name):
-        service('unmask', service_name)
-        service('enable', service_name)
+        if service('is-enabled', service_name):
+            log('service {} already enabled'.format(service_name), level=DEBUG)
+        else:
+            service('unmask', service_name)
+            service('enable', service_name)
     elif os.path.exists(upstart_file):
         override_path = os.path.join(
             init_dir, '{}.override'.format(service_name))

tests/core/test_host.py

@@ -238,8 +238,18 @@ def test_resumes_a_stopped_systemd_unit(self, service, systemd,
         service_name = 'foo-service'
         service_running.return_value = False
         systemd.return_value = True
+
+        def fake_service(action, name):
+            if action == 'is-enabled':
+                return False
+
+            if action == 'start':
+                return True
+
+        service.side_effect = fake_service
         self.assertTrue(host.service_resume(service_name))
         service.assert_has_calls([
+            call('is-enabled', service_name),
             call('unmask', service_name),
             # Ensures a package starts up if disabled but not masked,
             # per lp:1692178
@@ -455,6 +465,23 @@ def test_resume_a_running_sysv_service(self, service, check_call,
         self.assertFalse(service.called)
         check_call.assert_called_with(["update-rc.d", service_name, "enable"])
 
+    @patch.object(host, 'service_running')
+    @patch.object(host, 'init_is_systemd')
+    @patch.object(host, 'service')
+    def test_resume_an_enabled_systemd_service(self, service, systemd,
+                                               service_running):
+        service_name = 'foo-service'
+        systemd.return_value = True
+        service_running.return_value = True
+
+        def fake_service(action, name):
+            if action == 'is-enabled':
+                return True
+
+        service.side_effect = fake_service
+        self.assertTrue(host.service_resume(service_name))
+        service.assert_has_calls([call('is-enabled', service_name)])
+
     @patch.object(host, 'service_running')
     @patch.object(host, 'init_is_systemd')
     @patch.object(host, 'service')