From cab51a9fcc022f2e4bb634277dd20e90e2dc78f7 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Mon, 31 Oct 2022 11:19:08 +0100
Subject: [PATCH] Fix the invalid glance client test

It relied on mocking tenacity.retry, but it's executed on class
initialization. Depending on the ordering, it may do nothing or
it may replace ImageService.call with a mock.

Instead, add a new tenacity helper that loads an option in runtime.
As a nice side effect, [glance]num_retries is now mutable.

Change-Id: I2e02231d294997e824db77c998ef8d352fa69075
---
 ironic/common/glance_service/image_service.py |  3 ++-
 ironic/common/utils.py                        | 15 +++++++++++
 ironic/conf/glance.py                         |  1 +
 .../tests/unit/common/test_glance_service.py  | 25 ++++++++-----------
 4 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/ironic/common/glance_service/image_service.py b/ironic/common/glance_service/image_service.py
index 0a32eaf0a8..1d9d6d4bc4 100644
--- a/ironic/common/glance_service/image_service.py
+++ b/ironic/common/glance_service/image_service.py
@@ -33,6 +33,7 @@ from ironic.common.glance_service import service_utils
 from ironic.common.i18n import _
 from ironic.common import keystone
 from ironic.common import swift
+from ironic.common import utils
 from ironic.conf import CONF
 
 TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement',
@@ -114,7 +115,7 @@ class GlanceImageService(object):
     @tenacity.retry(
         retry=tenacity.retry_if_exception_type(
             exception.GlanceConnectionFailed),
-        stop=tenacity.stop_after_attempt(CONF.glance.num_retries + 1),
+        stop=utils.stop_after_retries('num_retries', group='glance'),
         wait=tenacity.wait_fixed(1),
         reraise=True
     )
diff --git a/ironic/common/utils.py b/ironic/common/utils.py
index e4d83c9b6f..9ae88d4d6b 100644
--- a/ironic/common/utils.py
+++ b/ironic/common/utils.py
@@ -681,3 +681,18 @@ def is_fips_enabled():
     except Exception:
         pass
     return False
+
+
+def stop_after_retries(option, group=None):
+    """A tenacity retry helper that stops after retries specified in conf."""
+    # NOTE(dtantsur): fetch the option inside of the nested call, otherwise it
+    # cannot be changed in runtime.
+    def should_stop(retry_state):
+        if group:
+            conf = getattr(CONF, group)
+        else:
+            conf = CONF
+        num_retries = getattr(conf, option)
+        return retry_state.attempt_number >= num_retries + 1
+
+    return should_stop
diff --git a/ironic/conf/glance.py b/ironic/conf/glance.py
index a3286b1eb4..317f213bc1 100644
--- a/ironic/conf/glance.py
+++ b/ironic/conf/glance.py
@@ -114,6 +114,7 @@ opts = [
                       'will determine how many containers are created.')),
     cfg.IntOpt('num_retries',
                default=0,
+               mutable=True,
                help=_('Number of retries when downloading an image from '
                       'glance.')),
 ]
diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py
index 327125a812..f9e713e911 100644
--- a/ironic/tests/unit/common/test_glance_service.py
+++ b/ironic/tests/unit/common/test_glance_service.py
@@ -24,7 +24,6 @@ from glanceclient import exc as glance_exc
 from keystoneauth1 import loading as ks_loading
 from oslo_config import cfg
 from oslo_utils import uuidutils
-import tenacity
 import testtools
 
 from ironic.common import context
@@ -202,20 +201,18 @@ class TestGlanceImageService(base.TestCase):
         image_id = uuidutils.generate_uuid()
         writer = NullWriter()
 
-        with mock.patch.object(tenacity, 'retry', autospec=True) as mock_retry:
-            # When retries are disabled, we should get an exception
-            self.config(num_retries=0, group='glance')
-            self.assertRaises(exception.GlanceConnectionFailed,
-                              stub_service.download, image_id, writer)
+        # When retries are disabled, we should get an exception
+        self.config(num_retries=0, group='glance')
+        self.assertRaises(exception.GlanceConnectionFailed,
+                          stub_service.download, image_id, writer)
 
-            # Now lets enable retries. No exception should happen now.
-            self.config(num_retries=1, group='glance')
-            importlib.reload(image_service)
-            stub_service = image_service.GlanceImageService(stub_client,
-                                                            stub_context)
-            tries = [0]
-            stub_service.download(image_id, writer)
-            mock_retry.assert_called_once()
+        # Now lets enable retries. No exception should happen now.
+        self.config(num_retries=1, group='glance')
+        importlib.reload(image_service)
+        stub_service = image_service.GlanceImageService(stub_client,
+                                                        stub_context)
+        tries = [0]
+        stub_service.download(image_id, writer)
 
     def test_download_no_data(self):
         self.client.fake_wrapped = None