diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 13ee0c16b7f..2ac50b5f04c 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/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()) diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index 07c26c8ce6a..e4ec408b729 100644 --- a/cinder/volume/drivers/rbd.py +++ b/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.""" diff --git a/releasenotes/notes/bug-1957073-0d1307a8637a62b7.yaml b/releasenotes/notes/bug-1957073-0d1307a8637a62b7.yaml new file mode 100644 index 00000000000..fc5d41a73a2 --- /dev/null +++ b/releasenotes/notes/bug-1957073-0d1307a8637a62b7.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + RBD Driver `bug #1957073 + `_: Snapshot delete now + succeeds and skips updating the source volume image if the image + does not exist in the backend.