From 51a06f3fcc30d0ceb9ccb98560993146e2273090 Mon Sep 17 00:00:00 2001 From: Sofia Enriquez Date: Wed, 28 Aug 2019 17:38:50 -0300 Subject: [PATCH] Allow removing NFS snapshots in error status The NFS driver doesn't allow to delete snapshots in error state when snapshot support is disable. I'm facing some scenarios where snapshots are enabled then a snapshot is created and after that then snapshots are disabled. This results in error state snapshots that can't be deleted. Another scenario is - with nfs snapshot support disable from the beginning- the API layer allows the snapshot to be created but the NFS driver sets the status to "error" leaving the snapshot in the DB with error state. Because of that, I have snapshots in error state that I'm not able to delete. The purpose of this fix allow deleting snapshots in error state. It makes sense to block creating snapshots when snapshot support is False. However, when deleting snapshot we don't want to block attempts to delete a broken snapshot DB entry, which is necessary to clean up a failed snapshot create. Closes-Bug: #1842088 Change-Id: Ieb19d5e3f58ae2343b6b145772fec33cb7517ab5 (cherry picked from commit 5714bdb8447a38440585116f784a322143711959) (cherry picked from commit 5a4f3ac53a554ecaa18cbd0a49c0606dff4e1137) (cherry picked from commit fb0efc42149fab524a9d3a95f01b7b71e954f314) (cherry picked from commit e9ef536958d9920b70cbbf27bc7faf548ad42d36) --- cinder/tests/unit/volume/drivers/test_nfs.py | 3 +++ cinder/volume/drivers/nfs.py | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cinder/tests/unit/volume/drivers/test_nfs.py b/cinder/tests/unit/volume/drivers/test_nfs.py index 1b2acb83da8..d6844089e55 100644 --- a/cinder/tests/unit/volume/drivers/test_nfs.py +++ b/cinder/tests/unit/volume/drivers/test_nfs.py @@ -1378,6 +1378,8 @@ class NfsDriverTestCase(test.TestCase): mock.patch.object(drv, '_read_info_file', return_value={}), \ mock.patch.object(drv, '_do_create_snapshot') \ as mock_do_create_snapshot, \ + mock.patch.object(drv, '_check_snapshot_support') \ + as mock_check_support, \ mock.patch.object(drv, '_write_info_file') \ as mock_write_info_file, \ mock.patch.object(drv, 'get_active_image_from_info', @@ -1386,6 +1388,7 @@ class NfsDriverTestCase(test.TestCase): return_value=snap_path): self._driver.create_snapshot(fake_snap) + mock_check_support.assert_called_once() mock_do_create_snapshot.assert_called_with(fake_snap, volume['name'], snap_path) mock_write_info_file.assert_called_with( diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 287f3efb5f2..ca4e8ed8b67 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -574,7 +574,6 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed): def delete_snapshot(self, snapshot): """Apply locking to the delete snapshot operation.""" - self._check_snapshot_support() return self._delete_snapshot(snapshot) def _copy_volume_from_snapshot(self, snapshot, volume, volume_size):