Merge "Fixes the possibility of leaving orphaned data"

glance/scrubber.py

@@ -17,6 +17,7 @@ import calendar
 import time
 
 import eventlet
+from glance_store import exceptions as store_exceptions
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import encodeutils
@@ -300,25 +301,44 @@ class Scrubber(object):
         LOG.info(_LI("Scrubbing image %(id)s from %(count)d locations.") %
                  {'id': image_id, 'count': len(delete_jobs)})
 
+        success = True
         for img_id, loc_id, uri in delete_jobs:
-            self._delete_image_location_from_backend(img_id, loc_id, uri)
+            try:
+                self._delete_image_location_from_backend(img_id, loc_id, uri)
+            except Exception:
+                success = False
 
-        image = self.registry.get_image(image_id)
+        if success:
-        if image['status'] == 'pending_delete':
+            image = self.registry.get_image(image_id)
-            self.registry.update_image(image_id, {'status': 'deleted'})
+            if image['status'] == 'pending_delete':
+                self.registry.update_image(image_id, {'status': 'deleted'})
+            LOG.info(_LI("Image %s has been scrubbed successfully") % image_id)
+        else:
+            LOG.warn(_LW("One or more image locations couldn't be scrubbed "
+                         "from backend. Leaving image '%s' in 'pending_delete'"
+                         " status") % image_id)
 
     def _delete_image_location_from_backend(self, image_id, loc_id, uri):
         if CONF.metadata_encryption_key:
             uri = crypt.urlsafe_decrypt(CONF.metadata_encryption_key, uri)
-
         try:
-            LOG.debug("Deleting URI from image %s." % image_id)
+            LOG.debug("Scrubbing image %s from a location." % image_id)
-            self.store_api.delete_from_backend(uri, self.admin_context)
+            try:
+                self.store_api.delete_from_backend(uri, self.admin_context)
+            except store_exceptions.NotFound:
+                LOG.info(_LI("Image location for image '%s' not found in "
+                             "backend; Marking image location deleted in "
+                             "db.") % image_id)
+
             if loc_id != '-':
                 db_api.get_api().image_location_delete(self.admin_context,
                                                        image_id,
                                                        int(loc_id),
                                                        'deleted')
-            LOG.info(_LI("Image %s has been deleted.") % image_id)
+            LOG.info(_LI("Image %s is scrubbed from a location.") % image_id)
-        except Exception:
+        except Exception as e:
-            LOG.warn(_LW("Unable to delete URI from image %s.") % image_id)
+            LOG.error(_LE("Unable to scrub image %(id)s from a location. "
+                          "Reason: %(exc)s ") %
+                      {'id': image_id,
+                       'exc': encodeutils.exception_to_unicode(e)})
+            raise

glance/tests/unit/test_scrubber.py

@@ -22,7 +22,6 @@ from oslo_config import cfg
 # NOTE(jokke): simplified transition to py3, behaves like py2 xrange
 from six.moves import range
 
-from glance.common import exception
 from glance import scrubber
 from glance.tests import utils as test_utils
 
@@ -61,13 +60,55 @@ class TestScrubber(test_utils.BaseTestCase):
         scrub._scrub_image(id, [(id, '-', uri)])
         self.mox.VerifyAll()
 
-    def test_store_delete_unsupported_backend_exception(self):
+    def test_store_delete_successful(self):
-        ex = glance_store.UnsupportedBackend()
+        uri = 'file://some/path/%s' % uuid.uuid4()
-        self._scrubber_cleanup_with_store_delete_exception(ex)
+        id = 'helloworldid'
+
+        scrub = scrubber.Scrubber(glance_store)
+        scrub.registry = self.mox.CreateMockAnything()
+        scrub.registry.get_image(id).AndReturn({'status': 'pending_delete'})
+        scrub.registry.update_image(id, {'status': 'deleted'})
+        self.mox.StubOutWithMock(glance_store, "delete_from_backend")
+        glance_store.delete_from_backend(uri, mox.IgnoreArg()).AndReturn('')
+        self.mox.ReplayAll()
+        scrub._scrub_image(id, [(id, '-', uri)])
+        self.mox.VerifyAll()
+
+    def test_store_delete_store_exceptions(self):
+        # While scrubbing image data, all store exceptions, other than
+        # NotFound, cause image scrubbing to fail. Essentially, no attempt
+        # would be made to update the status of image.
+
+        uri = 'file://some/path/%s' % uuid.uuid4()
+        id = 'helloworldid'
+        ex = glance_store.GlanceStoreException()
+
+        scrub = scrubber.Scrubber(glance_store)
+        scrub.registry = self.mox.CreateMockAnything()
+        self.mox.StubOutWithMock(glance_store, "delete_from_backend")
+        glance_store.delete_from_backend(
+            uri,
+            mox.IgnoreArg()).AndRaise(ex)
+        self.mox.ReplayAll()
+        scrub._scrub_image(id, [(id, '-', uri)])
+        self.mox.VerifyAll()
 
     def test_store_delete_notfound_exception(self):
-        ex = exception.NotFound()
+        # While scrubbing image data, NotFound exception is ignored and image
-        self._scrubber_cleanup_with_store_delete_exception(ex)
+        # scrubbing succeeds
+        uri = 'file://some/path/%s' % uuid.uuid4()
+        id = 'helloworldid'
+        ex = glance_store.NotFound(message='random')
+
+        scrub = scrubber.Scrubber(glance_store)
+        scrub.registry = self.mox.CreateMockAnything()
+        scrub.registry.get_image(id).AndReturn({'status': 'pending_delete'})
+        scrub.registry.update_image(id, {'status': 'deleted'})
+        self.mox.StubOutWithMock(glance_store, "delete_from_backend")
+        glance_store.delete_from_backend(uri, mox.IgnoreArg()).AndRaise(ex)
+        self.mox.ReplayAll()
+        scrub._scrub_image(id, [(id, '-', uri)])
+        self.mox.VerifyAll()
 
 
 class TestScrubDBQueue(test_utils.BaseTestCase):