From e4b4c27c6baa8c397885953c458215589eaaca57 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Thu, 19 Feb 2015 09:52:12 +0000 Subject: [PATCH] Sync charmhelpers and use validate_service_config_changed rather than service_restarted to remove false-positives due to noop config writes --- tests/basic_deployment.py | 54 ++-------- tests/charmhelpers/contrib/amulet/utils.py | 114 +++++++++++++++++++-- 2 files changed, 117 insertions(+), 51 deletions(-) diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index d6c98e90..d0a847bc 100755 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -16,7 +16,7 @@ from charmhelpers.contrib.openstack.amulet.utils import ( # noqa ) # Use DEBUG to turn on debug logging -u = OpenStackAmuletUtils(DEBUG) +u = OpenStackAmuletUtils(ERROR) class CinderBasicDeployment(OpenStackAmuletDeployment): @@ -103,42 +103,6 @@ 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 authenticate_cinder_admin(self, username, password, tenant): """Authenticates admin user with cinder.""" # NOTE(beisner): need to move to charmhelpers, and adjust calls here. @@ -521,17 +485,21 @@ class CinderBasicDeployment(OpenStackAmuletDeployment): easier because restarting services requires re-authorization. ''' u.log.debug('making charm config change') + mtime = u.get_sentry_time(self.cinder_sentry) self.d.configure('cinder', {'verbose': 'True', 'debug': 'True'}) - - if not u.service_restarted(self.cinder_sentry, 'cinder-api', - '/etc/cinder/cinder.conf', - sleep_time=120): + if not u.validate_service_config_changed(self.cinder_sentry, + mtime, + 'cinder-api', + '/etc/cinder/cinder.conf'): self.d.configure('cinder', {'verbose': 'False', 'debug': 'False'}) msg = "cinder-api service didn't restart after config change" amulet.raise_status(amulet.FAIL, msg=msg) - if not u.service_restarted(self.cinder_sentry, 'cinder-volume', - '/etc/cinder/cinder.conf', sleep_time=0): + if not u.validate_service_config_changed(self.cinder_sentry, + mtime, + 'cinder-volume', + '/etc/cinder/cinder.conf', + sleep_time=0): self.d.configure('cinder', {'verbose': 'False', 'debug': 'False'}) msg = "cinder-volume service didn't restart after conf change" amulet.raise_status(amulet.FAIL, msg=msg) diff --git a/tests/charmhelpers/contrib/amulet/utils.py b/tests/charmhelpers/contrib/amulet/utils.py index ae96330b..65219d33 100644 --- a/tests/charmhelpers/contrib/amulet/utils.py +++ b/tests/charmhelpers/contrib/amulet/utils.py @@ -171,20 +171,46 @@ class AmuletUtils(object): cmd = 'pgrep -o {}'.format(service) cmd = cmd + ' | grep -v pgrep || exit 0' cmd_out = sentry_unit.run(cmd) + self.log.debug('CMDout: ' + str(cmd_out)) if cmd_out[0]: + self.log.debug('Pid for %s %s' % (service, str(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, retry_count=2): + pgrep_full=False, sleep_time=20): """Check if service was restarted. 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. """ - self.log.debug('Checking %s restarted after %s was changed' - % (service, filename)) + time.sleep(sleep_time) + if (self._get_proc_start_time(sentry_unit, service, pgrep_full) >= + self._get_file_mtime(sentry_unit, filename)): + return True + else: + return False + + def service_restarted_since(self, sentry_unit, mtime, service, + pgrep_full=False, sleep_time=20, + retry_count=2): + """Check if service was been started after a given time. + + Args: + sentry_unit (sentry): The sentry unit to check for the service on + mtime (float): The epoch time to check against + service (string): service name to look for in process table + pgrep_full (boolean): Use full command line search mode with pgrep + sleep_time (int): Seconds to sleep before looking for process + retry_count (int): If service is not found, how many times to retry + + Returns: + bool: True if service found and its start time it newer than mtime, + False if service is older than mtime or if service was + not found. + """ + self.log.debug('Checking %s restarted since %s' % (service, mtime)) time.sleep(sleep_time) proc_start_time = self._get_proc_start_time(sentry_unit, service, pgrep_full) @@ -200,15 +226,87 @@ class AmuletUtils(object): self.log.warn('No proc start time found, assuming service did ' 'not start') return False - if proc_start_time >= self._get_file_mtime(sentry_unit, filename): - self.log.debug('proc start time is newer than config changed ' - 'time') + if proc_start_time >= mtime: + self.log.debug('proc start time is newer than provided mtime' + '(%s >= %s)' % (proc_start_time, mtime)) return True else: - self.log.warn('proc start time is older than config changed ' - 'time, service did not restart') + self.log.warn('proc start time (%s) is older than provided mtime ' + '(%s), service did not restart' % (proc_start_time, + mtime)) return False + def config_updated_since(self, sentry_unit, filename, mtime, + sleep_time=20): + """Check if file was modified after a given time. + + Args: + sentry_unit (sentry): The sentry unit to check the file mtime on + filename (string): The file to check mtime of + mtime (float): The epoch time to check against + sleep_time (int): Seconds to sleep before looking for process + + Returns: + bool: True if file was modified more recently than mtime, False if + file was modified before mtime, + """ + self.log.debug('Checking %s updated since %s' % (filename, mtime)) + time.sleep(sleep_time) + file_mtime = self._get_file_mtime(sentry_unit, filename) + if file_mtime >= mtime: + self.log.debug('File mtime is newer than provided mtime ' + '(%s >= %s)' % (file_mtime, mtime)) + return True + else: + self.log.warn('File mtime %s is older than provided mtime %s' + % (file_mtime, mtime)) + return False + + def validate_service_config_changed(self, sentry_unit, mtime, service, + filename, pgrep_full=False, + sleep_time=20, retry_count=2): + """Check service and file were updated after mtime + + Args: + sentry_unit (sentry): The sentry unit to check for the service on + mtime (float): The epoch time to check against + service (string): service name to look for in process table + filename (string): The file to check mtime of + pgrep_full (boolean): Use full command line search mode with pgrep + sleep_time (int): Seconds to sleep before looking for process + retry_count (int): If service is not found, how many times to retry + + Typical Usage: + u = OpenStackAmuletUtils(ERROR) + ... + mtime = u.get_sentry_time(self.cinder_sentry) + self.d.configure('cinder', {'verbose': 'True', 'debug': 'True'}) + if not u.validate_service_config_changed(self.cinder_sentry, + mtime, + 'cinder-api', + '/etc/cinder/cinder.conf') + amulet.raise_status(amulet.FAIL, msg='update failed') + Returns: + bool: True if both service and file where updated/restarted after + mtime, False if service is older than mtime or if service was + not found or if filename was modified before mtime. + """ + self.log.debug('Checking %s restarted since %s' % (service, mtime)) + time.sleep(sleep_time) + service_restart = self.service_restarted_since(sentry_unit, mtime, + service, + pgrep_full=pgrep_full, + sleep_time=0, + retry_count=retry_count) + config_update = self.config_updated_since(sentry_unit, filename, mtime, + sleep_time=0) + return service_restart and config_update + + def get_sentry_time(self, sentry_unit): + """Return current epoch time on a sentry""" + cmd = "date +'%s'" + return float(sentry_unit.run(cmd)[0]) + def relation_error(self, name, data): return 'unexpected relation data in {} - {}'.format(name, data)