From 91ec859e908714f08f30180278beee23c5da0d57 Mon Sep 17 00:00:00 2001
From: Brian Haley <brianphaley@gmail.com>
Date: Fri, 19 Apr 2024 12:02:37 -0400
Subject: [PATCH] Skip service_enable if already enabled (#887)

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

Co-authored-by: Edward Hope-Morley <edward.hope-morley@canonical.com>
---
 charmhelpers/core/host.py |  7 +++++--
 tests/core/test_host.py   | 27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/charmhelpers/core/host.py b/charmhelpers/core/host.py
index e8fb78cca..2c9132fd1 100644
--- a/charmhelpers/core/host.py
+++ b/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))
diff --git a/tests/core/test_host.py b/tests/core/test_host.py
index a246f3471..b5adb5fb3 100644
--- a/tests/core/test_host.py
+++ b/tests/core/test_host.py
@@ -225,8 +225,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
@@ -442,6 +452,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')