Fix iSCSI cleanup issue when using discovery

With the latest iSCSI disconnect refactoring [1] we have introduced a
regression where not all devices are properly cleaned up on a multipath
connection if we are using target discovery.

On connection we do the discovery so we get the targets, but on
disconnection  the code is expecting to have all the ips, iqns, and luns
information on the connection properties, so it will only flush the
multipath and disconnect volumes that were connected using the
information from the connection property.

This patch fixes this retrieving the targets information from the
discoverydb that was gathered during the connection phase.

Output from discoverydb query looks like this:

 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.

[1] https://review.openstack.org/455392

Change-Id: Iada5d4fbeb07aeaf3afb953a289b6b89778c382c
Closes-Bug: #1699061
This commit is contained in:
Gorka Eguileor 2017-06-20 14:15:26 +02:00
parent 9728815feb
commit 1905398f61
2 changed files with 235 additions and 22 deletions

View File

@ -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

View File

@ -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')