openstack
/
paunch
Code Issues Proposed changes

Corrected how health check unit and link are removed 

I681fa0eb225d43ecbaa1b765f6761dd07031bce1 introduced some logic and
links between main service and its health check. The removal wasn't that
clean, so this patch intends to make things faster and cleaner.

We also remove the "systemctl daemon-reload" call, since it's useless:
its only value is for modified units, and here we're stopping, disabling
and deleting units.

This last change allows to get a cleaner, safer removal of the health
check dependency.

Change-Id: Ib5d61ed0e3b85dfbc256cb46cbfb611f8a785d71
This commit is contained in:
Cédric Jeanneret 2019-05-28 10:03:34 +02:00
parent e6e7048fd2
commit 52d0b2d5f5
2 changed files with 14 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
paunch/tests/test_utils_systemd.py
View File

@ -70,47 +70,37 @@ class TestUtilsSystemd(base.TestCase):
os.rmdir(tempdir)
@mock.patch('os.rmdir', autospec=True)
@mock.patch('shutil.rmtree', autospec=True)
@mock.patch('os.remove', autospec=True)
@mock.patch('os.path.exists', autospec=True)
@mock.patch('os.path.isfile', autospec=True)
@mock.patch('subprocess.check_call', autospec=True)
def test_service_delete(self, mock_subprocess_check_call, mock_isfile,
mock_exists, mock_rm, mock_rmdir):
mock_exists, mock_rm, mock_rmtree):
mock_isfile.return_value = True
container = 'my_app'
service = 'tripleo_' + container
tempdir = tempfile.mkdtemp()
service_requires_d = service + '.service.requires'
service_health_timer_f = service + '_healthcheck.timer'
systemd.service_delete(container, tempdir)
mock_rm.assert_has_calls([
mock.call(tempdir + service_requires_d + '/' +
service_health_timer_f),
mock.call(tempdir + service + '.service'),
mock.call(tempdir + service + '_healthcheck.service'),
mock.call(tempdir + service + '_healthcheck.timer'),
])
mock_rmdir.assert_has_calls([
mock.call(os.path.join(tempdir, service_requires_d)),
])
mock_subprocess_check_call.assert_has_calls([
mock.call(['systemctl', 'stop', service + '_healthcheck.timer']),
mock.call(['systemctl', 'disable',
service + '_healthcheck.timer']),
mock.call(['systemctl', 'daemon-reload']),
mock.call(['systemctl', 'stop', service + '.service']),
mock.call(['systemctl', 'disable', service + '.service']),
mock.call(['systemctl', 'daemon-reload']),
mock.call(['systemctl', 'stop', service + '_healthcheck.service']),
mock.call(['systemctl', 'disable', service +
'_healthcheck.service']),
mock.call(['systemctl', 'daemon-reload']),
mock.call(['systemctl', 'stop', service + '_healthcheck.timer']),
mock.call(['systemctl', 'disable', service +
'_healthcheck.timer']),
mock.call(['systemctl', 'daemon-reload']),
])
mock_rmtree.assert_has_calls([
mock.call(os.path.join(tempdir, service_requires_d)),
])
@mock.patch('os.chmod')

23
paunch/utils/systemd.py
View File

@ -14,6 +14,7 @@
# under the License.
import os
import shutil
from paunch import constants
from paunch.utils import common
@ -115,32 +116,26 @@ def service_delete(container, sysdir=constants.SYSTEMD_DIR, log=None):
sysd_health_f = systemctl.format_name(service + '_healthcheck')
sysd_timer_f = service + '_healthcheck.timer'
sysd_health_req_d = sysd_unit_f + '.requires'
sysd_health_req_f = sysd_health_req_d + '/' + sysd_timer_f
for sysd_f in sysd_health_req_f, sysd_unit_f, sysd_health_f, sysd_timer_f:
for sysd_f in sysd_unit_f, sysd_health_f, sysd_timer_f:
if os.path.isfile(sysdir + sysd_f):
log.debug('Stopping and disabling systemd service for %s' %
service)
sysd_unit = os.path.basename(sysd_f)
try:
systemctl.stop(sysd_unit)
systemctl.disable(sysd_unit)
systemctl.stop(sysd_f)
systemctl.disable(sysd_f)
except systemctl.SystemctlException:
log.exception("systemctl failed")
raise
log.debug('Removing systemd unit file %s' % sysd_f)
if os.path.exists(sysdir + sysd_f):
os.remove(sysdir + sysd_f)
try:
systemctl.daemon_reload()
except systemctl.SystemctlException:
log.exception("systemctl failed")
raise
os.remove(sysdir + sysd_f)
else:
log.info('No systemd unit file was found for %s' % sysd_f)
# Now that the service is removed, we can remove its ".requires"
if os.path.exists(os.path.join(sysdir, sysd_health_req_d)):
log.info('Removing %s.requires' % service)
os.rmdir(os.path.join(sysdir, sysd_health_req_d))
log.info('Removing healthcheck require for %s' % service)
shutil.rmtree(os.path.join(sysdir, sysd_health_req_d))
def healthcheck_create(container, sysdir='/etc/systemd/system/',