openstack/cinder
Code Issues Proposed changes

NetApp: Return all iSCSI targets-portals

Browse Source 
NetApp iSCSI drivers use discovery mode for multipathing, which means
that we'll fail to attach a volume if the IP provided for the discovery
is not accessible from the host.

Something similar would happen when using singlepath, as we are only
trying to connect to one target/portal.

This patch changes NetApp drivers so the return target_iqns,
target_portals, and target_luns parameters whenever there are more than
one option.

Changes to be committed:
modified:   cinder/volume/drivers/netapp/eseries/library.py
As part eseries driver  code is still in the tree.  Need to propose
an additional patch to the stable branch, notice that patch
would not be in master, even though both have the same issue.

Closes-Bug: #1806398
Change-Id: If6b5ad23c899032dbf7535ed91251cbfe54a223a
(cherry picked from commit c658696265)
(cherry picked from commit dee7292b08)
This commit is contained in:
Gorka Eguileor 2018-12-03 12:16:10 +01:00 committed by Pablo Caruana
parent 0be8ae6c59
commit 8be2bb6d2a
10 changed files with 120 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py
View File

@ -685,8 +685,8 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
self.zapi_client.get_iscsi_target_details.return_value = (
target_details_list)
self.mock_object(block_base.NetAppBlockStorageLibrary,
'_get_preferred_target_from_list',
return_value=target_details_list[1])
'_get_targets_from_list',
return_value=target_details_list)
self.zapi_client.get_iscsi_service_details.return_value = (
fake.ISCSI_SERVICE_IQN)
self.mock_object(na_utils,
@ -722,7 +722,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
fake.ISCSI_VOLUME['name'], [fake.ISCSI_CONNECTOR['initiator']],
'iscsi', None)
self.zapi_client.get_iscsi_target_details.assert_called_once_with()
block_base.NetAppBlockStorageLibrary._get_preferred_target_from_list\
block_base.NetAppBlockStorageLibrary._get_targets_from_list\
.assert_called_once_with(
target_details_list)
self.zapi_client.get_iscsi_service_details.assert_called_once_with()
@ -734,7 +734,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
return_value=fake.ISCSI_LUN['lun_id'])
self.zapi_client.get_iscsi_target_details.return_value = None
self.mock_object(block_base.NetAppBlockStorageLibrary,
'_get_preferred_target_from_list')
'_get_targets_from_list')
self.mock_object(na_utils,
'get_iscsi_connection_properties',
return_value=fake.ISCSI_CONNECTION_PROPERTIES)
@ -745,7 +745,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
self.assertEqual(
0, block_base.NetAppBlockStorageLibrary
._get_preferred_target_from_list.call_count)
._get_targets_from_list.call_count)
self.assertEqual(
0, self.zapi_client.get_iscsi_service_details.call_count)
self.assertEqual(
@ -758,7 +758,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
return_value=fake.ISCSI_LUN['lun_id'])
self.zapi_client.get_iscsi_target_details.return_value = None
self.mock_object(block_base.NetAppBlockStorageLibrary,
'_get_preferred_target_from_list',
'_get_targets_from_list',
return_value=None)
self.mock_object(na_utils, 'get_iscsi_connection_properties')
@ -780,8 +780,8 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
self.zapi_client.get_iscsi_target_details.return_value = (
target_details_list)
self.mock_object(block_base.NetAppBlockStorageLibrary,
'_get_preferred_target_from_list',
return_value=target_details_list[1])
'_get_targets_from_list',
return_value=target_details_list)
self.zapi_client.get_iscsi_service_details.return_value = None
self.mock_object(na_utils, 'get_iscsi_connection_properties')
@ -794,53 +794,49 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
fake.ISCSI_VOLUME['name'], [fake.ISCSI_CONNECTOR['initiator']],
'iscsi', None)
self.zapi_client.get_iscsi_target_details.assert_called_once_with()
block_base.NetAppBlockStorageLibrary._get_preferred_target_from_list\
block_base.NetAppBlockStorageLibrary._get_targets_from_list\
.assert_called_once_with(target_details_list)
def test_get_target_details_list(self):
target_details_list = fake.ISCSI_TARGET_DETAILS_LIST
result = self.library._get_preferred_target_from_list(
target_details_list)
result = self.library._get_targets_from_list(target_details_list)
self.assertEqual(target_details_list[0], result)
self.assertEqual(target_details_list, result)
def test_get_preferred_target_from_empty_list(self):
target_details_list = []
result = self.library._get_preferred_target_from_list(
target_details_list)
result = self.library._get_targets_from_list(target_details_list)
self.assertIsNone(result)
self.assertFalse(bool(result))
def test_get_preferred_target_from_list_with_one_interface_disabled(self):
def test_get_targets_from_list_with_one_interface_disabled(self):
target_details_list = copy.deepcopy(fake.ISCSI_TARGET_DETAILS_LIST)
target_details_list[0]['interface-enabled'] = 'false'
result = self.library._get_preferred_target_from_list(
target_details_list)
result = self.library._get_targets_from_list(target_details_list)
self.assertEqual(target_details_list[1], result)
self.assertEqual(target_details_list[1:], result)
def test_get_preferred_target_from_list_with_all_interfaces_disabled(self):
def test_get_targets_from_list_with_all_interfaces_disabled(self):
target_details_list = copy.deepcopy(fake.ISCSI_TARGET_DETAILS_LIST)
for target in target_details_list:
target['interface-enabled'] = 'false'
result = self.library._get_preferred_target_from_list(
target_details_list)
result = self.library._get_targets_from_list(target_details_list)
self.assertEqual(target_details_list[0], result)
self.assertEqual(target_details_list, result)
def test_get_preferred_target_from_list_with_filter(self):
def test_get_targets_from_list_with_filter(self):
target_details_list = fake.ISCSI_TARGET_DETAILS_LIST
filter = [target_detail['address']
for target_detail in target_details_list[1:]]
result = self.library._get_preferred_target_from_list(
target_details_list, filter)
result = self.library._get_targets_from_list(target_details_list,
filter)
self.assertEqual(target_details_list[1], result)
self.assertEqual(target_details_list[1:], result)
@mock.patch.object(na_utils, 'check_flags', mock.Mock())
@mock.patch.object(block_base, 'LOG', mock.Mock())

