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
This commit is contained in:
Dmitry Tantsur 2022-10-31 11:19:08 +01:00
parent 5d5ae59538
commit cab51a9fcc
4 changed files with 29 additions and 15 deletions

View File

@ -33,6 +33,7 @@ from ironic.common.glance_service import service_utils
from ironic.common.i18n import _ from ironic.common.i18n import _
from ironic.common import keystone from ironic.common import keystone
from ironic.common import swift from ironic.common import swift
from ironic.common import utils
from ironic.conf import CONF from ironic.conf import CONF
TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement', TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement',
@ -114,7 +115,7 @@ class GlanceImageService(object):
@tenacity.retry( @tenacity.retry(
retry=tenacity.retry_if_exception_type( retry=tenacity.retry_if_exception_type(
exception.GlanceConnectionFailed), 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), wait=tenacity.wait_fixed(1),
reraise=True reraise=True
) )

View File

@ -681,3 +681,18 @@ def is_fips_enabled():
except Exception: except Exception:
pass pass
return False 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

View File

@ -114,6 +114,7 @@ opts = [
'will determine how many containers are created.')), 'will determine how many containers are created.')),
cfg.IntOpt('num_retries', cfg.IntOpt('num_retries',
default=0, default=0,
mutable=True,
help=_('Number of retries when downloading an image from ' help=_('Number of retries when downloading an image from '
'glance.')), 'glance.')),
] ]

View File

@ -24,7 +24,6 @@ from glanceclient import exc as glance_exc
from keystoneauth1 import loading as ks_loading from keystoneauth1 import loading as ks_loading
from oslo_config import cfg from oslo_config import cfg
from oslo_utils import uuidutils from oslo_utils import uuidutils
import tenacity
import testtools import testtools
from ironic.common import context from ironic.common import context
@ -202,20 +201,18 @@ class TestGlanceImageService(base.TestCase):
image_id = uuidutils.generate_uuid() image_id = uuidutils.generate_uuid()
writer = NullWriter() 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): def test_download_no_data(self):
self.client.fake_wrapped = None self.client.fake_wrapped = None