From acaeb62a4dc310f561091bd9b0f4e8f9f8aa5928 Mon Sep 17 00:00:00 2001
From: Felipe Reyes <felipe.reyes@canonical.com>
Date: Tue, 8 Mar 2022 10:11:51 -0300
Subject: [PATCH] Stop retrying if LB provisioning status reached ERROR (#682)

* Stop retrying if LB provisioning status reached ERROR

The method `wait_for_lb_resource()` retries for 15m while the load
balancer could have reached to ERROR during the provisioning in the
first few minutes, this approach makes the testing take longer for no
reason.

This change makes the ERROR state in provisioning_status final and
abort raising a ValueError() exception.

More details of the provisioning_status possible states can be found at:
https://docs.openstack.org/api-ref/load-balancer/v2/#provisioning-status-codes

* Use LoadBalancerUnexpectedState and LoadBalancerUnrecoverableError.

Drop the (re-)use of AssertionError and ValueError to identify when a
load balancer status is in a state where the test needs to retry or
break and fail respectively.

This change introduces 2 new exceptions to be explicit of what the code
is trying to do.

- LoadBalancerUnexpectedState is raised when the status of the load
  balancer is in a state different from the one requested by the caller,
  but said state can be considered as transitory.
- LoadBalancerUnrecoverableError is raised when the status of the load
  balancer is in ERROR state and said state is final for the
  proviniong_status property, hence retrying only delays the failure.
---
 zaza/openstack/charm_tests/octavia/tests.py | 38 ++++++++++++++-------
 zaza/openstack/utilities/exceptions.py      | 12 +++++++
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/zaza/openstack/charm_tests/octavia/tests.py b/zaza/openstack/charm_tests/octavia/tests.py
index 6158375..a46ce83 100644
--- a/zaza/openstack/charm_tests/octavia/tests.py
+++ b/zaza/openstack/charm_tests/octavia/tests.py
@@ -29,6 +29,10 @@ import zaza.openstack.utilities.openstack as openstack_utils
 
 from zaza.openstack.utilities import generic as generic_utils
 from zaza.openstack.utilities import ObjectRetrierWraps
+from zaza.openstack.utilities.exception import (
+    LoadBalancerUnexpectedState,
+    LoadBalancerUnrecoverableError,
+)
 
 LBAAS_ADMIN_ROLE = 'load-balancer_admin'
 
@@ -279,24 +283,34 @@ class LBAASv2Test(test_utils.OpenStackBaseTest):
         super(LBAASv2Test, self).resource_cleanup()
 
     @staticmethod
-    @tenacity.retry(retry=tenacity.retry_if_exception_type(AssertionError),
-                    wait=tenacity.wait_fixed(1), reraise=True,
-                    stop=tenacity.stop_after_delay(900))
+    @tenacity.retry(
+        retry=tenacity.retry_if_exception_type(LoadBalancerUnexpectedState),
+        wait=tenacity.wait_fixed(1), reraise=True,
+        stop=tenacity.stop_after_delay(900))
     def wait_for_lb_resource(octavia_show_func, resource_id,
                              provisioning_status=None, operating_status=None):
         """Wait for loadbalancer resource to reach expected status."""
         provisioning_status = provisioning_status or 'ACTIVE'
         resp = octavia_show_func(resource_id)
-        logging.info(resp['provisioning_status'])
-        assert resp['provisioning_status'] == provisioning_status, (
-            'load balancer resource has not reached '
-            'expected provisioning status: {}'
-            .format(resp))
+        logging.info("Current provisioning status: {}, waiting for {}"
+                     .format(resp['provisioning_status'], provisioning_status))
+
+        msg = ('load balancer resource has not reached '
+               'expected provisioning status: {}'.format(resp))
+
+        # ERROR is a final state, once it's reached there is no reason to keep
+        # retrying and delaying the failure.
+        if resp['provisioning_status'] == 'ERROR':
+            raise LoadBalancerUnrecoverableError(msg)
+
+        assert resp['provisioning_status'] == provisioning_status, msg
         if operating_status:
-            logging.info(resp['operating_status'])
-            assert resp['operating_status'] == operating_status, (
-                'load balancer resource has not reached '
-                'expected operating status: {}'.format(resp))
+            logging.info('Current operating status: {}, waiting for {}'
+                         .format(resp['operating_status'], operating_status))
+            if not resp['operating_status'] == operating_status:
+                raise LoadBalancerUnexpectedState((
+                    'load balancer resource has not reached '
+                    'expected operating status: {}'.format(resp)))
 
         return resp
 
diff --git a/zaza/openstack/utilities/exceptions.py b/zaza/openstack/utilities/exceptions.py
index 7f73be4..c638c2d 100644
--- a/zaza/openstack/utilities/exceptions.py
+++ b/zaza/openstack/utilities/exceptions.py
@@ -208,3 +208,15 @@ class CACERTNotFound(Exception):
     """Could not find cacert."""
 
     pass
+
+
+class LoadBalancerUnexpectedState(Exception):
+    """The LoadBalancer is in a unexpected state."""
+
+    pass
+
+
+class LoadBalancerUnrecoverableError(Exception):
+    """The LoadBalancer has reached to an unrecoverable error state."""
+
+    pass