From 09ee55efb1adf39f9442228c8f962cf0451a157b Mon Sep 17 00:00:00 2001 From: Joe Guo Date: Wed, 4 Sep 2019 22:13:48 +1200 Subject: [PATCH] use tenacity for retries These code snippets were using a hard-coded sleep time, wrapped by a for loop to retry, which is bad code smell. Use tenacity retries to simplify the code, and be consistent with the established approach to similar needs in other tests. Closes #46 Signed-off-by: Joe Guo --- zaza/openstack/charm_tests/mysql/tests.py | 39 +++++++++++++---------- zaza/openstack/charm_tests/vault/tests.py | 32 +++++++++++-------- zaza/openstack/charm_tests/vault/utils.py | 30 ++++++++--------- 3 files changed, 54 insertions(+), 47 deletions(-) diff --git a/zaza/openstack/charm_tests/mysql/tests.py b/zaza/openstack/charm_tests/mysql/tests.py index 1e11845..32305fe 100644 --- a/zaza/openstack/charm_tests/mysql/tests.py +++ b/zaza/openstack/charm_tests/mysql/tests.py @@ -17,7 +17,8 @@ import logging import os import re -import time + +import tenacity import zaza.charm_lifecycle.utils as lifecycle_utils import zaza.model @@ -349,6 +350,23 @@ class PerconaClusterColdStartTest(PerconaClusterTest): states=test_config.get("target_deploy_status", {})) +@tenacity.retry( + retry=tenacity.retry_if_result(lambda is_new: is_new is False), + wait=tenacity.wait_fixed(5), # interval between retries + stop=tenacity.stop_after_attempt(10)) # retry times +def retry_is_new_crm_master(test, old_crm_master): + """Check new crm master with retries. + + Return True if a new crm master detected, retry 10 times if False. + """ + new_crm_master = test.get_crm_master() + if new_crm_master and new_crm_master != old_crm_master: + logging.info( + "New crm_master unit detected on {}".format(new_crm_master)) + return True + return False + + class PerconaClusterScaleTests(PerconaClusterTest): """Percona Cluster scale tests.""" @@ -376,22 +394,9 @@ class PerconaClusterScaleTests(PerconaClusterTest): zaza.model.run_on_unit(old_crm_master, cmd) logging.info("looking for the new crm_master") - i = 0 - while i < 10: - i += 1 - # XXX time.sleep roundup - # https://github.com/openstack-charmers/zaza-openstack-tests/issues/46 - time.sleep(5) # give some time to pacemaker to react - new_crm_master = self.get_crm_master() - - if (new_crm_master and new_crm_master != old_crm_master): - logging.info( - "New crm_master unit detected" - " on {}".format(new_crm_master) - ) - break - else: - assert False, "The crm_master didn't change" + self.assertTrue( + retry_is_new_crm_master(self, old_crm_master), + msg="The crm_master didn't change") # Check connectivity on the VIP # \ is required due to pep8 and parenthesis would make the assertion diff --git a/zaza/openstack/charm_tests/vault/tests.py b/zaza/openstack/charm_tests/vault/tests.py index ec398b7..dfd4cc2 100644 --- a/zaza/openstack/charm_tests/vault/tests.py +++ b/zaza/openstack/charm_tests/vault/tests.py @@ -16,13 +16,14 @@ """Collection of tests for vault.""" -import hvac -import time import unittest import uuid import tempfile import requests +import tenacity +from hvac.exceptions import InternalServerError + import zaza.charm_lifecycle.utils as lifecycle_utils import zaza.openstack.charm_tests.test_utils as test_utils import zaza.openstack.charm_tests.vault.utils as vault_utils @@ -31,6 +32,21 @@ import zaza.openstack.utilities.openstack import zaza.model +@tenacity.retry( + retry=tenacity.retry_if_exception_type(InternalServerError), + retry_error_callback=lambda retry_state: False, + wait=tenacity.wait_fixed(2), # interval between retries + stop=tenacity.stop_after_attempt(10)) # retry 10 times +def retry_hvac_client_authenticated(client): + """Check hvac client is authenticated with retry. + + If is_authenticated() raise exception for all retries, + return False(which is done by `retry_error_callback`). + Otherwise, return whatever the returned value. + """ + return client.hvac_client.is_authenticated() + + class BaseVaultTest(unittest.TestCase): """Base class for vault tests.""" @@ -128,17 +144,7 @@ class VaultTest(BaseVaultTest): def test_all_clients_authenticated(self): """Check all vault clients are authenticated.""" for client in self.clients: - for i in range(1, 10): - try: - self.assertTrue(client.hvac_client.is_authenticated()) - except hvac.exceptions.InternalServerError: - # XXX time.sleep roundup - # https://github.com/openstack-charmers/zaza-openstack-tests/issues/46 - time.sleep(2) - else: - break - else: - self.assertTrue(client.hvac_client.is_authenticated()) + self.assertTrue(retry_hvac_client_authenticated(client)) def check_read(self, key, value): """Check reading the key from all vault units.""" diff --git a/zaza/openstack/charm_tests/vault/utils.py b/zaza/openstack/charm_tests/vault/utils.py index cdc68b1..26bbb2f 100644 --- a/zaza/openstack/charm_tests/vault/utils.py +++ b/zaza/openstack/charm_tests/vault/utils.py @@ -20,9 +20,9 @@ import base64 import hvac import requests import tempfile -import time import urllib3 import yaml +import tenacity import collections @@ -117,6 +117,15 @@ def get_clients(units=None, cacert=None): return clients +@tenacity.retry( + retry=tenacity.retry_if_exception_type(( + ConnectionRefusedError, + urllib3.exceptions.NewConnectionError, + urllib3.exceptions.MaxRetryError, + requests.exceptions.ConnectionError)), + reraise=True, # if all retries failed, reraise the last exception + wait=tenacity.wait_fixed(2), # interval between retries + stop=tenacity.stop_after_attempt(10)) # retry 10 times def is_initialized(client): """Check if vault is initialized. @@ -124,23 +133,10 @@ def is_initialized(client): :type client: CharmVaultClient :returns: Whether vault is initialized :rtype: bool + + Raise the last exception if no value returned after retries. """ - initialized = False - for i in range(1, 10): - try: - initialized = client.hvac_client.is_initialized() - except (ConnectionRefusedError, - urllib3.exceptions.NewConnectionError, - urllib3.exceptions.MaxRetryError, - requests.exceptions.ConnectionError): - # XXX time.sleep roundup - # https://github.com/openstack-charmers/zaza-openstack-tests/issues/46 - time.sleep(2) - else: - break - else: - raise Exception("Cannot connect") - return initialized + return client.hvac_client.is_initialized() def ensure_secret_backend(client):