Merge "[RBD] Clone v2: Image is unusable if deletion fails"

4 files changed, 59 insertions, 0 deletions

diff --git a/glance_store/_drivers/rbd.py b/glance_store/_drivers/rbd.py
index b5431fc..9c1d45b 100644
--- a/glance_store/_drivers/rbd.py
+++ b/glance_store/_drivers/rbd.py
@@ -434,6 +434,14 @@ class Store(driver.Store):
             'snapshot': DEFAULT_SNAPNAME,
         }, self.conf)
 
+    def _snapshot_has_external_reference(self, image, snapshot_name):
+        """Returns True if snapshot has external reference else False.
+        """
+        image.set_snap(snapshot_name)
+        has_references = bool(image.list_children())
+        image.set_snap(None)
+        return has_references
+
     def _delete_image(self, target_pool, image_name,
                       snapshot_name=None, context=None):
         """
@@ -453,6 +461,14 @@ 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:
diff --git a/glance_store/tests/unit/test_multistore_rbd.py b/glance_store/tests/unit/test_multistore_rbd.py
index 384fca0..822c1b5 100644
--- a/glance_store/tests/unit/test_multistore_rbd.py
+++ b/glance_store/tests/unit/test_multistore_rbd.py
@@ -113,6 +113,12 @@ class MockRBD(object):
         def remove_snap(self, *args, **kwargs):
             pass
 
+        def set_snap(self, *args, **kwargs):
+            pass
+
+        def list_children(self, *args, **kwargs):
+            pass
+
         def protect_snap(self, *args, **kwargs):
             pass
 
@@ -417,6 +423,15 @@ class TestMultiStore(base.MultiStoreBaseTest,
 
             self.called_commands_expected = ['unprotect_snap']
 
+    def test_delete_image_snap_has_external_references(self):
+        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.location.image,
+                              snapshot_name='snap')
+
     def test_delete_image_w_snap_exc_image_has_snap(self):
         def _fake_remove(*args, **kwargs):
             self.called_commands_actual.append('remove')
diff --git a/glance_store/tests/unit/test_rbd_store.py b/glance_store/tests/unit/test_rbd_store.py
index 2d1cae6..cbd34ac 100644
--- a/glance_store/tests/unit/test_rbd_store.py
+++ b/glance_store/tests/unit/test_rbd_store.py
@@ -114,6 +114,12 @@ class MockRBD(object):
         def remove_snap(self, *args, **kwargs):
             pass
 
+        def set_snap(self, *args, **kwargs):
+            pass
+
+        def list_children(self, *args, **kwargs):
+            pass
+
         def protect_snap(self, *args, **kwargs):
             pass
 
@@ -623,6 +629,15 @@ class TestStore(base.StoreBaseTest,
 
             self.called_commands_expected = ['unprotect_snap']
 
+    def test_delete_image_snap_has_external_references(self):
+        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.location.image,
+                              snapshot_name='snap')
+
     def test_delete_image_w_snap_exc_image_has_snap(self):
         def _fake_remove(*args, **kwargs):
             self.called_commands_actual.append('remove')
diff --git a/releasenotes/notes/bug-1954883-3666d63a3c0233f1.yaml b/releasenotes/notes/bug-1954883-3666d63a3c0233f1.yaml
new file mode 100644
index 0000000..fc58976
--- /dev/null
+++ b/releasenotes/notes/bug-1954883-3666d63a3c0233f1.yaml
@@ -0,0 +1,13 @@
+---
+fixes:
+  - |
+    * Bug 1954883_: [RBD] Image is unusable if deletion fails
+
+    .. _1954883: https://code.launchpad.net/bugs/1954883
+
+upgrade:
+  - |
+    Deployments which are using Ceph V2 clone feature (i.e. RBD backend for
+    glance_store as well as cinder driver is RBD or nova is using RBD driver)
+    and minimum ceph client version is greater than 'luminous' need to grant
+    glance osd read access to the cinder and nova RBD pool.