iscsi-enhancements.diff

commit a654e58c5c25cbf2baade37bebb2eda9388444c5
Author: Patrick East <patrick.east@purestorage.com>
Date:   Fri Aug 19 18:50:04 2016 -0700

    Kilo back-port of the iSCSI multipath enhancements

    Inspired by https://github.com/openstack/cinder/commit/72273183871a244426f3237118a9002aa53063e7

diff --git a/cinder/tests/test_pure.py b/cinder/tests/test_pure.py
index 2157ce6..b97b6a3 100644
--- a/cinder/tests/test_pure.py
+++ b/cinder/tests/test_pure.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.

+from copy import deepcopy
 import sys

 import mock
@@ -118,14 +119,19 @@ SPACE_INFO_EMPTY = {"capacity": TOTAL_CAPACITY * units.Gi,
                     "total": 0
                     }

-CONNECTION_INFO = {"driver_volume_type": "iscsi",
-                   "data": {"target_iqn": TARGET_IQN,
-                            "target_portal": ISCSI_IPS[0] + ":" + TARGET_PORT,
-                            "target_lun": 1,
-                            "target_discovered": True,
-                            "access_mode": "rw",
-                            },
-                   }
+CONNECTION_INFO = {
+    "driver_volume_type": "iscsi",
+    "data": {
+        "target_discovered": False,
+        "access_mode": "rw",
+        "target_luns": [1, 1, 1, 1],
+        "target_iqns": [TARGET_IQN, TARGET_IQN, TARGET_IQN, TARGET_IQN],
+        "target_portals": [ISCSI_IPS[0] + ":" + TARGET_PORT,
+                           ISCSI_IPS[1] + ":" + TARGET_PORT,
+                           ISCSI_IPS[2] + ":" + TARGET_PORT,
+                           ISCSI_IPS[3] + ":" + TARGET_PORT],
+        },
+    }


 class FakePureStorageHTTPError(Exception):
@@ -153,9 +159,7 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.purestorage_module = pure.purestorage
         self.purestorage_module.PureHTTPError = FakePureStorageHTTPError

-    @mock.patch(DRIVER_OBJ + "._choose_target_iscsi_port")
-    def test_do_setup(self, mock_choose_target_iscsi_port):
-        mock_choose_target_iscsi_port.return_value = ISCSI_PORTS[0]
+    def test_do_setup(self):
         self.purestorage_module.FlashArray.return_value = self.array
         self.array.get_rest_version.return_value = \
             self.driver.SUPPORTED_REST_API_VERSIONS[0]
@@ -169,15 +173,6 @@ class PureISCSIDriverTestCase(test.TestCase):
             self.driver.SUPPORTED_REST_API_VERSIONS,
             self.purestorage_module.FlashArray.supported_rest_versions
         )