13
cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py
View File

@ -336,19 +336,6 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase):
fake.VOLUME_ID, fake.LUN_ID, fake.LUN_SIZE, fake.LUN_METADATA,
None)
def test_get_preferred_target_from_list(self):
target_details_list = fake.ISCSI_TARGET_DETAILS_LIST
operational_addresses = [
target['address']
for target in target_details_list[2:]]
self.zapi_client.get_operational_lif_addresses = (
mock.Mock(return_value=operational_addresses))
result = self.library._get_preferred_target_from_list(
target_details_list)
self.assertEqual(target_details_list[2], result)
@ddt.data({'replication_backends': [], 'cluster_credentials': False},
{'replication_backends': ['target_1', 'target_2'],
'cluster_credentials': True})

10
cinder/tests/unit/volume/drivers/netapp/eseries/test_driver.py
View File

@ -258,17 +258,17 @@ class NetAppESeriesDriverTestCase(object):
vol_nomatch = {'id': 'vol_id', 'currentManager': 'ctrl3'}
portals = [{'controller': 'ctrl2', 'iqn': 'iqn2'},
{'controller': 'ctrl1', 'iqn': 'iqn1'}]
portal = self.library._get_iscsi_portal_for_vol(volume, portals)
self.assertEqual({'controller': 'ctrl1', 'iqn': 'iqn1'}, portal)
portal = self.library._get_iscsi_portal_for_vol(vol_nomatch, portals)
self.assertEqual({'controller': 'ctrl2', 'iqn': 'iqn2'}, portal)
portal = self.library._get_iscsi_portals_for_vol(volume, portals)
self.assertEqual({'controller': 'ctrl1', 'iqn': 'iqn1'}, portal[0])
portal = self.library._get_iscsi_portals_for_vol(vol_nomatch, portals)
self.assertEqual({'controller': 'ctrl2', 'iqn': 'iqn2'}, portal[0])
def test_portal_for_vol_any_false(self):
vol_nomatch = {'id': 'vol_id', 'currentManager': 'ctrl3'}
portals = [{'controller': 'ctrl2', 'iqn': 'iqn2'},
{'controller': 'ctrl1', 'iqn': 'iqn1'}]
self.assertRaises(exception.NetAppDriverException,
self.library._get_iscsi_portal_for_vol,
self.library._get_iscsi_portals_for_vol,
vol_nomatch, portals, False)
def test_do_setup_all_default(self):

14
cinder/tests/unit/volume/drivers/netapp/fakes.py
View File

