From c9ec9b9bd755f042dbe1ccbdc5e3ff87fa60269c Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Thu, 28 Sep 2017 14:00:56 +0200
Subject: [PATCH] Kaminario K2: Add non discovery iSCSI multipath

Current Kaminario K2 driver only supports iSCSI multipathing using
discovery (sendtargets), which in some cases may be undesirable, since a
broken primary path could prevent an attachment.

This code adds the possibility of not using discovery mode and return
all connection information when `initialize_connection` is called, which
should provide increased reliability.

Change-Id: I7932b1d952844b49cfdb504ed0ba188b489d7f44
---
 .../unit/volume/drivers/test_kaminario.py     | 97 ++++++++++++++-----
 .../drivers/kaminario/kaminario_common.py     |  4 +
 .../drivers/kaminario/kaminario_iscsi.py      | 36 ++++---
 ...k2-disable-discovery-bca0d65b5672ec7b.yaml |  6 ++
 4 files changed, 101 insertions(+), 42 deletions(-)
 create mode 100644 releasenotes/notes/k2-disable-discovery-bca0d65b5672ec7b.yaml

diff --git a/cinder/tests/unit/volume/drivers/test_kaminario.py b/cinder/tests/unit/volume/drivers/test_kaminario.py
index 3e68ccff0ce..74b062a4ef6 100644
--- a/cinder/tests/unit/volume/drivers/test_kaminario.py
+++ b/cinder/tests/unit/volume/drivers/test_kaminario.py
@@ -15,6 +15,7 @@
 """Unit tests for kaminario driver."""
 import re
 
+import ddt
 import mock
 from oslo_utils import units
 import time
@@ -47,12 +48,13 @@ class FakeK2Obj(object):
 
 class FakeSaveObject(FakeK2Obj):
     def __init__(self, *args, **kwargs):
+        item = kwargs.pop('item', 1)
         self.ntype = kwargs.get('ntype')
-        self.ip_address = '10.0.0.1'
+        self.ip_address = '10.0.0.%s' % item
         self.iscsi_qualified_target_name = "xyztlnxyz"
         self.snapshot = FakeK2Obj()
         self.name = 'test'
-        self.pwwn = '50024f4053300300'
+        self.pwwn = '50024f405330030%s' % item
         self.volume_group = self
         self.is_dedup = True
         self.size = units.Mi
@@ -83,8 +85,8 @@ class FakeSaveObjectExp(FakeSaveObject):
 
 
 class FakeSearchObject(object):
-    hits = [FakeSaveObject()]
-    total = 1
+    hits = [FakeSaveObject(item=1), FakeSaveObject(item=2)]
+    total = 2
 
     def __init__(self, *args):
         if args and "mappings" in args[0]:
@@ -119,14 +121,14 @@ class Replication(object):
     rpo = 500
 
 
-class TestKaminarioISCSI(test.TestCase):
+class TestKaminarioCommon(test.TestCase):
     driver = None
     conf = None
 
     def setUp(self):
         self._setup_config()
         self._setup_driver()
-        super(TestKaminarioISCSI, self).setUp()
+        super(TestKaminarioCommon, self).setUp()
         self.context = context.get_admin_context()
         self.vol = fake_volume.fake_volume_obj(self.context)
         self.vol.volume_type = fake_volume.fake_volume_type_obj(self.context)
@@ -140,6 +142,7 @@ class TestKaminarioISCSI(test.TestCase):
         self.conf.kaminario_dedup_type_name = "dedup"
         self.conf.volume_dd_blocksize = 2
         self.conf.unique_fqdn_network = True
+        self.conf.disable_discovery = False
 
     def _setup_driver(self):
         self.driver = (kaminario_iscsi.
@@ -258,12 +261,6 @@ class TestKaminarioISCSI(test.TestCase):
         self.assertRaises(exception.KaminarioCinderDriverException,
                           self.driver.extend_volume, self.vol, new_size)
 
-    def test_initialize_connection(self):
-        """Test initialize_connection."""
-        conn_info = self.driver.initialize_connection(self.vol, CONNECTOR)
-        self.assertIn('data', conn_info)
-        self.assertIn('target_iqn', conn_info['data'])
-
     def test_initialize_connection_with_exception(self):
         """Test initialize_connection_with_exception."""
         self.driver.client = FakeKrestException()
@@ -271,11 +268,6 @@ class TestKaminarioISCSI(test.TestCase):
                           self.driver.initialize_connection, self.vol,
                           CONNECTOR)
 
