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 <guoqiao@gmail.com>

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

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."""

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):