@ -23,8 +23,10 @@ import cinder.volume.drivers.netapp.options as na_opts
ISCSI_FAKE_LUN_ID = 1
ISCSI_FAKE_IQN = 'iqn.1993-08.org.debian:01:10'
ISCSI_FAKE_IQN2 = 'iqn.1993-08.org.debian:01:11'
ISCSI_FAKE_ADDRESS_IPV4 = '10.63.165.216'
ISCSI_FAKE_ADDRESS2_IPV4 = '10.63.165.217'
ISCSI_FAKE_ADDRESS_IPV6 = 'fe80::72a4:a152:aad9:30d9'
ISCSI_FAKE_PORT = '2232'
@ -38,6 +40,18 @@ ISCSI_FAKE_TARGET['port'] = ISCSI_FAKE_PORT
ISCSI_FAKE_VOLUME = {'id': 'fake_id', 'provider_auth': 'None stack password'}
ISCSI_FAKE_VOLUME_NO_AUTH = {'id': 'fake_id', 'provider_auth': ''}
ISCSI_MP_TARGET_INFO_DICT = {'target_discovered': False,
'target_portal': '10.63.165.216:2232',
'target_portals': ['10.63.165.216:2232',
'10.63.165.217:2232'],
'target_iqn': ISCSI_FAKE_IQN,
'target_iqns': [ISCSI_FAKE_IQN, ISCSI_FAKE_IQN2],
'target_lun': ISCSI_FAKE_LUN_ID,
'target_luns': [ISCSI_FAKE_LUN_ID] * 2,
'volume_id': ISCSI_FAKE_VOLUME['id'],
'auth_method': 'None', 'auth_username': 'stack',
'auth_password': 'password'}
FC_ISCSI_TARGET_INFO_DICT = {'target_discovered': False,
'target_portal': '10.63.165.216:2232',
'target_iqn': ISCSI_FAKE_IQN,

32
cinder/tests/unit/volume/drivers/netapp/test_utils.py
View File

@ -119,40 +119,56 @@ class NetAppDriverUtilsTestCase(test.TestCase):
self.assertAlmostEqual(na_utils.round_down(-5.567, '0'), -5)
def test_iscsi_connection_properties(self):
actual_properties = na_utils.get_iscsi_connection_properties(
fake.ISCSI_FAKE_LUN_ID, fake.ISCSI_FAKE_VOLUME,
fake.ISCSI_FAKE_IQN, fake.ISCSI_FAKE_ADDRESS_IPV4,
fake.ISCSI_FAKE_PORT)
[fake.ISCSI_FAKE_IQN, fake.ISCSI_FAKE_IQN2],
[fake.ISCSI_FAKE_ADDRESS_IPV4, fake.ISCSI_FAKE_ADDRESS2_IPV4],
[fake.ISCSI_FAKE_PORT, fake.ISCSI_FAKE_PORT])
actual_properties_mapped = actual_properties['data']
self.assertDictEqual(actual_properties_mapped,
fake.FC_ISCSI_TARGET_INFO_DICT)
fake.ISCSI_MP_TARGET_INFO_DICT)
def test_iscsi_connection_properties_single_iqn(self):
actual_properties = na_utils.get_iscsi_connection_properties(
fake.ISCSI_FAKE_LUN_ID, fake.ISCSI_FAKE_VOLUME,
fake.ISCSI_FAKE_IQN,
[fake.ISCSI_FAKE_ADDRESS_IPV4, fake.ISCSI_FAKE_ADDRESS2_IPV4],
[fake.ISCSI_FAKE_PORT, fake.ISCSI_FAKE_PORT])
actual_properties_mapped = actual_properties['data']
expected = fake.ISCSI_MP_TARGET_INFO_DICT.copy()
expected['target_iqns'][1] = expected['target_iqns'][0]
self.assertDictEqual(actual_properties_mapped,
fake.ISCSI_MP_TARGET_INFO_DICT)
def test_iscsi_connection_lun_id_type_str(self):
FAKE_LUN_ID = '1'
actual_properties = na_utils.get_iscsi_connection_properties(
FAKE_LUN_ID, fake.ISCSI_FAKE_VOLUME, fake.ISCSI_FAKE_IQN,
fake.ISCSI_FAKE_ADDRESS_IPV4, fake.ISCSI_FAKE_PORT)
[fake.ISCSI_FAKE_ADDRESS_IPV4], [fake.ISCSI_FAKE_PORT])
actual_properties_mapped = actual_properties['data']
self.assertIs(int, type(actual_properties_mapped['target_lun']))
self.assertDictEqual(actual_properties_mapped,
fake.FC_ISCSI_TARGET_INFO_DICT)
def test_iscsi_connection_lun_id_type_dict(self):
FAKE_LUN_ID = {'id': 'fake_id'}
self.assertRaises(TypeError, na_utils.get_iscsi_connection_properties,
FAKE_LUN_ID, fake.ISCSI_FAKE_VOLUME,
fake.ISCSI_FAKE_IQN, fake.ISCSI_FAKE_ADDRESS_IPV4,
fake.ISCSI_FAKE_PORT)
fake.ISCSI_FAKE_IQN, [fake.ISCSI_FAKE_ADDRESS_IPV4],
[fake.ISCSI_FAKE_PORT])
def test_iscsi_connection_properties_ipv6(self):
actual_properties = na_utils.get_iscsi_connection_properties(
'1', fake.ISCSI_FAKE_VOLUME_NO_AUTH, fake.ISCSI_FAKE_IQN,
fake.ISCSI_FAKE_ADDRESS_IPV6, fake.ISCSI_FAKE_PORT)
[fake.ISCSI_FAKE_ADDRESS_IPV6], [fake.ISCSI_FAKE_PORT])
self.assertDictEqual(actual_properties['data'],
fake.FC_ISCSI_TARGET_INFO_DICT_IPV6)

