CA-386479: ensure we login to all iSCSI Target Portal Groups

From: Mark Syms <mark.syms@citrix.com>

Some arrays may have multiple, entirely independent, Target Portal
Groups within a single controller IQN space. When performing discovery
to portals within these portal groups they respond only with the
portal addresses within that TQG and do not include the portal
addresses in other TPGs in the controller or information about TPGs
in any other controller IQN spaces.

In order to ensure that all the expected iSCSI paths and sessions are
activated it is necessary to make the checks for IQN connectivity also
check for the specific target portal address and not just check that
the target IQN is served by an active session

Signed-off-by: Mark Syms <mark.syms@citrix.com>
---
 drivers/BaseISCSI.py            |   24 ++++---
 drivers/LVHDoISCSISR.py         |    2 -
 tests/shared_iscsi_test_base.py |   64 +++++++++++++++++
 tests/test_BaseISCSI.py         |  144 ++++++++++++++++++++++++++++++---------
 tests/test_LVHDoISCSISR.py      |   53 +-------------
 5 files changed, 196 insertions(+), 91 deletions(-)
 create mode 100644 tests/shared_iscsi_test_base.py

diff --git a/drivers/BaseISCSI.py b/drivers/BaseISCSI.py
index bc760e7..652387f 100755
--- a/drivers/BaseISCSI.py
+++ b/drivers/BaseISCSI.py
@@ -45,7 +45,7 @@ class BaseISCSISR(SR.SR):
     def attached(self):
         if not self._attached:
             self._attached = False
-            self._attached = iscsilib._checkTGT(self.targetIQN)
+            self._attached = iscsilib._checkTGT(self.targetIQN, self.target)
         return self._attached
 
     @attached.setter
@@ -325,27 +325,30 @@ class BaseISCSISR(SR.SR):
             iscsilib.ensure_daemon_running_ok(self.localIQN)
 
             # Check to see if auto attach was set
-            if not iscsilib._checkTGT(self.targetIQN) or multiTargets:
+            if not iscsilib._checkTGT(self.targetIQN, tgt=self.target) or multiTargets:
                 try:
-                    map = []
+                    iqn_map = []
                     if 'any' != self.targetIQN:
                         try:
-                            map = iscsilib.get_node_records(self.targetIQN)
+                            iqn_map = iscsilib.get_node_records(self.targetIQN)
                         except:
                             # Pass the exception that is thrown, when there
                             # are no nodes
                             pass
