From 2095527e33e6e905846e33afd1a957986da18800 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 19 Apr 2024 12:01:53 -0400 Subject: [PATCH] Skip service_enable if already enabled (#884) 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 --- 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 70dde6a53..def403c51 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 57c3ac0b9..50c742c03 100644 --- a/tests/core/test_host.py +++ b/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')