]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Make glance image service check base exception classes
authorJohn Griffith <john.griffith@solidfire.com>
Wed, 22 Aug 2012 22:26:15 +0000 (16:26 -0600)
committerJohn Griffith <john.griffith@solidfire.com>
Wed, 22 Aug 2012 22:27:56 +0000 (16:27 -0600)
Fixes bug 1039675

glanceclient can raise HTTPNotFound (as well as NotFound) it appears,
but glance image service is only converting NotFound ->
InstanceNotFound.  Same applies to 'Forbidden' and other exceptions.

This patch converts 'exc_type is NotFound'-like checks to use
'isinstance' instead, which will cover HTTPNotFound, etc.

Also add missing glance/image tests that weren't in Cinder

Change-Id: I8787598daccf401a26d9397946ecd55f89ff62b3

cinder/image/glance.py
cinder/tests/glance/__init__.py [new file with mode: 0644]
cinder/tests/glance/stubs.py [new file with mode: 0644]
cinder/tests/image/test_glance.py [new file with mode: 0644]

index 32a9b9bf30e8fcf38eb29ad9bee0f3e6e2d7c7a8..c81be9e40460a9e1ae2051d4493536784d551cd5 100644 (file)
@@ -365,35 +365,35 @@ def _remove_read_only(image_meta):
 def _reraise_translated_image_exception(image_id):
     """Transform the exception for the image but keep its traceback intact."""
     exc_type, exc_value, exc_trace = sys.exc_info()
-    new_exc = _translate_image_exception(image_id, exc_type, exc_value)
+    new_exc = _translate_image_exception(image_id, exc_value)
     raise new_exc, None, exc_trace
 
 
 def _reraise_translated_exception():
     """Transform the exception but keep its traceback intact."""
     exc_type, exc_value, exc_trace = sys.exc_info()
-    new_exc = _translate_plain_exception(exc_type, exc_value)
+    new_exc = _translate_plain_exception(exc_value)
     raise new_exc, None, exc_trace
 
 
-def _translate_image_exception(image_id, exc_type, exc_value):
-    if exc_type in (glanceclient.exc.Forbidden,
-                    glanceclient.exc.Unauthorized):
+def _translate_image_exception(image_id, exc_value):
+    if isinstance(exc_value, (glanceclient.exc.Forbidden,
+                    glanceclient.exc.Unauthorized)):
         return exception.ImageNotAuthorized(image_id=image_id)
-    if exc_type is glanceclient.exc.NotFound:
+    if isinstance(exc_value, glanceclient.exc.NotFound):
         return exception.ImageNotFound(image_id=image_id)
-    if exc_type is glanceclient.exc.BadRequest:
+    if isinstance(exc_value, glanceclient.exc.BadRequest):
         return exception.Invalid(exc_value)
     return exc_value
 
 
-def _translate_plain_exception(exc_type, exc_value):
-    if exc_type in (glanceclient.exc.Forbidden,
-                    glanceclient.exc.Unauthorized):
+def _translate_plain_exception(exc_value):
+    if isinstance(exc_value, (glanceclient.exc.Forbidden,
+                    glanceclient.exc.Unauthorized)):
         return exception.NotAuthorized(exc_value)
-    if exc_type is glanceclient.exc.NotFound:
+    if isinstance(exc_value, glanceclient.exc.NotFound):
         return exception.NotFound(exc_value)
-    if exc_type is glanceclient.exc.BadRequest:
+    if isinstance(exc_value, glanceclient.exc.BadRequest):
         return exception.Invalid(exc_value)
     return exc_value
 
