Merge "Replace glanceclient usage with openstacksdk"

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

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 keystoneauth1 import exceptions as ks_exception
-from glanceclient import exc as glance_exc
+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,
+    if isinstance(exc_value, (openstack_exc.ForbiddenException)):
-                              glance_exc.Unauthorized)):
         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,
+        sess = keystone.get_session('glance',
-                                    auth=user_auth or service_auth,
+                                    auth=user_auth or service_auth)
-                                    endpoint_override=self.endpoint,
+        conn = openstack.connection.Connection(
-                                    global_request_id=self.context.global_id)
+            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)
+            return getattr(self.client, method)(*args, **kwargs)
-        except retry_excs as e:
+        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)
+        image_size = 0
-        # NOTE(dtantsur): when using Glance V2, image_chunks is a wrapper
+        image_data = None
-        # around real data, so we have to check the wrapped data for None.
+        if data:
-        if image_chunks.wrapped is None:
+            image_chunks = self.call('download_image', image_id, stream=True)
-            raise exception.ImageDownloadFailed(
-                image_href=image_href, reason=_('image contains no data.'))
-
-        if data is None:
-            return image_chunks
-        else:
             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)

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',

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 keystoneauth1 import exceptions as ks_exception
-from glanceclient import exc as glance_exc
 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):
+            def get_image(self, image_id):
-                raise glance_exc.Forbidden(image_id)
+                raise openstack_exc.ForbiddenException()
-
-        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)
 
         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):
+            def get_image(self, image_id):
-                raise glance_exc.NotFound(image_id)
+                raise openstack_exc.NotFoundException()
-
-        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)
 
         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',
+            image_endpoint_override=url,
-            auth=mock.sentinel.sauth if user else mock.sentinel.auth,
+            image_api_version='2')
-            endpoint_override=url)
 
     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)
+        self._assert_connnection_call(mock_gclient, 'glance_url')
-        mock_sess.assert_called_once_with('glance')
+        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')
+        self._assert_connnection_call(mock_gclient, 'foo')
-        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)
         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()

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
+    def get_image(self, image_id):
-        self.images = lambda: None
-        for fn in ('get', 'data'):
-            setattr(self.images, fn, getattr(self, fn))
-
-    def get(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):
+    def download_image(self, image_id, stream=False):
-        self.get(image_id)
+        self.get_image(image_id)
-        return _GlanceWrapper(self.fake_wrapped)
+        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):

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`.

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