openstack
/
cinder
Code Issues Proposed changes

3PAR volume detach with host in a host set 

When deleting the last VLUN for a host, try to
delete the host.  If the host is in a host set
this will fail.  When that happens, log a nice
message including the reason, but don't fail
or hang the delete VLUN.  Leave hosts in host
sets.

Closes-Bug: #1317134

Change-Id: Ie3f8f94c05a6a318de866598f9e9c4c5d84926f4
This commit is contained in:
Mark Sturdevant 2014-05-27 14:49:34 -07:00
parent aa9e6dca05
commit ffa12c9897
3 changed files with 108 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
cinder/tests/fake_hp_client_exceptions.py
View File

@ -20,6 +20,13 @@ class HTTPConflict(Exception):
http_status = 409
message = "Conflict"
def __init__(self, error=None):
if error and 'message' in error:
self._error_desc = error['message']
def get_description(self):
return self._error_desc
class HTTPNotFound(Exception):
http_status = 404
@ -31,9 +38,8 @@ class HTTPForbidden(Exception):
message = "Forbidden"
def __init__(self, error=None):
if error:
if 'code' in error:
self._error_code = error['code']
if error and 'code' in error:
self._error_code = error['code']
def get_code(self):
return self._error_code
@ -49,9 +55,8 @@ class HTTPServerError(Exception):
message = "Error"
def __init__(self, error=None):
if error:
if 'message' in error:
self._error_desc = error['message']
if error and 'message' in error:
self._error_desc = error['message']
def get_description(self):
return self._error_desc

48
cinder/tests/test_hp3par.py
View File

@ -1047,11 +1047,6 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
'volumeName': self.VOLUME_3PAR_NAME,
'lun': None, 'type': 0}]
self.driver.terminate_connection(
self.volume,
self.connector,
force=True)
expected = [
mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS),
mock.call.getHostVLUNs(self.FAKE_HOST),
@ -1063,7 +1058,50 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
mock.call.getPorts(),
mock.call.logout()]
self.driver.terminate_connection(self.volume, self.connector)
mock_client.assert_has_calls(expected)
mock_client.reset_mock()
# mock some deleteHost exceptions that are handled
delete_with_vlun = hpexceptions.HTTPConflict(
error={'message': "has exported VLUN"})
delete_with_hostset = hpexceptions.HTTPConflict(
error={'message': "host is a member of a set"})
mock_client.deleteHost = mock.Mock(
side_effect=[delete_with_vlun, delete_with_hostset])
self.driver.terminate_connection(self.volume, self.connector)
mock_client.assert_has_calls(expected)
mock_client.reset_mock()
self.driver.terminate_connection(self.volume, self.connector)
mock_client.assert_has_calls(expected)
def test_terminate_connection_more_vols(self):
mock_client = self.setup_driver()
# mock more than one vlun on the host (don't even try to remove host)
mock_client.getHostVLUNs.return_value = \
[
{'active': True,
'volumeName': self.VOLUME_3PAR_NAME,
'lun': None, 'type': 0},
{'active': True,
'volumeName': 'there-is-another-volume',
'lun': None, 'type': 0},
]
expect_less = [
mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS),
mock.call.getHostVLUNs(self.FAKE_HOST),
mock.call.deleteVLUN(
self.VOLUME_3PAR_NAME,
None,
self.FAKE_HOST),
mock.call.getPorts(),
mock.call.logout()]
self.driver.terminate_connection(self.volume, self.connector)
mock_client.assert_has_calls(expect_less)
def test_get_volume_stats(self):
# setup_mock_client drive with default configuration

74
cinder/volume/drivers/san/hp/hp_3par_common.py
View File

@ -128,10 +128,11 @@ class HP3PARCommon(object):
2.0.9 - Remove unused 3PAR driver method bug #1310807
2.0.10 - Fixed an issue with 3PAR vlun location bug #1315542
2.0.11 - Remove hp3parclient requirement from unit tests #1315195
2.0.12 - Volume detach hangs when host is in a host set bug #1317134
"""
VERSION = "2.0.11"
VERSION = "2.0.12"
stats = {}
@ -492,7 +493,7 @@ class HP3PARCommon(object):
if found_vlun is None:
msg = (_("3PAR vlun %(name)s not found on host %(host)s") %
{'name': volume_name, 'host': hostname})
LOG.warn(msg)
LOG.info(msg)
return found_vlun
def create_vlun(self, volume, host, nsp=None):
@ -506,26 +507,59 @@ class HP3PARCommon(object):
def delete_vlun(self, volume, hostname):
volume_name = self._get_3par_vol_name(volume['id'])
vlun = self._get_vlun(volume_name, hostname)
vluns = self.client.getHostVLUNs(hostname)
if vlun is not None:
# VLUN Type of MATCHED_SET 4 requires the port to be provided
if self.VLUN_TYPE_MATCHED_SET == vlun['type']:
self.client.deleteVLUN(volume_name, vlun['lun'], hostname,
vlun['portPos'])
else:
self.client.deleteVLUN(volume_name, vlun['lun'], hostname)
for vlun in vluns:
if volume_name in vlun['volumeName']:
break
else:
msg = (
_("3PAR vlun for volume %(name)s not found on host %(host)s") %
{'name': volume_name, 'host': hostname})
LOG.info(msg)
return
try:
self._delete_3par_host(hostname)
self._remove_hosts_naming_dict_host(hostname)
except hpexceptions.HTTPConflict as ex:
# host will only be removed after all vluns
# have been removed
if 'has exported VLUN' in ex.get_description():
pass
else:
raise
# VLUN Type of MATCHED_SET 4 requires the port to be provided
if self.VLUN_TYPE_MATCHED_SET == vlun['type']:
self.client.deleteVLUN(volume_name, vlun['lun'], hostname,
vlun['portPos'])
else:
self.client.deleteVLUN(volume_name, vlun['lun'], hostname)
# Determine if there are other volumes attached to the host.
# This will determine whether we should try removing host from host set
# and deleting the host.
for vlun in vluns:
if volume_name not in vlun['volumeName']:
# Found another volume
break
else:
# We deleted the last vlun, so try to delete the host too.
# This check avoids the old unnecessary try/fail when vluns exist
# but adds a minor race condition if a vlun is manually deleted
# externally at precisely the wrong time. Worst case is leftover
# host, so it is worth the unlikely risk.
try:
self._delete_3par_host(hostname)
self._remove_hosts_naming_dict_host(hostname)
except Exception as ex:
# Any exception down here is only logged. The vlun is deleted.
# If the host is in a host set, the delete host will fail and
# the host will remain in the host set. This is desired
# because cinder was not responsible for the host set
# assignment. The host set could be used outside of cinder
# for future needs (e.g. export volume to host set).
# The log info explains why the host was left alone.
msg = (_("3PAR vlun for volume '%(name)s' was deleted, "
"but the host '%(host)s' was not deleted because: "
"%(reason)s") %
{'name': volume_name,
'host': hostname,
'reason': ex.get_description()})
LOG.info(msg)
def _remove_hosts_naming_dict_host(self, hostname):
items = self.hosts_naming_dict.items()