PureISCSIDriver needs to disconnect hosts before deleting volumes.

Some error conditions can cause a situation where the volume is
“connected” to a Purity host, but the volume failed to attach on the
initiator side. If we then try to delete this volume it will cause
errors and leave orphaned volumes behind on the Flash Array.

The solution is to check for any connections and then remove them prior
to making a call to delete the volume.

Closes-Bug: 1388260
Change-Id: I8f5b3b28900d79228fd91f2ad53535224f263657

cinder/tests/test_pure.py

@@ -211,6 +211,29 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.assert_error_propagates([self.array.destroy_volume],
                                      self.driver.delete_volume, VOLUME)
 
+    def test_delete_connected_volume(self):
+        vol_name = VOLUME["name"] + "-cinder"
+        host_name_a = "ha"
+        host_name_b = "hb"
+        self.array.list_volume_hosts.return_value = [{
+            "host": host_name_a,
+            "lun": 7,
+            "name": vol_name,
+            "size": 3221225472
+        }, {
+            "host": host_name_b,
+            "lun": 2,
+            "name": vol_name,
+            "size": 3221225472
+        }]
+
+        self.driver.delete_volume(VOLUME)
+        expected = [mock.call.list_volume_hosts(vol_name),
+                    mock.call.disconnect_host(host_name_a, vol_name),
+                    mock.call.disconnect_host(host_name_b, vol_name),
+                    mock.call.destroy_volume(vol_name)]
+        self.array.assert_has_calls(expected)
+
     def test_create_snapshot(self):
         vol_name = SRC_VOL["name"] + "-cinder"
         self.driver.create_snapshot(SNAPSHOT)
@@ -337,12 +360,13 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.assertFalse(self.array.delete_host.called)
         # Branch with host added to host group
         self.array.reset_mock()
+        self.array.list_host_connections.return_value = []
         mock_host.return_value = PURE_HOST.copy()
         mock_host.return_value.update(hgroup="some-group")
         self.driver.terminate_connection(VOLUME, CONNECTOR)
         self.array.disconnect_host.assert_called_with(PURE_HOST_NAME, vol_name)
-        self.assertFalse(self.array.list_host_connections.called)
-        self.assertFalse(self.array.delete_host.called)
+        self.assertTrue(self.array.list_host_connections.called)
+        self.assertTrue(self.array.delete_host.called)
         # Branch with host still having connected volumes
         self.array.reset_mock()
         self.array.list_host_connections.return_value = [
@@ -370,6 +394,15 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.array.list_host_connections.assert_called_with(PURE_HOST_NAME,
                                                             private=True)
         self.array.delete_host.assert_called_with(PURE_HOST_NAME)
+        # Branch where an unexpected exception occurs
+        self.array.reset_mock()
+        self.array.disconnect_host.side_effect = exception.PureAPIException(
+            code=500, reason="unexpected exception")
+        self.assertRaises(exception.PureAPIException,
+                          self.driver.terminate_connection, VOLUME, CONNECTOR)
+        self.array.disconnect_host.assert_called_with(PURE_HOST_NAME, vol_name)
+        self.assertFalse(self.array.list_host_connections.called)
+        self.assertFalse(self.array.delete_host.called)
 
     def test_get_volume_stats(self):
         self.assertEqual(self.driver.get_volume_stats(), {})
@@ -686,6 +719,14 @@ class FlashArrayRESTTestCase(FlashArrayBaseTestCase):
         self.assert_error_propagates([mock_req], self.array.list_ports,
                                      **self.kwargs)
 
+    def test_list_volume_hosts(self, mock_req):
+        mock_req.return_value = self.result
+        result = self.array.list_volume_hosts("vol-name")
+        self.assertEqual(result, self.result)
+        mock_req.assert_called_with(self.array, "GET", "volume/vol-name/host")
+        self.assert_error_propagates([mock_req], self.array.list_volume_hosts,
+                                     "vol-name")
+
 
 class FakeFlashArray(pure.FlashArray):
 

cinder/volume/drivers/pure.py

@@ -130,9 +130,15 @@ class PureISCSIDriver(san.SanISCSIDriver):
             self._array.extend_volume(vol_name, vol_size)
 
     def delete_volume(self, volume):
-        """Deletes a volume."""
+        """Disconnect all hosts and delete the volume"""
         LOG.debug("Enter PureISCSIDriver.delete_volume.")
         vol_name = _get_vol_name(volume)
+
+        connected_hosts = self._array.list_volume_hosts(vol_name)
+        for host_info in connected_hosts:
+            host_name = host_info["host"]
+            self._disconnect_host(host_name, vol_name)
+
         try:
             self._array.destroy_volume(vol_name)
         except exception.PureAPIException as err:
@@ -252,27 +258,31 @@ class PureISCSIDriver(san.SanISCSIDriver):
         host = self._get_host(connector)
         if host:
             host_name = host["name"]
-            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(_LE("Disconnection failed "
-                                      "with message: {msg}."
-                                      ).format(msg=err.msg))
-            if (GENERATED_NAME.match(host_name) and not host["hgroup"] and
-                not self._array.list_host_connections(host_name,
-                                                      private=True)):
-                LOG.info(_LI("Deleting unneeded host %(host_name)r.") %
-                         {"host_name": host_name})
-                self._array.delete_host(host_name)
+            self._disconnect_host(host_name, vol_name)
         else:
             LOG.error(_LE("Unable to find host object in Purity with IQN: "
                           "{iqn}.").format(iqn=connector["initiator"]))
         LOG.debug("Leave PureISCSIDriver.terminate_connection.")
 
+    def _disconnect_host(self, host_name, vol_name):
+        LOG.debug("Enter PureISCSIDriver._disconnect_host.")
+        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(_LE("Disconnection failed with message: "
+                                  "%(msg)s.") % {"msg": err.msg})
+        if (GENERATED_NAME.match(host_name) and
+            not self._array.list_host_connections(host_name,
+                                                  private=True)):
+            LOG.info(_LI("Deleting unneeded host %(host_name)r.") %
+                     {"host_name": host_name})
+            self._array.delete_host(host_name)
+        LOG.debug("Leave PureISCSIDriver._disconnect_host.")
+
     def get_volume_stats(self, refresh=False):
         """Return the current state of the volume service.
 
@@ -448,3 +458,7 @@ class FlashArray(object):
     def list_ports(self, **kwargs):
         """Return a list of dictionaries describing ports."""
         return self._http_request("GET", "port", kwargs)
+
+    def list_volume_hosts(self, volume):
+        """Return a list of dictionaries describing connected hosts."""
+        return self._http_request("GET", "volume/%s/host" % volume)