diff --git a/cinder/tests/glance/__init__.py b/cinder/tests/glance/__init__.py
new file mode 100644 (file)
index 0000000..ef9fa05
--- /dev/null
@@ -0,0 +1,20 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+#    Copyright (c) 2011 Citrix Systems, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+:mod:`glance` -- Stubs for Glance
+=================================
+"""
diff --git a/cinder/tests/glance/stubs.py b/cinder/tests/glance/stubs.py
new file mode 100644 (file)
index 0000000..076afef
--- /dev/null
@@ -0,0 +1,112 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright (c) 2011 Citrix Systems, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import glanceclient.exc
+
+
+NOW_GLANCE_FORMAT = "2010-10-11T10:30:22"
+
+
+class StubGlanceClient(object):
+
+    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 ('list', 'get', 'data', 'create', 'update', 'delete'):
+            setattr(self.images, fn, getattr(self, fn))
+
+    #TODO(bcwaldon): implement filters
+    def list(self, filters=None, marker=None, limit=30):
+        if marker is None:
+            index = 0
+        else:
+            for index, image in enumerate(self._images):
+                if image.id == str(marker):
+                    index += 1
+                    break
+            else:
+                raise glanceclient.exc.BadRequest('Marker not found')
+
+        return self._images[index:index + limit]
+
+    def get(self, image_id):
+        for image in self._images:
+            if image.id == str(image_id):
+                return image
+        raise glanceclient.exc.NotFound(image_id)
+
+    def data(self, image_id):
+        self.get(image_id)
+        return []
+
+    def create(self, **metadata):
+        metadata['created_at'] = NOW_GLANCE_FORMAT
+        metadata['updated_at'] = NOW_GLANCE_FORMAT
+
+        self._images.append(FakeImage(metadata))
+
+        try:
+            image_id = str(metadata['id'])
+        except KeyError:
+            # auto-generate an id if one wasn't provided
+            image_id = str(len(self._images))
+
+        self._images[-1].id = image_id
+
+        return self._images[-1]
+
+    def update(self, image_id, **metadata):
+        for i, image in enumerate(self._images):
+            if image.id == str(image_id):
+                for k, v in metadata.items():
+                    setattr(self._images[i], k, v)
+                return self._images[i]
+        raise glanceclient.exc.NotFound(image_id)
+
+    def delete(self, image_id):
+        for i, image in enumerate(self._images):
+            if image.id == image_id:
+                del self._images[i]
+                return
+        raise glanceclient.exc.NotFound(image_id)
+
+
+class FakeImage(object):
+    def __init__(self, metadata):
+        IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner',
+                            'container_format', 'checksum', 'id',
+                            'name', 'created_at', 'updated_at',
+                            'deleted', 'status',
+                            'min_disk', 'min_ram', 'is_public']
+        raw = dict.fromkeys(IMAGE_ATTRIBUTES)
+        raw.update(metadata)
+        self.__dict__['raw'] = raw
+
+    def __getattr__(self, key):
+        try:
+            return self.__dict__['raw'][key]
+        except KeyError:
+            raise AttributeError(key)
+
+    def __setattr__(self, key, value):
+        try:
+            self.__dict__['raw'][key] = value
+        except KeyError:
+            raise AttributeError(key)
diff --git a/cinder/tests/image/test_glance.py b/cinder/tests/image/test_glance.py
new file mode 100644 (file)
index 0000000..4f2d5f1
--- /dev/null
@@ -0,0 +1,546 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+import datetime
+import random
+import time
+
+import glanceclient.exc
+
+from cinder import context
+from cinder import exception
+from cinder.image import glance
+from cinder import test
+from cinder.tests.api.openstack import fakes
+from cinder.tests.glance import stubs as glance_stubs
+
+
+class NullWriter(object):
+    """Used to test ImageService.get which takes a writer object"""
+
+    def write(self, *arg, **kwargs):
+        pass
+
+
+class TestGlanceSerializer(test.TestCase):
+    def test_serialize(self):
+        metadata = {'name': 'image1',
+                    'is_public': True,
+                    'foo': 'bar',
+                    'properties': {
+                        'prop1': 'propvalue1',
+                        'mappings': [
+                            {'virtual': 'aaa',
+                             'device': 'bbb'},
+                            {'virtual': 'xxx',
+                             'device': 'yyy'}],
+                        'block_device_mapping': [
+                            {'virtual_device': 'fake',
+                             'device_name': '/dev/fake'},
+                            {'virtual_device': 'ephemeral0',
+                             'device_name': '/dev/fake0'}]}}
+
+        converted_expected = {
+            'name': 'image1',
+            'is_public': True,
+            'foo': 'bar',
+            'properties': {
+                'prop1': 'propvalue1',
+                'mappings':
+                '[{"device": "bbb", "virtual": "aaa"}, '
+                '{"device": "yyy", "virtual": "xxx"}]',
+                'block_device_mapping':
+                '[{"virtual_device": "fake", "device_name": "/dev/fake"}, '
+                '{"virtual_device": "ephemeral0", '
+                '"device_name": "/dev/fake0"}]'}}
+        converted = glance._convert_to_string(metadata)
+        self.assertEqual(converted, converted_expected)
+        self.assertEqual(glance._convert_from_string(converted), metadata)
+
+
+class TestGlanceImageService(test.TestCase):
+    """
+    Tests the Glance image service.
+
+    At a high level, the translations involved are:
+
+        1. Glance -> ImageService - This is needed so we can support
+           multple ImageServices (Glance, Local, etc)
+
+        2. ImageService -> API - This is needed so we can support multple
+           APIs (OpenStack, EC2)
+
+    """
+    NOW_GLANCE_OLD_FORMAT = "2010-10-11T10:30:22"
+    NOW_GLANCE_FORMAT = "2010-10-11T10:30:22.000000"
+
+    class tzinfo(datetime.tzinfo):
+        @staticmethod
+        def utcoffset(*args, **kwargs):
+            return datetime.timedelta()
+
+    NOW_DATETIME = datetime.datetime(2010, 10, 11, 10, 30, 22, tzinfo=tzinfo())
+
+    def setUp(self):
+        super(TestGlanceImageService, self).setUp()
+        #fakes.stub_out_compute_api_snapshot(self.stubs)
+
+        client = glance_stubs.StubGlanceClient()
+        self.service = self._create_image_service(client)
+        self.context = context.RequestContext('fake', 'fake', auth_token=True)
+
+    def _create_image_service(self, client):
+        def _fake_create_glance_client(context, host, port, version):
+            return client
+
+        self.stubs.Set(glance, '_create_glance_client',
+                _fake_create_glance_client)
+
+        client_wrapper = glance.GlanceClientWrapper(
+                'fake', 'fake_host', 9292)
+        return glance.GlanceImageService(client=client_wrapper)
+
+    @staticmethod
+    def _make_fixture(**kwargs):
+        fixture = {'name': None,
+                   'properties': {},
+                   'status': None,
+                   'is_public': None}
+        fixture.update(kwargs)
+        return fixture
+
+    def _make_datetime_fixture(self):
+        return self._make_fixture(created_at=self.NOW_GLANCE_FORMAT,
+                                  updated_at=self.NOW_GLANCE_FORMAT,
+                                  deleted_at=self.NOW_GLANCE_FORMAT)
+
+    def test_create_with_instance_id(self):
+        """Ensure instance_id is persisted as an image-property"""
+        fixture = {'name': 'test image',
+                   'is_public': False,
+                   'properties': {'instance_id': '42', 'user_id': 'fake'}}
+
+        image_id = self.service.create(self.context, fixture)['id']
+        image_meta = self.service.show(self.context, image_id)
+        expected = {
+            'id': image_id,
+            'name': 'test image',
+            'is_public': False,
+            'size': None,
+            'min_disk': None,
+            'min_ram': None,
+            'disk_format': None,
+            'container_format': None,
+            'checksum': None,
+            'created_at': self.NOW_DATETIME,
+            'updated_at': self.NOW_DATETIME,
+            'deleted_at': None,
+            'deleted': None,
+            'status': None,
+            'properties': {'instance_id': '42', 'user_id': 'fake'},
+            'owner': None,
+        }
+        self.assertDictMatch(image_meta, expected)
+
+        image_metas = self.service.detail(self.context)
+        self.assertDictMatch(image_metas[0], expected)
+
+    def test_create_without_instance_id(self):
+        """
+        Ensure we can create an image without having to specify an
+        instance_id. Public images are an example of an image not tied to an
+        instance.
+        """
+        fixture = {'name': 'test image', 'is_public': False}
+        image_id = self.service.create(self.context, fixture)['id']
+
+        expected = {
+            'id': image_id,
+            'name': 'test image',
+            'is_public': False,
+            'size': None,
+            'min_disk': None,
+            'min_ram': None,
+            'disk_format': None,
+            'container_format': None,
+            'checksum': None,
+            'created_at': self.NOW_DATETIME,
+            'updated_at': self.NOW_DATETIME,
+            'deleted_at': None,
+            'deleted': None,
+            'status': None,
+            'properties': {},
+            'owner': None,
+        }
+        actual = self.service.show(self.context, image_id)
+        self.assertDictMatch(actual, expected)
+
+    def test_create(self):
+        fixture = self._make_fixture(name='test image')
+        num_images = len(self.service.detail(self.context))
+        image_id = self.service.create(self.context, fixture)['id']
+
+        self.assertNotEquals(None, image_id)
+        self.assertEquals(num_images + 1,
+                          len(self.service.detail(self.context)))
+
+    def test_create_and_show_non_existing_image(self):
+        fixture = self._make_fixture(name='test image')
+        image_id = self.service.create(self.context, fixture)['id']
+
+        self.assertNotEquals(None, image_id)
+        self.assertRaises(exception.ImageNotFound,
+                          self.service.show,
+                          self.context,
+                          'bad image id')
+
+    def test_detail_private_image(self):
+        fixture = self._make_fixture(name='test image')
+        fixture['is_public'] = False
+        properties = {'owner_id': 'proj1'}
+        fixture['properties'] = properties
+
+        self.service.create(self.context, fixture)['id']
+
+        proj = self.context.project_id
+        self.context.project_id = 'proj1'
+
+        image_metas = self.service.detail(self.context)
+
+        self.context.project_id = proj
+
+        self.assertEqual(1, len(image_metas))
+        self.assertEqual(image_metas[0]['name'], 'test image')
+        self.assertEqual(image_metas[0]['is_public'], False)
+
+    def test_detail_marker(self):
+        fixtures = []
+        ids = []
+        for i in range(10):
+            fixture = self._make_fixture(name='TestImage %d' % (i))
+            fixtures.append(fixture)
+            ids.append(self.service.create(self.context, fixture)['id'])
+
+        image_metas = self.service.detail(self.context, marker=ids[1])
+        self.assertEquals(len(image_metas), 8)
+        i = 2
+        for meta in image_metas:
+            expected = {
+                'id': ids[i],
+                'status': None,
+                'is_public': None,
+                'name': 'TestImage %d' % (i),
+                'properties': {},
+                'size': None,
+                'min_disk': None,
+                'min_ram': None,
+                'disk_format': None,
+                'container_format': None,
+                'checksum': None,
+                'created_at': self.NOW_DATETIME,
+                'updated_at': self.NOW_DATETIME,
+                'deleted_at': None,
+                'deleted': None,
+                'owner': None,
+            }
+
+            self.assertDictMatch(meta, expected)
+            i = i + 1
+
+    def test_detail_limit(self):
+        fixtures = []
+        ids = []
+        for i in range(10):
+            fixture = self._make_fixture(name='TestImage %d' % (i))
+            fixtures.append(fixture)
+            ids.append(self.service.create(self.context, fixture)['id'])
+
+        image_metas = self.service.detail(self.context, limit=5)
+        self.assertEquals(len(image_metas), 5)
+
+    def test_detail_default_limit(self):
+        fixtures = []
+        ids = []
+        for i in range(10):
+            fixture = self._make_fixture(name='TestImage %d' % (i))
+            fixtures.append(fixture)
+            ids.append(self.service.create(self.context, fixture)['id'])
+
+        image_metas = self.service.detail(self.context)
+        for i, meta in enumerate(image_metas):
+            self.assertEqual(meta['name'], 'TestImage %d' % (i))
+
+    def test_detail_marker_and_limit(self):
+        fixtures = []
+        ids = []
+        for i in range(10):
+            fixture = self._make_fixture(name='TestImage %d' % (i))
+            fixtures.append(fixture)
+            ids.append(self.service.create(self.context, fixture)['id'])
+
+        image_metas = self.service.detail(self.context, marker=ids[3], limit=5)
+        self.assertEquals(len(image_metas), 5)
+        i = 4
+        for meta in image_metas:
+            expected = {
+                'id': ids[i],
+                'status': None,
+                'is_public': None,
+                'name': 'TestImage %d' % (i),
+                'properties': {},
+                'size': None,
+                'min_disk': None,
+                'min_ram': None,
+                'disk_format': None,
+                'container_format': None,
+                'checksum': None,
+                'created_at': self.NOW_DATETIME,
+                'updated_at': self.NOW_DATETIME,
+                'deleted_at': None,
+                'deleted': None,
+                'owner': None,
+            }
+            self.assertDictMatch(meta, expected)
+            i = i + 1
+
+    def test_detail_invalid_marker(self):
+        fixtures = []
+        ids = []
+        for i in range(10):
+            fixture = self._make_fixture(name='TestImage %d' % (i))
+            fixtures.append(fixture)
+            ids.append(self.service.create(self.context, fixture)['id'])
+
+        self.assertRaises(exception.Invalid, self.service.detail,
+                          self.context, marker='invalidmarker')
+
+    def test_update(self):
+        fixture = self._make_fixture(name='test image')
+        image = self.service.create(self.context, fixture)
+        print image
+        image_id = image['id']
+        fixture['name'] = 'new image name'
+        self.service.update(self.context, image_id, fixture)
+
+        new_image_data = self.service.show(self.context, image_id)
+        self.assertEquals('new image name', new_image_data['name'])
+
+    def test_delete(self):
+        fixture1 = self._make_fixture(name='test image 1')
+        fixture2 = self._make_fixture(name='test image 2')
+        fixtures = [fixture1, fixture2]
+
+        num_images = len(self.service.detail(self.context))
+        self.assertEquals(0, num_images)
+
+        ids = []
+        for fixture in fixtures:
+            new_id = self.service.create(self.context, fixture)['id']
+            ids.append(new_id)
+
+        num_images = len(self.service.detail(self.context))
+        self.assertEquals(2, num_images)
+
+        self.service.delete(self.context, ids[0])
+
+        num_images = len(self.service.detail(self.context))
+        self.assertEquals(1, num_images)
+
+    def test_show_passes_through_to_client(self):
+        fixture = self._make_fixture(name='image1', is_public=True)
+        image_id = self.service.create(self.context, fixture)['id']
+
+        image_meta = self.service.show(self.context, image_id)
+        expected = {
+            'id': image_id,
+            'name': 'image1',
+            'is_public': True,
+            'size': None,
+            'min_disk': None,
+            'min_ram': None,
+            'disk_format': None,
+            'container_format': None,
+            'checksum': None,
+            'created_at': self.NOW_DATETIME,
+            'updated_at': self.NOW_DATETIME,
+            'deleted_at': None,
+            'deleted': None,
+            'status': None,
+            'properties': {},
+            'owner': None,
+        }
+        self.assertEqual(image_meta, expected)
+
+    def test_show_raises_when_no_authtoken_in_the_context(self):
+        fixture = self._make_fixture(name='image1',
+                                     is_public=False,
+                                     properties={'one': 'two'})
+        image_id = self.service.create(self.context, fixture)['id']
+        self.context.auth_token = False
+        self.assertRaises(exception.ImageNotFound,
+                          self.service.show,
+                          self.context,
+                          image_id)
+
+    def test_detail_passes_through_to_client(self):
+        fixture = self._make_fixture(name='image10', is_public=True)
+        image_id = self.service.create(self.context, fixture)['id']
+        image_metas = self.service.detail(self.context)
+        expected = [
+            {
+                'id': image_id,
+                'name': 'image10',
+                'is_public': True,
+                'size': None,
+                'min_disk': None,
+                'min_ram': None,
+                'disk_format': None,
+                'container_format': None,
+                'checksum': None,
+                'created_at': self.NOW_DATETIME,
+                'updated_at': self.NOW_DATETIME,
+                'deleted_at': None,
+                'deleted': None,
+                'status': None,
+                'properties': {},
+                'owner': None,
+            },
+        ]
+        self.assertEqual(image_metas, expected)
+
+    def test_show_makes_datetimes(self):
+        fixture = self._make_datetime_fixture()
+        image_id = self.service.create(self.context, fixture)['id']
+        image_meta = self.service.show(self.context, image_id)
+        self.assertEqual(image_meta['created_at'], self.NOW_DATETIME)
+        self.assertEqual(image_meta['updated_at'], self.NOW_DATETIME)
+
+    def test_detail_makes_datetimes(self):
+        fixture = self._make_datetime_fixture()
+        self.service.create(self.context, fixture)
+        image_meta = self.service.detail(self.context)[0]
+        self.assertEqual(image_meta['created_at'], self.NOW_DATETIME)
+        self.assertEqual(image_meta['updated_at'], self.NOW_DATETIME)
+
+    def test_download_with_retries(self):
+        tries = [0]
+
+        class MyGlanceStubClient(glance_stubs.StubGlanceClient):
+            """A client that fails the first time, then succeeds."""
+            def get(self, image_id):
+                if tries[0] == 0:
+                    tries[0] = 1
+                    raise glanceclient.exc.ServiceUnavailable('')
+                else:
+                    return {}
+
+        client = MyGlanceStubClient()
+        service = self._create_image_service(client)
+        image_id = 1  # doesn't matter
+        writer = NullWriter()
+
+        # When retries are disabled, we should get an exception
+        self.flags(glance_num_retries=0)
+        self.assertRaises(exception.GlanceConnectionFailed,
+                service.download, self.context, image_id, writer)
+
+        # Now lets enable retries. No exception should happen now.
+        tries = [0]
+        self.flags(glance_num_retries=1)
+        service.download(self.context, image_id, writer)
+
+    def test_client_forbidden_converts_to_imagenotauthed(self):
+        class MyGlanceStubClient(glance_stubs.StubGlanceClient):
+            """A client that raises a Forbidden exception."""
+            def get(self, image_id):
+                raise glanceclient.exc.Forbidden(image_id)
+
+        client = MyGlanceStubClient()
+        service = self._create_image_service(client)
+        image_id = 1  # doesn't matter
+        writer = NullWriter()
+        self.assertRaises(exception.ImageNotAuthorized, service.download,
+                          self.context, image_id, writer)
+
+    def test_client_httpforbidden_converts_to_imagenotauthed(self):
+        class MyGlanceStubClient(glance_stubs.StubGlanceClient):
+            """A client that raises a HTTPForbidden exception."""
+            def get(self, image_id):
+                raise glanceclient.exc.HTTPForbidden(image_id)
+
+        client = MyGlanceStubClient()
+        service = self._create_image_service(client)
+        image_id = 1  # doesn't matter
+        writer = NullWriter()
+        self.assertRaises(exception.ImageNotAuthorized, service.download,
+                          self.context, image_id, writer)
+
+    def test_client_notfound_converts_to_imagenotfound(self):
+        class MyGlanceStubClient(glance_stubs.StubGlanceClient):
+            """A client that raises a NotFound exception."""
+            def get(self, image_id):
+                raise glanceclient.exc.NotFound(image_id)
+
+        client = MyGlanceStubClient()
+        service = self._create_image_service(client)
+        image_id = 1  # doesn't matter
+        writer = NullWriter()
+        self.assertRaises(exception.ImageNotFound, service.download,
+                          self.context, image_id, writer)
+
+    def test_client_httpnotfound_converts_to_imagenotfound(self):
+        class MyGlanceStubClient(glance_stubs.StubGlanceClient):
+            """A client that raises a HTTPNotFound exception."""
+            def get(self, image_id):
+                raise glanceclient.exc.HTTPNotFound(image_id)
+
+        client = MyGlanceStubClient()
+        service = self._create_image_service(client)
+        image_id = 1  # doesn't matter
+        writer = NullWriter()
+        self.assertRaises(exception.ImageNotFound, service.download,
+                          self.context, image_id, writer)
+
+    def test_glance_client_image_id(self):
+        fixture = self._make_fixture(name='test image')
+        image_id = self.service.create(self.context, fixture)['id']
+        (service, same_id) = glance.get_remote_image_service(
+                self.context, image_id)
+        self.assertEquals(same_id, image_id)
+
+    def test_glance_client_image_ref(self):
+        fixture = self._make_fixture(name='test image')
+        image_id = self.service.create(self.context, fixture)['id']
+        image_url = 'http://something-less-likely/%s' % image_id
+        (service, same_id) = glance.get_remote_image_service(
+                self.context, image_url)
+        self.assertEquals(same_id, image_id)
+        self.assertEquals(service._client.host,
+                'something-less-likely')
+
+
+def _create_failing_glance_client(info):
+    class MyGlanceStubClient(glance_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 glanceclient.exc.ServiceUnavailable('')
+            return {}
+
+    return MyGlanceStubClient()