diff --git a/os_brick/initiator/connectors/iscsi.py b/os_brick/initiator/connectors/iscsi.py index 04ecef05a..df407038b 100644 --- a/os_brick/initiator/connectors/iscsi.py +++ b/os_brick/initiator/connectors/iscsi.py @@ -16,6 +16,7 @@ import collections import glob import os +import re import threading import time @@ -26,6 +27,7 @@ from oslo_utils import excutils from oslo_utils import strutils from os_brick import exception +from os_brick.i18n import _ from os_brick import initiator from os_brick.initiator.connectors import base from os_brick.initiator.connectors import base_iscsi @@ -159,16 +161,36 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): # entry: [tcp, [1], 192.168.121.250:3260,1 ...] return [entry[2] for entry in self._get_iscsi_sessions_full()] - def _get_ips_iqns_luns(self, connection_properties): + def _get_ips_iqns_luns(self, connection_properties, discover=True): """Build a list of ips, iqns, and luns. + Used only when doing multipath, we have 3 cases: + + - All information is in the connection properties + - We have to do an iSCSI discovery to get the information + - We don't want to do another discovery and we query the discoverydb + :param connection_properties: The dictionary that describes all of the target volume attributes. :type connection_properties: dict + :param discover: Wheter doing an iSCSI discovery is acceptable. + :type discover: bool :returns: list of tuples of (ip, iqn, lun) """ try: - ips_iqns_luns = self._discover_iscsi_portals(connection_properties) + if ('target_portals' in connection_properties and + 'target_iqns' in connection_properties): + # Use targets specified by connection_properties + ips_iqns_luns = list( + zip(connection_properties['target_portals'], + connection_properties['target_iqns'], + self._get_luns(connection_properties))) + else: + method = (self._discover_iscsi_portals if discover + else self._get_discoverydb_portals) + ips_iqns_luns = method(connection_properties) + except exception.TargetPortalsNotFound: + raise except Exception: if 'target_portals' in connection_properties: raise exception.TargetPortalsNotFound( @@ -289,14 +311,74 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): num_luns = len(con_props['target_iqns']) if iqns is None else len(iqns) return luns or [con_props.get('target_lun')] * num_luns - def _discover_iscsi_portals(self, connection_properties): - if all([key in connection_properties for key in ('target_portals', - 'target_iqns')]): - # Use targets specified by connection_properties - return list(zip(connection_properties['target_portals'], - connection_properties['target_iqns'], - self._get_luns(connection_properties))) + def _get_discoverydb_portals(self, connection_properties): + """Retrieve iscsi portals information from the discoverydb. + Example of discoverydb command output: + + SENDTARGETS: + DiscoveryAddress: 192.168.1.33,3260 + DiscoveryAddress: 192.168.1.2,3260 + Target: iqn.2004-04.com.qnap:ts-831x:iscsi.cinder-20170531114245.9eff88 + Portal: 192.168.1.3:3260,1 + Iface Name: default + Portal: 192.168.1.2:3260,1 + Iface Name: default + Target: iqn.2004-04.com.qnap:ts-831x:iscsi.cinder-20170531114447.9eff88 + Portal: 192.168.1.3:3260,1 + Iface Name: default + Portal: 192.168.1.2:3260,1 + Iface Name: default + DiscoveryAddress: 192.168.1.38,3260 + iSNS: + No targets found. + STATIC: + No targets found. + FIRMWARE: + No targets found. + + :param connection_properties: The dictionary that describes all + of the target volume attributes. + :type connection_properties: dict + :returns: list of tuples of (ip, iqn, lun) + """ + out = self._run_iscsiadm_bare(['-m', 'discoverydb', + '-o', 'show', + '-P', 1])[0] or "" + regex = ('^SENDTARGETS:\n.*?^DiscoveryAddress: ' + + connection_properties['target_portal'] + + '.*?\n(.*?)^(?:DiscoveryAddress|iSNS):.*') + info = re.search(regex, out, re.DOTALL | re.MULTILINE) + + ips = [] + iqns = [] + + if info: + iscsi_transport = ('iser' if self._get_transport() == 'iser' + else 'default') + iface = 'Iface Name: ' + iscsi_transport + current_iqn = '' + current_ip = '' + for line in info.group(1).splitlines(): + line = line.strip() + if line.startswith('Target:'): + current_iqn = line[8:] + elif line.startswith('Portal:'): + current_ip = line[8:].split(',')[0] + elif line.startswith(iface): + if current_iqn and current_ip: + iqns.append(current_iqn) + ips.append(current_ip) + current_ip = '' + + if not iqns: + raise exception.TargetPortalsNotFound( + _('Unable to find target portals information on discoverydb.')) + + luns = self._get_luns(connection_properties, iqns) + return list(zip(ips, iqns, luns)) + + def _discover_iscsi_portals(self, connection_properties): out = None iscsi_transport = ('iser' if self._get_transport() == 'iser' else 'default') @@ -640,9 +722,21 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): If ips_iqns_luns parameter is provided connection_properties won't be used to get them. + + When doing multipath we may not have all the information on the + connection properties (sendtargets was used on connect) so we may have + to retrieve the info from the discoverydb. Call _get_ips_iqns_luns to + do the right things. """ if not ips_iqns_luns: - ips_iqns_luns = self._get_all_targets(connection_properties) + if self.use_multipath: + # We are only called from disconnect, so don't discover + ips_iqns_luns = self._get_ips_iqns_luns(connection_properties, + discover=False) + else: + ips_iqns_luns = self._get_all_targets(connection_properties) + LOG.debug('Getting connected devices for (ips,iqns,luns)=%s', + ips_iqns_luns) nodes = self._get_iscsi_nodes() sessions = self._get_iscsi_sessions_full() # Use (portal, iqn) to map the session value @@ -673,6 +767,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): else: others.add(device) + LOG.debug('Resulting device map %s', device_map) return device_map @utils.trace diff --git a/os_brick/tests/initiator/connectors/test_iscsi.py b/os_brick/tests/initiator/connectors/test_iscsi.py index 61f1caa50..6dd15de0c 100644 --- a/os_brick/tests/initiator/connectors/test_iscsi.py +++ b/os_brick/tests/initiator/connectors/test_iscsi.py @@ -27,6 +27,10 @@ from os_brick.tests.initiator import test_connector @ddt.ddt class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): + SINGLE_CON_PROPS = {'volume_id': 'vol_id', + 'target_portal': 'ip1:port1', + 'target_iqn': 'tgt1', + 'target_lun': '1'} CON_PROPS = { 'volume_id': 'vol_id', 'target_portal': 'ip1:port1', @@ -108,11 +112,15 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): res = self.connector._get_iscsi_nodes() self.assertEqual([], res) + @mock.patch.object(iscsi.ISCSIConnector, '_get_ips_iqns_luns') @mock.patch('glob.glob') @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions_full') @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_nodes') def test_get_connection_devices(self, nodes_mock, sessions_mock, - glob_mock): + glob_mock, iql_mock): + self.connector.use_multipath = True + iql_mock.return_value = self.connector._get_all_targets(self.CON_PROPS) + # List sessions from other targets and non tcp sessions sessions_mock.return_value = [ ('non-tcp:', '0', 'ip1:port1', '1', 'tgt1'), @@ -136,6 +144,7 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): ('ip2:port2', 'tgt2'): ({'sdb'}, {'sdc'}), ('ip3:port3', 'tgt3'): (set(), set())} self.assertDictEqual(expected, res) + iql_mock.assert_called_once_with(self.CON_PROPS, discover=False) @mock.patch('glob.glob') @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions_full') @@ -525,6 +534,109 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): force=mock.sentinel.Force, ignore_errors=mock.sentinel.ignore_errors) + @ddt.data(True, False) + @mock.patch.object(iscsi.ISCSIConnector, '_get_transport') + @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare') + def test_get_discoverydb_portals(self, is_iser, iscsiadm_mock, + transport_mock): + params = {'iqn1': self.SINGLE_CON_PROPS['target_iqn'], + 'iqn2': 'iqn.2004-04.com.qnap:ts-831x:iscsi.cinder-2017.9ef', + 'ip1': self.SINGLE_CON_PROPS['target_portal'], + 'ip2': '192.168.1.3:3260', + 'transport': 'iser' if is_iser else 'default', + 'other_transport': 'default' if is_iser else 'iser'} + + iscsiadm_mock.return_value = ( + 'SENDTARGETS:\n' + 'DiscoveryAddress: 192.168.1.33,3260\n' + 'DiscoveryAddress: %(ip1)s\n' + 'Target: %(iqn1)s\n' + ' Portal: %(ip2)s,1\n' + ' Iface Name: %(transport)s\n' + ' Portal: %(ip1)s,1\n' + ' Iface Name: %(transport)s\n' + ' Portal: %(ip1)s,1\n' + ' Iface Name: %(other_transport)s\n' + 'Target: %(iqn2)s\n' + ' Portal: %(ip2)s,1\n' + ' Iface Name: %(transport)s\n' + ' Portal: %(ip1)s,1\n' + ' Iface Name: %(transport)s\n' + 'DiscoveryAddress: 192.168.1.38,3260\n' + 'iSNS:\n' + 'No targets found.\n' + 'STATIC:\n' + 'No targets found.\n' + 'FIRMWARE:\n' + 'No targets found.\n' % params, None) + transport_mock.return_value = 'iser' if is_iser else 'non-iser' + + res = self.connector._get_discoverydb_portals(self.SINGLE_CON_PROPS) + expected = [(params['ip2'], params['iqn1'], + self.SINGLE_CON_PROPS['target_lun']), + (params['ip1'], params['iqn1'], + self.SINGLE_CON_PROPS['target_lun']), + (params['ip2'], params['iqn2'], + self.SINGLE_CON_PROPS['target_lun']), + (params['ip1'], params['iqn2'], + self.SINGLE_CON_PROPS['target_lun'])] + self.assertListEqual(expected, res) + iscsiadm_mock.assert_called_once_with( + ['-m', 'discoverydb', '-o', 'show', '-P', 1]) + transport_mock.assert_called_once_with() + + @mock.patch.object(iscsi.ISCSIConnector, '_get_transport', return_value='') + @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare') + def test_get_discoverydb_portals_error(self, iscsiadm_mock, + transport_mock): + """DiscoveryAddress is not present.""" + iscsiadm_mock.return_value = ( + 'SENDTARGETS:\n' + 'DiscoveryAddress: 192.168.1.33,3260\n' + 'DiscoveryAddress: 192.168.1.38,3260\n' + 'iSNS:\n' + 'No targets found.\n' + 'STATIC:\n' + 'No targets found.\n' + 'FIRMWARE:\n' + 'No targets found.\n', None) + + self.assertRaises(exception.TargetPortalsNotFound, + self.connector._get_discoverydb_portals, + self.SINGLE_CON_PROPS) + iscsiadm_mock.assert_called_once_with( + ['-m', 'discoverydb', '-o', 'show', '-P', 1]) + transport_mock.assert_not_called() + + @mock.patch.object(iscsi.ISCSIConnector, '_get_transport', return_value='') + @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare') + def test_get_discoverydb_portals_error_is_present(self, iscsiadm_mock, + transport_mock): + """DiscoveryAddress is present but wrong iterface.""" + params = {'iqn': self.SINGLE_CON_PROPS['target_iqn'], + 'ip': self.SINGLE_CON_PROPS['target_portal']} + iscsiadm_mock.return_value = ( + 'SENDTARGETS:\n' + 'DiscoveryAddress: 192.168.1.33,3260\n' + 'DiscoveryAddress: %(ip)s\n' + 'Target: %(iqn)s\n' + ' Portal: %(ip)s,1\n' + ' Iface Name: iser\n' + 'DiscoveryAddress: 192.168.1.38,3260\n' + 'iSNS:\n' + 'No targets found.\n' + 'STATIC:\n' + 'No targets found.\n' + 'FIRMWARE:\n' + 'No targets found.\n' % params, None) + + self.assertRaises(exception.TargetPortalsNotFound, + self.connector._get_discoverydb_portals, + self.SINGLE_CON_PROPS) + iscsiadm_mock.assert_called_once_with( + ['-m', 'discoverydb', '-o', 'show', '-P', 1]) + transport_mock.assert_called_once_with() + @mock.patch.object(iscsi.ISCSIConnector, '_disconnect_connection') @mock.patch.object(iscsi.ISCSIConnector, '_get_connection_devices') @mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device') @@ -793,32 +905,38 @@ Setting up iSCSI targets: unused @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') def test_get_ips_iqns_luns_with_target_iqns(self, discover_mock): res = self.connector._get_ips_iqns_luns(self.CON_PROPS) - self.assertEqual(discover_mock.return_value, res) + expected = list(self.connector._get_all_targets(self.CON_PROPS)) + self.assertListEqual(expected, res) + discover_mock.assert_not_called() + + @mock.patch.object(iscsi.ISCSIConnector, '_get_discoverydb_portals') + @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') + def test_get_ips_iqns_luns_discoverydb(self, discover_mock, + db_portals_mock): + db_portals_mock.return_value = [('ip1:port1', 'tgt1', '1'), + ('ip2:port2', 'tgt2', '2')] + res = self.connector._get_ips_iqns_luns(self.SINGLE_CON_PROPS, + discover=False) + self.assertListEqual(db_portals_mock.return_value, res) + db_portals_mock.assert_called_once_with(self.SINGLE_CON_PROPS) + discover_mock.assert_not_called() @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') def test_get_ips_iqns_luns_no_target_iqns_share_iqn(self, discover_mock): - con_props = {'volume_id': 'vol_id', - 'target_portal': 'ip1:port1', - 'target_iqn': 'tgt1', - 'target_lun': '1'} discover_mock.return_value = [('ip1:port1', 'tgt1', '1'), ('ip1:port1', 'tgt2', '1'), ('ip2:port2', 'tgt1', '2'), ('ip2:port2', 'tgt2', '2')] - res = self.connector._get_ips_iqns_luns(con_props) + res = self.connector._get_ips_iqns_luns(self.SINGLE_CON_PROPS) expected = {('ip1:port1', 'tgt1', '1'), ('ip2:port2', 'tgt1', '2')} self.assertEqual(expected, set(res)) @mock.patch.object(iscsi.ISCSIConnector, '_discover_iscsi_portals') def test_get_ips_iqns_luns_no_target_iqns_diff_iqn(self, discover_mock): - con_props = {'volume_id': 'vol_id', - 'target_portal': 'ip1:port1', - 'target_iqn': 'tgt1', - 'target_lun': '1'} discover_mock.return_value = [('ip1:port1', 'tgt1', '1'), ('ip2:port2', 'tgt2', '2')] - res = self.connector._get_ips_iqns_luns(con_props) + res = self.connector._get_ips_iqns_luns(self.SINGLE_CON_PROPS) self.assertEqual(discover_mock.return_value, res) @mock.patch.object(iscsi.ISCSIConnector, '_get_iscsi_sessions_full')