Merge "PowerMax Driver - Re-use existing initiator group/host."

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

@@ -634,12 +634,19 @@ class PowerMaxData(object):
     # vmax data
     # sloprovisioning
     compression_info = {'symmetrixId': ['000197800128']}
-    inititiatorgroup = [{'initiator': [wwpn1],
+    initiator_group_fc = {
-                         'hostId': initiatorgroup_name_f,
+        'initiator': [wwpn1],
-                         'maskingview': [masking_view_name_f]},
+        'hostId': initiatorgroup_name_f,
-                        {'initiator': [initiator],
+        'maskingview': [masking_view_name_f]}
-                         'hostId': initiatorgroup_name_i,
+    initiator_group_iscsi = {
-                         'maskingview': [masking_view_name_i]}]
+        'initiator': [initiator],
+        'hostId': initiatorgroup_name_i,
+        'maskingview': [masking_view_name_i]}
+    initiator_group_empty = {
+        'hostId': initiatorgroup_name_i,
+    }
+    initiator_group_list = [
+        initiator_group_fc, initiator_group_iscsi]
 
     initiator_list = [{'host': initiatorgroup_name_f,
                        'initiatorId': wwpn1,

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

@@ -196,7 +196,7 @@ class FakeRequestsSession(object):
 
     def _sloprovisioning_ig(self, url):
         return_object = None
-        for ig in self.data.inititiatorgroup:
+        for ig in self.data.initiator_group_list:
             if ig['hostId'] in url:
                 return_object = ig
                 break

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

@@ -399,27 +399,83 @@ class PowerMaxMaskingTest(test.TestCase):
         self.assertIsNotNone(msg)
         self.assertEqual(2, mock_get_pg.call_count)
 
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_initiator_group',
+        side_effect=[None, tpd.PowerMaxData.initiator_group_iscsi])
     @mock.patch.object(
         masking.PowerMaxMasking, '_find_initiator_group',
         side_effect=[tpd.PowerMaxData.initiatorgroup_name_i, None, None])
     @mock.patch.object(
         masking.PowerMaxMasking, '_create_initiator_group',
         side_effect=([tpd.PowerMaxData.initiatorgroup_name_i, None]))
-    def test_get_or_create_initiator_group(self, mock_create_ig, mock_find_ig):
+    def test_get_or_create_initiator_group(
+            self, mock_create_ig, mock_find_ig, mock_get_ig):
         self.driver.masking._get_or_create_initiator_group(
             self.data.array, self.data.initiatorgroup_name_i,
             self.data.connector, self.extra_specs)
         mock_create_ig.assert_not_called()
+        found_init_group_name, msg = (
+            self.driver.masking._get_or_create_initiator_group(
+                self.data.array, self.data.initiatorgroup_name_i,
+                self.data.connector, self.extra_specs))
+        self.assertIsNone(msg)
+        found_init_group_name, msg = (
+            self.driver.masking._get_or_create_initiator_group(
+                self.data.array, self.data.initiatorgroup_name_i,
+                self.data.connector, self.extra_specs))
+        self.assertIsNone(msg)
+
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_initiator_group',
+        return_value=tpd.PowerMaxData.initiator_group_iscsi)
+    @mock.patch.object(
+        masking.PowerMaxMasking, '_find_initiator_group',
+        return_value=None)
+    @mock.patch.object(
+        masking.PowerMaxMasking, '_create_initiator_group')
+    def test_get_or_create_initiator_group_not_logged_in(
+            self, mock_create_ig, mock_find_ig, mock_get_ig):
+        found_init_group, msg = (
+            self.driver.masking._get_or_create_initiator_group(
+                self.data.array, self.data.initiatorgroup_name_i,
+                self.data.connector, self.extra_specs))
+        mock_create_ig.assert_not_called()
+        self.assertIsNone(msg)
+
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_initiator_group',
+        side_effect=[tpd.PowerMaxData.initiator_group_fc,
+                     tpd.PowerMaxData.initiator_group_empty])
+    @mock.patch.object(
+        masking.PowerMaxMasking, '_find_initiator_group',
+        return_value=None)
+    @mock.patch.object(
+        masking.PowerMaxMasking, '_create_initiator_group')
+    def test_get_or_create_initiator_group_not_logged_in_errors(
+            self, mock_create_ig, mock_find_ig, mock_get_ig):
+        found_init_group, msg = (
+            self.driver.masking._get_or_create_initiator_group(
+                self.data.array, self.data.initiatorgroup_name_i,
+                self.data.connector, self.extra_specs))
+        expected_msg = (
+            "Found initiator group OS-HostX-I-IG, but could not find "
+            "initiator_names ['iqn.1993-08.org.debian:01:222'] in "
+            "the login table. The contained initiators ['123456789012345'] "
+            "do match up with those in the connector object. Delete initiator "
+            "group OS-HostX-I-IG and retry.")
+        self.assertEqual(expected_msg, msg)
+        mock_create_ig.assert_not_called()
         found_init_group, msg = (
             self.driver.masking._get_or_create_initiator_group(
                 self.data.array, self.data.initiatorgroup_name_i,
                 self.data.connector, self.extra_specs))
