From 57892623dee9d64c0ac52e2830ff28b866f10d65 Mon Sep 17 00:00:00 2001 From: Silvan Kaiser Date: Wed, 6 May 2020 11:10:49 +0200 Subject: [PATCH] Disallow extension of attached volumes for NFS & Quobyte drivers NFS and Quobyte drivers no longer allow extension of an attached volume. The extend operation raises an ExtendVolumeError in the Cinder Volume service in case the volume to be extended is currently attached. This should be reverted once a a more capable solution for bug #1870367 has been found. Partial-bug: #1870367 Change-Id: Ib2a7c1cdf269b4907ff8adff1b9d900072eedde2 --- cinder/tests/unit/volume/drivers/test_nfs.py | 22 +++++++++++++++++++ .../tests/unit/volume/drivers/test_quobyte.py | 22 ++++++++++++++----- cinder/volume/drivers/nfs.py | 6 +++++ cinder/volume/drivers/quobyte.py | 6 +++++ doc/source/reference/support-matrix.ini | 4 ++-- .../notes/bug_1870367-49b74d10a9bfcf07.yaml | 7 ++++++ 6 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/bug_1870367-49b74d10a9bfcf07.yaml diff --git a/cinder/tests/unit/volume/drivers/test_nfs.py b/cinder/tests/unit/volume/drivers/test_nfs.py index 9de17b3c114..9c4b82cb532 100644 --- a/cinder/tests/unit/volume/drivers/test_nfs.py +++ b/cinder/tests/unit/volume/drivers/test_nfs.py @@ -997,6 +997,28 @@ class NfsDriverTestCase(test.TestCase): resize.assert_called_once_with(path, newSize, run_as_root=True) + def test_extend_volume_attached_fail(self): + """Extend a volume by 1.""" + self._set_driver() + drv = self._driver + volume = fake_volume.fake_volume_obj( + self.context, + id='80ee16b6-75d2-4d54-9539-ffc1b4b0fb10', + size=1, + provider_location='nfs_share') + path = 'path' + newSize = volume['size'] + 1 + + with mock.patch.object(drv, 'local_path', return_value=path): + with mock.patch.object(drv, '_is_share_eligible', + return_value=True): + with mock.patch.object(drv, '_is_file_size_equal', + return_value=True): + with mock.patch.object(drv, '_is_volume_attached', + return_value=True): + self.assertRaises(exception.ExtendVolumeError, + drv.extend_volume, volume, newSize) + def test_extend_volume_failure(self): """Error during extend operation.""" self._set_driver() diff --git a/cinder/tests/unit/volume/drivers/test_quobyte.py b/cinder/tests/unit/volume/drivers/test_quobyte.py index bc68631c8a6..3782ffdde21 100644 --- a/cinder/tests/unit/volume/drivers/test_quobyte.py +++ b/cinder/tests/unit/volume/drivers/test_quobyte.py @@ -927,7 +927,10 @@ class QuobyteDriverTestCase(test.TestCase): drv._ensure_share_mounted.assert_not_called() drv._execute.assert_not_called() - def test_extend_volume(self): + @ddt.data(True, False) + @mock.patch.object(remotefs.RemoteFSSnapDriverDistributed, + "_is_volume_attached") + def test_extend_volume(self, is_attached, mock_remote_attached): drv = self._driver volume = self._simple_volume() @@ -948,12 +951,19 @@ class QuobyteDriverTestCase(test.TestCase): image_utils.qemu_img_info = mock.Mock(return_value=img_info) image_utils.resize_image = mock.Mock() - drv.extend_volume(volume, 3) + mock_remote_attached.return_value = is_attached - image_utils.qemu_img_info.assert_called_once_with(volume_path, - force_share=True, - run_as_root=False) - image_utils.resize_image.assert_called_once_with(volume_path, 3) + if is_attached: + self.assertRaises(exception.ExtendVolumeError, drv.extend_volume, + volume, 3) + else: + drv.extend_volume(volume, 3) + + image_utils.qemu_img_info.assert_called_once_with(volume_path, + force_share=True, + run_as_root=False + ) + image_utils.resize_image.assert_called_once_with(volume_path, 3) def test_copy_volume_from_snapshot(self): drv = self._driver diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 287f3efb5f2..6529d7d1998 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -363,6 +363,12 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed): def extend_volume(self, volume, new_size): """Extend an existing volume to the new size.""" + if self._is_volume_attached(volume): + # NOTE(kaisers): no attached extensions until #1870367 is fixed + msg = (_("Cannot extend volume %s while it is attached.") + % volume['id']) + raise exception.ExtendVolumeError(msg) + LOG.info('Extending volume %s.', volume.id) extend_by = int(new_size) - volume.size if not self._is_share_eligible(volume.provider_location, diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index 1f53c192eb7..002a122cf82 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -540,6 +540,12 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriverDistributed): @utils.synchronized('quobyte', external=False) def extend_volume(self, volume, size_gb): + if self._is_volume_attached(volume): + # NOTE(kaisers): no attached extensions until #1870367 is fixed + msg = (_("Cannot extend volume %s while it is attached.") + % volume['id']) + raise exception.ExtendVolumeError(msg) + volume_path = self.local_path(volume) info = self._qemu_img_info(volume_path, volume.name) diff --git a/doc/source/reference/support-matrix.ini b/doc/source/reference/support-matrix.ini index 67ba999e812..a21bf482bc2 100644 --- a/doc/source/reference/support-matrix.ini +++ b/doc/source/reference/support-matrix.ini @@ -298,12 +298,12 @@ driver.nec=complete driver.netapp_ontap=missing driver.netapp_solidfire=complete driver.nexenta=complete -driver.nfs=complete +driver.nfs=missing driver.nimble=complete driver.prophetstor=complete driver.pure=complete driver.qnap=complete -driver.quobyte=complete +driver.quobyte=missing driver.rbd=complete driver.sandstone=complete driver.seagate=complete diff --git a/releasenotes/notes/bug_1870367-49b74d10a9bfcf07.yaml b/releasenotes/notes/bug_1870367-49b74d10a9bfcf07.yaml new file mode 100644 index 00000000000..584370eef1f --- /dev/null +++ b/releasenotes/notes/bug_1870367-49b74d10a9bfcf07.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `Bug #1870367 `_ : + Partially fixed NFS and Quobyte drivers by no longer allowing extending a + volume while it is attached, to prevent failures due to Qemu internal + locking mechanisms.