-    def test_terminate_connection(self):
-        """Test terminate_connection."""
-        result = self.driver.terminate_connection(self.vol, CONNECTOR)
-        self.assertIsNone(result)
-
     def test_get_lun_number(self):
         """Test _get_lun_number."""
         host, host_rs, host_name = self.driver._get_host_object(CONNECTOR)
@@ -291,20 +283,15 @@ class TestKaminarioISCSI(test.TestCase):
         """Test _get_host_object."""
         host, host_rs, host_name = self.driver._get_host_object(CONNECTOR)
         self.assertEqual(548, host.id)
-        self.assertEqual(1, host_rs.total)
+        self.assertEqual(2, host_rs.total)
         self.assertEqual('test-k2', host_name)
 
-    def test_get_target_info(self):
-        """Test get_target_info."""
-        iscsi_portal, target_iqn = self.driver.get_target_info(self.vol)
-        self.assertEqual('10.0.0.1:3260', iscsi_portal)
-        self.assertEqual('xyztlnxyz', target_iqn)
-
     def test_k2_initialize_connection(self):
         """Test k2_initialize_connection."""
         result = self.driver.k2_initialize_connection(self.vol, CONNECTOR)
         self.assertEqual(548, result)
 
+    @mock.patch.object(FakeSearchObject, 'total', 1)
     def test_manage_existing(self):
         """Test manage_existing."""
         self.driver._get_replica_status = mock.Mock(return_value=False)
@@ -543,7 +530,64 @@ class TestKaminarioISCSI(test.TestCase):
         self.assertEqual(expected, result)
 
 
-class TestKaminarioFC(TestKaminarioISCSI):
+@ddt.ddt
+class TestKaminarioISCSI(TestKaminarioCommon):
+    def test_get_target_info(self):
+        """Test get_target_info."""
+        iscsi_portals, target_iqns = self.driver.get_target_info(self.vol)
+        self.assertEqual(['10.0.0.1:3260', '10.0.0.2:3260'],
+                         iscsi_portals)
+        self.assertEqual(['xyztlnxyz', 'xyztlnxyz'],
+                         target_iqns)
+
+    @ddt.data(True, False)
+    def test_initialize_connection(self, multipath):
+        """Test initialize_connection."""
+        connector = CONNECTOR.copy()
+        connector['multipath'] = multipath
+        self.driver.configuration.disable_discovery = False
+
+        conn_info = self.driver.initialize_connection(self.vol, CONNECTOR)
+        expected = {
+            'data': {
+                'target_discovered': True,
+                'target_iqn': 'xyztlnxyz',
+                'target_lun': 548,
+                'target_portal': '10.0.0.1:3260',
+            },
+            'driver_volume_type': 'iscsi',
+        }
+        self.assertEqual(expected, conn_info)
+
+    def test_initialize_connection_multipath(self):
+        """Test initialize_connection with multipath."""
+        connector = CONNECTOR.copy()
+        connector['multipath'] = True
+        self.driver.configuration.disable_discovery = True
+
+        conn_info = self.driver.initialize_connection(self.vol, connector)
+
+        expected = {
+            'data': {
+                'target_discovered': True,
+                'target_iqn': 'xyztlnxyz',
+                'target_iqns': ['xyztlnxyz', 'xyztlnxyz'],
+                'target_lun': 548,
+                'target_luns': [548, 548],
+                'target_portal': '10.0.0.1:3260',
+                'target_portals': ['10.0.0.1:3260', '10.0.0.2:3260'],
+            },
+            'driver_volume_type': 'iscsi',
+        }
+        self.assertEqual(expected, conn_info)
+
+    def test_terminate_connection(self):
+        """Test terminate_connection."""
+        result = self.driver.terminate_connection(self.vol, CONNECTOR)
+        self.assertIsNone(result)
+
+
+class TestKaminarioFC(TestKaminarioCommon):
 
     def _setup_driver(self):
         self.driver = (kaminario_fc.
@@ -562,7 +606,8 @@ class TestKaminarioFC(TestKaminarioISCSI):
     def test_get_target_info(self):
         """Test get_target_info."""
         target_wwpn = self.driver.get_target_info(self.vol)
-        self.assertEqual(['50024f4053300300'], target_wwpn)
+        self.assertEqual(['50024f4053300301', '50024f4053300302'],
+                         target_wwpn)
 
     def test_terminate_connection(self):
         """Test terminate_connection."""
diff --git a/cinder/volume/drivers/kaminario/kaminario_common.py b/cinder/volume/drivers/kaminario/kaminario_common.py
index 0dac5db42f2..3c46b6ac119 100644
--- a/cinder/volume/drivers/kaminario/kaminario_common.py
+++ b/cinder/volume/drivers/kaminario/kaminario_common.py
@@ -59,6 +59,10 @@ kaminario_opts = [
                      "FQDN.  When true this will create host entries on K2 "
                      "using the FQDN, when false it will use the reversed "
                      "IQN/WWNN."),
+    cfg.BoolOpt('disable_discovery',
+                default=False,
+                help="Disabling iSCSI discovery (sendtargets) for multipath "
+                     "connections on K2 driver."),
 ]
 
 CONF = cfg.CONF