-        self.assertIsNone(msg)
+        expected_msg = (
-        found_init_group, msg = (
+            "Found initiator group OS-HostX-I-IG, but could not find "
-            self.driver.masking._get_or_create_initiator_group(
+            "initiator_names ['iqn.1993-08.org.debian:01:222'] in the login "
-                self.data.array, self.data.initiatorgroup_name_i,
+            "table. There are no initiators in OS-HostX-I-IG. Delete "
-                self.data.connector, self.extra_specs))
+            "initiator group OS-HostX-I-IG and retry.")
-        self.assertIsNotNone(msg)
+        self.assertEqual(expected_msg, msg)
+        mock_create_ig.assert_not_called()
 
     def test_check_existing_initiator_group(self):
         with mock.patch.object(

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

@@ -1012,7 +1012,7 @@ class PowerMaxRestTest(test.TestCase):
     def test_get_initiator_group(self):
         array = self.data.array
         ig_name = self.data.initiatorgroup_name_f
-        ref_ig = self.data.inititiatorgroup[0]
+        ref_ig = self.data.initiator_group_fc
         response_ig = self.rest.get_initiator_group(array, ig_name)
         self.assertEqual(ref_ig, response_ig)
 

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

@@ -651,28 +651,70 @@ class PowerMaxMasking(object):
         LOG.debug("The initiator name(s) are: %(initiatorNames)s.",
                   {'initiatorNames': initiator_names})
 
-        found_init_group = self._find_initiator_group(
+        found_init_group_name = self._find_initiator_group(
             serial_number, initiator_names)
 
         # If you cannot find an initiator group that matches the connector
         # info, create a new initiator group.
-        if found_init_group is None:
+        if found_init_group_name is None:
-            found_init_group = self._create_initiator_group(
+            # Check if the initiator group exists even if the initiators
-                serial_number, init_group_name, initiator_names, extra_specs)
+            # not found. This will happen if there is no entry for them
-            LOG.info("Created new initiator group name: %(init_group_name)s.",
+            # in the login table
-                     {'init_group_name': init_group_name})
+            initiator_group = self.rest.get_initiator_group(
+                serial_number, initiator_group=init_group_name)
+            if not initiator_group:
+                found_init_group_name = self._create_initiator_group(
+                    serial_number, init_group_name, initiator_names,
+                    extra_specs)
+                LOG.info("Created new initiator group name: "
+                         "%(init_group_name)s.",
+                         {'init_group_name': init_group_name})
+            else:
+                initiator_list = initiator_group.get(
+                    'initiator', list()) if initiator_group else list()
+                if initiator_list:
+                    if set(initiator_list) == set(initiator_names):
+                        LOG.debug(
+                            "Found initiator group %(ign)s, but could not "
+                            "find initiator_names %(ins)s in the login "
+                            "table. The contained initiator(s) are the "
+                            "same as those supplied by OpenStack, therefore "
+                            "reusing %(ign)s.",
+                            {'ign': init_group_name,
+                             'ins': initiator_names})
+                    else:
+                        msg = ("Found initiator group %(ign)s, but could not "
+                               "find initiator_names %(ins)s in the login "
+                               "table. The contained initiators %(ins_host)s "
+                               "do match up with those in the connector "
+                               "object. Delete initiator group %(ign)s and "
+                               "retry." % {'ign': init_group_name,
+                                           'ins': initiator_names,
+                                           'ins_host': initiator_list})
+                        LOG.error(msg)
+                        return None, msg
+                else:
+                    msg = ("Found initiator group %(ign)s, but could not "
+                           "find initiator_names %(ins)s in the login "
+                           "table. There are no initiators in %(ign)s. "
+                           "Delete initiator group %(ign)s and retry."
+                           % {'ign': init_group_name, 'ins': initiator_names})
+                    LOG.error(msg)
+                    return None, msg
+
+                found_init_group_name = initiator_group.get('hostId')
         else:
             LOG.info("Using existing initiator group name: "
                      "%(init_group_name)s.",
-                     {'init_group_name': found_init_group})
+                     {'init_group_name': found_init_group_name})
 
-        if found_init_group is None:
+        if found_init_group_name is None:
             msg = ("Cannot get or create initiator group: "
                    "%(init_group_name)s. "
                    % {'init_group_name': init_group_name})
             LOG.error(msg)
 
-        return found_init_group, msg
+        return found_init_group_name, msg
 
     def _check_existing_initiator_group(
             self, serial_number, maskingview_name, masking_view_dict,

releasenotes/notes/powermax-existing-host-092f7daf29053d82.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    PowerMax driver : Enhancement to use an existing initiator group even if
+    there is no entry for the contained initiator(s) in the login table. This
+    is permissable so long as the initiator(s) in the connector object match.