openstack/nova
Code Issues Proposed changes

Better glance exception handling

Browse Source 
- 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
This commit is contained in:
Mark Washenberger 2012-03-02 15:57:55 -05:00
parent 0d78045e72
commit f56cef93ea
5 changed files with 124 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
nova/api/openstack/compute/images.py
View File

@ -171,8 +171,11 @@ class Controller(wsgi.Controller):
for key, val in page_params.iteritems():
params[key] = val
images = self._image_service.index(context, filters=filters,
**page_params)
try:
images = self._image_service.index(context, filters=filters,
**page_params)
except exception.Invalid as e:
raise webob.exc.HTTPBadRequest(explanation=str(e))
return self._view_builder.index(req, images)
@wsgi.serializers(xml=ImagesTemplate)
@ -188,8 +191,11 @@ class Controller(wsgi.Controller):
page_params = common.get_pagination_params(req)
for key, val in page_params.iteritems():
params[key] = val
images = self._image_service.detail(context, filters=filters,
**page_params)
try:
images = self._image_service.detail(context, filters=filters,
**page_params)
except exception.Invalid as e:
raise webob.exc.HTTPBadRequest(explanation=str(e))
return self._view_builder.detail(req, images)

59
nova/image/glance.py
View File

@ -23,6 +23,7 @@ import copy
import datetime
import json
import random
import sys
import time
import urlparse
@ -202,7 +203,10 @@ class GlanceImageService(object):
def _fetch_images(self, fetch_func, **kwargs):
"""Paginate through results from glance server"""
images = fetch_func(**kwargs)
try:
images = fetch_func(**kwargs)
except Exception:
_reraise_translated_exception()
if not images:
# break out of recursive loop to end pagination
@ -234,8 +238,8 @@ class GlanceImageService(object):
try:
image_meta = self._call_retry(context, 'get_image_meta',
image_id)
except glance_exception.NotFound:
raise exception.ImageNotFound(image_id=image_id)
except Exception:
_reraise_translated_image_exception(image_id)
if not self._is_image_available(context, image_meta):
raise exception.ImageNotFound(image_id=image_id)
@ -256,8 +260,8 @@ class GlanceImageService(object):
try:
image_meta, image_chunks = self._call_retry(context, 'get_image',
image_id)
except glance_exception.NotFound:
raise exception.ImageNotFound(image_id=image_id)
except Exception:
_reraise_translated_image_exception(image_id)
for chunk in image_chunks:
data.write(chunk)
@ -296,14 +300,11 @@ class GlanceImageService(object):
# NOTE(vish): show is to check if image is available
self.show(context, image_id)
image_meta = self._translate_to_glance(image_meta)
client = self._get_client(context)
try:
client = self._get_client(context)
image_meta = client.update_image(image_id, image_meta, data)
except glance_exception.NotFound:
raise exception.ImageNotFound(image_id=image_id)
# NOTE(vish): this gets raised for public images
except glance_exception.MissingCredentialError:
raise exception.ImageNotAuthorized(image_id=image_id)
except Exception:
_reraise_translated_image_exception(image_id)
base_image_meta = self._translate_from_glance(image_meta)
return base_image_meta
@ -468,3 +469,39 @@ def _remove_read_only(image_meta):
if attr in output:
del output[attr]
return output
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)
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)
raise new_exc, None, exc_trace
def _translate_image_exception(image_id, exc_type, exc_value):
if exc_type in (glance_exception.NotAuthorized,
glance_exception.MissingCredentialError):
return exception.ImageNotAuthorized(image_id=image_id)
if exc_type is glance_exception.NotFound:
return exception.ImageNotFound(image_id=image_id)
if exc_type is glance_exception.Invalid:
return exception.Invalid(exc_value)
return exc_value
def _translate_plain_exception(exc_type, exc_value):
if exc_type in (glance_exception.NotAuthorized,
glance_exception.MissingCredentialError):
return exception.NotAuthorized(exc_value)
if exc_type is glance_exception.NotFound:
return exception.NotFound(exc_value)
if exc_type is glance_exception.Invalid:
return exception.Invalid(exc_value)
return exc_value

25
nova/tests/api/openstack/compute/test_images.py
View File

@ -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')

10
nova/tests/glance/stubs.py
View File

@ -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()

37
nova/tests/image/test_glance.py
View File

@ -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']