From ca0c9ff887a3bf946db637465cf2551f88b29706 Mon Sep 17 00:00:00 2001 From: Kaifeng Wang Date: Sat, 4 Jan 2020 16:07:20 +0800 Subject: [PATCH] Refactor glance retry code to use retrying lib Also removed the reference to the deprecated import in tests. Change-Id: I2fc6ee10d9d26a8bdec105d298474b047b56d6e1 --- ironic/common/glance_service/image_service.py | 43 +++++++++---------- ironic/common/image_service.py | 6 +-- .../tests/unit/common/test_glance_service.py | 40 ++++++++++------- 3 files changed, 47 insertions(+), 42 deletions(-) diff --git a/ironic/common/glance_service/image_service.py b/ironic/common/glance_service/image_service.py index e1a9bbf87e..3f9ae31870 100644 --- a/ironic/common/glance_service/image_service.py +++ b/ironic/common/glance_service/image_service.py @@ -25,6 +25,7 @@ from glanceclient import client from glanceclient import exc as glance_exc from oslo_log import log from oslo_utils import uuidutils +import retrying import sendfile from swiftclient import utils as swift_utils @@ -111,6 +112,12 @@ class GlanceImageService(object): self.context = context self.endpoint = None + @retrying.retry( + stop_max_attempt_number=CONF.glance.num_retries + 1, + retry_on_exception=lambda e: isinstance( + e, exception.GlanceConnectionFailed), + wait_fixed=1000 + ) def call(self, method, *args, **kwargs): """Call a glance client method. @@ -131,29 +138,21 @@ class GlanceImageService(object): glance_exc.Unauthorized, glance_exc.NotFound, glance_exc.BadRequest) - num_attempts = 1 + CONF.glance.num_retries - # TODO(pas-ha) use retrying lib here - for attempt in range(1, num_attempts + 1): - try: - return getattr(self.client.images, method)(*args, **kwargs) - except retry_excs as e: - error_msg = ("Error contacting glance endpoint " - "%(endpoint)s for '%(method)s', attempt " - "%(attempt)s of %(num_attempts)s failed.") - LOG.exception(error_msg, {'endpoint': self.endpoint, - 'num_attempts': num_attempts, - 'attempt': attempt, - 'method': method}) - if attempt == num_attempts: - raise exception.GlanceConnectionFailed( - endpoint=self.endpoint, reason=e) - time.sleep(1) - except image_excs: - exc_type, exc_value, exc_trace = sys.exc_info() - new_exc = _translate_image_exception( - args[0], exc_value) - raise type(new_exc)(new_exc).with_traceback(exc_trace) + try: + return getattr(self.client.images, method)(*args, **kwargs) + except retry_excs as e: + error_msg = ("Error contacting glance endpoint " + "%(endpoint)s for '%(method)s'") + LOG.exception(error_msg, {'endpoint': self.endpoint, + 'method': method}) + raise exception.GlanceConnectionFailed( + endpoint=self.endpoint, reason=e) + except image_excs: + exc_type, exc_value, exc_trace = sys.exc_info() + new_exc = _translate_image_exception( + args[0], exc_value) + raise type(new_exc)(new_exc).with_traceback(exc_trace) @check_image_service def show(self, image_href): diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index c9c7026afd..e67aca83c2 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -28,7 +28,7 @@ import requests import sendfile from ironic.common import exception -from ironic.common.glance_service import image_service +from ironic.common.glance_service.image_service import GlanceImageService from ironic.common.i18n import _ from ironic.common import utils @@ -40,10 +40,6 @@ SENDFILE_CHUNK_SIZE = 1024 * 1024 * 1024 # 1Gb LOG = log.getLogger(__name__) -# TODO(dtantsur): temporary re-import, refactor the code and remove it. -GlanceImageService = image_service.GlanceImageService - - class BaseImageService(object, metaclass=abc.ABCMeta): """Provides retrieval of disk images.""" diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index 9e5c93a439..02e4914bc1 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -15,6 +15,7 @@ import datetime +import importlib import time from glanceclient import client as glance_client @@ -23,13 +24,13 @@ from keystoneauth1 import loading as kaloading import mock from oslo_config import cfg from oslo_utils import uuidutils +import retrying import testtools from ironic.common import context from ironic.common import exception from ironic.common.glance_service import image_service from ironic.common.glance_service import service_utils -from ironic.common import image_service as service from ironic.tests import base from ironic.tests.unit import stubs @@ -92,7 +93,8 @@ class TestGlanceImageService(base.TestCase): self.context = context.RequestContext(auth_token=True) self.context.user_id = 'fake' self.context.project_id = 'fake' - self.service = service.GlanceImageService(self.client, self.context) + self.service = image_service.GlanceImageService(self.client, + self.context) @staticmethod def _make_fixture(**kwargs): @@ -175,7 +177,7 @@ class TestGlanceImageService(base.TestCase): self.assertRaises(exception.ImageUnacceptable, self.service.show, image_id) - @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(retrying.time, 'sleep', autospec=True) def test_download_with_retries(self, mock_sleep): tries = [0] @@ -192,7 +194,8 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, stub_context) + stub_service = image_service.GlanceImageService(stub_client, + stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() @@ -202,8 +205,11 @@ class TestGlanceImageService(base.TestCase): stub_service.download, image_id, writer) # Now lets enable retries. No exception should happen now. - tries = [0] 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) self.assertTrue(mock_sleep.called) @@ -238,8 +244,8 @@ class TestGlanceImageService(base.TestCase): stub_context.project_id = 'fake' stub_client = MyGlanceStubClient() - stub_service = service.GlanceImageService(stub_client, - context=stub_context) + stub_service = image_service.GlanceImageService(stub_client, + context=stub_context) image_id = uuidutils.generate_uuid() self.config(allowed_direct_url_schemes=['file'], group='glance') @@ -274,7 +280,8 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, stub_context) + stub_service = image_service.GlanceImageService(stub_client, + stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotAuthorized, stub_service.download, @@ -290,7 +297,8 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, stub_context) + stub_service = image_service.GlanceImageService(stub_client, + stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotAuthorized, stub_service.download, @@ -306,7 +314,8 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, stub_context) + stub_service = image_service.GlanceImageService(stub_client, + stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotFound, stub_service.download, @@ -322,7 +331,8 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, stub_context) + stub_service = image_service.GlanceImageService(stub_client, + stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotFound, stub_service.download, @@ -341,7 +351,7 @@ class CheckImageServiceTestCase(base.TestCase): def setUp(self): super(CheckImageServiceTestCase, self).setUp() self.context = context.RequestContext(global_request_id='global') - self.service = service.GlanceImageService(None, self.context) + self.service = image_service.GlanceImageService(None, self.context) # NOTE(pas-ha) register keystoneauth dynamic options manually plugin = kaloading.get_plugin_loader('password') opts = kaloading.get_auth_plugin_conf_options(plugin) @@ -469,7 +479,7 @@ class TestGlanceSwiftTempURL(base.TestCase): client = stubs.StubGlanceClient() self.context = context.RequestContext() self.context.auth_token = 'fake' - self.service = service.GlanceImageService(client, self.context) + self.service = image_service.GlanceImageService(client, self.context) self.config(swift_temp_url_key='correcthorsebatterystaple', group='glance') self.config(swift_endpoint_url='https://swift.example.com', @@ -734,8 +744,8 @@ class TestSwiftTempUrlCache(base.TestCase): group='glance') self.config(swift_store_multiple_containers_seed=0, group='glance') - self.glance_service = service.GlanceImageService(client, - context=self.context) + self.glance_service = image_service.GlanceImageService( + client, context=self.context) @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) def test_add_items_to_cache(self, tempurl_mock):