openstack
/
cinder
Code Issues Proposed changes

Fix exception handling in PureISCSIDriver 

PureISCSIDriver had some error logging messages that make use of empty
replacement fields "{}" in strings on which the .format() method is
called, as allowed in Python 2.7+. Python 2.6 requires explicitly
naming or enumerating fields, i.e., "{}" must be replaced with "{0}".
This change fixes this so PureISCSIDriver is Python 2.6 compatible.

PureISCSIDriver.terminate_connection() also changed to catch
errors raised by _get_host_name(). Exception handling generally changed
to be more correct.

This change also adds testing of these code path to the unit tests,
to make sure it's actually correct.

Change-Id: I84179a8bc59dcf5593664ab11c90b07c451fd360
Closes-Bug: 1357004
This commit is contained in:
Victor A. Ying 2014-08-14 15:03:58 -07:00
parent 4b973e90e0
commit be0cee3b95
2 changed files with 36 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
cinder/tests/test_pure.py
View File

@ -185,6 +185,10 @@ class PureISCSIDriverTestCase(test.TestCase):
self.driver.delete_volume(VOLUME)
expected = [mock.call.destroy_volume(vol_name)]
self.array.assert_has_calls(expected)
self.array.destroy_volume.side_effect = exception.PureAPIException(
code=400, reason="reason")
self.driver.delete_snapshot(SNAPSHOT)
self.array.destroy_volume.side_effect = None
self.assert_error_propagates([self.array.destroy_volume],
self.driver.delete_volume, VOLUME)
@ -201,6 +205,10 @@ class PureISCSIDriverTestCase(test.TestCase):
self.driver.delete_snapshot(SNAPSHOT)
expected = [mock.call.destroy_volume(snap_name)]
self.array.assert_has_calls(expected)
self.array.destroy_volume.side_effect = exception.PureAPIException(
code=400, reason="reason")
self.driver.delete_snapshot(SNAPSHOT)
self.array.destroy_volume.side_effect = None
self.assert_error_propagates([self.array.destroy_volume],
self.driver.delete_snapshot, SNAPSHOT)
@ -289,6 +297,15 @@ class PureISCSIDriverTestCase(test.TestCase):
mock_host.return_value = HOST_NAME
self.driver.terminate_connection(VOLUME, CONNECTOR)
self.array.disconnect_host.assert_called_with(HOST_NAME, vol_name)
self.array.disconnect_host.side_effect = exception.PureAPIException(
code=400, reason="reason")
self.driver.terminate_connection(VOLUME, CONNECTOR)
self.array.disconnect_host.assert_called_with(HOST_NAME, vol_name)
self.array.disconnect_host.side_effect = None
self.array.disconnect_host.reset_mock()
mock_host.side_effect = exception.PureDriverException(reason="reason")
self.assertFalse(self.array.disconnect_host.called)
mock_host.side_effect = None
self.assert_error_propagates(
[self.array.disconnect_host],
self.driver.terminate_connection, VOLUME, CONNECTOR)

33
cinder/volume/drivers/pure.py
View File

@ -123,11 +123,11 @@ class PureISCSIDriver(san.SanISCSIDriver):
try:
self._array.destroy_volume(vol_name)
except exception.PureAPIException as err:
with excutils.save_and_reraise_exception as ctxt:
with excutils.save_and_reraise_exception() as ctxt:
if err.kwargs["code"] == 400:
# This happens if the volume does not exist.
# Happens if the volume does not exist.
ctxt.reraise = False
LOG.error(_("Disconnection failed with message: {}"
LOG.error(_("Volume deletion failed with message: {0}"
).format(err.msg))
LOG.debug("Leave PureISCSIDriver.delete_volume.")
@ -145,11 +145,11 @@ class PureISCSIDriver(san.SanISCSIDriver):
try:
self._array.destroy_volume(snap_name)
except exception.PureAPIException as err:
with excutils.save_and_reraise_exception as ctxt:
with excutils.save_and_reraise_exception() as ctxt:
if err.kwargs["code"] == 400:
# This happens if the snapshot does not exist.
# Happens if the snapshot does not exist.
ctxt.reraise = False
LOG.error(_("Disconnection failed with message: {}"
LOG.error(_("Snapshot deletion failed with message: {0}"
).format(err.msg))
LOG.debug("Leave PureISCSIDriver.delete_snapshot.")
@ -223,16 +223,21 @@ class PureISCSIDriver(san.SanISCSIDriver):
"""Terminate connection."""
LOG.debug("Enter PureISCSIDriver.terminate_connection.")
vol_name = _get_vol_name(volume)
message = _("Disconnection failed with message: {0}")
try:
host_name = self._get_host_name(connector)
self._array.disconnect_host(host_name, vol_name)
except exception.PureAPIException as err:
with excutils.save_and_reraise_exception as ctxt:
if err.kwargs["code"] == 400:
# This happens if the host and volume are not connected
ctxt.reraise = False
LOG.error(_("Disconnection failed with message: {}"
).format(err.msg))
except exception.PureDriverException as err:
# Happens if the host object is missing.
LOG.error(message.format(err.msg))
else:
try:
self._array.disconnect_host(host_name, vol_name)
except exception.PureAPIException as err:
with excutils.save_and_reraise_exception() as ctxt:
if err.kwargs["code"] == 400:
# Happens if the host and volume are not connected.
ctxt.reraise = False
LOG.error(message.format(err.msg))
LOG.debug("Leave PureISCSIDriver.terminate_connection.")
def get_volume_stats(self, refresh=False):