openstack
/
cinder
Code Issues Proposed changes

PowerMax driver - handle special case where IG exists 

An 'Initiator' does not show up in the initiatior list unless
it is part of a masking view or has previously been logged on.
However these initiators can still be part of an 'Initiator Group'
and an attempt to create a new 'Initiator Group' will fail as
an 'Initiator' cannot be belong to more than one 'Initiator Group'.
Handling this special case and bringing the initiator group check
before the storage group logic so rollback, if required, is tidier.

Change-Id: I62f8a5d75d82352875b179f3e6e1a3f343bb531d
This commit is contained in:
Helen Walsh 2019-01-25 16:48:14 +00:00
parent 74ed84ba4a
commit 75ba4f0db6
4 changed files with 60 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax.py
View File

@ -7320,28 +7320,30 @@ class PowerMaxMaskingTest(test.TestCase):
self.data.defaultstoragegroup_name, self.extra_specs)
mock_create_mv.assert_called_once()
@mock.patch.object(
masking.PowerMaxMasking,
'_get_or_create_storage_group',
side_effect=["Storage group not found", None,
"Storage group not found", None, None, 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), (None, None), (None, None)])
@mock.patch.object(
masking.PowerMaxMasking,
'_get_or_create_storage_group',
side_effect=["Storage group not found", None,
"Storage group not found", "Storage group not found",
None, None, None, None, None, None, None])
@mock.patch.object(
masking.PowerMaxMasking,
'_move_vol_from_default_sg',
side_effect=["Storage group error", None, "Storage group error"])
side_effect=["Storage group error", None, "Storage group error",
None])
@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_create_SG):
self, mock_create_mv, mock_move, mock_create_SG,
mock_create_IG):
for x in range(0, 6):
self.driver.masking._create_new_masking_view(
self.data.array, self.maskingviewdict,
@ -7705,6 +7707,17 @@ class PowerMaxMaskingTest(test.TestCase):
self.extra_specs)
self.assertEqual(self.data.initiatorgroup_name_i, ret_init_group_name)
@mock.patch.object(rest.PowerMaxRest, 'create_initiator_group',
side_effect=([exception.VolumeBackendAPIException(
masking.CREATE_IG_ERROR)]))
def test_create_initiator_group_exception(self, mock_create_ig):
initiator_names = self.mask.find_initiator_names(self.data.connector)
self.assertRaises(
exception.VolumeBackendAPIException,
self.mask._create_initiator_group,
self.data.array, self.data.initiatorgroup_name_i, initiator_names,
self.extra_specs)
@mock.patch.object(masking.PowerMaxMasking,
'_last_volume_delete_initiator_group')
def test_check_ig_rollback(self, mock_last_volume):

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

