Removes capture of exception from eventlet in _upload_and_activate(), which catches the exceptions that come from the _safe_kill() method properly.
Also fixes an incorrect call to _safe_kill() with mapping instead of image ID in the block of code that kills an image if a bad checksum is given. Fixes bug #759018.
This commit is contained in:
@@ -227,17 +227,20 @@ class Controller(wsgi.Controller):
|
|||||||
:raises HTTPConflict if image already exists
|
:raises HTTPConflict if image already exists
|
||||||
:retval The location where the image was stored
|
:retval The location where the image was stored
|
||||||
"""
|
"""
|
||||||
|
image_id = image_meta['id']
|
||||||
content_type = req.headers.get('content-type', 'notset')
|
content_type = req.headers.get('content-type', 'notset')
|
||||||
if content_type != 'application/octet-stream':
|
if content_type != 'application/octet-stream':
|
||||||
raise HTTPBadRequest(
|
self._safe_kill(req, image_id)
|
||||||
"Content-Type must be application/octet-stream")
|
msg = ("Content-Type must be application/octet-stream")
|
||||||
|
logger.error(msg)
|
||||||
|
raise HTTPBadRequest(msg, content_type="text/plain",
|
||||||
|
request=req)
|
||||||
|
|
||||||
store_name = req.headers.get(
|
store_name = req.headers.get(
|
||||||
'x-image-meta-store', self.options['default_store'])
|
'x-image-meta-store', self.options['default_store'])
|
||||||
|
|
||||||
store = self.get_store_or_400(req, store_name)
|
store = self.get_store_or_400(req, store_name)
|
||||||
|
|
||||||
image_id = image_meta['id']
|
|
||||||
logger.debug("Setting image %s to status 'saving'"
|
logger.debug("Setting image %s to status 'saving'"
|
||||||
% image_id)
|
% image_id)
|
||||||
registry.update_image_metadata(self.options, image_id,
|
registry.update_image_metadata(self.options, image_id,
|
||||||
@@ -257,7 +260,8 @@ class Controller(wsgi.Controller):
|
|||||||
"checksum generated from uploaded image "
|
"checksum generated from uploaded image "
|
||||||
"(%(checksum)s) did not match. Setting image "
|
"(%(checksum)s) did not match. Setting image "
|
||||||
"status to 'killed'.") % locals()
|
"status to 'killed'.") % locals()
|
||||||
self._safe_kill(req, image_meta)
|
logger.error(msg)
|
||||||
|
self._safe_kill(req, image_id)
|
||||||
raise HTTPBadRequest(msg, content_type="text/plain",
|
raise HTTPBadRequest(msg, content_type="text/plain",
|
||||||
request=req)
|
request=req)
|
||||||
|
|
||||||
@@ -272,8 +276,15 @@ class Controller(wsgi.Controller):
|
|||||||
|
|
||||||
return location
|
return location
|
||||||
except exception.Duplicate, e:
|
except exception.Duplicate, e:
|
||||||
logger.error("Error adding image to store: %s", str(e))
|
msg = ("Attempt to upload duplicate image: %s") % str(e)
|
||||||
raise HTTPConflict(str(e), request=req)
|
logger.error(msg)
|
||||||
|
self._safe_kill(req, image_id)
|
||||||
|
raise HTTPConflict(msg, request=req)
|
||||||
|
except Exception, e:
|
||||||
|
msg = ("Error uploading image: %s") % str(e)
|
||||||
|
logger.error(msg)
|
||||||
|
self._safe_kill(req, image_id)
|
||||||
|
raise HTTPBadRequest(msg, request=req)
|
||||||
|
|
||||||
def _activate(self, req, image_id, location):
|
def _activate(self, req, image_id, location):
|
||||||
"""
|
"""
|
||||||
@@ -329,22 +340,9 @@ class Controller(wsgi.Controller):
|
|||||||
|
|
||||||
:retval Mapping of updated image data
|
:retval Mapping of updated image data
|
||||||
"""
|
"""
|
||||||
try:
|
image_id = image_meta['id']
|
||||||
image_id = image_meta['id']
|
location = self._upload(req, image_meta)
|
||||||
location = self._upload(req, image_meta)
|
return self._activate(req, image_id, location)
|
||||||
return self._activate(req, image_id, location)
|
|
||||||
except: # unqualified b/c we're re-raising it
|
|
||||||
exc_type, exc_value, exc_traceback = sys.exc_info()
|
|
||||||
self._safe_kill(req, image_id)
|
|
||||||
# NOTE(sirp): _safe_kill uses httplib which, in turn, uses
|
|
||||||
# Eventlet's GreenSocket. Eventlet subsequently clears exceptions
|
|
||||||
# by calling `sys.exc_clear()`.
|
|
||||||
#
|
|
||||||
# This is why we can't use a raise with no arguments here: our
|
|
||||||
# exception context was destroyed by Eventlet. To work around
|
|
||||||
# this, we need to 'memorize' the exception context, and then
|
|
||||||
# re-raise here.
|
|
||||||
raise exc_type(exc_value)
|
|
||||||
|
|
||||||
def create(self, req):
|
def create(self, req):
|
||||||
"""
|
"""
|
||||||
|
@@ -415,6 +415,8 @@ class TestCurlApi(functional.FunctionalTest):
|
|||||||
"Size was supposed to be %d. Got:\n%s."
|
"Size was supposed to be %d. Got:\n%s."
|
||||||
% (FIVE_GB, out))
|
% (FIVE_GB, out))
|
||||||
|
|
||||||
|
self.stop_servers()
|
||||||
|
|
||||||
def test_traceback_not_consumed(self):
|
def test_traceback_not_consumed(self):
|
||||||
"""
|
"""
|
||||||
A test that errors coming from the POST API do not
|
A test that errors coming from the POST API do not
|
||||||
@@ -447,3 +449,5 @@ class TestCurlApi(functional.FunctionalTest):
|
|||||||
expected = "Content-Type must be application/octet-stream"
|
expected = "Content-Type must be application/octet-stream"
|
||||||
self.assertTrue(expected in out,
|
self.assertTrue(expected in out,
|
||||||
"Could not find '%s' in '%s'" % (expected, out))
|
"Could not find '%s' in '%s'" % (expected, out))
|
||||||
|
|
||||||
|
self.stop_servers()
|
||||||
|
Reference in New Issue
Block a user