From ccb39178ebc987979f4f2e236b94628df06bcda2 Mon Sep 17 00:00:00 2001 From: Patrick East Date: Fri, 22 Aug 2014 16:43:20 -0700 Subject: [PATCH] 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 | 121 +++++++++++++++++++++++++--------- cinder/volume/drivers/pure.py | 79 +++++++++++++++++----- 2 files changed, 155 insertions(+), 45 deletions(-) diff --git a/cinder/tests/test_pure.py b/cinder/tests/test_pure.py index bcf3af7ba..f8a172405 100644 --- a/cinder/tests/test_pure.py +++ b/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) - def test_connect(self, mock_host): + @mock.patch(DRIVER_OBJ + "._get_host", autospec=True) + @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.assert_error_propagates([mock_host, self.array.connect_host], - self.driver._connect, - VOLUME, CONNECTOR) + self.assertFalse(mock_generate.called) + self.assertFalse(self.array.create_host.called) + self.array.connect_host.assert_called_with(PURE_HOST_NAME, vol_name) + # 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): - good_host = {"name": HOST_NAME, - "iqn": ["another-wrong-iqn", INITIATOR_IQN]} + def test_get_host(self): + good_host = PURE_HOST.copy() + 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, - self.driver._get_host_name, CONNECTOR) + real_result = self.driver._get_host(CONNECTOR) + self.assertIs(real_result, None) self.array.list_hosts.return_value.append(good_host) - real_result = self.driver._get_host_name(CONNECTOR) - self.assertEqual(real_result, good_host["name"]) + real_result = self.driver._get_host(CONNECTOR) + 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("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(HOST_NAME, vol_name) + 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.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) + 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) def test_get_volume_stats(self): self.assertEqual(self.driver.get_volume_stats(), {}) diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 7c57be6c3..489dc8f0b 100644 --- a/cinder/volume/drivers/pure.py +++ b/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): - """Return dictionary describing the Purity host with initiator IQN.""" + def _get_host(self, connector): + """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"] - raise exception.PureDriverException( - reason=(_("No host object on target array with IQN: ") + - connector["initiator"])) + return host + return None 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}") - try: - host_name = self._get_host_name(connector) - except exception.PureDriverException as err: - # Happens if the host object is missing. - LOG.error(message.format(err.msg)) - else: + 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: @@ -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) -- 2.45.2