-        mock_choose_target_iscsi_port.assert_called_with()
-        self.assertEqual(ISCSI_PORTS[0], self.driver._iscsi_port)
-        self.assert_error_propagates(
-            [
-                self.purestorage_module.FlashArray,
-                mock_choose_target_iscsi_port
-            ],
-            self.driver.do_setup, None
-        )

     def assert_error_propagates(self, mocks, func, *args, **kwargs):
         """Assert that errors from mocks propagate to func.
@@ -392,29 +387,33 @@ class PureISCSIDriverTestCase(test.TestCase):
                                      self.driver.delete_snapshot, SNAPSHOT)

     @mock.patch(DRIVER_OBJ + "._connect")
-    @mock.patch(DRIVER_OBJ + "._get_target_iscsi_port")
-    def test_initialize_connection(self, mock_get_iscsi_port, mock_connection):
-        mock_get_iscsi_port.return_value = ISCSI_PORTS[0]
-        mock_connection.return_value = {"vol": VOLUME["name"] + "-cinder",
-                                        "lun": 1,
-                                        }
-        result = CONNECTION_INFO
+    @mock.patch(DRIVER_OBJ + "._get_target_iscsi_ports")
+    def test_initialize_connection(self, mock_get_iscsi_ports,
+                                   mock_connection):
+        mock_get_iscsi_ports.return_value = ISCSI_PORTS
+        lun = 1
+        connection = {
+            "vol": VOLUME["name"] + "-cinder",
+            "lun": lun,
+        }
+        mock_connection.return_value = connection
+        result = deepcopy(CONNECTION_INFO)
         real_result = self.driver.initialize_connection(VOLUME, CONNECTOR)
         self.assertDictMatch(result, real_result)
-        mock_get_iscsi_port.assert_called_with()
+        mock_get_iscsi_ports.assert_called_with()
         mock_connection.assert_called_with(VOLUME, CONNECTOR, None)
-        self.assert_error_propagates([mock_get_iscsi_port, mock_connection],
+        self.assert_error_propagates([mock_get_iscsi_ports, mock_connection],
                                      self.driver.initialize_connection,
                                      VOLUME, CONNECTOR)

     @mock.patch(DRIVER_OBJ + "._connect")
-    @mock.patch(DRIVER_OBJ + "._get_target_iscsi_port")
-    def test_initialize_connection_with_auth(self, mock_get_iscsi_port,
+    @mock.patch(DRIVER_OBJ + "._get_target_iscsi_ports")
+    def test_initialize_connection_with_auth(self, mock_get_iscsi_ports,
                                              mock_connection):
         auth_type = "CHAP"
         chap_username = CONNECTOR["host"]
         chap_password = "password"
-        mock_get_iscsi_port.return_value = ISCSI_PORTS[0]
+        mock_get_iscsi_ports.return_value = ISCSI_PORTS
         initiator_update = [{"key": pure.CHAP_SECRET_KEY,
                             "value": chap_password}]
         mock_connection.return_value = {
@@ -423,7 +422,7 @@ class PureISCSIDriverTestCase(test.TestCase):
             "auth_username": chap_username,
             "auth_password": chap_password,
         }
-        result = CONNECTION_INFO.copy()
+        result = deepcopy(CONNECTION_INFO)
         result["data"]["auth_method"] = auth_type
         result["data"]["auth_username"] = chap_username
         result["data"]["auth_password"] = chap_password
@@ -444,37 +443,57 @@ class PureISCSIDriverTestCase(test.TestCase):
         mock_connection.assert_called_with(VOLUME, CONNECTOR, None)
         self.assertDictMatch(result, real_result)

-        self.assert_error_propagates([mock_get_iscsi_port, mock_connection],
+        self.assert_error_propagates([mock_get_iscsi_ports, mock_connection],
                                      self.driver.initialize_connection,
                                      VOLUME, CONNECTOR)

-    @mock.patch(DRIVER_OBJ + "._choose_target_iscsi_port")
-    @mock.patch(DRIVER_OBJ + "._run_iscsiadm_bare")
-    def test_get_target_iscsi_port(self, mock_iscsiadm, mock_choose_port):
-        self.driver._iscsi_port = ISCSI_PORTS[1]
-        self.assertEqual(self.driver._get_target_iscsi_port(), ISCSI_PORTS[1])
-        mock_iscsiadm.assert_called_with(["-m", "discovery",
-                                          "-t", "sendtargets",
-                                          "-p", ISCSI_PORTS[1]["portal"]])
-        self.assertFalse(mock_choose_port.called)
-        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])
-        mock_choose_port.assert_called_with()
-        mock_iscsiadm.side_effect = processutils.ProcessExecutionError
-        self.assert_error_propagates([mock_choose_port],
-                                     self.driver._get_target_iscsi_port)
-
-    @mock.patch(DRIVER_OBJ + "._run_iscsiadm_bare")
-    def test_choose_target_iscsi_port(self, mock_iscsiadm):
+    @mock.patch(DRIVER_OBJ + "._connect")
+    @mock.patch(DRIVER_OBJ + "._get_target_iscsi_ports")
+    def test_initialize_connection_multipath(self,
+                                             mock_get_iscsi_ports,
+                                             mock_connection):
+        mock_get_iscsi_ports.return_value = ISCSI_PORTS
+        lun = 1
+        connection = {
+            "vol": VOLUME["name"] + "-cinder",
+            "lun": lun,
+        }
+        mock_connection.return_value = connection
+        multipath_connector = deepcopy(CONNECTOR)
+        multipath_connector["multipath"] = True
+        result = deepcopy(CONNECTION_INFO)
+
+        real_result = self.driver.initialize_connection(VOLUME,
+                                                        multipath_connector)
+        self.assertDictMatch(result, real_result)
+        mock_get_iscsi_ports.assert_called_with()
+        mock_connection.assert_called_with(VOLUME, multipath_connector, None)
+
+        multipath_connector["multipath"] = False
+        self.driver.initialize_connection(VOLUME, multipath_connector)
+
+    def test_get_target_iscsi_ports(self):
+        self.array.list_ports.return_value = ISCSI_PORTS
+        ret = self.driver._get_target_iscsi_ports()
+        self.assertEqual(ISCSI_PORTS, ret)
+
+    def test_get_target_iscsi_ports_with_iscsi_and_fc(self):
+        self.array.list_ports.return_value = PORTS_WITH
+        ret = self.driver._get_target_iscsi_ports()
+        self.assertEqual(ISCSI_PORTS, ret)
+
+    def test_get_target_iscsi_ports_with_no_ports(self):
+        # Should raise an exception if there are no ports
+        self.array.list_ports.return_value = []
+        self.assertRaises(exception.PureDriverException,
+                          self.driver._get_target_iscsi_ports)
+
+    def test_get_target_iscsi_ports_with_only_fc_ports(self):
+        # Should raise an exception of there are no iscsi ports
         self.array.list_ports.return_value = PORTS_WITHOUT
         self.assertRaises(exception.PureDriverException,
-                          self.driver._choose_target_iscsi_port)
-        self.array.list_ports.return_value = PORTS_WITH
-        self.assertEqual(ISCSI_PORTS[0],
-                         self.driver._choose_target_iscsi_port())
-        self.assert_error_propagates([mock_iscsiadm, self.array.list_ports],
-                                     self.driver._choose_target_iscsi_port)
+                          self.driver._get_target_iscsi_ports)
+

     @mock.patch(DRIVER_PATH + "._generate_chap_secret", autospec=True)
     @mock.patch(DRIVER_OBJ + "._get_host", autospec=True)
diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py
index dfa0ab7..ccc0f94 100644
--- a/cinder/volume/drivers/pure.py
+++ b/cinder/volume/drivers/pure.py
@@ -118,7 +118,6 @@ class PureISCSIDriver(san.SanISCSIDriver):
         super(PureISCSIDriver, self).__init__(execute=execute, *args, **kwargs)
         self.configuration.append_config_values(PURE_OPTS)
         self._array = None
-        self._iscsi_port = None
         self._backend_name = (self.configuration.volume_backend_name or
                               self.__class__.__name__)

@@ -136,7 +135,6 @@ class PureISCSIDriver(san.SanISCSIDriver):
         self._array = purestorage.FlashArray(
             self.configuration.san_ip,
             api_token=self.configuration.pure_api_token)
-        self._iscsi_port = self._choose_target_iscsi_port()

     def check_for_setup_error(self):
         # Avoid inheriting check_for_setup_error from SanDriver, which checks
@@ -250,18 +248,11 @@ class PureISCSIDriver(san.SanISCSIDriver):
     def initialize_connection(self, volume, connector, initiator_data=None):
         """Allow connection to connector and return connection info."""
         LOG.debug("Enter PureISCSIDriver.initialize_connection.")
-        target_port = self._get_target_iscsi_port()
+        target_ports = self._get_target_iscsi_ports()
         connection = self._connect(volume, connector, initiator_data)
-        properties = {
-            "driver_volume_type": "iscsi",
-            "data": {
-                "target_iqn": target_port["iqn"],
-                "target_portal": target_port["portal"],
-                "target_lun": connection["lun"],
-                "target_discovered": True,
-                "access_mode": "rw",
-            },
-        }
+
+        properties = self._build_connection_properties(connection,
+                                                       target_ports)

         if self.configuration.use_chap_auth:
             properties["data"]["auth_method"] = "CHAP"
@@ -275,43 +266,42 @@ class PureISCSIDriver(san.SanISCSIDriver):
         LOG.debug("Leave PureISCSIDriver.initialize_connection.")
         return properties

-    def _get_target_iscsi_port(self):
-        """Return dictionary describing iSCSI-enabled port on target array."""
-        try:
-            self._run_iscsiadm_bare(["-m", "discovery", "-t", "sendtargets",
-                                     "-p", self._iscsi_port["portal"]])
-        except processutils.ProcessExecutionError as err:
-            LOG.warn(_LW("iSCSI discovery of port %(port_name)s at "
-                         "%(port_portal)s failed with error: %(err_msg)s"),
-                     {"port_name": self._iscsi_port["name"],
-                      "port_portal": self._iscsi_port["portal"],
-                      "err_msg": err.stderr})
-            self._iscsi_port = self._choose_target_iscsi_port()
-        return self._iscsi_port
-
-    @utils.retry(exception.PureDriverException, retries=3)
-    def _choose_target_iscsi_port(self):
-        """Find a reachable iSCSI-enabled port on target array."""
+    def _build_connection_properties(self, connection, target_ports):
+        props = {
+            "driver_volume_type": "iscsi",
+            "data": {
+                "target_discovered": False,
+                "access_mode": "rw",
+            },
+        }
+
+        port_iter = iter(target_ports)
+
+        target_luns = []
+        target_iqns = []
+        target_portals = []
+
+        for port in port_iter:
+            target_luns.append(connection["lun"])
+            target_iqns.append(port["iqn"])
+            target_portals.append(port["portal"])
+
+        # If we have multiple ports always report them.
+        if target_luns and target_iqns and target_portals:
+            props["data"]["target_luns"] = target_luns
+            props["data"]["target_iqns"] = target_iqns
+            props["data"]["target_portals"] = target_portals
+
+        return props
+
+    def _get_target_iscsi_ports(self):
+        """Return list of iSCSI-enabled port descriptions."""
         ports = self._array.list_ports()
         iscsi_ports = [port for port in ports if port["iqn"]]
-        for port in iscsi_ports:
-            try:
-                self._run_iscsiadm_bare(["-m", "discovery",
-                                         "-t", "sendtargets",
-                                         "-p", port["portal"]])
-            except processutils.ProcessExecutionError as err:
-                LOG.debug(("iSCSI discovery of port %(port_name)s at "
-                           "%(port_portal)s failed with error: %(err_msg)s"),
-                          {"port_name": port["name"],
-                           "port_portal": port["portal"],
-                           "err_msg": err.stderr})
-            else:
-                LOG.info(_LI("Using port %(name)s on the array at %(portal)s "
-                             "for iSCSI connectivity."),
-                         {"name": port["name"], "portal": port["portal"]})
-                return port
-        raise exception.PureDriverException(
-            reason=_("No reachable iSCSI-enabled ports on target array."))
+        if not iscsi_ports:
+            raise exception.PureDriverException(
+                reason=_("No iSCSI-enabled ports on target array."))
+        return iscsi_ports

     def _get_chap_credentials(self, host, data):
         initiator_updates = None