summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTakashi Kajinami <tkajinam@redhat.com>2022-01-11 21:22:52 +0900
committerMasayuki Igawa <masayuki@igawa.io>2022-10-17 19:12:16 +0900
commit43fd2fadb7ce06a435967884124a69c96bb47d00 (patch)
tree688c8bdfe317387390d82ba0644114d4555575ec
parent1fa65ac7e28c4039dccf02e4c146d90891d7f2b1 (diff)
downloadcinder-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.py20
-rw-r--r--cinder/volume/drivers/rbd.py60
-rw-r--r--releasenotes/notes/bug-1957073-0d1307a8637a62b7.yaml6
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.