rbd: Fix snapshot delete when the source volume doesn't exist

Currently snapshot delete requires access to the source volume and
the operation fails if the source volume doesn't exist in the backend.
This prevents some snapshots from being deleted when the source volume
image is deleted from the backend for some reason (for example, after
cluster format).

This change makes the rbd driver to skip updating the source volume
if it doesn't exist. A warning log is left so that operators can be
aware of any skip event.

Closes-Bug: #1957073
Change-Id: Icd9dad9ad7b3ad71b3962b078e5b94670ac41c87

cinder/tests/unit/volume/drivers/test_rbd.py

@@ -84,6 +84,10 @@ class MockImageHasSnapshotsException(MockException):
     """Used as mock for rbd.ImageHasSnapshots."""
 
 
+class MockInvalidArgument(MockException):
+    """Used as mock for rbd.InvalidArgument."""
+
+
 class KeyObject(object):
     def get_encoded(arg):
         return "asdf".encode('utf-8')
@@ -113,7 +117,7 @@ def common_mocks(f):
             inst.mock_rbd.ImageNotFound = MockImageNotFoundException
             inst.mock_rbd.ImageExists = MockImageExistsException
             inst.mock_rbd.ImageHasSnapshots = MockImageHasSnapshotsException
-            inst.mock_rbd.InvalidArgument = MockImageNotFoundException
+            inst.mock_rbd.InvalidArgument = MockInvalidArgument
             inst.mock_rbd.PermissionError = MockPermissionError
 
             inst.driver.rbd = inst.mock_rbd
@@ -1081,7 +1085,7 @@ class RBDTestCase(test.TestCase):
 
         self.driver.delete_snapshot(self.snapshot)
 
-        proxy.remove_snap.assert_called_with(self.snapshot.name)
+        proxy.remove_snap.assert_not_called()
         proxy.unprotect_snap.assert_called_with(self.snapshot.name)
 
     @common_mocks
@@ -1140,6 +1144,18 @@ class RBDTestCase(test.TestCase):
                 self.assertTrue(proxy.unprotect_snap.called)
                 self.assertFalse(proxy.remove_snap.called)
 
+    @common_mocks
+    @mock.patch('cinder.objects.Volume.get_by_id')
+    def test_delete_snapshot_volume_not_found(self, volume_get_by_id):
+        volume_get_by_id.return_value = self.volume_a
+        proxy = self.mock_proxy.return_value
+        proxy.__enter__.side_effect = self.mock_rbd.ImageNotFound
+
+        self.driver.delete_snapshot(self.snapshot)
+
+        proxy.remove_snap.assert_not_called()
+        proxy.unprotect_snap.assert_not_called()
+
     @common_mocks
     def test_snapshot_revert_use_temp_snapshot(self):
         self.assertFalse(self.driver.snapshot_revert_use_temp_snapshot())

cinder/volume/drivers/rbd.py

@@ -1286,34 +1286,38 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
         volume_name = utils.convert_str(snapshot.volume_name)
         snap_name = utils.convert_str(snapshot.name)
 
-        with RBDVolumeProxy(self, volume_name) as volume:
-            try:
-                volume.unprotect_snap(snap_name)
-            except self.rbd.InvalidArgument:
-                LOG.info(
-                    "InvalidArgument: Unable to unprotect snapshot %s.",
-                    snap_name)
-            except self.rbd.ImageNotFound:
-                LOG.info(
-                    "ImageNotFound: Unable to unprotect snapshot %s.",
-                    snap_name)
-            except self.rbd.ImageBusy:
-                children_list = self._get_children_info(volume, snap_name)
+        try:
+            with RBDVolumeProxy(self, volume_name) as volume:
+                try:
+                    volume.unprotect_snap(snap_name)
+                except self.rbd.InvalidArgument:
+                    LOG.info(
+                        "InvalidArgument: Unable to unprotect snapshot %s.",
+                        snap_name)
+                except self.rbd.ImageNotFound:
+                    LOG.info("Snapshot %s does not exist in backend.",
+                             snap_name)
+                    return
+                except self.rbd.ImageBusy:
+                    children_list = self._get_children_info(volume, snap_name)
 
-                if children_list:
-                    for (pool, image) in children_list:
-                        LOG.info('Image %(pool)s/%(image)s is dependent '
-                                 'on the snapshot %(snap)s.',
-                                 {'pool': pool,
-                                  'image': image,
-                                  'snap': snap_name})
+                    if children_list:
+                        for (pool, image) in children_list:
+                            LOG.info('Image %(pool)s/%(image)s is dependent '
+                                     'on the snapshot %(snap)s.',
+                                     {'pool': pool,
+                                      'image': image,
+                                      'snap': snap_name})
 
-                raise exception.SnapshotIsBusy(snapshot_name=snap_name)
-            try:
-                volume.remove_snap(snap_name)
-            except self.rbd.ImageNotFound:
-                LOG.info("Snapshot %s does not exist in backend.",
-                         snap_name)
+                    raise exception.SnapshotIsBusy(snapshot_name=snap_name)
+
+                try:
+                    volume.remove_snap(snap_name)
+                except self.rbd.ImageNotFound:
+                    LOG.info("Snapshot %s does not exist in backend.",
+                             snap_name)
+        except self.rbd.ImageNotFound:
+            LOG.warning("Volume %s does not exist in backend.", volume_name)
 
     def snapshot_revert_use_temp_snapshot(self):
         """Disable the use of a temporary snapshot on revert."""

releasenotes/notes/bug-1957073-0d1307a8637a62b7.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    RBD Driver `bug #1957073
+    <https://bugs.launchpad.net/cinder/+bug/1957073>`_: Snapshot delete now
+    succeeds and skips updating the source volume image if the image
+    does not exist in the backend.