Add possibility to delete image from single store

This change introduces new 'v2/stores/<store_id>/<image_id>'
endpoint that accepts 'DELETE' method request. Once successful
the request will delete the image <image_id>'s location that
matches the store <store_id>. If the store is not read-only
or return image in use exception the image data will be
deleted. In the case of read-only store, the location will
be removed and if the image in use is raised, the call will
fail.

bp: delete-from-store

Co-authored-by: Brian Rosmaita <rosmaita.fossdev@gmail.com>

Change-Id: I1cb45026489a96a283b82e8e7efc9975c181fceb

doc/source/user/glanceapi.rst

@@ -771,6 +771,20 @@ New API Calls
   where <STATUS_VALUE> is ``pending``, ``accepted``, or ``rejected``.
   The {memberId} is the tenant ID of the image member.
 
+Images v2 Stores API
+--------------------
+
+Version 2.10 of the OpenStack Images API introduces new /v2/stores/ endpoint
+when multiple stores is configured. The endpoint is used to delete image from
+specific store.
+
+Delete from Store
+*****************
+
+A user wants to delete image from specific store. The user issues a ``DELETE``
+request to ``/v2/stores/<STORE_ID>/<IMAGE_ID>``. NOTE: request body is not
+accepted.
+
 Images v2 Tasks API
 -------------------
 

glance/api/v2/images.py

@@ -397,6 +397,68 @@ class ImagesController(object):
                    cinder_encryption_key_id)
             LOG.warn(msg)
 
+    @utils.mutating
+    def delete_from_store(self, req, store_id, image_id):
+        if not CONF.enabled_backends:
+            raise webob.exc.HTTPNotFound()
+        if store_id not in CONF.enabled_backends:
+            msg = (_("The selected store %s is not available on this node.") %
+                   store_id)
+            raise webob.exc.HTTPConflict(explanation=msg)
+
+        image_repo = self.gateway.get_repo(req.context)
+        try:
+            image = image_repo.get(image_id)
+        except exception.NotAuthenticated as e:
+            raise webob.exc.HTTPUnauthorized(explanation=e.msg)
+        except exception.NotFound:
+            msg = (_("Failed to find image %(image_id)s") %
+                   {'image_id': image_id})
+            raise webob.exc.HTTPNotFound(explanation=msg)
+
+        if image.status != 'active':
+            msg = _("It's not allowed to remove image data from store if "
+                    "image status is not 'active'")
+            raise webob.exc.HTTPConflict(explanation=msg)
+
+        if len(image.locations) == 1:
+            LOG.debug("User forbidden to remove last location of image %s",
+                      image_id)
+            msg = _("Cannot delete image data from the only store containing "
+                    "it. Consider deleting the image instead.")
+            raise webob.exc.HTTPForbidden(explanation=msg)
+
+        try:
+            # NOTE(jokke): Here we go through the locations list and act on
+            # the first hit. image.locations.pop() will actually remove the
+            # data from the backend as well as remove the location object
+            # from the list.
+            for pos, loc in enumerate(image.locations):
+                if loc['metadata'].get('store') == store_id:
+                    image.locations.pop(pos)
+                    break
+            else:
+                msg = (_("Image %(iid)s is not stored in store %(sid)s.") %
+                       {'iid': image_id, 'sid': store_id})
+                raise exception.Invalid(msg)
+        except exception.Forbidden as e:
+            raise webob.exc.HTTPForbidden(explanation=e.msg)
+        except exception.Invalid as e:
+            raise webob.exc.HTTPNotFound(explanation=e.msg)
+        except glance_store.exceptions.HasSnapshot as e:
+            raise webob.exc.HTTPConflict(explanation=e.msg)
+        except glance_store.exceptions.InUseByStore as e:
+            msg = ("The data for Image %(id)s could not be deleted "
+                   "because it is in use: %(exc)s" % {"id": image_id,
+                                                      "exc": e.msg})
+            LOG.warning(msg)
+            raise webob.exc.HTTPConflict(explanation=msg)
+        except Exception as e:
+            raise webob.exc.HTTPInternalServerError(
+                explanation=encodeutils.exception_to_unicode(e))
+
+        image_repo.save(image)
+
     @utils.mutating
     def delete(self, req, image_id):
         image_repo = self.gateway.get_repo(req.context)
