Merge "Cleanup chunks for deleted image that was 'saving'" into stable/icehouse
This commit is contained in:
commit
49e3c0a56d
|
@ -147,10 +147,10 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo):
|
|||
raise exception.Forbidden(message
|
||||
% self.image.image_id)
|
||||
|
||||
def save(self, image_member):
|
||||
def save(self, image_member, from_state=None):
|
||||
if (self.context.is_admin or
|
||||
self.context.owner == image_member.member_id):
|
||||
self.member_repo.save(image_member)
|
||||
self.member_repo.save(image_member, from_state=from_state)
|
||||
else:
|
||||
message = _("You cannot update image member %s")
|
||||
raise exception.Forbidden(message % image_member.member_id)
|
||||
|
|
|
@ -182,9 +182,9 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
|
|||
self.policy.enforce(self.context, 'get_images', {})
|
||||
return super(ImageRepoProxy, self).list(*args, **kwargs)
|
||||
|
||||
def save(self, image):
|
||||
def save(self, image, from_state=None):
|
||||
self.policy.enforce(self.context, 'modify_image', {})
|
||||
return super(ImageRepoProxy, self).save(image)
|
||||
return super(ImageRepoProxy, self).save(image, from_state=from_state)
|
||||
|
||||
def add(self, image):
|
||||
self.policy.enforce(self.context, 'add_image', {})
|
||||
|
@ -283,9 +283,9 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo):
|
|||
self.policy.enforce(self.context, 'get_member', {})
|
||||
return self.member_repo.get(member_id)
|
||||
|
||||
def save(self, member):
|
||||
def save(self, member, from_state=None):
|
||||
self.policy.enforce(self.context, 'modify_member', {})
|
||||
self.member_repo.save(member)
|
||||
self.member_repo.save(member, from_state=from_state)
|
||||
|
||||
def list(self, *args, **kwargs):
|
||||
self.policy.enforce(self.context, 'get_members', {})
|
||||
|
|
|
@ -146,14 +146,21 @@ def upload_data_to_store(req, image_meta, image_data, store, notifier):
|
|||
update_data = {'checksum': checksum,
|
||||
'size': size}
|
||||
try:
|
||||
image_meta = registry.update_image_metadata(req.context,
|
||||
image_id,
|
||||
update_data,
|
||||
from_state='saving')
|
||||
|
||||
except exception.NotFound as e:
|
||||
msg = _("Image %s could not be found after upload. The image may "
|
||||
"have been deleted during the upload.") % image_id
|
||||
try:
|
||||
state = 'saving'
|
||||
image_meta = registry.update_image_metadata(req.context,
|
||||
image_id,
|
||||
update_data,
|
||||
from_state=state)
|
||||
except exception.Duplicate:
|
||||
image = registry.get_image_metadata(req.context, image_id)
|
||||
if image['status'] == 'deleted':
|
||||
raise exception.NotFound()
|
||||
else:
|
||||
raise
|
||||
except exception.NotFound:
|
||||
msg = _("Image %s could not be found after upload. The image may"
|
||||
" have been deleted during the upload.") % image_id
|
||||
LOG.info(msg)
|
||||
|
||||
# NOTE(jculp): we need to clean up the datastore if an image
|
||||
|
|
|
@ -22,6 +22,7 @@ from glance.common import wsgi
|
|||
import glance.db
|
||||
import glance.gateway
|
||||
import glance.notifier
|
||||
from glance.openstack.common import excutils
|
||||
import glance.openstack.common.log as logging
|
||||
import glance.store
|
||||
|
||||
|
@ -66,13 +67,12 @@ class ImageDataController(object):
|
|||
try:
|
||||
image_repo.save(image)
|
||||
image.set_data(data, size)
|
||||
image_repo.save(image)
|
||||
except exception.NotFound as e:
|
||||
msg = (_("Image %(id)s could not be found after upload."
|
||||
"The image may have been deleted during the upload: "
|
||||
"%(error)s Cleaning up the chunks uploaded") %
|
||||
{'id': image_id,
|
||||
'error': e})
|
||||
image_repo.save(image, from_state='saving')
|
||||
except (exception.NotFound, exception.Conflict):
|
||||
msg = (_("Image %s could not be found after upload. "
|
||||
"The image may have been deleted during the "
|
||||
"upload, cleaning up the chunks uploaded.") %
|
||||
image_id)
|
||||
LOG.warn(msg)
|
||||
# NOTE(sridevi): Cleaning up the uploaded chunks.
|
||||
try:
|
||||
|
@ -131,6 +131,10 @@ class ImageDataController(object):
|
|||
raise webob.exc.HTTPServiceUnavailable(explanation=msg,
|
||||
request=req)
|
||||
|
||||
except webob.exc.HTTPGone as e:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.error(_("Failed to upload image data due to HTTP error"))
|
||||
|
||||
except webob.exc.HTTPError as e:
|
||||
LOG.error(_("Failed to upload image data due to HTTP error"))
|
||||
self._restore(image_repo, image)
|
||||
|
|
|
@ -162,7 +162,7 @@ class ImageRepo(object):
|
|||
image.created_at = new_values['created_at']
|
||||
image.updated_at = new_values['updated_at']
|
||||
|
||||
def save(self, image):
|
||||
def save(self, image, from_state=None):
|
||||
image_values = self._format_image_to_db(image)
|
||||
if image_values['size'] > CONF.image_size_cap:
|
||||
raise exception.ImageSizeLimitExceeded
|
||||
|
@ -170,7 +170,8 @@ class ImageRepo(object):
|
|||
new_values = self.db_api.image_update(self.context,
|
||||
image.image_id,
|
||||
image_values,
|
||||
purge_props=True)
|
||||
purge_props=True,
|
||||
from_state=from_state)
|
||||
except (exception.NotFound, exception.Forbidden):
|
||||
msg = _("No image found with ID %s") % image.image_id
|
||||
raise exception.NotFound(msg)
|
||||
|
@ -263,7 +264,7 @@ class ImageMemberRepo(object):
|
|||
msg = _("The specified member %s could not be found")
|
||||
raise exception.NotFound(msg % image_member.id)
|
||||
|
||||
def save(self, image_member):
|
||||
def save(self, image_member, from_state=None):
|
||||
image_member_values = self._format_image_member_to_db(image_member)
|
||||
try:
|
||||
new_values = self.db_api.image_member_update(self.context,
|
||||
|
|
|
@ -94,9 +94,9 @@ class Repo(object):
|
|||
result = self.base.add(base_item)
|
||||
return self.helper.proxy(result)
|
||||
|
||||
def save(self, item):
|
||||
def save(self, item, from_state=None):
|
||||
base_item = self.helper.unproxy(item)
|
||||
result = self.base.save(base_item)
|
||||
result = self.base.save(base_item, from_state=from_state)
|
||||
return self.helper.proxy(result)
|
||||
|
||||
def remove(self, item):
|
||||
|
|
|
@ -178,8 +178,8 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
|
|||
item_proxy_class=ImageProxy,
|
||||
item_proxy_kwargs=proxy_kwargs)
|
||||
|
||||
def save(self, image):
|
||||
super(ImageRepoProxy, self).save(image)
|
||||
def save(self, image, from_state=None):
|
||||
super(ImageRepoProxy, self).save(image, from_state=from_state)
|
||||
self.notifier.info('image.update',
|
||||
format_image_notification(image))
|
||||
|
||||
|
|
|
@ -96,9 +96,9 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
|
|||
raise exception.ImagePropertyLimitExceeded(attempted=attempted,
|
||||
maximum=maximum)
|
||||
|
||||
def save(self, image):
|
||||
def save(self, image, from_state=None):
|
||||
self._enforce_image_property_quota(image)
|
||||
super(ImageRepoProxy, self).save(image)
|
||||
return super(ImageRepoProxy, self).save(image, from_state=from_state)
|
||||
|
||||
def add(self, image):
|
||||
self._enforce_image_property_quota(image)
|
||||
|
|
|
@ -468,7 +468,7 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
|
|||
self._set_acls(image)
|
||||
return result
|
||||
|
||||
def save(self, image):
|
||||
def save(self, image, from_state=None):
|
||||
result = super(ImageRepoProxy, self).save(image)
|
||||
self._set_acls(image)
|
||||
return result
|
||||
|
|
|
@ -74,7 +74,7 @@ class TestProxyRepoPlain(test_utils.BaseTestCase):
|
|||
self._test_method('add', 'snuff', 'enough')
|
||||
|
||||
def test_save(self):
|
||||
self._test_method('save', 'snuff', 'enough')
|
||||
self._test_method('save', 'snuff', 'enough', from_state=None)
|
||||
|
||||
def test_remove(self):
|
||||
self._test_method('add', None, 'flying')
|
||||
|
@ -121,14 +121,14 @@ class TestProxyRepoWrapping(test_utils.BaseTestCase):
|
|||
self.assertEqual(results[i].args, tuple())
|
||||
self.assertEqual(results[i].kwargs, {'a': 1})
|
||||
|
||||
def _test_method_with_proxied_argument(self, name, result):
|
||||
def _test_method_with_proxied_argument(self, name, result, **kwargs):
|
||||
self.fake_repo.result = result
|
||||
item = FakeProxy('snoop')
|
||||
method = getattr(self.proxy_repo, name)
|
||||
proxy_result = method(item)
|
||||
|
||||
self.assertEqual(self.fake_repo.args, ('snoop',))
|
||||
self.assertEqual(self.fake_repo.kwargs, {})
|
||||
self.assertEqual(('snoop',), self.fake_repo.args)
|
||||
self.assertEqual(kwargs, self.fake_repo.kwargs)
|
||||
|
||||
if result is None:
|
||||
self.assertTrue(proxy_result is None)
|
||||
|
@ -145,10 +145,12 @@ class TestProxyRepoWrapping(test_utils.BaseTestCase):
|
|||
self._test_method_with_proxied_argument('add', None)
|
||||
|
||||
def test_save(self):
|
||||
self._test_method_with_proxied_argument('save', 'dog')
|
||||
self._test_method_with_proxied_argument('save', 'dog',
|
||||
from_state=None)
|
||||
|
||||
def test_save_with_no_result(self):
|
||||
self._test_method_with_proxied_argument('save', None)
|
||||
self._test_method_with_proxied_argument('save', None,
|
||||
from_state=None)
|
||||
|
||||
def test_remove(self):
|
||||
self._test_method_with_proxied_argument('remove', 'dog')
|
||||
|
|
|
@ -69,7 +69,7 @@ class MemberRepoStub(object):
|
|||
def get(self, *args, **kwargs):
|
||||
return 'member_repo_get'
|
||||
|
||||
def save(self, image_member):
|
||||
def save(self, image_member, from_state=None):
|
||||
image_member.output = 'member_repo_save'
|
||||
|
||||
def list(self, *args, **kwargs):
|
||||
|
|
|
@ -290,7 +290,8 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
|
|||
self.image.extra_properties = {'foo': 'bar'}
|
||||
self.image_repo_proxy.save(self.image)
|
||||
|
||||
self.image_repo_mock.save.assert_called_once_with(self.base_image)
|
||||
self.image_repo_mock.save.assert_called_once_with(self.base_image,
|
||||
from_state=None)
|
||||
|
||||
def test_save_image_too_many_image_properties(self):
|
||||
self.config(image_property_quota=1)
|
||||
|
@ -306,7 +307,8 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
|
|||
self.image.extra_properties = {'foo': 'bar'}
|
||||
self.image_repo_proxy.save(self.image)
|
||||
|
||||
self.image_repo_mock.save.assert_called_once_with(self.base_image)
|
||||
self.image_repo_mock.save.assert_called_once_with(self.base_image,
|
||||
from_state=None)
|
||||
|
||||
def test_add_image_with_image_property(self):
|
||||
self.config(image_property_quota=1)
|
||||
|
|
|
@ -34,7 +34,7 @@ class ImageRepoStub(object):
|
|||
def add(self, image):
|
||||
return image
|
||||
|
||||
def save(self, image):
|
||||
def save(self, image, from_state=None):
|
||||
return image
|
||||
|
||||
|
||||
|
|
|
@ -1633,8 +1633,7 @@ class TestGlanceAPI(base.IsolatedUnitTest):
|
|||
|
||||
self.assertEqual(1, mock_store_add_to_backend.call_count)
|
||||
|
||||
def test_delete_during_image_upload(self):
|
||||
req = unit_test_utils.get_fake_request()
|
||||
def _check_delete_during_image_upload(self, is_admin=False):
|
||||
|
||||
fixture_headers = {'x-image-meta-store': 'file',
|
||||
'x-image-meta-disk-format': 'vhd',
|
||||
|
@ -1668,30 +1667,18 @@ class TestGlanceAPI(base.IsolatedUnitTest):
|
|||
mock_initiate_deletion)
|
||||
|
||||
orig_update_image_metadata = registry.update_image_metadata
|
||||
ctlr = glance.api.v1.controller.BaseController
|
||||
orig_get_image_meta_or_404 = ctlr.get_image_meta_or_404
|
||||
|
||||
data = "somedata"
|
||||
|
||||
def mock_update_image_metadata(*args, **kwargs):
|
||||
|
||||
if args[2].get('status', None) == 'deleted':
|
||||
|
||||
# One shot.
|
||||
def mock_get_image_meta_or_404(*args, **kwargs):
|
||||
ret = orig_get_image_meta_or_404(*args, **kwargs)
|
||||
ret['status'] = 'queued'
|
||||
self.stubs.Set(ctlr, 'get_image_meta_or_404',
|
||||
orig_get_image_meta_or_404)
|
||||
return ret
|
||||
|
||||
self.stubs.Set(ctlr, 'get_image_meta_or_404',
|
||||
mock_get_image_meta_or_404)
|
||||
|
||||
req = webob.Request.blank("/images/%s" % image_id)
|
||||
req.method = 'PUT'
|
||||
req.headers['Content-Type'] = 'application/octet-stream'
|
||||
req.body = "somedata"
|
||||
if args[2].get('size', None) == len(data):
|
||||
path = "/images/%s" % image_id
|
||||
req = unit_test_utils.get_fake_request(path=path,
|
||||
method='DELETE',
|
||||
is_admin=is_admin)
|
||||
res = req.get_response(self.api)
|
||||
self.assertEqual(res.status_int, 200)
|
||||
self.assertEqual(200, res.status_int)
|
||||
|
||||
self.stubs.Set(registry, 'update_image_metadata',
|
||||
orig_update_image_metadata)
|
||||
|
@ -1701,20 +1688,30 @@ class TestGlanceAPI(base.IsolatedUnitTest):
|
|||
self.stubs.Set(registry, 'update_image_metadata',
|
||||
mock_update_image_metadata)
|
||||
|
||||
req = webob.Request.blank("/images/%s" % image_id)
|
||||
req.method = 'DELETE'
|
||||
req = unit_test_utils.get_fake_request(path="/images/%s" % image_id,
|
||||
method='PUT')
|
||||
req.headers['Content-Type'] = 'application/octet-stream'
|
||||
req.body = data
|
||||
res = req.get_response(self.api)
|
||||
self.assertEqual(res.status_int, 200)
|
||||
self.assertEqual(412, res.status_int)
|
||||
self.assertFalse(res.location)
|
||||
|
||||
self.assertTrue(called['initiate_deletion'])
|
||||
|
||||
req = webob.Request.blank("/images/%s" % image_id)
|
||||
req.method = 'HEAD'
|
||||
req = unit_test_utils.get_fake_request(path="/images/%s" % image_id,
|
||||
method='HEAD',
|
||||
is_admin=True)
|
||||
res = req.get_response(self.api)
|
||||
self.assertEqual(res.status_int, 200)
|
||||
self.assertEqual(res.headers['x-image-meta-deleted'], 'True')
|
||||
self.assertEqual(res.headers['x-image-meta-status'], 'deleted')
|
||||
|
||||
def test_delete_during_image_upload_by_normal_user(self):
|
||||
self._check_delete_during_image_upload(is_admin=False)
|
||||
|
||||
def test_delete_during_image_upload_by_admin(self):
|
||||
self._check_delete_during_image_upload(is_admin=True)
|
||||
|
||||
def test_disable_purge_props(self):
|
||||
"""
|
||||
Test the special x-glance-registry-purge-props header controls
|
||||
|
|
|
@ -79,7 +79,7 @@ class FakeImageRepo(object):
|
|||
else:
|
||||
return self.result
|
||||
|
||||
def save(self, image):
|
||||
def save(self, image, from_state=None):
|
||||
self.saved_image = image
|
||||
|
||||
|
||||
|
@ -180,17 +180,21 @@ class TestImagesController(base.StoreClearingUnitTest):
|
|||
request, unit_test_utils.UUID1, 'YYYY', 4)
|
||||
|
||||
def test_upload_non_existent_image_during_save_initiates_deletion(self):
|
||||
def fake_save(self):
|
||||
def fake_save_not_found(self):
|
||||
raise exception.NotFound()
|
||||
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = FakeImage('abcd', locations=['http://example.com/image'])
|
||||
self.image_repo.result = image
|
||||
self.image_repo.save = fake_save
|
||||
image.delete = mock.Mock()
|
||||
self.assertRaises(webob.exc.HTTPGone, self.controller.upload,
|
||||
request, str(uuid.uuid4()), 'ABC', 3)
|
||||
self.assertTrue(image.delete.called)
|
||||
def fake_save_conflict(self):
|
||||
raise exception.Conflict()
|
||||
|
||||
for fun in [fake_save_not_found, fake_save_conflict]:
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = FakeImage('abcd', locations=['http://example.com/image'])
|
||||
self.image_repo.result = image
|
||||
self.image_repo.save = fun
|
||||
image.delete = mock.Mock()
|
||||
self.assertRaises(webob.exc.HTTPGone, self.controller.upload,
|
||||
request, str(uuid.uuid4()), 'ABC', 3)
|
||||
self.assertTrue(image.delete.called)
|
||||
|
||||
def test_upload_non_existent_image_before_save(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
|
|
Loading…
Reference in New Issue