From 978afbd5a1bba71181c5a0a7cb3ebfb20132c438 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Tue, 23 Apr 2024 10:14:27 +1200 Subject: [PATCH] Replace glanceclient usage with openstacksdk Closes-Bug: #2042495 Change-Id: Ic8421bd937a3a1ab6c3b86c259cd929810c0532e --- doc/source/admin/report.txt | 1 - ironic/common/glance_service/image_service.py | 77 +++++++------ ironic/conf/opts.py | 1 - .../tests/unit/common/test_glance_service.py | 104 ++++++------------ ironic/tests/unit/stubs.py | 40 +++---- ...glanceclient-removal-33b382ad03772530.yaml | 5 + requirements.txt | 1 - 7 files changed, 96 insertions(+), 133 deletions(-) create mode 100644 releasenotes/notes/glanceclient-removal-33b382ad03772530.yaml diff --git a/doc/source/admin/report.txt b/doc/source/admin/report.txt index a1c96e2cce..05ca307df0 100644 --- a/doc/source/admin/report.txt +++ b/doc/source/admin/report.txt @@ -229,7 +229,6 @@ default: amqp=WARNING amqplib=WARNING eventlet.wsgi.server=INFO - glanceclient=WARNING iso8601=WARNING keystoneauth.session=INFO keystonemiddleware.auth_token=INFO diff --git a/ironic/common/glance_service/image_service.py b/ironic/common/glance_service/image_service.py index 7ef032d778..c2f7f53090 100644 --- a/ironic/common/glance_service/image_service.py +++ b/ironic/common/glance_service/image_service.py @@ -21,8 +21,9 @@ import sys import time from urllib import parse as urlparse -from glanceclient import client -from glanceclient import exc as glance_exc +from keystoneauth1 import exceptions as ks_exception +import openstack +from openstack.connection import exceptions as openstack_exc from oslo_log import log from oslo_utils import uuidutils import tenacity @@ -44,12 +45,11 @@ _GLANCE_SESSION = None def _translate_image_exception(image_id, exc_value): - if isinstance(exc_value, (glance_exc.Forbidden, - glance_exc.Unauthorized)): + if isinstance(exc_value, (openstack_exc.ForbiddenException)): return exception.ImageNotAuthorized(image_id=image_id) - if isinstance(exc_value, glance_exc.NotFound): + if isinstance(exc_value, openstack_exc.NotFoundException): return exception.ImageNotFound(image_id=image_id) - if isinstance(exc_value, glance_exc.BadRequest): + if isinstance(exc_value, openstack_exc.BadRequestException): return exception.Invalid(exc_value) return exc_value @@ -70,8 +70,6 @@ def check_image_service(func): if not _GLANCE_SESSION: _GLANCE_SESSION = keystone.get_session('glance') - # NOTE(pas-ha) glanceclient uses Adapter-based SessionClient, - # so we can pass session and auth separately, makes things easier service_auth = keystone.get_auth('glance') self.endpoint = keystone.get_endpoint('glance', @@ -85,10 +83,15 @@ def check_image_service(func): if self.context.auth_token: user_auth = keystone.get_service_auth(self.context, self.endpoint, service_auth) - self.client = client.Client(2, session=_GLANCE_SESSION, - auth=user_auth or service_auth, - endpoint_override=self.endpoint, - global_request_id=self.context.global_id) + sess = keystone.get_session('glance', + auth=user_auth or service_auth) + conn = openstack.connection.Connection( + session=sess, + image_endpoint_override=self.endpoint, + image_api_version='2') + + self.client = conn.global_request(self.context.global_id).image + return func(self, *args, **kwargs) return wrapper @@ -130,28 +133,22 @@ class GlanceImageService(object): :raises: GlanceConnectionFailed """ - retry_excs = (glance_exc.ServiceUnavailable, - glance_exc.InvalidEndpoint, - glance_exc.CommunicationError) - image_excs = (glance_exc.Forbidden, - glance_exc.Unauthorized, - glance_exc.NotFound, - glance_exc.BadRequest) - try: - return getattr(self.client.images, method)(*args, **kwargs) - except retry_excs as e: + return getattr(self.client, method)(*args, **kwargs) + except openstack_exc.SDKException: + exc_type, exc_value, exc_trace = sys.exc_info() + new_exc = _translate_image_exception( + args[0], exc_value) + if isinstance(new_exc, exception.IronicException): + # exception has been translated to a new one, raise it + raise type(new_exc)(new_exc).with_traceback(exc_trace) + except ks_exception.ClientException 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): @@ -167,7 +164,7 @@ class GlanceImageService(object): image_href) image_id = service_utils.parse_image_id(image_href) - image = self.call('get', image_id) + image = self.call('get_image', image_id) if not service_utils.is_image_active(image): raise exception.ImageUnacceptable( @@ -198,18 +195,20 @@ class GlanceImageService(object): os.sendfile(data.fileno(), f.fileno(), 0, filesize) return - image_chunks = self.call('data', image_id) - # NOTE(dtantsur): when using Glance V2, image_chunks is a wrapper - # around real data, so we have to check the wrapped data for None. - if image_chunks.wrapped is None: - raise exception.ImageDownloadFailed( - image_href=image_href, reason=_('image contains no data.')) - - if data is None: - return image_chunks - else: + image_size = 0 + image_data = None + if data: + image_chunks = self.call('download_image', image_id, stream=True) for chunk in image_chunks: data.write(chunk) + image_size += len(chunk) + else: + image_data = self.call('download_image', image_id).content + image_size = len(image_data) + if image_size == 0: + raise exception.ImageDownloadFailed( + image_href=image_href, reason=_('image contains no data.')) + return image_data def _generate_temp_url(self, path, seconds, key, method, endpoint, image_id): @@ -400,7 +399,7 @@ class GlanceImageService(object): Returns the direct url representing the backend storage location, or None if this attribute is not shown by Glance. """ - image_meta = self.call('get', image_id) + image_meta = self.call('get_image', image_id) if not service_utils.is_image_available(self.context, image_meta): raise exception.ImageNotFound(image_id=image_id) diff --git a/ironic/conf/opts.py b/ironic/conf/opts.py index 0d8aeafd20..d04f9ddfcc 100644 --- a/ironic/conf/opts.py +++ b/ironic/conf/opts.py @@ -82,7 +82,6 @@ def update_opt_defaults(): 'eventlet.wsgi.server=INFO', 'iso8601=WARNING', 'requests=WARNING', - 'glanceclient=WARNING', 'urllib3.connectionpool=WARNING', 'keystonemiddleware.auth_token=INFO', 'keystoneauth.session=INFO', diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index ab55aacb42..e02156ae1b 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -19,9 +19,10 @@ import importlib import time from unittest import mock -from glanceclient import client as glance_client -from glanceclient import exc as glance_exc +from keystoneauth1 import exceptions as ks_exception from keystoneauth1 import loading as ks_loading +import openstack +from openstack.connection import exceptions as openstack_exc from oslo_config import cfg from oslo_utils import uuidutils import testtools @@ -150,7 +151,7 @@ class TestGlanceImageService(base.TestCase): with mock.patch.object(self.service, 'call', autospec=True): self.service.call.return_value = image image_meta = self.service.show(image_id) - self.service.call.assert_called_with('get', image_id) + self.service.call.assert_called_with('get_image', image_id) self.assertEqual(expected, image_meta) def test_show_makes_datetimes(self): @@ -159,7 +160,7 @@ class TestGlanceImageService(base.TestCase): with mock.patch.object(self.service, 'call', autospec=True): self.service.call.return_value = image image_meta = self.service.show(image_id) - self.service.call.assert_called_with('get', image_id) + self.service.call.assert_called_with('get_image', image_id) self.assertEqual(self.NOW_DATETIME, image_meta['created_at']) self.assertEqual(self.NOW_DATETIME, image_meta['updated_at']) @@ -185,10 +186,10 @@ class TestGlanceImageService(base.TestCase): class MyGlanceStubClient(stubs.StubGlanceClient): """A client that fails the first time, then succeeds.""" - def get(self, image_id): + def get_image(self, image_id): if tries[0] == 0: tries[0] = 1 - raise glance_exc.ServiceUnavailable('') + raise ks_exception.ServiceUnavailable() else: return {} @@ -216,11 +217,11 @@ class TestGlanceImageService(base.TestCase): stub_service.download(image_id, writer) def test_download_no_data(self): - self.client.fake_wrapped = None + self.client.image_data = b'' image_id = uuidutils.generate_uuid() image = self._make_datetime_fixture() - with mock.patch.object(self.client, 'get', return_value=image, + with mock.patch.object(self.client, 'get_image', return_value=image, autospec=True): self.assertRaisesRegex(exception.ImageDownloadFailed, 'image contains no data', @@ -237,7 +238,7 @@ class TestGlanceImageService(base.TestCase): s_tmpfname = '/whatever/source' - def get(self, image_id): + def get_image(self, image_id): return type('GlanceTestDirectUrlMeta', (object,), {'direct_url': 'file://%s' + self.s_tmpfname}) @@ -275,25 +276,8 @@ class TestGlanceImageService(base.TestCase): def test_client_forbidden_converts_to_imagenotauthed(self): class MyGlanceStubClient(stubs.StubGlanceClient): """A client that raises a Forbidden exception.""" - def get(self, image_id): - raise glance_exc.Forbidden(image_id) - - stub_client = MyGlanceStubClient() - stub_context = context.RequestContext(auth_token=True) - stub_context.user_id = 'fake' - stub_context.project_id = 'fake' - stub_service = image_service.GlanceImageService(stub_client, - stub_context) - image_id = uuidutils.generate_uuid() - writer = NullWriter() - self.assertRaises(exception.ImageNotAuthorized, stub_service.download, - image_id, writer) - - def test_client_httpforbidden_converts_to_imagenotauthed(self): - class MyGlanceStubClient(stubs.StubGlanceClient): - """A client that raises a HTTPForbidden exception.""" - def get(self, image_id): - raise glance_exc.HTTPForbidden(image_id) + def get_image(self, image_id): + raise openstack_exc.ForbiddenException() stub_client = MyGlanceStubClient() stub_context = context.RequestContext(auth_token=True) @@ -309,25 +293,8 @@ class TestGlanceImageService(base.TestCase): def test_client_notfound_converts_to_imagenotfound(self): class MyGlanceStubClient(stubs.StubGlanceClient): """A client that raises a NotFound exception.""" - def get(self, image_id): - raise glance_exc.NotFound(image_id) - - stub_client = MyGlanceStubClient() - stub_context = context.RequestContext(auth_token=True) - stub_context.user_id = 'fake' - stub_context.project_id = 'fake' - stub_service = image_service.GlanceImageService(stub_client, - stub_context) - image_id = uuidutils.generate_uuid() - writer = NullWriter() - self.assertRaises(exception.ImageNotFound, stub_service.download, - image_id, writer) - - def test_client_httpnotfound_converts_to_imagenotfound(self): - class MyGlanceStubClient(stubs.StubGlanceClient): - """A client that raises a HTTPNotFound exception.""" - def get(self, image_id): - raise glance_exc.HTTPNotFound(image_id) + def get_image(self, image_id): + raise openstack_exc.NotFoundException() stub_client = MyGlanceStubClient() stub_context = context.RequestContext(auth_token=True) @@ -348,7 +315,7 @@ class TestGlanceImageService(base.TestCase): @mock.patch('ironic.common.keystone.get_adapter', autospec=True) @mock.patch('ironic.common.keystone.get_session', autospec=True, return_value=mock.sentinel.session) -@mock.patch.object(glance_client, 'Client', autospec=True) +@mock.patch.object(openstack.connection, 'Connection', autospec=True) class CheckImageServiceTestCase(base.TestCase): def setUp(self): super(CheckImageServiceTestCase, self).setUp() @@ -385,13 +352,11 @@ class CheckImageServiceTestCase(base.TestCase): self.assertEqual(0, mock_auth.call_count) self.assertEqual(0, mock_sauth.call_count) - def _assert_client_call(self, mock_gclient, url, user=False): + def _assert_connnection_call(self, mock_gclient, url): mock_gclient.assert_called_once_with( - 2, session=mock.sentinel.session, - global_request_id='global', - auth=mock.sentinel.sauth if user else mock.sentinel.auth, - endpoint_override=url) + image_endpoint_override=url, + image_api_version='2') def test_check_image_service__config_auth(self, mock_gclient, mock_sess, mock_adapter, mock_sauth, @@ -406,9 +371,12 @@ class CheckImageServiceTestCase(base.TestCase): wrapped_func = image_service.check_image_service(func) self.assertEqual(((), params), wrapped_func(self.service, **params)) - self._assert_client_call(mock_gclient, 'glance_url') + self._assert_connnection_call(mock_gclient, 'glance_url') mock_auth.assert_called_once_with('glance') - mock_sess.assert_called_once_with('glance') + mock_sess.assert_has_calls([ + mock.call('glance'), + mock.call('glance', auth=mock.sentinel.auth) + ]) mock_adapter.assert_called_once_with('glance', session=mock.sentinel.session, auth=mock.sentinel.auth) @@ -430,8 +398,11 @@ class CheckImageServiceTestCase(base.TestCase): wrapped_func = image_service.check_image_service(func) self.assertEqual(((), params), wrapped_func(self.service, **params)) - self._assert_client_call(mock_gclient, 'glance_url', user=True) - mock_sess.assert_called_once_with('glance') + self._assert_connnection_call(mock_gclient, 'glance_url') + mock_sess.assert_has_calls([ + mock.call('glance'), + mock.call('glance', auth=mock.sentinel.sauth) + ]) mock_adapter.assert_called_once_with('glance', session=mock.sentinel.session, auth=mock.sentinel.auth) @@ -455,26 +426,17 @@ class CheckImageServiceTestCase(base.TestCase): wrapped_func = image_service.check_image_service(func) self.assertEqual(((), params), wrapped_func(self.service, **params)) self.assertEqual('none', image_service.CONF.glance.auth_type) - self._assert_client_call(mock_gclient, 'foo') - mock_sess.assert_called_once_with('glance') + self._assert_connnection_call(mock_gclient, 'foo') + mock_sess.assert_has_calls([ + mock.call('glance'), + mock.call('glance', auth=mock.sentinel.auth) + ]) mock_adapter.assert_called_once_with('glance', session=mock.sentinel.session, auth=mock.sentinel.auth) self.assertEqual(0, mock_sauth.call_count) -def _create_failing_glance_client(info): - class MyGlanceStubClient(stubs.StubGlanceClient): - """A client that fails the first time, then succeeds.""" - def get(self, image_id): - info['num_calls'] += 1 - if info['num_calls'] == 1: - raise glance_exc.ServiceUnavailable('') - return {} - - return MyGlanceStubClient() - - class TestGlanceSwiftTempURL(base.TestCase): def setUp(self): super(TestGlanceSwiftTempURL, self).setUp() diff --git a/ironic/tests/unit/stubs.py b/ironic/tests/unit/stubs.py index f048b916aa..6927f38d2a 100644 --- a/ironic/tests/unit/stubs.py +++ b/ironic/tests/unit/stubs.py @@ -12,43 +12,43 @@ # License for the specific language governing permissions and limitations # under the License. -from glanceclient import exc as glance_exc +import io + +from openstack.connection import exceptions as openstack_exc NOW_GLANCE_FORMAT = "2010-10-11T10:30:22" -class _GlanceWrapper(object): - def __init__(self, wrapped): - self.wrapped = wrapped - - def __iter__(self): - return iter(()) - - class StubGlanceClient(object): - fake_wrapped = object() + image_data = b'this is an image' def __init__(self, images=None): self._images = [] _images = images or [] map(lambda image: self.create(**image), _images) - # NOTE(bcwaldon): HACK to get client.images.* to work - self.images = lambda: None - for fn in ('get', 'data'): - setattr(self.images, fn, getattr(self, fn)) - - def get(self, image_id): + def get_image(self, image_id): for image in self._images: if image.id == str(image_id): return image - raise glance_exc.NotFound(image_id) + raise openstack_exc.NotFoundException(image_id) - def data(self, image_id): - self.get(image_id) - return _GlanceWrapper(self.fake_wrapped) + def download_image(self, image_id, stream=False): + self.get_image(image_id) + if stream: + return io.BytesIO(self.image_data) + else: + return FakeImageDownload(self.image_data) + + +class FakeImageDownload(object): + + content = None + + def __init__(self, content): + self.content = content class FakeImage(dict): diff --git a/releasenotes/notes/glanceclient-removal-33b382ad03772530.yaml b/releasenotes/notes/glanceclient-removal-33b382ad03772530.yaml new file mode 100644 index 0000000000..34e8849ddf --- /dev/null +++ b/releasenotes/notes/glanceclient-removal-33b382ad03772530.yaml @@ -0,0 +1,5 @@ +--- +upgrade: + - | + `python-glanceclient` is no longer a dependency, all OpenStack Glance + operations are now done using `openstacksdk`. \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index b93b3f9ef9..f39f153d67 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,6 @@ automaton>=1.9.0 # Apache-2.0 eventlet>=0.30.1 # MIT WebOb>=1.7.1 # MIT python-cinderclient!=4.0.0,>=3.3.0 # Apache-2.0 -python-glanceclient>=2.8.0 # Apache-2.0 keystoneauth1>=4.2.0 # Apache-2.0 ironic-lib>=6.0.0 # Apache-2.0 stevedore>=1.29.0 # Apache-2.0