diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py index 074cff8c661..6581e484de5 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py @@ -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()) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py index 1935c3bdec1..0786bf2df9d 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py @@ -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}) diff --git a/cinder/tests/unit/volume/drivers/netapp/fakes.py b/cinder/tests/unit/volume/drivers/netapp/fakes.py index 6f74f55ae21..28536f12703 100644 --- a/cinder/tests/unit/volume/drivers/netapp/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/fakes.py @@ -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, diff --git a/cinder/tests/unit/volume/drivers/netapp/test_utils.py b/cinder/tests/unit/volume/drivers/netapp/test_utils.py index 85478004c2c..be8051de321 100644 --- a/cinder/tests/unit/volume/drivers/netapp/test_utils.py +++ b/cinder/tests/unit/volume/drivers/netapp/test_utils.py @@ -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) diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index c6147969931..8ca86bf5c58 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -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. diff --git a/cinder/volume/drivers/netapp/dataontap/block_cmode.py b/cinder/volume/drivers/netapp/dataontap/block_cmode.py index fec8c297358..e1058eb0866 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_cmode.py +++ b/cinder/volume/drivers/netapp/dataontap/block_cmode.py @@ -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( diff --git a/cinder/volume/drivers/netapp/utils.py b/cinder/volume/drivers/netapp/utils.py index c647e53a09d..94f756addf0 100644 --- a/cinder/volume/drivers/netapp/utils.py +++ b/cinder/volume/drivers/netapp/utils.py @@ -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() diff --git a/releasenotes/notes/netapp-non-discovery-19af4e10f7b190ea.yaml b/releasenotes/notes/netapp-non-discovery-19af4e10f7b190ea.yaml new file mode 100644 index 00000000000..6bd34e0b19d --- /dev/null +++ b/releasenotes/notes/netapp-non-discovery-19af4e10f7b190ea.yaml @@ -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.