From a8386d5cbe732a4e3f4daf836a6efdf69ee6e11f Mon Sep 17 00:00:00 2001 From: Tomoki Sekiyama Date: Wed, 3 Jun 2015 13:42:34 -0400 Subject: [PATCH] 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 Related-Bug: #1382440 Change-Id: I4c9b173cfab387a55772709242eed69c10168c0f --- os_brick/initiator/connector.py | 20 ++++++++++- os_brick/tests/initiator/test_connector.py | 42 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/os_brick/initiator/connector.py b/os_brick/initiator/connector.py index 8018c83b0..e7eef34e9 100644 --- a/os_brick/initiator/connector.py +++ b/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 diff --git a/os_brick/tests/initiator/test_connector.py b/os_brick/tests/initiator/test_connector.py index e745225ef..6c3762826 100644 --- a/os_brick/tests/initiator/test_connector.py +++ b/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,