openstack/cinder
Code Issues Proposed changes

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

Browse Source 
When we query an initiator group/host based on it's contained
initiator(s) it will not be found unless there is an entry
in the login table.  It is possible for an initiator
group/host to exist and not be in the login table, so when we
attempt to create it using it's OpenStack name
OS-<shortHostName>-<protocol>-IG, it may fail with an
'Host already exists' error, and need to be manually deleted.

This fix checks for the existence of the initiator group/host
based on it's OpenStack name OS-<shortHostName>-<protocol>-IG.
If it already exists, we will check that the contained
initiator(s) match those in the connector object and reuse it.

Change-Id: I925697cca1a4a10dcccd630dc14d24c1735b3a0a
This commit is contained in:
Helen Walsh 2021-04-23 15:43:35 +01:00
parent 84af44ed09
commit 97cd6052ae
6 changed files with 135 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py
View File

@ -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,

2
cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py
View File

@ -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

70
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py
View File

@ -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(

2
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
View File

@ -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)

60
cinder/volume/drivers/dell_emc/powermax/masking.py
View File

@ -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,

6
releasenotes/notes/powermax-existing-host-092f7daf29053d82.yaml Normal file
View File

@ -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.