diff options
author | Takashi Kajinami <tkajinam@redhat.com> | 2022-01-11 21:22:52 +0900 |
---|---|---|
committer | Masayuki Igawa <masayuki@igawa.io> | 2022-10-17 19:12:16 +0900 |
commit | 43fd2fadb7ce06a435967884124a69c96bb47d00 (patch) | |
tree | 688c8bdfe317387390d82ba0644114d4555575ec | |
parent | 1fa65ac7e28c4039dccf02e4c146d90891d7f2b1 (diff) | |
download | cinder-43fd2fadb7ce06a435967884124a69c96bb47d00.tar.gz |
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.
This commit also includes a squash to add a releasenote from
I40f6eb1f104da4410d32410940824a88f7d1ec62 to reduce backporting
workload.
Closes-Bug: #1957073
Change-Id: Icd9dad9ad7b3ad71b3962b078e5b94670ac41c87
(cherry picked from commit 3ddf7ca9ea9587318b8c903e2c43b1879846d1c2)
(cherry picked from commit cf108981432d35049bf28010d3191fd9f4b82ccc)
-rw-r--r-- | cinder/tests/unit/volume/drivers/test_rbd.py | 20 | ||||
-rw-r--r-- | cinder/volume/drivers/rbd.py | 60 | ||||
-rw-r--r-- | releasenotes/notes/bug-1957073-0d1307a8637a62b7.yaml | 6 |
3 files changed, 56 insertions, 30 deletions
diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 78b619ceb..f2307017a 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 @@ -1126,7 +1130,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 @@ -1186,6 +1190,18 @@ class RBDTestCase(test.TestCase): 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 e0fe18987..8db8a81a7 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -1366,34 +1366,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) - - 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) + 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}) + + 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 000000000..92c4584f5 --- /dev/null +++ b/releasenotes/notes/bug-1957073-0d1307a8637a62b7.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + RBD Driver `bug #1957073 + <https://bugs.launchpad.net/cinder/+bug/1957073>`_: Fixed snapshot deletion + failure when its volume doesn't exist. |