PowerMax Driver - Fix for renaming GVG

When a Generic Volume Group is renamed in OpenStack, the
corresponding storage group on the PowerMax does not reflect
this new name. The name format is <user-supplied-name><uuid>
on the PowerMax and now the former section has been changed.

To fix this issue we search based on the uuid alone and not
the combination of user supplied name and uuid. Once
the modified storage group object is returned successfully,
the storage group is renamed on the PowerMax.  Any subsequent
operation on the Generic Volume Group will proceed as before.

Closes-Bug: #1936848
Change-Id: Ia49ac163e6d9995368ef7931e51835f753b6623e

7 files changed, 149 insertions, 4 deletions

diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py
index d032902bf..161f9b31b 100644
--- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py
+++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py
@@ -2866,6 +2866,8 @@ class PowerMaxCommonTest(test.TestCase):
         mck_update.assert_called_once_with(group, add_vols, remove_vols)
         self.assertEqual(ref_model_update, model_update)
 
+    @mock.patch.object(common.PowerMaxCommon, '_find_volume_group',
+                       return_value=tpd.PowerMaxData.test_rep_group)
     @mock.patch.object(rest.PowerMaxRest, 'is_volume_in_storagegroup',
                        return_value=True)
     @mock.patch.object(
@@ -2882,7 +2884,7 @@ class PowerMaxCommonTest(test.TestCase):
         masking.PowerMaxMasking, 'remove_volumes_from_storage_group')
     def test_update_group_promotion(
             self, mck_rem, mock_cg_type, mock_type_check, mck_setup, mck_rep,
-            mck_in_sg):
+            mck_in_sg, mck_group):
         group = self.data.test_rep_group
         add_vols = []
         remove_vols = [self.data.test_volume_group_member]
diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py
index 66c52ea6d..a5eae2b7c 100644
--- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py
+++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py
@@ -695,7 +695,10 @@ class PowerMaxReplicationTest(test.TestCase):
         mck_validate.assert_called_once_with(
             self.common.rep_configs, extra_specs_list)
 
-    def test_enable_replication(self):
+    @mock.patch.object(common.PowerMaxCommon, '_find_volume_group',
+                       side_effect=[tpd.PowerMaxData.test_group,
+                                    None])
+    def test_enable_replication(self, mock_vg):
         # Case 1: Group not replicated
         with mock.patch.object(volume_utils, 'is_group_a_type',
                                return_value=False):
@@ -720,7 +723,10 @@ class PowerMaxReplicationTest(test.TestCase):
             self.assertEqual(fields.ReplicationStatus.ERROR,
                              model_update['replication_status'])
 
-    def test_disable_replication(self):
+    @mock.patch.object(common.PowerMaxCommon, '_find_volume_group',
+                       side_effect=[tpd.PowerMaxData.test_group,
+                                    None])
+    def test_disable_replication(self, mock_vg):
         # Case 1: Group not replicated
         with mock.patch.object(volume_utils, 'is_group_a_type',
                                return_value=False):
diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
index 45084ae4e..a3fac79c9 100644
--- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
+++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
@@ -2469,3 +2469,69 @@ class PowerMaxRestTest(test.TestCase):
                 self.data.array, self.data.device_id,
                 self.data.test_snapshot_snap_name)
             self.assertEqual('0', snap_id)
+
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group_list')
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group_rep',
+        side_effect=[{'rdf': False}, None])
+    def test_get_or_rename_storage_group_rep(
+            self, mock_sg_rep, mock_sg_list):
+        # Success - no need for rename
+        rep_info = self.rest.get_or_rename_storage_group_rep(
+            self.data.array, self.data.storagegroup_name_f,
+            self.data.extra_specs)
+        mock_sg_list.assert_not_called()
+        self.assertIsNotNone(rep_info)
+
+        # Fail - cannot find sg but no filter set
+        rep_info = self.rest.get_or_rename_storage_group_rep(
+            self.data.array, self.data.storagegroup_name_f,
+            self.data.extra_specs)
+        mock_sg_list.assert_not_called()
+        self.assertIsNone(rep_info)
+
+    @mock.patch.object(
+        rest.PowerMaxRest, '_rename_storage_group')
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group_list',
+        return_value=({'storageGroupId': ['user-name+uuid']}))
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group_rep',
+        side_effect=[None, ({'rdf': False}), ({'rdf': False})])
+    def test_get_or_rename_storage_group_rep_exists(
+            self, mock_sg_rep, mock_sg_list, mock_rename):
+        sg_filter = '<like>uuid'
+        rep_info = self.rest.get_or_rename_storage_group_rep(
+            self.data.array, self.data.storagegroup_name_f,
+            self.data.extra_specs, sg_filter=sg_filter)
+        mock_sg_list.assert_called_once_with(
+            self.data.array,
+            params={'storageGroupId': sg_filter})
+        group_list_return = {'storageGroupId': ['user-name+uuid']}
+        mock_rename.assert_called_once_with(
+            self.data.array,
+            group_list_return['storageGroupId'][0],
+            self.data.storagegroup_name_f,
+            self.data.extra_specs)
+        self.assertIsNotNone(rep_info)
+
+    @mock.patch.object(
+        rest.PowerMaxRest, '_rename_storage_group')
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group_list',
+        return_value=({'storageGroupId': ['user-name+uuid']}))
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group_rep',
+        side_effect=[None, None])
+    def test_get_or_rename_storage_group_rep_does_not_exist(
+            self, mock_sg_rep, mock_sg_list, mock_rename):
+        sg_filter = '<like>uuid'
+        rep_info = self.rest.get_or_rename_storage_group_rep(
+            self.data.array, self.data.storagegroup_name_f,
+            self.data.extra_specs, sg_filter=sg_filter)
+        mock_sg_list.assert_called_once_with(
+            self.data.array,
+            params={'storageGroupId': sg_filter})
+        mock_rename.assert_not_called()
+        self.assertIsNone(rep_info)
diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py
index 1935ef35b..2022621ed 100644
--- a/cinder/volume/drivers/dell_emc/powermax/common.py
+++ b/cinder/volume/drivers/dell_emc/powermax/common.py
@@ -535,6 +535,7 @@ class PowerMaxCommon(object):
         if group_id is not None:
             if group and (volume_utils.is_group_a_cg_snapshot_type(group)
                           or group.is_replicated):
