From 98acb8222135ed01d0d89c5f49247b4fd7d85add Mon Sep 17 00:00:00 2001 From: Sofia Enriquez Date: Tue, 23 Feb 2021 00:24:55 +0000 Subject: [PATCH] RBD: Fix _show_msg_check_clone_v2_api There might be a case where the exception raised by `op_features` could be other than AttributeError. When any exception other than AttributeError is raised, we should log and avoid raising an exception. Closes-Bug: #1942210 Co-Authored-By: Gorka Eguileor Change-Id: I513abe980b73d7e7b1a3cd9c7ff89490f7fd6b08 --- cinder/tests/unit/volume/drivers/test_rbd.py | 26 +++++++++++++++++++ cinder/volume/drivers/rbd.py | 21 ++++++++------- ...aise-attribute-error-40efd74bb92b9482.yaml | 10 +++++++ 3 files changed, 47 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/bug-1942210-show-msg-check-clone-v2-api-raise-attribute-error-40efd74bb92b9482.yaml diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 78b619ceb7f..21766541ceb 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -117,6 +117,9 @@ def common_mocks(f): inst.mock_rbd.PermissionError = MockPermissionError inst.driver.rbd = inst.mock_rbd + aux = inst.driver.rbd + aux.Image.return_value.stripe_unit.return_value = 4194304 + inst.driver.rados = inst.mock_rados return f(inst, *args, **kwargs) @@ -1102,6 +1105,29 @@ class RBDTestCase(test.TestCase): flatten_mock.assert_called_once_with(self.cfg.rbd_pool, self.volume_a.name) + @common_mocks + @mock.patch('cinder.objects.Volume.get_by_id') + @mock.patch.object(driver.RBDDriver, '_resize', mock.Mock()) + def test_log_create_vol_from_snap_raise(self, volume_get_by_id): + volume_get_by_id.return_value = self.volume_a + + self.mock_proxy().__enter__().volume.op_features.side_effect = \ + Exception + self.mock_rbd.RBD_OPERATION_FEATURE_CLONE_PARENT = 1 + + snapshot = self.snapshot + self.cfg.rbd_flatten_volume_from_snapshot = False + + with mock.patch.object(driver, 'LOG') as mock_log: + # Fist call + self.driver.create_volume_from_snapshot(self.volume_a, snapshot) + self.assertTrue(self.driver._clone_v2_api_checked) + # Second call + self.driver.create_volume_from_snapshot(self.volume_a, snapshot) + # Check that that the second call to create_volume_from_snapshot + # doesn't log anything + mock_log.warning.assert_called_once_with(mock.ANY) + @common_mocks @mock.patch('cinder.objects.Volume.get_by_id') def test_delete_snapshot(self, volume_get_by_id): diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index bc1b1b0ea96..265a51a6351 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -328,16 +328,17 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, self._clone_v2_api_checked = True with RBDVolumeProxy(self, volume_name, read_only=True) as volume: try: - if (volume.volume.op_features() & - self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT): - LOG.info('Using v2 Clone API') - return - except AttributeError: - pass - LOG.warning('Not using v2 clone API, please upgrade to' - ' mimic+ and set the OSD minimum client' - ' compat version to mimic for better' - ' performance, fewer deletion issues') + enabled = (volume.volume.op_features() & + self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT) + except Exception: + enabled = False + if enabled: + LOG.info('Using v2 Clone API') + else: + LOG.warning('Not using v2 clone API, please upgrade to' + ' mimic+ and set the OSD minimum client' + ' compat version to mimic for better' + ' performance, fewer deletion issues') def _get_target_config(self, target_id: Optional[str]) -> Dict[str, diff --git a/releasenotes/notes/bug-1942210-show-msg-check-clone-v2-api-raise-attribute-error-40efd74bb92b9482.yaml b/releasenotes/notes/bug-1942210-show-msg-check-clone-v2-api-raise-attribute-error-40efd74bb92b9482.yaml new file mode 100644 index 00000000000..ddc25f23037 --- /dev/null +++ b/releasenotes/notes/bug-1942210-show-msg-check-clone-v2-api-raise-attribute-error-40efd74bb92b9482.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + RBD driver `bug #1942210 + `_: When + creating a volume from a snapshot, the operation could + fail due to an uncaught exception being raised during a + check to see if the backend Ceph installation supported + the clone v2 API. The driver now handles this situation + gracefully.