openstack/cinder
Code Issues Proposed changes

PowerMax Driver - Initiator group contents check

Browse Source 
Previously, the checking the contents of an existing initiator group
was configurable.  If there was a mismatch between the initiators
from the connector object and the contents of the IG, without this
check, it left the system in an undesirable state.  Now, we check
for the mismatch and throw an exception if one occurs if the
initiator_check is not configured. If initiator_check is True, the
driver recreates the initiator group and masking view and the attach
operation can proceed.

Change-Id: I5e871679817b5aacbdd9aefdc888d1d43202b411
This commit is contained in:
Helen Walsh 2020-11-25 12:50:30 +00:00
parent 30501b9ecd
commit 4489ca3320
3 changed files with 201 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -394,13 +394,13 @@ class PowerMaxMaskingTest(test.TestCase):
def test_check_existing_initiator_group(self):
with mock.patch.object(
rest.PowerMaxRest, 'get_element_from_masking_view',
return_value=tpd.PowerMaxData.inititiatorgroup):
return_value=tpd.PowerMaxData.initiatorgroup_name_f):
ig_from_mv, msg = (
self.driver.masking._check_existing_initiator_group(
self.data.array, self.maskingviewdict['maskingview_name'],
self.maskingviewdict, self.data.storagegroup_name_i,
self.data.port_group_name_i, self.extra_specs))
self.assertEqual(self.data.inititiatorgroup, ig_from_mv)
self.assertEqual(self.data.initiatorgroup_name_f, ig_from_mv)
def test_check_adding_volume_to_storage_group(self):
with mock.patch.object(
@ -530,27 +530,88 @@ class PowerMaxMaskingTest(test.TestCase):
self.device_id, self.data.masking_view_dict_multiattach)
mock_return.assert_called_once()
@mock.patch.object(rest.PowerMaxRest, 'delete_masking_view')
@mock.patch.object(rest.PowerMaxRest, 'delete_initiator_group')
@mock.patch.object(rest.PowerMaxRest, 'get_initiator_group')
@mock.patch.object(masking.PowerMaxMasking, '_recreate_masking_view')
@mock.patch.object(rest.PowerMaxRest, 'get_initiator_group',
return_value=True)
def test_verify_initiator_group_from_masking_view(
self, mock_get_ig, mock_recreate_mv):
mv_dict = deepcopy(self.maskingviewdict)
mv_dict['initiator_check'] = True
self.mask._verify_initiator_group_from_masking_view(
self.data.array, mv_dict['maskingview_name'],
mv_dict, self.data.initiatorgroup_name_i,
self.data.storagegroup_name_i, self.data.port_group_name_i,
self.extra_specs)
mock_recreate_mv.assert_called()
@mock.patch.object(masking.PowerMaxMasking, '_recreate_masking_view')
@mock.patch.object(rest.PowerMaxRest, 'get_initiator_group',
return_value=True)
@mock.patch.object(
masking.PowerMaxMasking, '_find_initiator_group',
return_value=tpd.PowerMaxData.initiatorgroup_name_i)
def test_verify_initiator_group_from_masking_view(
self, mock_find_ig, mock_get_ig, mock_delete_ig, mock_delete_mv):
self.mask._verify_initiator_group_from_masking_view(
self.data.array, self.maskingviewdict['maskingview_name'],
self.maskingviewdict, self.data.initiatorgroup_name_i,
def test_verify_initiator_group_from_masking_view_no_recreate(
self, mock_find_ig, mock_get_ig, mock_recreate):
mv_dict = deepcopy(self.maskingviewdict)
mv_dict['initiator_check'] = False
self.assertRaises(
exception.VolumeBackendAPIException,
self.mask._verify_initiator_group_from_masking_view,
self.data.array, mv_dict['maskingview_name'],
mv_dict, 'OS-Wrong-Host-I-IG',
self.data.storagegroup_name_i, self.data.port_group_name_i,
self.extra_specs)
mock_get_ig.assert_not_called()
mock_get_ig.return_value = False
self.mask._verify_initiator_group_from_masking_view(
self.data.array, self.maskingviewdict['maskingview_name'],
self.maskingviewdict, 'OS-Wrong-Host-I-IG',
mock_recreate.assert_not_called()
@mock.patch.object(rest.PowerMaxRest, 'delete_initiator_group')
@mock.patch.object(rest.PowerMaxRest, 'get_initiator_group',
return_value=True)
def test_recreate_masking_view(
self, mock_get_ig, mock_delete_ig):
ig_from_conn = self.data.initiatorgroup_name_i
ig_from_mv = self.data.initiatorgroup_name_i
ig_openstack = self.data.initiatorgroup_name_i
self.mask._recreate_masking_view(
self.data.array, ig_from_conn, ig_from_mv,
ig_openstack, self.data.masking_view_name_i, [self.data.initiator],
self.data.storagegroup_name_i, self.data.port_group_name_i,
self.extra_specs)
mock_get_ig.assert_called()
mock_delete_ig.assert_not_called()
@mock.patch.object(rest.PowerMaxRest, 'delete_initiator_group')
@mock.patch.object(rest.PowerMaxRest, 'get_initiator_group',
return_value=True)
def test_recreate_masking_view_no_ig_from_connector(
self, mock_get_ig, mock_delete_ig):
ig_from_mv = self.data.initiatorgroup_name_i
ig_openstack = self.data.initiatorgroup_name_i
self.mask._recreate_masking_view(
self.data.array, None, ig_from_mv,
ig_openstack, self.data.masking_view_name_i, [self.data.initiator],
self.data.storagegroup_name_i, self.data.port_group_name_i,
self.extra_specs)
mock_delete_ig.assert_called()
@mock.patch.object(rest.PowerMaxRest, 'create_masking_view')
@mock.patch.object(rest.PowerMaxRest, 'get_initiator_group',
return_value=True)
def test_recreate_masking_view_wrong_host(
self, mock_get_ig, mock_create_mv):
ig_from_conn = 'OS-Wrong-Host-I-IG'
ig_from_mv = self.data.initiatorgroup_name_i
ig_openstack = self.data.initiatorgroup_name_i
self.mask._recreate_masking_view(
self.data.array, ig_from_conn, ig_from_mv,
ig_openstack, self.data.masking_view_name_i, [self.data.initiator],
self.data.storagegroup_name_i, self.data.port_group_name_i,
self.extra_specs)
mock_create_mv.assert_called()
@mock.patch.object(rest.PowerMaxRest, 'delete_masking_view')
@mock.patch.object(rest.PowerMaxRest, 'delete_initiator_group')
@ -559,23 +620,19 @@ class PowerMaxMaskingTest(test.TestCase):
@mock.patch.object(
masking.PowerMaxMasking, '_find_initiator_group',
return_value=tpd.PowerMaxData.initiatorgroup_name_i)
def test_verify_initiator_group_from_masking_view2(
def test_recreate_masking_view_delete_mv(
self, mock_find_ig, mock_get_ig, mock_delete_ig, mock_delete_mv):
mock_delete_mv.side_effect = [None, Exception]
self.mask._verify_initiator_group_from_masking_view(
self.data.array, self.maskingviewdict['maskingview_name'],
self.maskingviewdict, 'OS-Wrong-Host-I-IG',
mv_dict = deepcopy(self.maskingviewdict)
mv_dict['initiator_check'] = True
verify_flag = self.mask._verify_initiator_group_from_masking_view(
self.data.array, mv_dict['maskingview_name'],
mv_dict, 'OS-Wrong-Host-I-IG',
self.data.storagegroup_name_i, self.data.port_group_name_i,
self.extra_specs)
mock_delete_mv.assert_called()
_, found_ig_from_connector = (
self.mask._verify_initiator_group_from_masking_view(
self.data.array, self.maskingviewdict['maskingview_name'],
self.maskingviewdict, 'OS-Wrong-Host-I-IG',
self.data.storagegroup_name_i, self.data.port_group_name_i,
self.extra_specs))
self.assertEqual(self.data.initiatorgroup_name_i,
found_ig_from_connector)
self.assertTrue(verify_flag)
@mock.patch.object(rest.PowerMaxRest, 'create_initiator_group')
def test_create_initiator_group(self, mock_create_ig):

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

@ -645,19 +645,17 @@ class PowerMaxMasking(object):
msg = None
ig_from_mv = self.rest.get_element_from_masking_view(
serial_number, maskingview_name, host=True)
check_ig = masking_view_dict[utils.INITIATOR_CHECK]
if check_ig:
# First verify that the initiator group matches the initiators.
check, found_ig = self._verify_initiator_group_from_masking_view(
if not self._verify_initiator_group_from_masking_view(
serial_number, maskingview_name, masking_view_dict, ig_from_mv,
storagegroup_name, portgroup_name, extra_specs)
if not check:
storagegroup_name, portgroup_name, extra_specs):
msg = ("Unable to verify initiator group: %(ig_name)s "
"in masking view %(maskingview_name)s."
% {'ig_name': ig_from_mv,
'maskingview_name': maskingview_name})
LOG.error(msg)
return ig_from_mv, msg
def _check_adding_volume_to_storage_group(
@ -964,80 +962,116 @@ class PowerMaxMasking(object):
raise exception.VolumeBackendAPIException(message=error_message)
def _verify_initiator_group_from_masking_view(
self, serial_number, maskingview_name, maskingview_dict,
ig_from_mv, storagegroup_name, portgroup_name, extra_specs):
self, serial_number, masking_view_name, masking_view_dict,
ig_from_mv, storage_group_name, port_group_name, extra_specs):
"""Check that the initiator group contains the correct initiators.
:param serial_number: the array serial number
:param masking_view_name: name of the masking view
:param masking_view_dict: the masking view dict
:param ig_from_mv: the initiator group name
:param storage_group_name: the storage group
:param port_group_name: the port group
:param extra_specs: extra specifications
:returns: boolean
"""
connector = masking_view_dict['connector']
initiator_names = self.find_initiator_names(connector)
found_ig_from_connector = self._find_initiator_group(
serial_number, initiator_names)
if found_ig_from_connector != ig_from_mv:
check_ig_flag = masking_view_dict[utils.INITIATOR_CHECK]
if check_ig_flag:
return self._recreate_masking_view(
serial_number, found_ig_from_connector, ig_from_mv,
masking_view_dict['init_group_name'], masking_view_name,
initiator_names, storage_group_name, port_group_name,
extra_specs)
else:
msg = (_(
"Initiator group %(ig_conn)s contains initiators "
"%(init_list)s and does not match IG %(ig_mv)s "
"contained in masking view %(mv_name)s."
"Please delete the masking view or set 'initiator_check' "
"to True in the extra specs to let the driver do it for "
"you.")
% {'ig_conn': found_ig_from_connector,
'init_list': initiator_names,
'ig_mv': ig_from_mv,
'mv_name': masking_view_name})
LOG.error(msg)
raise exception.VolumeBackendAPIException(message=msg)
return True
def _recreate_masking_view(
self, serial_number, ig_from_conn, ig_from_mv, ig_name, mv_name,
initiator_names, sg_name, pg_name, extra_specs):
"""Recreate a masking view if the initiators do not match.
If using an existing masking view check that the initiator group
contains the correct initiators. If it does not contain the correct
initiators then we delete the initiator group from the masking view,
re-create it with the correct initiators and add it to the masking view
NOTE: PowerMax/VMAX does not support ModifyMaskingView so we must
first delete the masking view and recreate it.
:param serial_number: the array serial number
:param maskingview_name: name of the masking view
:param maskingview_dict: the masking view dict
:param ig_from_mv: the initiator group name
:param storagegroup_name: the storage group
:param portgroup_name: the port group
:param extra_specs: extra specifications
:returns: bool, found_ig_from_connector
"""
connector = maskingview_dict['connector']
initiator_names = self.find_initiator_names(connector)
found_ig_from_connector = self._find_initiator_group(
serial_number, initiator_names)
if found_ig_from_connector != ig_from_mv:
:param serial_number: the array serial number
:param ig_from_conn: initiator group from initiators in connector
:param ig_from_mv: initiator group from masking view
:param ig_name: drivers initiator group name by convention
:param mv_name: masking view
:param initiator_names: initiator(s) in the connector object
:param sg_name: storage group name
:param pg_name: port group name
:param extra_specs: extra specifications
:returns: boolean
"""
check_ig = self.rest.get_initiator_group(
serial_number, initiator_group=ig_from_mv)
if check_ig:
if found_ig_from_connector is None:
if not ig_from_conn:
# If the name of the current initiator group from the
# masking view matches the igGroupName supplied for the
# new group, the existing ig needs to be deleted before
# the new one with the correct initiators can be created.
if maskingview_dict['init_group_name'] == ig_from_mv:
if ig_name == ig_from_mv:
# Masking view needs to be deleted before IG
# can be deleted.
self.rest.delete_masking_view(
serial_number, maskingview_name)
serial_number, mv_name)
self.rest.delete_initiator_group(
serial_number, ig_from_mv)
found_ig_from_connector = (
ig_from_conn = (
self._create_initiator_group(
serial_number, ig_from_mv, initiator_names,
extra_specs))
if (found_ig_from_connector is not None and
storagegroup_name is not None and
portgroup_name is not None):
if ig_from_conn and sg_name and pg_name:
# Existing masking view (if still on the array) needs
# to be deleted before a new one can be created.
try:
self.rest.delete_masking_view(
serial_number, maskingview_name)
serial_number, mv_name)
except Exception:
pass
error_message = (
self.create_masking_view(
serial_number, maskingview_name, storagegroup_name,
portgroup_name,
maskingview_dict['init_group_name'],
serial_number, mv_name, sg_name, pg_name, ig_name,
extra_specs))
if not error_message:
LOG.debug(
"The old masking view has been replaced: "
"%(maskingview_name)s.",
{'maskingview_name': maskingview_name})
{'maskingview_name': mv_name})
else:
LOG.error(
"One of the components of the original masking view "
"%(maskingview_name)s cannot be retrieved so "
"please contact your system administrator to check "
"that the correct initiator(s) are part of masking.",
{'maskingview_name': maskingview_name})
{'maskingview_name': mv_name})
return False
return True, found_ig_from_connector
return True
def _create_initiator_group(
self, serial_number, init_group_name, initiator_names,

6
releasenotes/notes/powermax_initiator_check-249279d30e3f8322.yaml Normal file
View File

@ -0,0 +1,6 @@
fixes:
- |
PowerMax driver: Checking that the contents of the initiator group
match the contents of the connector regardless of the initiator_check
option being enabled. This will ensure an exception is raised if
there is a mismatch, in all scenarios.