Add automatic creation and deletion of Purity hosts for PureISCSIDriver

The driver will now be responsible to manage hosts for new initiators
which do not already have a host created for them. This will allow for
backwards compatibility with the previous version that relied on hosts
being pre-configured for use by Cinder.


Implements: blueprint pure-iscsi-automatic-host-creation
Change-Id: I04b554e1a21128d55eddc21afda7108a868e4248

cinder/tests/test_pure.py

@@ -33,7 +33,13 @@ API_TOKEN = "12345678-abcd-1234-abcd-1234567890ab"
 VOLUME_BACKEND_NAME = "Pure_iSCSI"
 PORT_NAMES = ["ct0.eth2", "ct0.eth3", "ct1.eth2", "ct1.eth3"]
 ISCSI_IPS = ["10.0.0." + str(i + 1) for i in range(len(PORT_NAMES))]
-HOST_NAME = "pure-host"
+HOSTNAME = "computenode1"
+PURE_HOST_NAME = pure._generate_purity_host_name(HOSTNAME)
+PURE_HOST = {"name": PURE_HOST_NAME,
+             "hgroup": None,
+             "iqn": [],
+             "wwn": [],
+             }
 REST_VERSION = "1.2"
 VOLUME_ID = "abcdabcd-1234-abcd-1234-abcdeffedcba"
 VOLUME = {"name": "volume-" + VOLUME_ID,
@@ -62,7 +68,7 @@ SNAPSHOT = {"name": "snapshot-" + SNAPSHOT_ID,
             "display_name": "fake_snapshot",
             }
 INITIATOR_IQN = "iqn.1993-08.org.debian:01:222"
-CONNECTOR = {"initiator": INITIATOR_IQN}
+CONNECTOR = {"initiator": INITIATOR_IQN, "host": HOSTNAME}
 TARGET_IQN = "iqn.2010-06.com.purestorage:flasharray.12345abc"
 TARGET_PORT = "3260"
 ISCSI_PORTS = [{"name": name,
@@ -129,6 +135,19 @@ class PureISCSIDriverTestCase(test.TestCase):
                               func, *args, **kwargs)
             mock_func.side_effect = None
 
+    def test_generate_purity_host_name(self):
+        generate = pure._generate_purity_host_name
+        result = generate("really-long-string-thats-a-bit-too-long")
+        self.assertTrue(result.startswith("really-long-string-that-"))
+        self.assertTrue(result.endswith("-cinder"))
+        self.assertEqual(len(result), 63)
+        self.assertTrue(pure.GENERATED_NAME.match(result))
+        result = generate("!@#$%^-invalid&*")
+        self.assertTrue(result.startswith("invalid---"))
+        self.assertTrue(result.endswith("-cinder"))
+        self.assertEqual(len(result), 49)
+        self.assertTrue(pure.GENERATED_NAME.match(result))
+
     def test_create_volume(self):
         self.driver.create_volume(VOLUME)
         self.array.create_volume.assert_called_with(
@@ -244,7 +263,6 @@ class PureISCSIDriverTestCase(test.TestCase):
                                           "-t", "sendtargets",
                                           "-p", ISCSI_PORTS[1]["portal"]])
         self.assertFalse(mock_choose_port.called)
-        mock_iscsiadm.reset_mock()
         mock_iscsiadm.side_effect = [processutils.ProcessExecutionError, None]
         mock_choose_port.return_value = ISCSI_PORTS[2]
         self.assertEqual(self.driver._get_target_iscsi_port(), ISCSI_PORTS[2])
@@ -264,51 +282,94 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.assert_error_propagates([mock_iscsiadm, self.array.list_ports],
                                      self.driver._choose_target_iscsi_port)
 
-    @mock.patch(DRIVER_OBJ + "._get_host_name", autospec=True)
+    @mock.patch(DRIVER_OBJ + "._get_host", autospec=True)
-    def test_connect(self, mock_host):
+    @mock.patch(DRIVER_PATH + "._generate_purity_host_name", autospec=True)
+    def test_connect(self, mock_generate, mock_host):
         vol_name = VOLUME["name"] + "-cinder"
         result = {"vol": vol_name, "lun": 1}
-        mock_host.return_value = HOST_NAME
+        # Branch where host already exists
+        mock_host.return_value = PURE_HOST
         self.array.connect_host.return_value = {"vol": vol_name, "lun": 1}
         real_result = self.driver._connect(VOLUME, CONNECTOR)
         self.assertEqual(result, real_result)
         mock_host.assert_called_with(self.driver, CONNECTOR)
-        self.array.connect_host.assert_called_with(HOST_NAME, vol_name)
+        self.assertFalse(mock_generate.called)
-        self.assert_error_propagates([mock_host, self.array.connect_host],
+        self.assertFalse(self.array.create_host.called)
-                                     self.driver._connect,
+        self.array.connect_host.assert_called_with(PURE_HOST_NAME, vol_name)
-                                     VOLUME, CONNECTOR)
+        # Branch where new host is created
+        mock_host.return_value = None
+        mock_generate.return_value = PURE_HOST_NAME
+        real_result = self.driver._connect(VOLUME, CONNECTOR)
+        mock_host.assert_called_with(self.driver, CONNECTOR)
+        mock_generate.assert_called_with(HOSTNAME)
+        self.array.create_host.assert_called_with(PURE_HOST_NAME,
+                                                  iqnlist=[INITIATOR_IQN])
+        self.assertEqual(result, real_result)
+        # Branch where host is needed
+        mock_generate.reset_mock()
+        self.array.reset_mock()
+        self.assert_error_propagates(
+            [mock_host, mock_generate, self.array.connect_host,
+             self.array.create_host],
+            self.driver._connect, VOLUME, CONNECTOR)
 
-    def test_get_host_name(self):
+    def test_get_host(self):
-        good_host = {"name": HOST_NAME,
+        good_host = PURE_HOST.copy()
-                     "iqn": ["another-wrong-iqn", INITIATOR_IQN]}
+        good_host.update(iqn=["another-wrong-iqn", INITIATOR_IQN])
         bad_host = {"name": "bad-host", "iqn": ["wrong-iqn"]}
         self.array.list_hosts.return_value = [bad_host]
-        self.assertRaises(exception.PureDriverException,
+        real_result = self.driver._get_host(CONNECTOR)
-                          self.driver._get_host_name, CONNECTOR)
+        self.assertIs(real_result, None)
         self.array.list_hosts.return_value.append(good_host)
-        real_result = self.driver._get_host_name(CONNECTOR)
+        real_result = self.driver._get_host(CONNECTOR)
-        self.assertEqual(real_result, good_host["name"])
+        self.assertEqual(real_result, good_host)
         self.assert_error_propagates([self.array.list_hosts],
-                                     self.driver._get_host_name, CONNECTOR)
+                                     self.driver._get_host, CONNECTOR)
 
