Merge "Fix the invalid glance client test"

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
     )

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

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.')),
 ]

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
-            # When retries are disabled, we should get an exception
+        self.config(num_retries=0, group='glance')
-            self.config(num_retries=0, group='glance')
+        self.assertRaises(exception.GlanceConnectionFailed,
-            self.assertRaises(exception.GlanceConnectionFailed,
+                          stub_service.download, image_id, writer)
-                              stub_service.download, image_id, writer)
 
-            # Now lets enable retries. No exception should happen now.
+        # Now lets enable retries. No exception should happen now.
-            self.config(num_retries=1, group='glance')
+        self.config(num_retries=1, group='glance')
-            importlib.reload(image_service)
+        importlib.reload(image_service)
-            stub_service = image_service.GlanceImageService(stub_client,
+        stub_service = image_service.GlanceImageService(stub_client,
-                                                            stub_context)
+                                                        stub_context)
-            tries = [0]
+        tries = [0]
-            stub_service.download(image_id, writer)
+        stub_service.download(image_id, writer)
-            mock_retry.assert_called_once()
 
     def test_download_no_data(self):
         self.client.fake_wrapped = None