Skip to content

Commit

Permalink
Skip service_enable if already enabled (#885)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
brianphaley and dosaboy authored Apr 19, 2024
1 parent f5efeef commit b884b20
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
7 changes: 5 additions & 2 deletions charmhelpers/core/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
27 changes: 27 additions & 0 deletions tests/core/test_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit b884b20

Please sign in to comment.