From 8fb25fce4330bb4e4373d5bc4fa166d0cd282de4 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 18 Feb 2015 10:13:00 +0000 Subject: [PATCH] Charmhelper sync and move to using charm helper service restart check --- Makefile | 2 +- charm-helpers-tests.yaml | 2 +- tests/basic_deployment.py | 80 +++++++++---------- tests/charmhelpers/contrib/amulet/utils.py | 24 ++++-- .../contrib/openstack/amulet/deployment.py | 7 +- 5 files changed, 66 insertions(+), 49 deletions(-) diff --git a/Makefile b/Makefile index 11a2609f..38df26e5 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,7 @@ bin/charm_helpers_sync.py: > bin/charm_helpers_sync.py sync: bin/charm_helpers_sync.py - @$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-hooks.yaml +# @$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-hooks.yaml @$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-tests.yaml publish: lint unit_test diff --git a/charm-helpers-tests.yaml b/charm-helpers-tests.yaml index 48b12f6f..08db8d35 100644 --- a/charm-helpers-tests.yaml +++ b/charm-helpers-tests.yaml @@ -1,4 +1,4 @@ -branch: lp:charm-helpers +branch: lp:~gnuoy/charm-helpers/1422386 destination: tests/charmhelpers include: - contrib.amulet diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index 509fca59..d85f2f7a 100755 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -103,41 +103,41 @@ class CinderBasicDeployment(OpenStackAmuletDeployment): # Wait for relations to settle sleep(120) - def service_restarted(self, sentry_unit, service, filename, - pgrep_full=False, sleep_time=60): - """Compare a service's start time vs a file's last modification time - (such as a config file for that service) to determine if the service - has been restarted (within 60s by default), return when verified.""" - - # NOTE(beisner): prev rev utilized sleep_time as an arbitrary wait with - # no debug feedback. Added checking timeout loop logic & debug output. - # Increased default timeout to 60s due to test failures. - - # NOTE(beisner): need to move to charmhelpers, and adjust calls here. - # It is backward compatible with prev rev by coreycb. - - proc_start_time = u._get_proc_start_time(sentry_unit, - service, pgrep_full) - file_mtime = u._get_file_mtime(sentry_unit, filename) - - tries = 0 - while proc_start_time < file_mtime and tries < (sleep_time/30): - sleep(30) - proc_start_time = u._get_proc_start_time(sentry_unit, - service, pgrep_full) - file_mtime = u._get_file_mtime(sentry_unit, filename) - u.log.debug('proc restart wait: {} {}'.format(tries, service)) - tries += 1 - - u.log.debug('proc-file time diff for {},{}: {}'.format(service, - filename, proc_start_time - file_mtime)) - - if proc_start_time >= file_mtime: - return True - else: - u.log.debug('service not restarted within ()s: {}'.format( - service, sleep_time)) - return False +# def service_restarted(self, sentry_unit, service, filename, +# pgrep_full=False, sleep_time=60): +# """Compare a service's start time vs a file's last modification time +# (such as a config file for that service) to determine if the service +# has been restarted (within 60s by default), return when verified.""" +# +# # NOTE(beisner): prev rev utilized sleep_time as an arbitrary wait with +# # no debug feedback. Added checking timeout loop logic & debug output. +# # Increased default timeout to 60s due to test failures. +# +# # NOTE(beisner): need to move to charmhelpers, and adjust calls here. +# # It is backward compatible with prev rev by coreycb. +# +# proc_start_time = u._get_proc_start_time(sentry_unit, +# service, pgrep_full) +# file_mtime = u._get_file_mtime(sentry_unit, filename) +# +# tries = 0 +# while proc_start_time < file_mtime and tries < (sleep_time/30): +# sleep(30) +# proc_start_time = u._get_proc_start_time(sentry_unit, +# service, pgrep_full) +# file_mtime = u._get_file_mtime(sentry_unit, filename) +# u.log.debug('proc restart wait: {} {}'.format(tries, service)) +# tries += 1 +# +# u.log.debug('proc-file time diff for {},{}: {}'.format(service, +# filename, proc_start_time - file_mtime)) +# +# if proc_start_time >= file_mtime: +# return True +# else: +# u.log.debug('service not restarted within ()s: {}'.format( +# service, sleep_time)) +# return False def authenticate_cinder_admin(self, username, password, tenant): """Authenticates admin user with cinder.""" @@ -524,16 +524,16 @@ class CinderBasicDeployment(OpenStackAmuletDeployment): self.d.configure('cinder', {'verbose': 'True'}) self.d.configure('cinder', {'debug': 'True'}) - if not self.service_restarted(self.cinder_sentry, 'cinder-api', - '/etc/cinder/cinder.conf', - sleep_time=120): + if not u.service_restarted(self.cinder_sentry, 'cinder-api', + '/etc/cinder/cinder.conf', + sleep_time=120): self.d.configure('cinder', {'verbose': 'False'}) self.d.configure('cinder', {'debug': 'False'}) msg = "cinder-api service didn't restart after config change" amulet.raise_status(amulet.FAIL, msg=msg) - if not self.service_restarted(self.cinder_sentry, 'cinder-volume', - '/etc/cinder/cinder.conf', sleep_time=0): + if not u.service_restarted(self.cinder_sentry, 'cinder-volume', + '/etc/cinder/cinder.conf', sleep_time=0): self.d.configure('cinder', {'verbose': 'False'}) self.d.configure('cinder', {'debug': 'False'}) msg = "cinder-volume service didn't restart after conf change" diff --git a/tests/charmhelpers/contrib/amulet/utils.py b/tests/charmhelpers/contrib/amulet/utils.py index 3464b873..d0213d9f 100644 --- a/tests/charmhelpers/contrib/amulet/utils.py +++ b/tests/charmhelpers/contrib/amulet/utils.py @@ -169,11 +169,14 @@ class AmuletUtils(object): cmd = 'pgrep -o -f {}'.format(service) else: cmd = 'pgrep -o {}'.format(service) - proc_dir = '/proc/{}'.format(sentry_unit.run(cmd)[0].strip()) - return self._get_dir_mtime(sentry_unit, proc_dir) + cmd = cmd + ' | grep -v pgrep || exit 0' + cmd_out = sentry_unit.run(cmd) + if cmd_out[0]: + proc_dir = '/proc/{}'.format(cmd_out[0].strip()) + return self._get_dir_mtime(sentry_unit, proc_dir) def service_restarted(self, sentry_unit, service, filename, - pgrep_full=False, sleep_time=20): + pgrep_full=False, sleep_time=20, retry_count=2): """Check if service was restarted. Compare a service's start time vs a file's last modification time @@ -181,8 +184,19 @@ class AmuletUtils(object): has been restarted. """ time.sleep(sleep_time) - if (self._get_proc_start_time(sentry_unit, service, pgrep_full) >= - self._get_file_mtime(sentry_unit, filename)): + proc_start_time = self._get_proc_start_time(sentry_unit, service, + pgrep_full) + while retry_count > 0 and not proc_start_time: + self.log.debug('No pid file found for service %s, will retry %i ' + 'more times' % (service, retry_count)) + time.sleep(30) + proc_start_time = self._get_proc_start_time(sentry_unit, service, + pgrep_full) + retry_count = retry_count - 1 + + if not proc_start_time: + return False + if proc_start_time >= self._get_file_mtime(sentry_unit, filename): return True else: return False diff --git a/tests/charmhelpers/contrib/openstack/amulet/deployment.py b/tests/charmhelpers/contrib/openstack/amulet/deployment.py index c50d3ec6..0cfeaa4c 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/deployment.py +++ b/tests/charmhelpers/contrib/openstack/amulet/deployment.py @@ -71,16 +71,19 @@ class OpenStackAmuletDeployment(AmuletDeployment): services.append(this_service) use_source = ['mysql', 'mongodb', 'rabbitmq-server', 'ceph', 'ceph-osd', 'ceph-radosgw'] + # Openstack subordinate charms do not expose an origin option as that + # is controlled by the principle + ignore = ['neutron-openvswitch'] if self.openstack: for svc in services: - if svc['name'] not in use_source: + if svc['name'] not in use_source + ignore: config = {'openstack-origin': self.openstack} self.d.configure(svc['name'], config) if self.source: for svc in services: - if svc['name'] in use_source: + if svc['name'] in use_source and svc['name'] not in ignore: config = {'source': self.source} self.d.configure(svc['name'], config)