Better glance exception handling

- Adds a conversion step that turns glance exceptions into nova
  exceptions
- Converts Invalid to HTTPBadRequest in /images and /images/detail,
  fixing bug 944846
- Makes stub glance client return glance exceptions instead of nova
  exceptions
- Rebased off of http://review.openstack.org/4788 to pick up
  MissingCredentialError handling as well, and added a test
- A few small, miscellaneous testing fixes for issues I noticed

Change-Id: I88eebfe7a7ac21cc5cd84ad84d64b311ddccf91e

3 files changed, 66 insertions, 6 deletions

diff --git a/nova/tests/api/openstack/compute/test_images.py b/nova/tests/api/openstack/compute/test_images.py
index 092485a10..0bcae01ce 100644
--- a/nova/tests/api/openstack/compute/test_images.py
+++ b/nova/tests/api/openstack/compute/test_images.py
@@ -25,6 +25,7 @@ import urlparse
 from lxml import etree
 import webob
 
+from nova import exception
 from nova import flags
 from nova.api.openstack.compute import images
 from nova.api.openstack.compute.views import images as images_view
@@ -903,10 +904,10 @@ class ImagesControllerTest(test.TestCase):
         request = fakes.HTTPRequest.blank('/v2/images?status=ACTIVE&'
                                           'UNSUPPORTEDFILTER=testname')
         context = request.environ['nova.context']
-        image_service.detail(context, filters=filters).AndReturn([])
+        image_service.index(context, filters=filters).AndReturn([])
         self.mox.ReplayAll()
         controller = images.Controller(image_service=image_service)
-        controller.detail(request)
+        controller.index(request)
 
     def test_image_no_filters(self):
         image_service = self.mox.CreateMockAnything()
@@ -918,6 +919,16 @@ class ImagesControllerTest(test.TestCase):
         controller = images.Controller(image_service=image_service)
         controller.index(request)
 
+    def test_image_invalid_marker(self):
+        class InvalidImageService(object):
+
+            def index(self, *args, **kwargs):
+                raise exception.Invalid('meow')
+
+        request = fakes.HTTPRequest.blank('/v2/images?marker=invalid')
+        controller = images.Controller(image_service=InvalidImageService())
+        self.assertRaises(webob.exc.HTTPBadRequest, controller.index, request)
+
     def test_image_detail_filter_with_name(self):
         image_service = self.mox.CreateMockAnything()
         filters = {'name': 'testname'}
@@ -1018,6 +1029,16 @@ class ImagesControllerTest(test.TestCase):
         controller = images.Controller(image_service=image_service)
         controller.detail(request)
 
+    def test_image_detail_invalid_marker(self):
+        class InvalidImageService(object):
+
+            def detail(self, *args, **kwargs):
+                raise exception.Invalid('meow')
+
+        request = fakes.HTTPRequest.blank('/v2/images?marker=invalid')
+        controller = images.Controller(image_service=InvalidImageService())
+        self.assertRaises(webob.exc.HTTPBadRequest, controller.detail, request)
+
     def test_generate_alternate_link(self):
         view = images_view.ViewBuilder()
         request = fakes.HTTPRequest.blank('/v2/fake/images/1')
diff --git a/nova/tests/glance/stubs.py b/nova/tests/glance/stubs.py
index 5c0f4f05e..ccf431b69 100644
--- a/nova/tests/glance/stubs.py
+++ b/nova/tests/glance/stubs.py
@@ -17,6 +17,8 @@
 import copy
 import StringIO
 
+from glance.common import exception as glance_exception
+
 from nova import exception
 from nova.image import glance
 
@@ -103,7 +105,7 @@ class StubGlanceClient(object):
         for image in self.images:
             if image['id'] == str(image_id):
                 return image
-        raise exception.ImageNotFound(image_id=image_id)
+        raise glance_exception.NotFound()
 
     #TODO(bcwaldon): implement filters
     def get_images_detailed(self, filters=None, marker=None, limit=3):
@@ -114,6 +116,8 @@ class StubGlanceClient(object):
                 if image['id'] == str(marker):
                     index += 1
                     break
+            else:
+                raise glance_exception.Invalid()
 
         return self.images[index:index + limit]
 
@@ -143,11 +147,11 @@ class StubGlanceClient(object):
                     metadata['id'] = str(metadata['id'])
                 self.images[i].update(metadata)
                 return self.images[i]
-        raise exception.ImageNotFound(image_id=image_id)
+        raise glance_exception.NotFound()
 
     def delete_image(self, image_id):
         for i, image in enumerate(self.images):
             if image['id'] == image_id:
                 del self.images[i]
                 return
-        raise exception.ImageNotFound(image_id=image_id)
+        raise glance_exception.NotFound()
diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py
index 6d965756a..6ed4c501f 100644
--- a/nova/tests/image/test_glance.py
+++ b/nova/tests/image/test_glance.py
@@ -18,6 +18,8 @@
 
 import datetime
 
+import glance.common.exception as glance_exception
+
 from nova.tests.api.openstack import fakes
 from nova import context
 from nova import exception
@@ -181,7 +183,7 @@ class TestGlanceImageService(test.TestCase):
         image_id = self.service.create(self.context, fixture)['id']
 
         self.assertNotEquals(None, image_id)
-        self.assertRaises(exception.NotFound,
+        self.assertRaises(exception.ImageNotFound,
                           self.service.show,
                           self.context,
                           'bad image id')
@@ -260,6 +262,17 @@ class TestGlanceImageService(test.TestCase):
             self.assertDictMatch(meta, expected)
             i = i + 1
 
+    def test_index_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.index,
+                          self.context, marker='invalidmarker')
+
     def test_index_private_image(self):
         fixture = self._make_fixture(name='test image')
         fixture['is_public'] = False
@@ -354,6 +367,17 @@ class TestGlanceImageService(test.TestCase):
             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_id = self.service.create(self.context, fixture)['id']
@@ -445,6 +469,17 @@ class TestGlanceImageService(test.TestCase):
                           self.context,
                           image_id)
 
+    def test_show_raises_on_missing_credential(self):
+        def raise_missing_credentials(*args, **kwargs):
+            raise glance_exception.MissingCredentialError()
+
+        self.stubs.Set(glance_stubs.StubGlanceClient, 'get_image_meta',
+                       raise_missing_credentials)
+        self.assertRaises(exception.ImageNotAuthorized,
+                          self.service.show,
+                          self.context,
+                          'test-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']