-    @mock.patch(DRIVER_OBJ + "._get_host_name", autospec=True)
+    @mock.patch(DRIVER_OBJ + "._get_host", autospec=True)
     def test_terminate_connection(self, mock_host):
         vol_name = VOLUME["name"] + "-cinder"
-        mock_host.return_value = HOST_NAME
+        mock_host.return_value = {"name": "some-host"}
+        # Branch with manually created host
         self.driver.terminate_connection(VOLUME, CONNECTOR)
-        self.array.disconnect_host.assert_called_with(HOST_NAME, vol_name)
+        self.array.disconnect_host.assert_called_with("some-host", vol_name)
+        self.assertFalse(self.array.list_host_connections.called)
+        self.assertFalse(self.array.delete_host.called)
+        # Branch with host added to host group
+        self.array.reset_mock()
+        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)
+        # Branch with host still having connected volumes
+        self.array.reset_mock()
+        self.array.list_host_connections.return_value = [
+            {"lun": 2, "name": PURE_HOST_NAME, "vol": "some-vol"}]
+        mock_host.return_value = PURE_HOST
+        self.driver.terminate_connection(VOLUME, CONNECTOR)
+        self.array.disconnect_host.assert_called_with(PURE_HOST_NAME, vol_name)
+        self.array.list_host_connections.assert_called_with(PURE_HOST_NAME,
+                                                            private=True)
+        self.assertFalse(self.array.delete_host.called)
+        # Branch where host gets deleted
+        self.array.reset_mock()
+        self.array.list_host_connections.return_value = []
+        self.driver.terminate_connection(VOLUME, CONNECTOR)
+        self.array.disconnect_host.assert_called_with(PURE_HOST_NAME, vol_name)
+        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 connection is missing and the host is still deleted
+        self.array.reset_mock()
         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.assert_called_with(PURE_HOST_NAME, vol_name)
-        self.array.disconnect_host.side_effect = None
+        self.array.list_host_connections.assert_called_with(PURE_HOST_NAME,
-        self.array.disconnect_host.reset_mock()
+                                                            private=True)
-        mock_host.side_effect = exception.PureDriverException(reason="reason")
+        self.array.delete_host.assert_called_with(PURE_HOST_NAME)
-        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)
 
     def test_get_volume_stats(self):
         self.assertEqual(self.driver.get_volume_stats(), {})

cinder/volume/drivers/pure.py

@@ -20,7 +20,9 @@ This driver requires Purity version 3.4.0 or later.
 
 import cookielib
 import json
+import re
 import urllib2
+import uuid
 
 from oslo.config import cfg
 
