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

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
@@ -1195,7 +1199,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
@@ -1254,6 +1258,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

@@ -1455,34 +1455,38 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
         volume_name = snapshot.volume_name
         snap_name = 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) -> bool:
         """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.