RBD: Trash image when snapshots prevent deletion

RBD snapshots automatically end up in the trash
when deleted when using RBD image format v2.

However, they prevent images from being deleted.

Instead, move RBD images to the trash in this
situation.  This trash must be purged by a scheduled
rbd trash purge operation outside of Glance.

This passes new cinder<->image dependency tests in
I5fee2395195 when using ceph require-min-compat-client
of "mimic", which enables clone v2.

Closes-Bug: #1959186
Change-Id: I34dcd90a09d43127ff2e8b477750c70f3cc01113

glance_store/_drivers/rbd.py

@@ -471,14 +471,6 @@ class Store(driver.Store):
                     if snapshot_name is not None:
                         with rbd.Image(ioctx, image_name) as image:
                             try:
-                                # NOTE(abhishekk): Check whether snapshot
-                                # has any external references
-                                if self._snapshot_has_external_reference(
-                                        image, snapshot_name):
-                                    raise rbd.ImageBusy(
-                                        "Image snapshot has external "
-                                        "references.")
-
                                 self._unprotect_snapshot(image, snapshot_name)
                                 image.remove_snap(snapshot_name)
                             except rbd.ImageNotFound as exc:
@@ -498,10 +490,19 @@ class Store(driver.Store):
                     # Then delete image.
                     rbd.RBD().remove(ioctx, image_name)
                 except rbd.ImageHasSnapshots:
-                    log_msg = (_LW("Remove image %(img_name)s failed. "
+                    log_msg = (_LW("Unable to remove image %(img_name)s: it "
-                                   "It has snapshot(s) left.") %
+                                   "has snapshot(s) left; trashing instead") %
                                {'img_name': image_name})
                     LOG.warning(log_msg)
+                    with rbd.Image(ioctx, image_name) as image:
+                        try:
+                            rbd.RBD().trash_move(ioctx, image_name)
+                            LOG.debug('Moved %s to trash', image_name)
+                        except rbd.ImageBusy:
+                            LOG.warning(_('Unable to move in-use image to '
+                                          'trash'))
+                            raise exceptions.InUseByStore()
+                        return
                     raise exceptions.HasSnapshot()
                 except rbd.ImageBusy:
                     log_msg = (_LW("Remove image %(img_name)s failed. "

glance_store/tests/unit/test_multistore_rbd.py

@@ -172,6 +172,9 @@ class MockRBD(object):
         def clone(self, *args, **kwargs):
             raise NotImplementedError()
 
+        def trash_move(self, *args, **kwargs):
+            pass
+
     RBD_FEATURE_LAYERING = 1
 
 
@@ -427,9 +430,8 @@ class TestMultiStore(base.MultiStoreBaseTest,
         with mock.patch.object(MockRBD.Image, 'list_children') as mocked:
             mocked.return_value = True
 
-            self.assertRaises(exceptions.InUseByStore,
+            self.store._delete_image('fake_pool',
-                              self.store._delete_image,
+                                     self.location.image,
-                              'fake_pool', self.location.image,
                                      snapshot_name='snap')
 
     def test_delete_image_w_snap_exc_image_has_snap(self):
@@ -439,8 +441,8 @@ class TestMultiStore(base.MultiStoreBaseTest,
 
         with mock.patch.object(MockRBD.RBD, 'remove') as remove:
             remove.side_effect = _fake_remove
-            self.assertRaises(exceptions.HasSnapshot, self.store._delete_image,
+            self.store._delete_image('fake_pool',
-                              'fake_pool', self.location.image)
+                                     self.location.image)
 
             self.called_commands_expected = ['remove']
 

glance_store/tests/unit/test_rbd_store.py

@@ -173,6 +173,9 @@ class MockRBD(object):
         def clone(self, *args, **kwargs):
             raise NotImplementedError()
 
+        def trash_move(self, *args, **kwargs):
+            pass
+
     RBD_FEATURE_LAYERING = 1
 
 
@@ -633,9 +636,8 @@ class TestStore(base.StoreBaseTest,
         with mock.patch.object(MockRBD.Image, 'list_children') as mocked:
             mocked.return_value = True
 
-            self.assertRaises(exceptions.InUseByStore,
+            self.store._delete_image('fake_pool',
-                              self.store._delete_image,
+                                     self.location.image,
-                              'fake_pool', self.location.image,
                                      snapshot_name='snap')
 
     def test_delete_image_w_snap_exc_image_has_snap(self):
@@ -643,13 +645,36 @@ class TestStore(base.StoreBaseTest,
             self.called_commands_actual.append('remove')
             raise MockRBD.ImageHasSnapshots()
 
+        mock.patch.object(MockRBD.RBD, 'trash_move').start()
+
         with mock.patch.object(MockRBD.RBD, 'remove') as remove:
             remove.side_effect = _fake_remove
-            self.assertRaises(exceptions.HasSnapshot, self.store._delete_image,
+            self.store._delete_image('fake_pool',
-                              'fake_pool', self.location.image)
+                                     self.location.image)
 
             self.called_commands_expected = ['remove']
 
+        MockRBD.RBD.trash_move.assert_called_once_with(mock.ANY, 'fake_image')
+
+    def test_delete_image_w_snap_exc_image_has_snap_2(self):
+        def _fake_remove(*args, **kwargs):
+            self.called_commands_actual.append('remove')
+            raise MockRBD.ImageHasSnapshots()
+
+        mock.patch.object(MockRBD.RBD, 'trash_move',
+                          side_effect=MockRBD.ImageBusy).start()
+
+        with mock.patch.object(MockRBD.RBD, 'remove') as remove:
+            remove.side_effect = _fake_remove
+            self.assertRaises(exceptions.InUseByStore,
+                              self.store._delete_image,
+                              'fake_pool',
+                              self.location.image)
+
+            self.called_commands_expected = ['remove']
+
+        MockRBD.RBD.trash_move.assert_called_once_with(mock.ANY, 'fake_image')
+
     def test_get_partial_image(self):
         loc = g_location.Location('test_rbd_store', rbd_store.StoreLocation,
                                   self.conf, store_specs=self.store_specs)

releasenotes/notes/rbd-trash-snapshots-158a39da4248fb0c.yaml

@@ -0,0 +1,10 @@
+---
+features:
+  - |
+    The RBD driver now moves images to the trash if they cannot be deleted
+    immediately due to having snapshots. This fixes the long-standing issue
+    where base images are unable to be deleted until/unless all snapshots of
+    it are also deleted. Moving the image to the trash allows Glance to
+    proceed with the deletion of the image (as far as it is concerned), mark
+    the RBD image for deletion, which will happen once the last snapshot that
+    uses it has been deleted.