@@ -43,6 +45,9 @@ PURE_OPTS = [
 CONF = cfg.CONF
 CONF.register_opts(PURE_OPTS)
 
+INVALID_CHARACTERS = re.compile(r"[^-a-zA-Z0-9]")
+GENERATED_NAME = re.compile(r".*-[a-f0-9]{32}-cinder$")
+
 
 def _get_vol_name(volume):
     """Return the name of the volume Purity will use."""
@@ -55,6 +60,15 @@ def _get_snap_name(snapshot):
                                    snapshot["name"])
 
 
+def _generate_purity_host_name(name):
+    """Return a valid Purity host name based on the name passed in."""
+    if len(name) > 23:
+        name = name[0:23]
+    name = INVALID_CHARACTERS.sub("-", name)
+    name = name.lstrip("-")
+    return "{name}-{uuid}-cinder".format(name=name, uuid=uuid.uuid4().hex)
+
+
 class PureISCSIDriver(san.SanISCSIDriver):
     """Performs volume management on Pure Storage FlashArray."""
 
@@ -205,31 +219,36 @@ class PureISCSIDriver(san.SanISCSIDriver):
 
     def _connect(self, volume, connector):
         """Connect the host and volume; return dict describing connection."""
-        host_name = self._get_host_name(connector)
         vol_name = _get_vol_name(volume)
+        host = self._get_host(connector)
+        if host:
+            host_name = host["name"]
+            LOG.info(_("Re-using existing purity host {host_name!r}"
+                       ).format(host_name=host_name))
+        else:
+            host_name = _generate_purity_host_name(connector["host"])
+            iqn = connector["initiator"]
+            LOG.info(_("Creating host object {host_name!r} with IQN: {iqn}."
+                       ).format(host_name=host_name, iqn=iqn))
+            self._array.create_host(host_name, iqnlist=[iqn])
+
         return self._array.connect_host(host_name, vol_name)
 
-    def _get_host_name(self, connector):
+    def _get_host(self, connector):
-        """Return dictionary describing the Purity host with initiator IQN."""
+        """Return dict describing existing Purity host object or None."""
         hosts = self._array.list_hosts()
         for host in hosts:
             if connector["initiator"] in host["iqn"]:
-                return host["name"]
+                return host
-        raise exception.PureDriverException(
+        return None
-            reason=(_("No host object on target array with IQN: ") +
-                    connector["initiator"]))
 
     def terminate_connection(self, volume, connector, **kwargs):
         """Terminate connection."""
         LOG.debug("Enter PureISCSIDriver.terminate_connection.")
         vol_name = _get_vol_name(volume)
-        message = _("Disconnection failed with message: {0}")
+        host = self._get_host(connector)
-        try:
+        if host:
-            host_name = self._get_host_name(connector)
+            host_name = host["name"]
-        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:
@@ -237,7 +256,17 @@ class PureISCSIDriver(san.SanISCSIDriver):
                     if err.kwargs["code"] == 400:
                         # Happens if the host and volume are not connected.
                         ctxt.reraise = False
-                        LOG.error(message.format(err.msg))
+                        LOG.error(_("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(_("Deleting unneeded host {host_name!r}.").format(
+                    host_name=host_name))
+                self._array.delete_host(host_name)
+        else:
+            LOG.error(_("Unable to find host object in Purity with IQN: "
+                        "{iqn}.").format(iqn=connector["initiator"]))
         LOG.debug("Leave PureISCSIDriver.terminate_connection.")
 
     def get_volume_stats(self, refresh=False):
@@ -386,6 +415,21 @@ class FlashArray(object):
         """Return a list of dictionaries describing each host."""
         return self._http_request("GET", "host", kwargs)
 
+    def list_host_connections(self, host, **kwargs):
+        """Return a list of dictionaries describing connected volumes."""
+        return self._http_request("GET", "host/{host_name}/volume".format(
+            host_name=host), kwargs)
+
+    def create_host(self, host, **kwargs):
+        """Create a host."""
+        return self._http_request("POST", "host/{host_name}".format(
+            host_name=host), kwargs)
+
+    def delete_host(self, host):
+        """Delete a host."""
+        return self._http_request("DELETE", "host/{host_name}".format(
+            host_name=host))
+
     def connect_host(self, host, volume, **kwargs):
         """Create a connection between a host and a volume."""
         return self._http_request("POST",
@@ -397,6 +441,11 @@ class FlashArray(object):
         return self._http_request("DELETE",
                                   "host/{0}/volume/{1}".format(host, volume))
 
+    def set_host(self, host, **kwargs):
+        """Set an attribute of a host."""
+        return self._http_request("PUT", "host/{host_name}".format(
+            host_name=host), kwargs)
+
     def list_ports(self, **kwargs):
         """Return a list of dictionaries describing ports."""
         return self._http_request("GET", "port", kwargs)