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

diff --git a/cinder/tests/test_pure.py b/cinder/tests/test_pure.py
index 612e49d20f2b5e4828706671d7c0a921f8378645..d56bc4329710be85e5d2bdd1624fffa7a1447b08 100644 (file)
--- 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 0c05254269a64a1235e9da21de545e451e116b68..7c57be6c33f6675ccad0e4d80a92b6a55b1d72ad 100644 (file)
--- 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):