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

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>

Author: MarkSymsCtx

drivers/BaseISCSI.py

@@ -52,7 +52,7 @@ def force_tapdisk(self):
     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
@@ -331,7 +331,7 @@ def attach(self, sr_uuid):
             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:
                     iqn_map = []
                     if 'any' != self.targetIQN:
@@ -341,7 +341,10 @@ def attach(self, sr_uuid):
                             # Pass the exception that is thrown, when there
                             # are no nodes
                             pass
-                    if len(iqn_map) == 0:
+
+                    # 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,
@@ -373,7 +376,7 @@ def attach(self, sr_uuid):
                             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')
 
@@ -394,6 +397,7 @@ def attach(self, sr_uuid):
                     IQNs += iqn.split(',')[2]
         else:
             IQNs.append(self.targetIQN)
+
         sessions = 0
         paths = iscsilib.get_IQN_paths()
         for path in paths:

drivers/LVHDoISCSISR.py

@@ -228,7 +228,7 @@ def create_iscsi_sessions(self, sr_uuid):
                 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

tests/shared_iscsi_test_base.py

@@ -0,0 +1,63 @@
+import unittest
+from unittest import mock
+
+import iscsilib
+from SRCommand import SRCommand
+
+
+class ISCSITestCase(unittest.TestCase):
+
+    def setUp(self):
+        iscsilib_patcher = mock.patch(f'{self.TEST_CLASS}.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.mock_iscsilib.parse_IP_port = iscsilib.parse_IP_port
+        self.discovery_data = {}
+        self.sessions = []
+
+        sleep_patcher = mock.patch(f'{self.TEST_CLASS}.time.sleep',
+                                   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(f'tcp: [{session_count}] {target},1 {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
+

tests/test_BaseISCSI.py

@@ -6,29 +6,36 @@
 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)
 
         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 @@ def deepcopy(to_copy):
 
         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().setUp()
 
     def setup_path_mocks(self):
         self.path_contents = {}
@@ -67,24 +58,31 @@ def setup_path_mocks(self):
         mock_listdir.side_effect = self.listdir
 
     def exists(self, path):
-        print(f'checking existance of {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
         self.setup_path_mocks()
         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,15 +92,97 @@ def test_attach_tgt_present_path_found(self, mock_init_paths):
     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:
             self.subject.attach(self.sr_uuid)
 
         # 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)