@ -19,6 +19,7 @@ import sys
import time
from oslo_log import log as logging
import re
import six
from cinder import coordination
@ -30,6 +31,8 @@ from cinder.volume import utils as volume_utils
LOG = logging.getLogger(__name__)
CREATE_IG_ERROR = "is already in use in another Initiator Group"
class PowerMaxMasking(object):
"""Masking class for Dell EMC PowerMax.
@ -239,6 +242,11 @@ class PowerMaxMasking(object):
LOG.info("Port Group in masking view operation: %(port_group_name)s.",
{'port_group_name': port_group_name})
init_group_name, error_message = (self._get_or_create_initiator_group(
serial_number, init_group_name, connector, extra_specs))
if error_message:
return error_message
# get or create parent sg
error_message = self._get_or_create_storage_group(
serial_number, masking_view_dict, parent_sg_name, extra_specs,
@ -252,11 +260,6 @@ class PowerMaxMasking(object):
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:
return error_message
# Only after the components of the MV have been validated,
# move the volume from the default storage group to the
# masking view storage group. This is necessary before
@ -978,8 +981,19 @@ class PowerMaxMasking(object):
:param extra_specs: the extra specifications
:returns: the initiator group name
"""
self.rest.create_initiator_group(
serial_number, init_group_name, initiator_names, extra_specs)
try:
self.rest.create_initiator_group(
serial_number, init_group_name, initiator_names, extra_specs)
except exception.VolumeBackendAPIException as ex:
if re.search(CREATE_IG_ERROR, ex.msg):
LOG.error("It is probable that initiator(s) %(initiators)s "
"belong to an existing initiator group (host) "
"that is neither logged into the array or part "
"of a masking view and as such cannot be queried. "
"Please delete this initiator group (host) and "
"re-run the operation.",
{'initiators': initiator_names})
raise exception.VolumeBackendAPIException(message=ex)
return init_group_name
def _check_ig_rollback(

2
cinder/volume/drivers/dell_emc/powermax/provision.py
View File

@ -495,7 +495,7 @@ class PowerMaxProvision(object):
"%(valid_slos)s.", {'slo': slo, 'valid_slos': valid_slos})
if not is_valid_workload:
LOG.error(
LOG.warning(
"Workload: %(workload)s is not valid. Valid values are "
"%(valid_workloads)s. Note you cannot "
"set a workload without an SLO.",

29
cinder/volume/drivers/dell_emc/powermax/rest.py
View File

@ -186,7 +186,7 @@ class PowerMaxRest(object):
"Cinder Volume service to revert back to the primary "
"Unisphere instance.")
self.u4p_failover_lock = False
raise exception.VolumeBackendAPIException(data=msg)
raise exception.VolumeBackendAPIException(message=msg)
def request(self, target_uri, method, params=None, request_object=None,
u4p_check=False, retry=False):
@ -287,8 +287,8 @@ class PowerMaxRest(object):
LOG.exception(msg, {'method': method, 'url': url,
'e': six.text_type(e)})
raise exception.VolumeBackendAPIException(
data=(msg, {'method': method, 'url': url,
'e': six.text_type(e)}))
message=(msg, {'method': method, 'url': url,
'e': six.text_type(e)}))
return status_code, message
@ -326,7 +326,7 @@ class PowerMaxRest(object):
exception_message = (_("Issue encountered waiting for job."))
LOG.exception(exception_message)
raise exception.VolumeBackendAPIException(
data=exception_message)
message=exception_message)
if retries > int(extra_specs[utils.RETRIES]):
LOG.error("_wait_for_job_complete failed after "
@ -389,7 +389,7 @@ class PowerMaxRest(object):
'operation': operation, 'sc': status_code,
'message': message})
raise exception.VolumeBackendAPIException(
data=exception_message)
message=exception_message)
def wait_for_job(self, operation, status_code, job, extra_specs):
"""Check if call is async, wait for it to complete.
@ -413,7 +413,7 @@ class PowerMaxRest(object):
'error': six.text_type(result), 'status': status})
LOG.error(exception_message)
raise exception.VolumeBackendAPIException(
data=exception_message)
message=exception_message)
return task
@staticmethod
@ -1029,7 +1029,7 @@ class PowerMaxRest(object):
'dv': qos_unit})
LOG.error(exception_message)
raise exception.VolumeBackendAPIException(
data=exception_message)
message=exception_message)
return property_dict
@staticmethod
@ -1048,7 +1048,7 @@ class PowerMaxRest(object):
'dl': dynamic_list})
LOG.error(exception_message)
raise exception.VolumeBackendAPIException(
data=exception_message)
message=exception_message)
return property_dict
def set_storagegroup_srp(
@ -1138,7 +1138,8 @@ class PowerMaxRest(object):
exception_message = (_("Volume %(deviceID)s not found.")
% {'deviceID': device_id})
LOG.error(exception_message)
raise exception.VolumeBackendAPIException(data=exception_message)
raise exception.VolumeBackendAPIException(
message=exception_message)
return volume_dict
def _get_private_volume(self, array, device_id):
@ -1160,7 +1161,8 @@ class PowerMaxRest(object):
exception_message = (_("Volume %(deviceID)s not found.")
% {'deviceID': device_id})
LOG.error(exception_message)
raise exception.VolumeBackendAPIException(data=exception_message)
raise exception.VolumeBackendAPIException(
message=exception_message)
return volume_dict
def get_volume_list(self, array, params):
@ -1609,7 +1611,8 @@ class PowerMaxRest(object):
else:
exception_message = (_("Error retrieving masking group."))
LOG.error(exception_message)
raise exception.VolumeBackendAPIException(data=exception_message)
raise exception.VolumeBackendAPIException(
message=exception_message)
return element
def get_common_masking_views(self, array, portgroup_name, ig_name):
@ -1922,7 +1925,7 @@ class PowerMaxRest(object):
"synchronization."))
LOG.exception(exception_message)
raise exception.VolumeBackendAPIException(
data=exception_message)
message=exception_message)
if kwargs['retries'] > int(extra_specs[utils.RETRIES]):
LOG.error("_wait_for_sync failed after %(retries)d "
@ -2185,7 +2188,7 @@ class PowerMaxRest(object):
exception_message = _("Issue encountered waiting for job.")
LOG.exception(exception_message)
raise exception.VolumeBackendAPIException(
data=exception_message)
message=exception_message)
if retries > int(extra_specs[utils.RETRIES]):
LOG.error("_wait_for_consistent_state failed after "