From be0cee3b95e875788cfc5561837cff766ca87bc7 Mon Sep 17 00:00:00 2001 From: "Victor A. Ying" Date: Thu, 14 Aug 2014 15:03:58 -0700 Subject: [PATCH] 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 --- cinder/tests/test_pure.py | 17 +++++++++++++++++ cinder/volume/drivers/pure.py | 33 +++++++++++++++++++-------------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/cinder/tests/test_pure.py b/cinder/tests/test_pure.py index 612e49d20f2..d56bc432971 100644 --- a/cinder/tests/test_pure.py +++ b/cinder/tests/test_pure.py @@ -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) diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 0c05254269a..7c57be6c33f 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -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):