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.

Closes-Bug: #1806398
Change-Id: If6b5ad23c899032dbf7535ed91251cbfe54a223a
This commit is contained in:
Gorka Eguileor 2018-12-03 12:16:10 +01:00
parent da34b35579
commit c658696265
8 changed files with 100 additions and 97 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})

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(

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

@ -165,18 +165,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.