diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 4d92afdcc03..dca373f23b4 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -294,6 +294,18 @@ class RBDTestCase(test.TestCase): self.qos_policy_b = {"read_iops_sec": "500", "write_iops_sec": "200"} + # For tests involving multiattach volume type + MULTIATTACH_FULL_FEATURES = ( + driver.RBDDriver.RBD_FEATURE_LAYERING | + driver.RBDDriver.RBD_FEATURE_EXCLUSIVE_LOCK | + driver.RBDDriver.RBD_FEATURE_OBJECT_MAP | + driver.RBDDriver.RBD_FEATURE_FAST_DIFF | + driver.RBDDriver.RBD_FEATURE_JOURNALING) + + MULTIATTACH_REDUCED_FEATURES = ( + driver.RBDDriver.RBD_FEATURE_LAYERING | + driver.RBDDriver.RBD_FEATURE_EXCLUSIVE_LOCK) + @ddt.data({'cluster_name': None, 'pool_name': 'rbd'}, {'cluster_name': 'volumes', 'pool_name': None}) @ddt.unpack @@ -737,6 +749,87 @@ class RBDTestCase(test.TestCase): self.assertEqual([self.mock_rbd.ImageNotFound], RAISED_EXCEPTIONS) + @common_mocks + def test_manage_existing_replicated_type(self): + client = self.mock_client.return_value + client.__enter__.return_value = client + + self.volume_a.volume_type = fake_volume.fake_volume_type_obj( + self.context, + id=fake.VOLUME_TYPE_ID, + extra_specs={'replication_enabled': ' True'}) + + with mock.patch.object(self.driver.rbd.RBD(), 'rename') as \ + mock_rbd_image_rename: + exist_volume = 'vol-exist' + existing_ref = {'source-name': exist_volume} + mock_rbd_image_rename.return_value = 0 + res = self.driver.manage_existing(self.volume_a, existing_ref) + mock_rbd_image_rename.assert_called_with( + client.ioctx, + exist_volume, + self.volume_a.name) + self.assertEqual('enabled', res['replication_status']) + + @common_mocks + def test_manage_existing_multiattach_type(self): + client = self.mock_client.return_value + client.__enter__.return_value = client + image = self.mock_proxy.return_value.__enter__.return_value + image_features = self.MULTIATTACH_FULL_FEATURES + image.features.return_value = image_features + expected_res = { + 'provider_location': "{\"saved_features\":%s}" % image_features} + + self.volume_a.volume_type = fake_volume.fake_volume_type_obj( + self.context, + id=fake.VOLUME_TYPE_ID, + extra_specs={'multiattach': ' True'}) + + with mock.patch.object(self.driver.rbd.RBD(), 'rename') as \ + mock_rbd_image_rename: + exist_volume = 'vol-exist' + existing_ref = {'source-name': exist_volume} + mock_rbd_image_rename.return_value = 0 + res = self.driver.manage_existing(self.volume_a, existing_ref) + mock_rbd_image_rename.assert_called_with( + client.ioctx, + exist_volume, + self.volume_a.name) + self.assertEqual(expected_res, res) + + @common_mocks + def test_manage_existing_invalid_type(self): + client = self.mock_client.return_value + client.__enter__.return_value = client + # Replication and multiattach are mutually exclusive + extra_specs = { + 'replication_enabled': ' True', + 'multiattach': ' True' + } + + self.volume_a.volume_type = fake_volume.fake_volume_type_obj( + self.context, + id=fake.VOLUME_TYPE_ID, + extra_specs=extra_specs) + + with mock.patch.object(self.driver.rbd.RBD(), 'rename') as \ + mock_rbd_image_rename: + exist_volume = 'vol-exist' + existing_ref = {'source-name': exist_volume} + mock_rbd_image_rename.return_value = 0 + res = self.assertRaises( + exception.ManageExistingVolumeTypeMismatch, + self.driver.manage_existing, self.volume_a, existing_ref) + self.assertIn( + "Manage existing volume failed due to volume type mismatch", + str(res)) + self.assertIn( + "Replication and Multiattach are mutually exclusive.", + str(res)) + # Ensure rename is not called + mock_rbd_image_rename.assert_not_called() + @common_mocks @mock.patch.object(driver.RBDDriver, '_get_image_status') def test_get_manageable_volumes(self, mock_get_image_status): @@ -3439,17 +3532,6 @@ class RBDTestCase(test.TestCase): self.driver.RBD_FEATURE_EXCLUSIVE_LOCK, self.driver.MULTIATTACH_EXCLUSIONS) - MULTIATTACH_FULL_FEATURES = ( - driver.RBDDriver.RBD_FEATURE_LAYERING | - driver.RBDDriver.RBD_FEATURE_EXCLUSIVE_LOCK | - driver.RBDDriver.RBD_FEATURE_OBJECT_MAP | - driver.RBDDriver.RBD_FEATURE_FAST_DIFF | - driver.RBDDriver.RBD_FEATURE_JOURNALING) - - MULTIATTACH_REDUCED_FEATURES = ( - driver.RBDDriver.RBD_FEATURE_LAYERING | - driver.RBDDriver.RBD_FEATURE_EXCLUSIVE_LOCK) - @ddt.data(MULTIATTACH_FULL_FEATURES, MULTIATTACH_REDUCED_FEATURES) @common_mocks def test_enable_multiattach(self, features): diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index 4ecd096eb5c..280e644fab8 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -2141,8 +2141,17 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, LOG.debug("Extend volume from %(old_size)s GB to %(new_size)s GB.", {'old_size': old_size, 'new_size': new_size}) + def _is_valid_type(self, volume_type): + want_replication = self._is_replicated_type(volume_type) + want_multiattach = self._is_multiattach_type(volume_type) + + if want_replication and want_multiattach: + return False + return True + def manage_existing(self, - volume: Volume, existing_ref: dict[str, str]) -> None: + volume: Volume, + existing_ref: dict[str, str]) -> dict[str, Any]: """Manages an existing image. Renames the image name to match the expected name for the volume. @@ -2154,12 +2163,17 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, existing_ref is a dictionary of the form: {'source-name': } """ + # check if the volume type is valid and fail fast if not + if not self._is_valid_type(volume.volume_type): + msg = _('Replication and Multiattach are mutually exclusive.') + raise exception.ManageExistingVolumeTypeMismatch(reason=msg) # Raise an exception if we didn't find a suitable rbd image. with RADOSClient(self) as client: rbd_name = existing_ref['source-name'] self.RBDProxy().rename(client.ioctx, utils.convert_str(rbd_name), volume.name) + return self._setup_volume(volume) def manage_existing_get_size(self, volume: Volume, diff --git a/releasenotes/notes/fix-manage-replicated-multiattach-9bc258d349e0f5a6.yaml b/releasenotes/notes/fix-manage-replicated-multiattach-9bc258d349e0f5a6.yaml new file mode 100644 index 00000000000..f3a6e53a99b --- /dev/null +++ b/releasenotes/notes/fix-manage-replicated-multiattach-9bc258d349e0f5a6.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + RBD `bug #2115985 + `_: Fixed + issue when managing a volume with ``multiattach`` or + ``replication_enabled`` properties in volume type.