From 5714bdb8447a38440585116f784a322143711959 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 --- 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 51514bc6b97..fb3481f4387 100644 --- a/cinder/tests/unit/volume/drivers/test_nfs.py +++ b/cinder/tests/unit/volume/drivers/test_nfs.py @@ -1520,6 +1520,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', @@ -1528,6 +1530,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 5174774e376..2315d899d67 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -594,7 +594,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,