diff --git a/cinder/volume/drivers/kaminario/kaminario_iscsi.py b/cinder/volume/drivers/kaminario/kaminario_iscsi.py
index 03165fe7a48..665e2410aee 100644
--- a/cinder/volume/drivers/kaminario/kaminario_iscsi.py
+++ b/cinder/volume/drivers/kaminario/kaminario_iscsi.py
@@ -61,18 +61,24 @@ class KaminarioISCSIDriver(common.KaminarioCinderDriver):
             temp_client = self.client
             self.client = self.target
         # Get target_portal and target iqn.
-        iscsi_portal, target_iqn = self.get_target_info(volume)
+        iscsi_portals, target_iqns = self.get_target_info(volume)
         # Map volume.
         lun = self.k2_initialize_connection(volume, connector)
         # To support replication failback
         if temp_client:
             self.client = temp_client
         # Return target volume information.
-        return {"driver_volume_type": "iscsi",
-                "data": {"target_iqn": target_iqn,
-                         "target_portal": iscsi_portal,
-                         "target_lun": lun,
-                         "target_discovered": True}}
+        result = {"driver_volume_type": "iscsi",
+                  "data": {"target_iqn": target_iqns[0],
+                           "target_portal": iscsi_portals[0],
+                           "target_lun": lun,
+                           "target_discovered": True}}
+
+        if self.configuration.disable_discovery and connector.get('multipath'):
+            result['data'].update(target_iqns=target_iqns,
+                                  target_portals=iscsi_portals,
+                                  target_luns=[lun] * len(target_iqns))
+        return result
 
     @kaminario_logger
     @coordination.synchronized('{self.k2_lock_name}')
@@ -92,28 +98,26 @@ class KaminarioISCSIDriver(common.KaminarioCinderDriver):
     def get_target_info(self, volume):
         LOG.debug("Searching first iscsi port ip without wan in K2.")
         iscsi_ip_rs = self.client.search("system/net_ips")
-        iscsi_ip = target_iqn = None
+        iscsi_portals = target_iqns = None
         if hasattr(iscsi_ip_rs, 'hits') and iscsi_ip_rs.total != 0:
-            for ip in iscsi_ip_rs.hits:
-                if not ip.wan_port:
-                    iscsi_ip = ip.ip_address
-                    break
-        if not iscsi_ip:
+            iscsi_portals = ['%s:%s' % (ip.ip_address, ISCSI_TCP_PORT)
+                             for ip in iscsi_ip_rs.hits if not ip.wan_port]
+        if not iscsi_portals:
             msg = _("Unable to get ISCSI IP address from K2.")
             LOG.error(msg)
             raise exception.KaminarioCinderDriverException(reason=msg)
-        iscsi_portal = "{0}:{1}".format(iscsi_ip, ISCSI_TCP_PORT)
         LOG.debug("Searching system state for target iqn in K2.")
         sys_state_rs = self.client.search("system/state")
 
         if hasattr(sys_state_rs, 'hits') and sys_state_rs.total != 0:
-            target_iqn = sys_state_rs.hits[0].iscsi_qualified_target_name
+            iqn = sys_state_rs.hits[0].iscsi_qualified_target_name
+            target_iqns = [iqn] * len(iscsi_portals)
 
-        if not target_iqn:
+        if not target_iqns:
             msg = _("Unable to get target iqn from K2.")
             LOG.error(msg)
             raise exception.KaminarioCinderDriverException(reason=msg)
-        return iscsi_portal, target_iqn
+        return iscsi_portals, target_iqns
 
     @kaminario_logger
     def _get_host_object(self, connector):
diff --git a/releasenotes/notes/k2-disable-discovery-bca0d65b5672ec7b.yaml b/releasenotes/notes/k2-disable-discovery-bca0d65b5672ec7b.yaml
new file mode 100644
index 00000000000..3f2c2bfcda8
--- /dev/null
+++ b/releasenotes/notes/k2-disable-discovery-bca0d65b5672ec7b.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Kaminario K2 iSCSI driver now supports non discovery multipathing (Nova and
+    Cinder won't use iSCSI sendtargets) which can be enabled by setting
+    `disable_discovery` to `true` in the configuration.