Merge "Delete image from backend store on delete."

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

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,
+            store_api.set_acls(req.context, location_uri, public=public,
-                     read_tenants=read_tenants,
+                               read_tenants=read_tenants,
-                     write_tenants=write_tenants)
+                               write_tenants=write_tenants)
         except exception.UnknownScheme:
             msg = _("Store for image_id not found: %s") % image_id
             raise webob.exc.HTTPBadRequest(explanation=msg,

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)

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):

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):
+def safe_delete_from_backend(uri, context, image_id, **kwargs):
-    """
+    """Given a uri, delete an image from the store."""
-    Given a uri and a time, schedule the deletion of an image.
+    try:
-    """
+        return delete_from_backend(context, uri, **kwargs)
-    if not CONF.delayed_delete:
+    except exception.NotFound:
-        registry.update_image_metadata(context, image_id,
+        msg = _('Failed to delete image in store at URI: %s')
-                                       {'status': 'deleted'})
+        LOG.warn(msg % uri)
-        try:
+    except exception.StoreDeleteNotSupported as e:
-            return delete_from_backend(context, uri, **kwargs)
+        LOG.warn(str(e))
-        except (UnsupportedBackend,
+    except UnsupportedBackend:
-                exception.StoreDeleteNotSupported,
+        exc_type = sys.exc_info()[0].__name__
-                exception.NotFound):
+        msg = (_('Failed to delete image at %s from store (%s)') %
-            exc_type = sys.exc_info()[0].__name__
+               (uri, exc_type))
-            msg = (_("Failed to delete image at %s from store (%s)") %
+        LOG.error(msg)
-                   (uri, exc_type))
-            LOG.error(msg)
-        finally:
-            # avoid falling through to the delayed deletion logic
-            return
 
+
+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)

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

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]
 

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,