@@ -1191,6 +1253,9 @@ class ResponseSerializer(wsgi.JSONResponseSerializer):
                                                          ensure_ascii=False))
         response.content_type = 'application/json'
 
+    def delete_from_store(self, response, result):
+        response.status_int = http.NO_CONTENT
+
     def delete(self, response, result):
         response.status_int = http.NO_CONTENT
 

glance/api/v2/router.py

@@ -433,6 +433,15 @@ class API(wsgi.Router):
                        controller=reject_method_resource,
                        action='reject',
                        allowed_methods='POST')
+        mapper.connect('/stores/{store_id}/{image_id}',
+                       controller=images_resource,
+                       action='delete_from_store',
+                       conditions={'method': ['DELETE']},
+                       body_reject=True)
+        mapper.connect('/stores/{store_id}/{image_id}',
+                       controller=reject_method_resource,
+                       action='reject',
+                       allowed_methods='DELETE')
 
         image_actions_resource = image_actions.create_resource()
         mapper.connect('/images/{image_id}/actions/deactivate',

glance/api/versions.py

@@ -75,10 +75,15 @@ class Controller(object):
         if CONF.enable_v2_api:
             if CONF.enabled_backends:
                 version_objs.extend([
+                    build_version_object(2.10, 'v2', 'CURRENT'),
+                    build_version_object(2.9, 'v2', 'SUPPORTED'),
                     build_version_object(2.8, 'v2', 'SUPPORTED')
                 ])
+            else:
                 version_objs.extend([
                     build_version_object(2.9, 'v2', 'CURRENT'),
+                ])
+            version_objs.extend([
                 build_version_object(2.7, 'v2', 'SUPPORTED'),
                 build_version_object(2.6, 'v2', 'SUPPORTED'),
                 build_version_object(2.5, 'v2', 'SUPPORTED'),

glance/common/store_utils.py

@@ -21,7 +21,7 @@ from oslo_utils import encodeutils
 import six.moves.urllib.parse as urlparse
 
 import glance.db as db_api
-from glance.i18n import _LE, _LW
+from glance.i18n import _LE
 from glance import scrubber
 
 LOG = logging.getLogger(__name__)
@@ -68,7 +68,9 @@ def safe_delete_from_backend(context, image_id, location):
                                                    location['id'], 'deleted')
         return ret
     except store_api.NotFound:
-        msg = _LW('Failed to delete image %s in store from URI') % image_id
+        msg = ("The image data for %(iid)s was not found in the store. "
+               "The image record has been updated to reflect "
+               "this." % {'iid': image_id})
         LOG.warn(msg)
     except store_api.StoreDeleteNotSupported as e:
         LOG.warn(encodeutils.exception_to_unicode(e))

glance/location.py

@@ -244,6 +244,17 @@ class StoreLocations(MutableSequence):
                 self.image_proxy.context,
                 self.image_proxy.image.image_id,
                 location)
+        except store.exceptions.NotFound:
+            # NOTE(rosmaita): This can happen if the data was deleted by an
+            # operator from the backend, or a race condition from multiple
+            # delete-from-store requests. The old way to deal with this was
+            # that the user could just delete the image when the data is gone,
+            # but with multi-store, that is no longer a good option. So we
+            # intentionally leave the location popped (in other words, the
+            # pop() succeeds) but we also reraise the NotFound so that the
+            # calling code knows what happened.
+            with excutils.save_and_reraise_exception():
+                pass
         except Exception:
             with excutils.save_and_reraise_exception():
                 self.value.insert(i, location)

glance/tests/unit/base.py

@@ -81,6 +81,8 @@ class MultiStoreClearingUnitTest(test_utils.BaseTestCase):
 
         self.config(filesystem_store_datadir=self.test_dir,
                     group='fast')
+        self.config(filesystem_store_datadir=self.test_dir2,
+                    group='cheap')
         store.create_multi_stores(CONF)
 
 

glance/tests/unit/v2/test_images_resource.py

@@ -2916,6 +2916,12 @@ class TestImagesController(base.IsolatedUnitTest):
         self.assertTrue(image['deleted'])
         self.assertEqual('deleted', image['status'])
 
+    def test_delete_from_store_no_multistore(self):
+        request = unit_test_utils.get_fake_request()
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.delete_from_store, request,
+                          "the IDs should", "not matter")
+
     def test_index_with_invalid_marker(self):
         fake_uuid = str(uuid.uuid4())
         request = unit_test_utils.get_fake_request()
