diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py index 1dca158b98f..4c3d7f2a407 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py +++ b/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], - 'hostId': initiatorgroup_name_f, - 'maskingview': [masking_view_name_f]}, - {'initiator': [initiator], - 'hostId': initiatorgroup_name_i, - 'maskingview': [masking_view_name_i]}] + initiator_group_fc = { + 'initiator': [wwpn1], + 'hostId': initiatorgroup_name_f, + 'maskingview': [masking_view_name_f]} + initiator_group_iscsi = { + '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, diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py index 794f548b0df..1358ac21839 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py +++ b/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 diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py index 40ce442760d..87da912ff15 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py +++ b/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) - 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.assertIsNotNone(msg) + 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. There are no initiators in OS-HostX-I-IG. Delete " + "initiator group OS-HostX-I-IG and retry.") + self.assertEqual(expected_msg, msg) + mock_create_ig.assert_not_called() def test_check_existing_initiator_group(self): with mock.patch.object( 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 45084ae4e38..a74e35de58c 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 @@ -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) diff --git a/cinder/volume/drivers/dell_emc/powermax/masking.py b/cinder/volume/drivers/dell_emc/powermax/masking.py index 5fad8cd212a..4904e564a4c 100644 --- a/cinder/volume/drivers/dell_emc/powermax/masking.py +++ b/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: - found_init_group = 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}) + if found_init_group_name is None: + # Check if the initiator group exists even if the initiators + # not found. This will happen if there is no entry for them + # in the login table + 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, diff --git a/releasenotes/notes/powermax-existing-host-092f7daf29053d82.yaml b/releasenotes/notes/powermax-existing-host-092f7daf29053d82.yaml new file mode 100644 index 00000000000..2a891dbcb20 --- /dev/null +++ b/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.