Fix connecting unnecessary iSCSI sessions issue

With "iscsi_use_multipath=true", attaching a multipath
iSCSI volume may create unnecessary iSCSI sessions.
The iscsiadm discovery command in connect_volume() returns all of
the targets in the Cinder node, not just the ones related to
the multipath volume which is specified by iqn. If the storage
has many targets, connecting to all these volumes will also
result in many unnecessary connections.

There are two types of iSCSI multipath devices.
One which shares the same iqn between multiple portals,
and the other which use different iqns on different portals.
connect_volume() needs to identify the type by checking iscsiadm
the output if the iqn is used by multiple portals.

This patch changes the behavior of attaching volume:

   1. Identify the type by checking the iscsiadm output.
   2. Connect to the correct targets by connect_to_iscsi_portal().

(Ported from I488ad0c09bf26a609e27d67b9ef60b65bb45e0ad)

Co-Authored-By: Hiroyuki Eguchi <h-eguchi@az.jp.nec.com>
Related-Bug: #1382440
Change-Id: I4c9b173cfab387a55772709242eed69c10168c0f

os_brick/initiator/connector.py

@@ -306,7 +306,25 @@ class ISCSIConnector(InitiatorConnector):
 
         if self.use_multipath:
             # Multipath installed, discovering other targets if available
-            for ip, iqn in self._discover_iscsi_portals(connection_properties):
+            ips_iqns = self._discover_iscsi_portals(connection_properties)
+
+            if not connection_properties.get('target_iqns'):
+                # There are two types of iSCSI multipath devices. One which
+                # shares the same iqn between multiple portals, and the other
+                # which use different iqns on different portals.
+                # Try to identify the type by checking the iscsiadm output
+                # if the iqn is used by multiple portals. If it is, it's
+                # the former, so use the supplied iqn. Otherwise, it's the
+                # latter, so try the ip,iqn combinations to find the targets
+                # which constitutes the multipath device.
+                main_iqn = connection_properties['target_iqn']
+                all_portals = set([ip for ip, iqn in ips_iqns])
+                match_portals = set([ip for ip, iqn in ips_iqns
+                                     if iqn == main_iqn])
+                if len(all_portals) == len(match_portals):
+                    ips_iqns = zip(all_portals, [main_iqn] * len(all_portals))
+
+            for ip, iqn in ips_iqns:
                 props = copy.deepcopy(connection_properties)
                 props['target_portal'] = ip
                 props['target_iqn'] = iqn

os_brick/tests/initiator/test_connector.py

@@ -535,6 +535,48 @@ class ISCSIConnectorTestCase(ConnectorTestCase):
         mock_iscsiadm.assert_any_call(props, ('--logout',),
                                       check_exit_code=[0, 21, 255])
 
+    @mock.patch.object(os.path, 'exists', return_value=True)
+    @mock.patch.object(connector.ISCSIConnector,
+                       '_get_target_portals_from_iscsiadm_output')
+    @mock.patch.object(connector.ISCSIConnector, '_connect_to_iscsi_portal')
+    @mock.patch.object(host_driver.HostDriver, 'get_all_block_devices')
+    @mock.patch.object(connector.ISCSIConnector, '_get_iscsi_devices')
+    @mock.patch.object(connector.ISCSIConnector, '_rescan_multipath')
+    @mock.patch.object(connector.ISCSIConnector, '_run_multipath')
+    @mock.patch.object(connector.ISCSIConnector, '_get_multipath_device_name')
+    def test_connect_volume_with_multipath_connecting(
+            self, mock_device_name, mock_run_multipath,
+            mock_rescan_multipath, mock_iscsi_devices, mock_devices,
+            mock_connect, mock_portals, mock_exists):
+        location1 = '10.0.2.15:3260'
+        location2 = '10.0.3.15:3260'
+        name1 = 'volume-00000001-1'
+        name2 = 'volume-00000001-2'
+        iqn1 = 'iqn.2010-10.org.openstack:%s' % name1
+        iqn2 = 'iqn.2010-10.org.openstack:%s' % name2
+        fake_multipath_dev = '/dev/mapper/fake-multipath-dev'
+        vol = {'id': 1, 'name': name1}
+        connection_properties = self.iscsi_connection(vol, location1, iqn1)
+        devs = ['/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location1, iqn1),
+                '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (location2, iqn2)]
+        mock_devices.return_value = devs
+        mock_iscsi_devices.return_value = devs
+        mock_device_name.return_value = fake_multipath_dev
+        mock_portals.return_value = [[location1, iqn1], [location2, iqn1],
+                                     [location2, iqn2]]
+
+        result = self.connector_with_multipath.connect_volume(
+            connection_properties['data'])
+        expected_result = {'path': fake_multipath_dev, 'type': 'block'}
+        props1 = connection_properties['data'].copy()
+        props2 = connection_properties['data'].copy()
+        locations = list(set([location1, location2]))  # order may change
+        props1['target_portal'] = locations[0]
+        props2['target_portal'] = locations[1]
+        expected_calls = [mock.call(props1), mock.call(props2)]
+        self.assertEqual(expected_result, result)
+        self.assertEqual(expected_calls, mock_connect.call_args_list)
+
     @mock.patch.object(time, 'sleep')
     @mock.patch.object(os.path, 'exists', return_value=False)
     def test_connect_volume_with_not_found_device(self, exists_mock,