PowerMax driver - do the portgroup check earlier

If a portgroup does not exist, then this check should be done early in
the creation of a new masking view process, so no rollback action is
required on failure.

Change-Id: I02d8a37aa0551e3105dbffa7d1421c799e9e1753

cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax.py

@@ -7153,6 +7153,30 @@ class PowerMaxMaskingTest(test.TestCase):
         super(PowerMaxMaskingTest, self).tearDown()
         common.PowerMaxCommon._gather_info = self._gather_info
 
+    def test_sanity_port_group_check_none(self):
+        self.assertRaises(
+            exception.VolumeBackendAPIException,
+            self.driver.masking._sanity_port_group_check,
+            self.data.array, None)
+
+    @mock.patch.object(
+        rest.PowerMaxRest,
+        'get_portgroup',
+        return_value=None)
+    def test_sanity_port_group_check_invalid_portgroup(self, mock_pg):
+        self.assertRaises(
+            exception.VolumeBackendAPIException,
+            self.driver.masking._sanity_port_group_check,
+            self.data.array, None)
+
+    @mock.patch.object(
+        rest.PowerMaxRest,
+        'get_portgroup',
+        return_value=PowerMaxCommonData.portgroup)
+    def test_sanity_port_group_check(self, mock_pg):
+        self.driver.masking._sanity_port_group_check(
+            self.data.array, self.data.port_group_name_f)
+
     @mock.patch.object(
         masking.PowerMaxMasking,
         'get_or_create_masking_view_and_map_lun')
@@ -7274,27 +7298,22 @@ class PowerMaxMaskingTest(test.TestCase):
         side_effect=["Storage group not found", None,
                      "Storage group not found", None, None, None,
                      None, None, None, None, None])
-    @mock.patch.object(
-        masking.PowerMaxMasking,
-        '_check_port_group',
-        side_effect=[(None, "Port group error"), (None, None), (None, None),
-                     (None, None)])
     @mock.patch.object(
         masking.PowerMaxMasking,
         '_get_or_create_initiator_group',
         side_effect=[(None, "Initiator group error"), (None, None),
-                     (None, None)])
+                     (None, None), (None, None), (None, None)])
     @mock.patch.object(
         masking.PowerMaxMasking,
         '_move_vol_from_default_sg',
-        side_effect=["Storage group error", None])
+        side_effect=["Storage group error", None, "Storage group error"])
     @mock.patch.object(
         masking.PowerMaxMasking,
         'create_masking_view',
         return_value=None)
     def test_create_new_masking_view(
             self, mock_create_mv, mock_move, mock_create_IG,
-            mock_check_PG, mock_create_SG):
+            mock_create_SG):
         for x in range(0, 6):
             self.driver.masking._create_new_masking_view(
                 self.data.array, self.maskingviewdict,

cinder/volume/drivers/dell_emc/powermax/common.py

@@ -1450,6 +1450,11 @@ class PowerMaxCommon(object):
             extra_specs[utils.WORKLOAD])
         masking_view_dict[utils.ARRAY] = extra_specs[utils.ARRAY]
         masking_view_dict[utils.SRP] = extra_specs[utils.SRP]
+        if not extra_specs[utils.PORTGROUPNAME]:
+            LOG.warning("You must supply a valid pre-created port group "
+                        "in cinder.conf or as an extra spec. Port group "
+                        "cannot be left empty as creating a new masking "
+                        "view will fail.")
         masking_view_dict[utils.PORTGROUPNAME] = (
             extra_specs[utils.PORTGROUPNAME])
         masking_view_dict[utils.INITIATOR_CHECK] = (

cinder/volume/drivers/dell_emc/powermax/masking.py

@@ -180,6 +180,9 @@ class PowerMaxMasking(object):
         masking_view_details = self.rest.get_masking_view(
             serial_number, masking_view_name=maskingview_name)
         if not masking_view_details:
+            self._sanity_port_group_check(
+                masking_view_dict[utils.PORTGROUPNAME], serial_number)
+
             error_message = self._create_new_masking_view(
                 serial_number, masking_view_dict, maskingview_name,
                 default_sg_name, extra_specs)
@@ -192,6 +195,30 @@ class PowerMaxMasking(object):
 
         return error_message
 
+    def _sanity_port_group_check(self, port_group_name, serial_number):
+        """Check if the port group exists
+
+        :param port_group_name: the port group name (can be None)
+        :param serial_number: the array serial number
+        """
+        exc_msg = None
+        if port_group_name:
+            portgroup = self.rest.get_portgroup(
+                serial_number, port_group_name)
+            if not portgroup:
+                exc_msg = ("Failed to get portgroup %(pg)s."
+                           % {'pg': port_group_name})
+        else:
+            exc_msg = "Port group cannot be left empty."
+        if exc_msg:
+            exception_message = (_(
+                "%(exc_msg)s You must supply a valid pre-created "
+                "port group in cinder.conf or as an extra spec.")
+                % {'exc_msg': exc_msg})
+            LOG.error(exception_message)
+            raise exception.VolumeBackendAPIException(
+                message=exception_message)
+
     def _create_new_masking_view(
             self, serial_number, masking_view_dict,
             maskingview_name, default_sg_name, extra_specs):
@@ -225,11 +252,6 @@ class PowerMaxMasking(object):
         if error_message:
             return error_message
 
-        __, error_message = self._check_port_group(
-            serial_number, port_group_name)
-        if error_message:
-            return error_message
-
         init_group_name, error_message = (self._get_or_create_initiator_group(
             serial_number, init_group_name, connector, extra_specs))
         if error_message: