diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 6d4cf0b9..353d1025 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -47,7 +47,8 @@ from glance import registry from glance.store import (create_stores, get_from_backend, get_size_from_backend, - schedule_delete_from_backend, + safe_delete_from_backend, + schedule_delayed_delete_from_backend, get_store_from_location, get_store_from_scheme) @@ -758,8 +759,15 @@ class Controller(controller.BaseController): # See https://bugs.launchpad.net/glance/+bug/747799 try: if image['location']: - schedule_delete_from_backend(image['location'], + if CONF.delayed_delete: + schedule_delayed_delete_from_backend(image['location'], id) + registry.update_image_metadata(req.context, id, + {'status': 'pending_delete'}) + else: + safe_delete_from_backend(image['location'], req.context, id) + registry.update_image_metadata(req.context, id, + {'status': 'deleted'}) registry.delete_image_metadata(req.context, id) except exception.NotFound, e: msg = ("Failed to find image to delete: %(e)s" % locals()) diff --git a/glance/api/v2/__init__.py b/glance/api/v2/__init__.py index 5683bfa3..b400c0f6 100644 --- a/glance/api/v2/__init__.py +++ b/glance/api/v2/__init__.py @@ -16,10 +16,9 @@ import webob.exc from glance.common import exception -from glance.store import set_acls -def update_image_read_acl(req, db_api, image): +def update_image_read_acl(req, store_api, db_api, image): """Helper function to set ACL permissions on images in the image store""" location_uri = image['location'] public = image['is_public'] @@ -36,9 +35,9 @@ def update_image_read_acl(req, db_api, image): write_tenants.append(member['member']) else: read_tenants.append(member['member']) - set_acls(req.context, location_uri, public=public, - read_tenants=read_tenants, - write_tenants=write_tenants) + store_api.set_acls(req.context, location_uri, public=public, + read_tenants=read_tenants, + write_tenants=write_tenants) except exception.UnknownScheme: msg = _("Store for image_id not found: %s") % image_id raise webob.exc.HTTPBadRequest(explanation=msg, diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py index 29fd23d1..be48c3bd 100644 --- a/glance/api/v2/image_data.py +++ b/glance/api/v2/image_data.py @@ -58,7 +58,7 @@ class ImageDataController(object): except exception.Duplicate: raise webob.exc.HTTPConflict() else: - v2.update_image_read_acl(req, self.db_api, image) + v2.update_image_read_acl(req, self.store_api, self.db_api, image) values = {'location': location, 'size': size, 'checksum': checksum} self.db_api.image_update(req.context, image_id, values) updated_image = self._get_image(req.context, image_id) diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 6db11640..a5184d08 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -32,6 +32,7 @@ from glance.openstack.common import cfg import glance.openstack.common.log as logging from glance.openstack.common import timeutils import glance.schema +import glance.store LOG = logging.getLogger(__name__) @@ -40,11 +41,14 @@ CONF = cfg.CONF class ImagesController(object): - def __init__(self, db_api=None, policy_enforcer=None, notifier=None): + def __init__(self, db_api=None, policy_enforcer=None, notifier=None, + store_api=None): self.db_api = db_api or glance.db.get_api() self.db_api.configure_db() self.policy = policy_enforcer or policy.Enforcer() self.notifier = notifier or glance.notifier.Notifier() + self.store_api = store_api or glance.store + self.store_api.create_stores() def _enforce(self, req, action): """Authorize an action against our policies""" @@ -101,7 +105,7 @@ class ImagesController(object): else: image['tags'] = [] - v2.update_image_read_acl(req, self.db_api, image) + v2.update_image_read_acl(req, self.store_api, self.db_api, image) image = self._normalize_properties(dict(image)) self.notifier.info('image.update', image) return image @@ -173,7 +177,7 @@ class ImagesController(object): raise webob.exc.HTTPNotFound() image = self._normalize_properties(dict(image)) - v2.update_image_read_acl(req, self.db_api, image) + v2.update_image_read_acl(req, self.store_api, self.db_api, image) if tags is not None: self.db_api.image_tag_set_all(req.context, image_id, tags) @@ -264,6 +268,18 @@ class ImagesController(object): % locals()) raise webob.exc.HTTPForbidden(explanation=msg) + if image['location']: + if CONF.delayed_delete: + self.store_api.schedule_delayed_delete_from_backend( + image['location'], id) + self.db_api.image_update(req.context, image_id, + {'status': 'pending_delete'}) + else: + self.store_api.safe_delete_from_backend(image['location'], + req.context, id) + self.db_api.image_update(req.context, image_id, + {'status': 'deleted'}) + try: self.db_api.image_destroy(req.context, image_id) except (exception.NotFound, exception.Forbidden): diff --git a/glance/store/__init__.py b/glance/store/__init__.py index 268de9ac..c2b04013 100644 --- a/glance/store/__init__.py +++ b/glance/store/__init__.py @@ -24,7 +24,6 @@ from glance.common import utils from glance.openstack.common import cfg from glance.openstack.common import importutils import glance.openstack.common.log as logging -from glance import registry from glance.store import location LOG = logging.getLogger(__name__) @@ -254,26 +253,24 @@ def get_store_from_location(uri): return loc.store_name -def schedule_delete_from_backend(uri, context, image_id, **kwargs): - """ - Given a uri and a time, schedule the deletion of an image. - """ - if not CONF.delayed_delete: - registry.update_image_metadata(context, image_id, - {'status': 'deleted'}) - try: - return delete_from_backend(context, uri, **kwargs) - except (UnsupportedBackend, - exception.StoreDeleteNotSupported, - exception.NotFound): - exc_type = sys.exc_info()[0].__name__ - msg = (_("Failed to delete image at %s from store (%s)") % - (uri, exc_type)) - LOG.error(msg) - finally: - # avoid falling through to the delayed deletion logic - return +def safe_delete_from_backend(uri, context, image_id, **kwargs): + """Given a uri, delete an image from the store.""" + try: + return delete_from_backend(context, uri, **kwargs) + except exception.NotFound: + msg = _('Failed to delete image in store at URI: %s') + LOG.warn(msg % uri) + except exception.StoreDeleteNotSupported as e: + LOG.warn(str(e)) + except UnsupportedBackend: + exc_type = sys.exc_info()[0].__name__ + msg = (_('Failed to delete image at %s from store (%s)') % + (uri, exc_type)) + LOG.error(msg) + +def schedule_delayed_delete_from_backend(uri, image_id, **kwargs): + """Given a uri, schedule the deletion of an image.""" datadir = CONF.scrubber_datadir delete_time = time.time() + CONF.scrub_time file_path = os.path.join(datadir, str(image_id)) @@ -289,9 +286,6 @@ def schedule_delete_from_backend(uri, context, image_id, **kwargs): os.chmod(file_path, 0600) os.utime(file_path, (delete_time, delete_time)) - registry.update_image_metadata(context, image_id, - {'status': 'pending_delete'}) - def add_to_backend(context, scheme, image_id, data, size): store = get_store_from_scheme(context, scheme) diff --git a/glance/tests/unit/test_http_store.py b/glance/tests/unit/test_http_store.py index d3a67e11..10de952b 100644 --- a/glance/tests/unit/test_http_store.py +++ b/glance/tests/unit/test_http_store.py @@ -23,7 +23,7 @@ from glance import context from glance.db.sqlalchemy import api as db_api from glance.registry import configure_registry_client from glance.store import (delete_from_backend, - schedule_delete_from_backend) + safe_delete_from_backend) from glance.store.http import Store, MAX_REDIRECTS from glance.store.location import get_location_from_uri from glance.tests.unit import base @@ -185,6 +185,6 @@ class TestHttpStore(base.StoreClearingUnitTest): ctx = context.RequestContext() stub_out_registry_image_update(self.stubs) try: - schedule_delete_from_backend(uri, ctx, 'image_id') + safe_delete_from_backend(uri, ctx, 'image_id') except exception.StoreDeleteNotSupported: self.fail('StoreDeleteNotSupported should be swallowed') diff --git a/glance/tests/unit/utils.py b/glance/tests/unit/utils.py index 7dacf93a..31335c9a 100644 --- a/glance/tests/unit/utils.py +++ b/glance/tests/unit/utils.py @@ -31,6 +31,8 @@ TENANT2 = '2c014f32-55eb-467d-8fcb-4bd706012f81' USER1 = '54492ba0-f4df-4e4e-be62-27f4d76b29cf' USER2 = '0b3b3006-cb76-4517-ae32-51397e22c754' +BASE_URI = 'swift+http://storeurl.com/container' + def get_fake_request(path='', method='POST', is_admin=False): req = wsgi.Request.blank(path) @@ -58,7 +60,7 @@ class FakeDB(object): def init_db(): images = [ {'id': UUID1, 'owner': TENANT1, - 'location': 'swift+http://storeurl.com/container/%s' % UUID1}, + 'location': '%s/%s' % (BASE_URI, UUID1)}, {'id': UUID2, 'owner': TENANT1}, ] [simple_db.image_create(None, image) for image in images] @@ -86,18 +88,30 @@ class FakeDB(object): class FakeStoreAPI(object): def __init__(self): self.data = { - 'swift+http://storeurl.com/container/%s' % UUID1: ('XXX', 3), + '%s/%s' % (BASE_URI, UUID1): ('XXX', 3), } def create_stores(self): pass + def set_acls(*_args, **_kwargs): + pass + def get_from_backend(self, context, location): try: return self.data[location] except KeyError: raise exception.NotFound() + def safe_delete_from_backend(self, uri, context, id, **kwargs): + try: + del self.data[uri] + except KeyError: + pass + + def schedule_delayed_delete_from_backend(self, uri, id, **kwargs): + pass + def get_size_from_backend(self, context, location): return self.get_from_backend(context, location)[1] diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index d07d0c0b..b7a54f2c 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -34,6 +34,8 @@ ISOTIME = '2012-05-16T15:27:36Z' CONF = cfg.CONF +BASE_URI = unit_test_utils.BASE_URI + UUID1 = 'c80a1a6c-bd1f-41c5-90ee-81afedb1d58d' UUID2 = 'a85abd86-55b3-4d5b-b0b4-5d0a6e6042fc' @@ -76,17 +78,19 @@ class TestImagesController(test_utils.BaseTestCase): self.db = unit_test_utils.FakeDB() self.policy = unit_test_utils.FakePolicyEnforcer() self.notifier = unit_test_utils.FakeNotifier() + self.store = unit_test_utils.FakeStoreAPI() self._create_images() self.controller = glance.api.v2.images.ImagesController(self.db, self.policy, - self.notifier) + self.notifier, + self.store) glance.store.create_stores() def _create_images(self): self.db.reset() self.images = [ _fixture(UUID1, owner=TENANT1, name='1', size=256, is_public=True, - location='swift+http://example.com/container/%s' % UUID1), + location='%s/%s' % (BASE_URI, UUID1)), _fixture(UUID2, owner=TENANT1, name='2', size=512, is_public=True), _fixture(UUID3, owner=TENANT3, name='3', size=512, is_public=True), _fixture(UUID4, owner=TENANT4, name='4', size=1024), @@ -534,6 +538,7 @@ class TestImagesController(test_utils.BaseTestCase): def test_delete(self): request = unit_test_utils.get_fake_request() + self.assertTrue(filter(lambda k: UUID1 in k, self.store.data)) try: image = self.controller.delete(request, UUID1) output_log = self.notifier.get_log() @@ -542,6 +547,39 @@ class TestImagesController(test_utils.BaseTestCase): except Exception as e: self.fail("Delete raised exception: %s" % e) + deleted_img = self.db.image_get(request.context, UUID1, + force_show_deleted=True) + self.assertTrue(deleted_img['deleted']) + self.assertEqual(deleted_img['status'], 'deleted') + self.assertFalse(filter(lambda k: UUID1 in k, self.store.data)) + + def test_delete_not_in_store(self): + request = unit_test_utils.get_fake_request() + self.assertTrue(filter(lambda k: UUID1 in k, self.store.data)) + for k in self.store.data: + if UUID1 in k: + del self.store.data[k] + break + + self.controller.delete(request, UUID1) + deleted_img = self.db.image_get(request.context, UUID1, + force_show_deleted=True) + self.assertTrue(deleted_img['deleted']) + self.assertEqual(deleted_img['status'], 'deleted') + self.assertFalse(filter(lambda k: UUID1 in k, self.store.data)) + + def test_delayed_delete(self): + self.config(delayed_delete=True) + request = unit_test_utils.get_fake_request() + self.assertTrue(filter(lambda k: UUID1 in k, self.store.data)) + + self.controller.delete(request, UUID1) + deleted_img = self.db.image_get(request.context, UUID1, + force_show_deleted=True) + self.assertTrue(deleted_img['deleted']) + self.assertEqual(deleted_img['status'], 'pending_delete') + self.assertTrue(filter(lambda k: UUID1 in k, self.store.data)) + def test_delete_non_existent(self): request = unit_test_utils.get_fake_request() self.assertRaises(webob.exc.HTTPNotFound, self.controller.delete,