+                self._find_volume_group(extra_specs[utils.ARRAY], group)
                 extra_specs[utils.FORCE_VOL_EDIT] = True
                 group_name = self._add_new_volume_to_volume_group(
                     volume, device_id, volume_name,
@@ -6071,8 +6072,12 @@ class PowerMaxCommon(object):
         :param group: the group object
         :returns: volume group dictionary
         """
+        __, interval_retries_dict = self._get_volume_group_info(group)
         group_name = self.utils.update_volume_group_name(group)
-        volume_group = self.rest.get_storage_group_rep(array, group_name)
+        sg_name_filter = utils.LIKE_FILTER + group.id
+        volume_group = self.rest.get_or_rename_storage_group_rep(
+            array, group_name, interval_retries_dict,
+            sg_filter=sg_name_filter)
         if not volume_group:
             LOG.warning("Volume group %(group_id)s cannot be found",
                         {'group_id': group_name})
diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py
index f18cf6b12..2ee58b8dd 100644
--- a/cinder/volume/drivers/dell_emc/powermax/rest.py
+++ b/cinder/volume/drivers/dell_emc/powermax/rest.py
@@ -3072,6 +3072,44 @@ class PowerMaxRest(object):
                 'src_device': device_id, 'tgt_device': r2_device_id,
                 'session_info': session_info}
 
+    def get_or_rename_storage_group_rep(
+            self, array, storage_group_name, extra_specs, sg_filter=None):
+        """Get storage group rep info if it exist.
+
+        If a generic volume group has been renamed we also need
+        to rename it on the array based on the uuid component.
+        We check for uuid if we cannot find it based on its old name.
+
+        :param array: the array serial number
+        :param storage_group_name: the name of the storage group
+        :param extra_specs: extra specification
+        :param sg_filter: uuid substring <like>
+        :returns: storage group dict or None
+        """
+        rep_details = self.get_storage_group_rep(array, storage_group_name)
+        if not rep_details:
+            # It is possible that the group has been renamed
+            if sg_filter:
+                sg_dict = self.get_storage_group_list(
+                    array, params={
+                        'storageGroupId': sg_filter})
+                sg_list = sg_dict.get('storageGroupId') if sg_dict else None
+                if sg_list and len(sg_list) == 1:
+                    rep_details = self.get_storage_group_rep(
+                        array, sg_list[0])
+                    # Update the new storage group name
+                    if rep_details:
+                        self._rename_storage_group(
+                            array, sg_list[0], storage_group_name, extra_specs)
+                        rep_details = self.get_storage_group_rep(
+                            array, storage_group_name)
+                        LOG.warning(
+                            "Volume group %(old)s has been renamed to %(new)s "
+                            "due to a rename operation in OpenStack.",
+                            {'old': sg_list[0], 'new': storage_group_name})
+
+        return rep_details
+
     def get_storage_group_rep(self, array, storage_group_name):
         """Given a name, return storage group details wrt replication.
 
@@ -3429,3 +3467,20 @@ class PowerMaxRest(object):
         """
         return (self.ucode_major_level >= utils.UCODE_5978 and
                 self.ucode_minor_level >= utils.UCODE_5978_HICKORY)
+
+    def _rename_storage_group(
+            self, array, old_name, new_name, extra_specs):
+        """Rename the storage group.
+
+        :param array: the array serial number
+        :param old_name: the original name
+        :param new_name: the new name
+        :param extra_specs: the extra specifications
+        """
+        payload = {"editStorageGroupActionParam": {
+            "renameStorageGroupParam": {
+                "new_storage_Group_name": new_name}}}
+        status_code, job = self.modify_storage_group(
+            array, old_name, payload)
+        self.wait_for_job(
+            'Rename storage group', status_code, job, extra_specs)
diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py
index 6956329ad..fc2b51294 100644
--- a/cinder/volume/drivers/dell_emc/powermax/utils.py
+++ b/cinder/volume/drivers/dell_emc/powermax/utils.py
@@ -216,6 +216,9 @@ REVERT_SS_EXC = 'Link must be fully copied for this operation to proceed'
 IS_TRUE = ['<is> True', 'True', 'true', True]
 IS_FALSE = ['<is> False', 'False', 'false', False]
 
+# <like> filter
+LIKE_FILTER = '<like>'
+
 
 class PowerMaxUtils(object):
     """Utility class for Rest based PowerMax volume drivers.
diff --git a/releasenotes/notes/bug-1936848-6ecc78e0e970419a.yaml b/releasenotes/notes/bug-1936848-6ecc78e0e970419a.yaml
new file mode 100644
index 000000000..5cf26d331
--- /dev/null
+++ b/releasenotes/notes/bug-1936848-6ecc78e0e970419a.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    PowerMax driver `bug #1936848
+    <https://bugs.launchpad.net/cinder/+bug/1936848>`_: Fixed
+    Generic Volume Group error where the name has been changed
+    in OpenStack and is not reflected on the corresponding storage
+    group on the PowerMax.