RBD: Fix issue with managing volume with type properties

Certain properties like multiattach, replication etc. were
not inherited by the volume when we manage it using a volume type.
This patch enables it by calling the ``_setup_volume`` method in
the manage workflow to apply the volume type properties to the
RBD image.

Closes-Bug: #2115985

Signed-off-by: Rajat Dhasmana <rajatdhasmana@gmail.com>
Change-Id: Ia58b12a48ff853c82a3fc0a9a3f8ad8a54e284f0

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': '<is> 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': '<is> 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': '<is> True',
+            'multiattach': '<is> 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):

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': <name of RBD image>}
         """
+        # 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,

releasenotes/notes/fix-manage-replicated-multiattach-9bc258d349e0f5a6.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    RBD `bug #2115985
+    <https://bugs.launchpad.net/cinder/+bug/2115985>`_: Fixed
+    issue when managing a volume with ``multiattach`` or
+    ``replication_enabled`` properties in volume type.