@@ -5104,7 +5110,7 @@ class TestMultiImagesController(base.MultiIsolatedUnitTest):
         self.store = store
         self._create_images()
         self._create_image_members()
-        stores = {'cheap': 'file', 'fast': 'file'}
+        stores = {'cheap': 'file', 'fast': 'file', 'empty': 'file'}
         self.config(enabled_backends=stores)
         self.store.register_store_opts(CONF)
         self.controller = glance.api.v2.images.ImagesController(self.db,
@@ -5216,6 +5222,68 @@ class TestMultiImagesController(base.MultiIsolatedUnitTest):
                           request, UUID1,
                           {'method': {'name': 'glance-direct'}})
 
+    def test_delete_from_store_as_non_owner(self):
+        request = unit_test_utils.get_fake_request()
+        self.assertRaises(webob.exc.HTTPForbidden,
+                          self.controller.delete_from_store,
+                          request,
+                          "fast",
+                          UUID6)
+
+    def test_delete_from_store_non_active(self):
+        request = unit_test_utils.get_fake_request(tenant=TENANT3)
+        self.assertRaises(webob.exc.HTTPConflict,
+                          self.controller.delete_from_store,
+                          request,
+                          "fast",
+                          UUID3)
+
+    def test_delete_from_store_no_image(self):
+        request = unit_test_utils.get_fake_request(tenant=TENANT3)
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.delete_from_store,
+                          request,
+                          "fast",
+                          "nonexisting")
+
+    def test_delete_from_store_invalid_store(self):
+        request = unit_test_utils.get_fake_request(tenant=TENANT3)
+        self.assertRaises(webob.exc.HTTPConflict,
+                          self.controller.delete_from_store,
+                          request,
+                          "burn",
+                          UUID6)
+
+    def test_delete_from_store_not_in_store(self):
+        request = unit_test_utils.get_fake_request(tenant=TENANT3)
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.delete_from_store,
+                          request,
+                          "empty",
+                          UUID6)
+
+    def test_delete_from_store_one_location(self):
+        request = unit_test_utils.get_fake_request(tenant=TENANT3)
+        self.assertRaises(webob.exc.HTTPForbidden,
+                          self.controller.delete_from_store,
+                          request,
+                          "fast",
+                          UUID7)
+
+    def test_delete_from_store_as_non_admin(self):
+        request = unit_test_utils.get_fake_request(tenant=TENANT3)
+        self.controller.delete_from_store(request, "fast", UUID6)
+        image = self.controller.show(request, UUID6)
+        self.assertEqual(1, len(image.locations))
+        self.assertEqual("cheap", image.locations[0]['metadata']['store'])
+
+    def test_delete_from_store_as_admin(self):
+        request = unit_test_utils.get_fake_request(is_admin=True)
+        self.controller.delete_from_store(request, "fast", UUID6)
+        image = self.controller.show(request, UUID6)
+        self.assertEqual(1, len(image.locations))
+        self.assertEqual("cheap", image.locations[0]['metadata']['store'])
+
     def test_image_lazy_loading_store(self):
         # assert existing image does not have store in metadata
         existing_image = self.images[1]
@@ -5319,10 +5387,12 @@ class TestMultiImagesController(base.MultiIsolatedUnitTest):
 
         with mock.patch.object(
                 glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
+            with mock.patch.object(self.store,
+                                   'get_store_from_store_identifier'):
                 mock_get.return_value = FakeImage(id=UUID7, status='active',
                                                   locations=locations)
-            self.assertIsNotNone(self.controller.import_image(request, UUID7,
+                self.assertIsNotNone(self.controller.import_image(
-                                 {'method': {'name': 'copy-image'},
+                    request, UUID7, {'method': {'name': 'copy-image'},
                                      'all_stores': True}))
 
     def test_copy_non_active_image(self):

releasenotes/notes/delete_from_store-a1d9b9bd5cf27546.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    As part of the multi-store efforts this release introduces deletion from
+    single store. Through new '/v2/stores' endpoint the API user can request
+    image to be deleted from single store instead of deleting the whole image.
+    This feature can be used to clean up store metadata in cases where the
+    image data has for some reason disappeared from the store already, except
+    410 Gone HTTP response.