-                    if len(map) == 0:
-                        map = iscsilib.discovery(self.target, self.port,
+
+                    # Check our current target is in the map
+                    portal = '%s:%d' % (self.target, self.port)
+                    if len(iqn_map) == 0 or not any([x[0] for x in iqn_map if x[0] == portal]):
+                        iqn_map = iscsilib.discovery(self.target, self.port,
                                                   self.chapuser, self.chappassword,
                                                   self.targetIQN,
                                                   iscsilib.get_iscsi_interfaces())
-                    if len(map) == 0:
+                    if len(iqn_map) == 0:
                         self._scan_IQNs()
                         raise xs_errors.XenError('ISCSIDiscovery', 
                                                  opterr='check target settings')
-                    for i in range(0,len(map)):
-                        (portal,tpgt,iqn) = map[i]
+                    for i in range(0,len(iqn_map)):
+                        (portal,tpgt,iqn) = iqn_map[i]
                         try:
                             (ipaddr, port) = iscsilib.parse_IP_port(portal)
                             if not self.multihomed and ipaddr != self.target:
@@ -367,7 +370,7 @@ class BaseISCSISR(SR.SR):
                             else:
                                 pass
 
-                    if not iscsilib._checkTGT(self.targetIQN):
+                    if not iscsilib._checkTGT(self.targetIQN, tgt=self.target):
                         raise xs_errors.XenError('ISCSIDevice', \
                                                  opterr='during login')
                 
@@ -387,6 +390,7 @@ class BaseISCSISR(SR.SR):
                 if len(iqn): IQNs += iqn.split(',')[2]
         else:
             IQNs.append(self.targetIQN)
+
         sessions = 0
         paths = iscsilib.get_IQN_paths()
         for path in paths:
diff --git a/drivers/LVHDoISCSISR.py b/drivers/LVHDoISCSISR.py
index 485a33c..1fa6f6f 100755
--- a/drivers/LVHDoISCSISR.py
+++ b/drivers/LVHDoISCSISR.py
@@ -217,7 +217,7 @@ class LVHDoISCSISR(LVHDSR.LVHDSR):
                 util.SMlog("path %s" % self.iscsi.path)
                 util.SMlog("iscsci data: targetIQN %s, portal %s" % (self.iscsi.targetIQN, self.iscsi.target))
                 iscsilib.ensure_daemon_running_ok(self.iscsi.localIQN)
-                if not iscsilib._checkTGT(self.iscsi.targetIQN):
+                if not iscsilib._checkTGT(self.iscsi.targetIQN, self.iscsi.target):
                     attempt_discovery = True
                     try:
                         # Ensure iscsi db has been populated
diff --git a/tests/shared_iscsi_test_base.py b/tests/shared_iscsi_test_base.py
new file mode 100644
index 0000000..eccb7df
--- /dev/null
+++ b/tests/shared_iscsi_test_base.py
@@ -0,0 +1,64 @@
+import mock
+import unittest
+
+
+import iscsilib
+from SRCommand import SRCommand
+
+
+class ISCSITestCase(unittest.TestCase):
+
+    def setUp(self):
+        iscsilib_patcher = mock.patch('%s.iscsilib' % (self.TEST_CLASS),
+                                      autospec=True)
+        self.mock_iscsilib = iscsilib_patcher.start()
+        self.mock_iscsilib.discovery.side_effect = self.discovery
+        self.mock_iscsilib._checkTGT.side_effect = self._checkTGT
+        self.mock_iscsilib.login.side_effect = self.iscsi_login
+        self.mock_iscsilib.parse_IP_port = iscsilib.parse_IP_port
+        self.discovery_data = {}
+        self.sessions = []
+
+        sleep_patcher = mock.patch('%s.time.sleep' % (self.TEST_CLASS),
+                                   autospec=True)
+        self.mock_sleep = sleep_patcher.start()
+
+    def _checkTGT(self, tgtIQN, tgt=''):
+        all_sessions = '\n'.join(self.sessions)
+        matched = iscsilib._compare_sessions_to_tgt(all_sessions, tgtIQN, tgt)
+        return matched
+
+    def discovery(self, target, port, chapuser, chappassword,
+                  targetIQN="any", interfaceArray=["default"]):
+        return self.discovery_data.get(target, [])
+
+    def iscsi_login(self, target, target_iqn, chauser, chappassword,
+                    incoming_user, incoming_password, mpath):
+        session_count = len(self.sessions)
+        self.sessions.append('tcp: [%s] %s,1 %s' % (session_count, target, target_iqn))
+
+    def create_sr_command(
+            self, additional_dconf=None, cmd=None,
+            target_iqn='iqn.2009-01.example.test:iscsi085e938a',
+            multihomelist='tgt1:3260,tgt2:3260', target="10.70.89.34"):
+
+        sr_cmd = mock.create_autospec(SRCommand)
+        sr_cmd.dconf = {
+            'SCSIid': '3600a098038313577792450384a4a6275',
+            'multihomelist': multihomelist,
+            'target': target,
+            'targetIQN': target_iqn,
+            'localIQN': 'iqn.2018-05.com.example:0d312804'
+        }
+        if additional_dconf:
+            sr_cmd.dconf.update(additional_dconf)
+
+        sr_cmd.params = {
+            'command': 'nop',
+            'session_ref': 'test_session',
+            'host_ref': 'test_host',
+            'sr_ref': 'sr_ref'
+        }
+        sr_cmd.cmd = cmd
+        return sr_cmd
+
diff --git a/tests/test_BaseISCSI.py b/tests/test_BaseISCSI.py
index 22a6d25..a7d0eff 100644
--- a/tests/test_BaseISCSI.py
+++ b/tests/test_BaseISCSI.py
@@ -6,13 +6,17 @@ import mock
 import unittest
 from uuid import uuid4
 
+import util
 from BaseISCSI import BaseISCSISR
 import SR
 import SRCommand
+from shared_iscsi_test_base import ISCSITestCase
 from util import CommandException
 
 
-class TestBaseISCSI(unittest.TestCase):
+class TestBaseISCSI(ISCSITestCase):
+
+    TEST_CLASS = 'BaseISCSI'
 
     def setUp(self):
         self.addCleanup(mock.patch.stopall)
@@ -20,15 +24,18 @@ class TestBaseISCSI(unittest.TestCase):
         util_patcher = mock.patch('BaseISCSI.util', autospec=True)
         self.mock_util = util_patcher.start()
         self.mock_util.CommandException = CommandException
+        self.mock_util.sessions_less_than_targets = util.sessions_less_than_targets
+        self.mock_util._convertDNS.side_effect = lambda x: x
+        # self.mock_util.SMlog.side_effect = print
+
+        scsi_util_patcher = mock.patch('BaseISCSI.scsiutil', autospec=True)
+        self.mock_scsiutil = scsi_util_patcher.start()
 
         self.mock_session = mock.MagicMock()
         xenapi_patcher = mock.patch('SR.XenAPI')
         mock_xenapi = xenapi_patcher.start()
         mock_xenapi.xapi_local.return_value = self.mock_session
 
-        iscsilib_patcher = mock.patch('BaseISCSI.iscsilib', autospec=True)
-        self.mock_iscsilib = iscsilib_patcher.start()
-
         copy_patcher = mock.patch('LVHDoISCSISR.SR.copy.deepcopy')
         self.mock_copy = copy_patcher.start()
 
@@ -37,25 +44,9 @@ class TestBaseISCSI(unittest.TestCase):
 
         self.mock_copy.side_effect = deepcopy
 
-        dummy_cmd = mock.create_autospec(SRCommand)
-        dummy_cmd.dconf = {
-            'SCSIid': '3600a098038313577792450384a4a6275',
-            'target': "10.70.89.34",
-            'targetIQN': 'iqn.2009-01.example.test:iscsi085e938a'
-        }
-        dummy_cmd.params = {
-            'command': 'nop',
-            'session_ref': 'test_session',
-            'host_ref': 'test_host',
-            'sr_ref': 'sr_ref'
-        }
-        dummy_cmd.cmd = None
-
         self.sr_uuid = str(uuid4())
 
-        self.subject = BaseISCSISR(
-            dummy_cmd, self.sr_uuid
-        )
+        super(TestBaseISCSI, self).setUp()
 
     def setup_path_mocks(self):
         self.path_contents = {}
@@ -67,12 +58,16 @@ class TestBaseISCSI(unittest.TestCase):
         mock_listdir.side_effect = self.listdir
 
     def exists(self, path):
-        print('checking existance of %s' % path)
         return path in self.path_contents
 
     def listdir(self, path):
         return self.path_contents[path]
 
+    def create_test_sr(self, sr_cmd):
+        self.sr_uuid = str(uuid4())
+        self.subject = BaseISCSISR(
+            sr_cmd, self.sr_uuid)
+
     @mock.patch('BaseISCSI.BaseISCSISR._initPaths', autospec=True)
     def test_attach_tgt_present_path_found(self, mock_init_paths):
         # Arrange
@@ -80,11 +75,14 @@ class TestBaseISCSI(unittest.TestCase):
         self.path_contents.update(
             {'/dev/disk/by-scsid/3600a098038313577792450384a4a6275': ['sdb']})
         self.mock_util._testHost.return_value = None
-        self.mock_util.sessions_less_than_targets.return_value = False
-        self.mock_iscsilib._checkTGT.return_value = True
-        self.mock_iscsilib.parse_IP_port.side_effect = [
-            ('tgt1', '3260')
-            ]
+        self.discovery_data = {
+            'tgt1': [
+                ('tgt1:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393')],
+        }
+
+        self.create_test_sr(self.create_sr_command(
+            cmd='sr_attach',
+            target_iqn='iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'))
 
         # Act
         self.subject.attach(self.sr_uuid)
@@ -94,11 +92,14 @@ class TestBaseISCSI(unittest.TestCase):
     def test_attach_tgt_present_path_not_found(self, mock_init_paths):
         # Arrange
         self.mock_util._testHost.return_value = None
-        self.mock_util.sessions_less_than_targets.return_value = False
-        self.mock_iscsilib._checkTGT.return_value = True
-        self.mock_iscsilib.parse_IP_port.side_effect = [
-            ('tgt1', '3260')
-            ]
+        self.discovery_data = {
+            'tgt1': [
+                ('tgt1:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393')],
+        }
+
+        self.create_test_sr(self.create_sr_command(
+            cmd='sr_attach',
+            target_iqn='iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'))
 
         # Act
         with self.assertRaises(SR.SROSError) as srose:
@@ -106,3 +107,82 @@ class TestBaseISCSI(unittest.TestCase):
 
         # Assert
         self.assertEqual(107, srose.exception.errno)
+
+    def test_sr_attach_multi_session(self):
+        # Arrange
+        self.mock_util.find_my_pbd.return_value = 'my_pbd'
+        additional_dconf = {
+            'multiSession': '10.207.6.60,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
+                            '10.207.3.65,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
+                            '10.207.3.61,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
+                            '10.207.6.61,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
+                            '10.207.3.63,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
+                            '10.207.6.62,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
+                            '10.207.3.62,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
+                            '10.207.3.60,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
+                            '10.207.6.64,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
+                            '10.207.6.65,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
+                            '10.207.3.64,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
+                            '10.207.6.63,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
+        }
+
+        tpg_data = [
+            [
+                ('10.207.3.60:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
+                ('10.207.3.61:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
+                ('10.207.3.62:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393')],
+            [
+                ('10.207.3.63:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
+                ('10.207.3.64:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
+                ('10.207.3.65:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394')],
+            [
+                ('10.207.6.60:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
+                ('10.207.6.61:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
+                ('10.207.6.62:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393')
+            ],
+            [
+                ('10.207.6.63:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
+                ('10.207.6.64:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
+                ('10.207.6.65:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394')
+            ]
+        ]
+
+        self.discovery_data = {
+            '10.207.3.60': tpg_data[0],
+            '10.207.3.61': tpg_data[0],
+            '10.207.3.62': tpg_data[0],
+            '10.207.3.63': tpg_data[1],
+            '10.207.3.64': tpg_data[1],
+            '10.207.3.65': tpg_data[1],
+            '10.207.6.60': tpg_data[2],
+            '10.207.6.61': tpg_data[2],
+            '10.207.6.62': tpg_data[2],
+            '10.207.6.63': tpg_data[3],
+            '10.207.6.64': tpg_data[3],
+            '10.207.6.65': tpg_data[3]
+        }
+
+        self.mock_scsiutil._genHostList.return_value = [1, 2]
+        self.mock_iscsilib.get_targetIQN.return_value = 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'
+        self.mock_scsiutil.cacheSCSIidentifiers.return_value = [
+            ['NONE', '0', '0', '0', '0', '0', '/dev/sdb']
+        ]
+        self.setup_path_mocks()
+        self.path_contents.update(
+            {'/dev/iscsi/iqn.2009-11.com.infinidat:storage:infinibox-sn-3393/10.207.3.60:3260': ['LUN0'],
+             '/dev/disk/by-scsid/3600a098038313577792450384a4a6275': []})
+
+        # Create SR
+        self.create_test_sr(self.create_sr_command(
+            additional_dconf=additional_dconf,
+            cmd='sr_attach',
+            multihomelist="10.207.3.62:3260,10.207.6.61:3260,10.207.6.62:3260,10.207.6.60:3260",
+            target='10.207.3.60',
+            target_iqn='iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'))
+
+        # Act
+        self.subject.attach(self.sr_uuid)
+
+        # Assert
+        self.assertEqual(1, self.mock_iscsilib.discovery.call_count)
+        self.assertEqual(1, self.mock_iscsilib.login.call_count)
diff --git a/tests/test_LVHDoISCSISR.py b/tests/test_LVHDoISCSISR.py
index 535090f..ba668d0 100644
--- a/tests/test_LVHDoISCSISR.py
+++ b/tests/test_LVHDoISCSISR.py
@@ -15,6 +15,7 @@ import util
 import xs_errors
 
 import testlib
+from shared_iscsi_test_base import ISCSITestCase
 from test_ISCSISR import NonInitingISCSISR
 
 TEST_SR_UUID = 'test_uuid'
@@ -127,7 +128,9 @@ class TestLVHDoISCSISR_load(unittest.TestCase):
         )
 
 
-class TestLVHDoISCSISR(unittest.TestCase):
+class TestLVHDoISCSISR(ISCSITestCase):
+
+    TEST_CLASS = 'LVHDoISCSISR'
 
     def setUp(self):
         util_patcher = mock.patch('LVHDoISCSISR.util', autospec=True)
@@ -143,15 +146,6 @@ class TestLVHDoISCSISR(unittest.TestCase):
         patched_baseiscsi.side_effect = self.baseiscsi
         lvhdsr_patcher = mock.patch ('LVHDoISCSISR.LVHDSR')
 
-        iscsilib_patcher = mock.patch('LVHDoISCSISR.iscsilib',
-                                      autospec=True)
-        self.mock_iscsilib = iscsilib_patcher.start()
-        self.mock_iscsilib.discovery.side_effect = self.discovery
-        self.mock_iscsilib._checkTGT.side_effect = self._checkTGT
-        self.mock_iscsilib.login.side_effect = self.iscsi_login
-        self.discovery_data = {}
-        self.sessions = []
-
         self.mock_lvhdsr = lvhdsr_patcher.start()
         self.mock_session = mock.MagicMock()
         xenapi_patcher = mock.patch('SR.XenAPI')
@@ -171,20 +165,7 @@ class TestLVHDoISCSISR(unittest.TestCase):
 
         self.addCleanup(mock.patch.stopall)
 
-    def _checkTGT(self, tgtIQN, tgt=''):
-        all_sessions = '\n'.join(self.sessions)
-        matched = iscsilib._compare_sessions_to_tgt(all_sessions, tgtIQN, tgt)
-        return matched
-
-    def discovery(self, target, port, chapuser, chappassword,
-                  targetIQN="any", interfaceArray=["default"]):
-        return self.discovery_data.get(target, [])
-
-    def iscsi_login(self, target, target_iqn, chauser, chappassword,
-                    incoming_user, incoming_password, mpath):
-        print("Logging in %s - %s" % (target, target_iqn))
-        session_count = len(self.sessions)
-        self.sessions.append('tcp: [%s] %s:3260,1 %s' % (session_count, target, target_iqn))
+        super(TestLVHDoISCSISR, self).setUp()
 
     @property
     def mock_baseiscsi(self):
@@ -218,30 +199,6 @@ class TestLVHDoISCSISR(unittest.TestCase):
         self.subject = LVHDoISCSISR.LVHDoISCSISR(
             sr_cmd, self.sr_uuid)
 
-    def create_sr_command(
-            self, additional_dconf=None, cmd=None,
-            target_iqn='iqn.2009-01.example.test:iscsi085e938a'):
-
-        sr_cmd = mock.create_autospec(SRCommand)
-        sr_cmd.dconf = {
-            'SCSIid': '3600a098038313577792450384a4a6275',
-            'multihomelist': 'tgt1:3260,tgt2:3260',
-            'target': "10.70.89.34",
-            'targetIQN': target_iqn,
-            'localIQN': 'iqn.2018-05.com.example:0d312804'
-        }
-        if additional_dconf:
-            sr_cmd.dconf.update(additional_dconf)
-
-        sr_cmd.params = {
-            'command': 'nop',
-            'session_ref': 'test_session',
-            'host_ref': 'test_host',
-            'sr_ref': 'sr_ref'
-        }
-        sr_cmd.cmd = cmd
-        return sr_cmd
-
     def test_check_sr_pbd_not_found(self):
         # Arrange
         self.mock_util.find_my_pbd.return_value = None