32
cinder/volume/drivers/netapp/dataontap/block_base.py
View File

@ -839,13 +839,12 @@ class NetAppBlockStorageLibrary(object):
"initiator %(initiator_name)s",
{'name': name, 'initiator_name': initiator_name})
preferred_target = self._get_preferred_target_from_list(
target_list)
if preferred_target is None:
targets = self._get_targets_from_list(target_list)
if not targets:
msg = _('Failed to get target portal for the LUN %s')
raise exception.VolumeBackendAPIException(data=msg % name)
(address, port) = (preferred_target['address'],
preferred_target['port'])
addresses = [target['address'] for target in targets]
ports = [target['port'] for target in targets]
iqn = self.zapi_client.get_iscsi_service_details()
if not iqn:
@ -853,8 +852,8 @@ class NetAppBlockStorageLibrary(object):
raise exception.VolumeBackendAPIException(data=msg % name)
properties = na_utils.get_iscsi_connection_properties(lun_id, volume,
iqn, address,
port)
iqn, addresses,
ports)
if self.configuration.use_chap_auth:
chap_username, chap_password = self._configure_chap(initiator_name)
@ -881,18 +880,13 @@ class NetAppBlockStorageLibrary(object):
properties['data']['discovery_auth_username'] = username
properties['data']['discovery_auth_password'] = password
def _get_preferred_target_from_list(self, target_details_list,
filter=None):
preferred_target = None
for target in target_details_list:
if filter and target['address'] not in filter:
continue
if target.get('interface-enabled', 'true') == 'true':
preferred_target = target
break
if preferred_target is None and len(target_details_list) > 0:
preferred_target = target_details_list[0]
return preferred_target
def _get_targets_from_list(self, target_details_list, filter=None):
targets = [target for target in target_details_list
if (not filter or target['address'] in filter) and
target.get('interface-enabled', 'true') == 'true']
if not targets and len(target_details_list) > 0:
targets = target_details_list
return targets
def terminate_connection_iscsi(self, volume, connector, **kwargs):
"""Driver entry point to unattach a volume from an instance.

23
cinder/volume/drivers/netapp/dataontap/block_cmode.py
View File

@ -390,29 +390,6 @@ class NetAppBlockStorageCmodeLibrary(block_base.NetAppBlockStorageLibrary,
msg = 'Deleted LUN with name %(name)s and QoS info %(qos)s'
LOG.debug(msg, {'name': volume['name'], 'qos': qos_policy_group_info})
def _get_preferred_target_from_list(self, target_details_list,
filter=None):
# cDOT iSCSI LIFs do not migrate from controller to controller
# in failover. Rather, an iSCSI LIF must be configured on each
# controller and the initiator has to take responsibility for
# using a LIF that is UP. In failover, the iSCSI LIF on the
# downed controller goes DOWN until the controller comes back up.
#
# Currently Nova only accepts a single target when obtaining
# target details from Cinder, so we pass back the first portal
# with an UP iSCSI LIF. There are plans to have Nova accept
# and try multiple targets. When that happens, we can and should
# remove this filter and return all targets since their operational
# state could change between the time we test here and the time
# Nova uses the target.
operational_addresses = (
self.zapi_client.get_operational_lif_addresses())
return (super(NetAppBlockStorageCmodeLibrary, self)
._get_preferred_target_from_list(target_details_list,
filter=operational_addresses))
def _setup_qos_for_volume(self, volume, extra_specs):
try:
qos_policy_group_info = na_utils.get_valid_qos_policy_group_info(

28
cinder/volume/drivers/netapp/eseries/library.py
View File

@ -1292,19 +1292,19 @@ class NetAppESeriesLibrary(object):
msg_fmt)
iscsi_details = self._get_iscsi_service_details()
iscsi_portal = self._get_iscsi_portal_for_vol(eseries_vol,
iscsi_details)
iscsi_portals = self._get_iscsi_portals_for_vol(eseries_vol,
iscsi_details)
LOG.debug("Successfully fetched target details for volume %(id)s and "
"initiator %(initiator_name)s.", msg_fmt)
iqn = iscsi_portal['iqn']
address = iscsi_portal['ip']
port = iscsi_portal['tcp_port']
iqns = [portal['iqn'] for portal in iscsi_portals]
addresses = [portal['ip'] for portal in iscsi_portals]
ports = [portal['tcp_port'] for portal in iscsi_portals]
properties = na_utils.get_iscsi_connection_properties(lun_id, volume,
iqn, address,
port)
iqns, addresses,
ports)
if self.configuration.use_chap_auth:
if self._client.features.CHAP_AUTHENTICATION:
chap_username, chap_password = self._configure_chap(iqn)
chap_username, chap_password = self._configure_chap(iqns[0])
properties['data']['auth_username'] = chap_username
properties['data']['auth_password'] = chap_password
properties['data']['auth_method'] = 'CHAP'
@ -1359,13 +1359,15 @@ class NetAppESeriesLibrary(object):
msg % self._client.get_system_id())
return ports
def _get_iscsi_portal_for_vol(self, volume, portals, anyController=True):
def _get_iscsi_portals_for_vol(self, volume, portals, anyController=True):
"""Get the iscsi portal info relevant to volume."""
for portal in portals:
if portal.get('controller') == volume.get('currentManager'):
return portal
manager = volume.get('currentManager')
current_manager_portals = [portal for portal in portals
if portal.get('controller') == manager]
if current_manager_portals:
return current_manager_portals
if anyController and portals:
return portals[0]
return portals
msg = _('No good iscsi portal found in supplied list for %s.')
raise exception.NetAppDriverException(
msg % self._client.get_system_id())

25
cinder/volume/drivers/netapp/utils.py
View File

@ -173,18 +173,29 @@ def log_extra_spec_warnings(extra_specs):
'new': DEPRECATED_SSC_SPECS[spec]})
def get_iscsi_connection_properties(lun_id, volume, iqn,
address, port):
def get_iscsi_connection_properties(lun_id, volume, iqns,
addresses, ports):
# literal ipv6 address
if netutils.is_valid_ipv6(address):
address = netutils.escape_ipv6(address)
addresses = [netutils.escape_ipv6(a) if netutils.is_valid_ipv6(a) else a
for a in addresses]
lun_id = int(lun_id)
if isinstance(iqns, six.string_types):
iqns = [iqns] * len(addresses)
target_portals = ['%s:%s' % (a, p) for a, p in zip(addresses, ports)]
properties = {}
properties['target_discovered'] = False
properties['target_portal'] = '%s:%s' % (address, port)
properties['target_iqn'] = iqn
properties['target_lun'] = int(lun_id)
properties['target_portal'] = target_portals[0]
properties['target_iqn'] = iqns[0]
properties['target_lun'] = lun_id
properties['volume_id'] = volume['id']
if len(addresses) > 1:
properties['target_portals'] = target_portals
properties['target_iqns'] = iqns
properties['target_luns'] = [lun_id] * len(addresses)
auth = volume['provider_auth']
if auth:
(auth_method, auth_username, auth_secret) = auth.split()

8
releasenotes/notes/netapp-non-discovery-19af4e10f7b190ea.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
NetApp iSCSI drivers no longer use the discovery mechanism for multipathing
and they always return all target/portals when attaching a volume. Thanks
to this, volumes will be successfully attached even if the target/portal
selected as primary is down, this will be the case for both, multipath and
single path connections.