diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 73a275ca..7cea75e6 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -252,26 +252,68 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): in nodes_info_list.get_children()] @na_utils.trace - def get_node_data_port(self, node): - """Get data port on the node.""" + def list_node_data_ports(self, node): + ports = self.get_node_data_ports(node) + return [port.get('port') for port in ports] + + @na_utils.trace + def get_node_data_ports(self, node): + """Get applicable data ports on the node.""" api_args = { 'query': { 'net-port-info': { 'node': node, - 'port-type': 'physical', + 'link-status': 'up', + 'port-type': 'physical|if_group', 'role': 'data', }, }, + 'desired-attributes': { + 'node-details-info': { + 'port': None, + 'node': None, + 'operational-speed': None, + 'ifgrp-port': None, + }, + }, } - port_info = self.send_request('net-port-get-iter', api_args) - try: - port = port_info.get_child_by_name( - 'attributes-list').get_child_by_name( - 'net-port-info').get_child_content('port') - except AttributeError: - msg = _("Data port does not exist for node %s.") - raise exception.NetAppException(msg % node) - return port + result = self.send_request('net-port-get-iter', api_args) + net_port_info_list = result.get_child_by_name( + 'attributes-list') or netapp_api.NaElement('none') + + ports = [] + for port_info in net_port_info_list.get_children(): + + # Skip physical ports that are part of interface groups. + if port_info.get_child_content('ifgrp-port'): + continue + + port = { + 'node': port_info.get_child_content('node'), + 'port': port_info.get_child_content('port'), + 'speed': port_info.get_child_content('operational-speed'), + } + ports.append(port) + + return self._sort_data_ports_by_speed(ports) + + @na_utils.trace + def _sort_data_ports_by_speed(self, ports): + + def sort_key(port): + value = port.get('speed') + if not (value and isinstance(value, six.string_types)): + return 0 + elif value.isdigit(): + return int(value) + elif value == 'auto': + return 3 + elif value == 'undef': + return 2 + else: + return 1 + + return sorted(ports, key=sort_key, reverse=True) @na_utils.trace def list_aggregates(self): diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py index 95441af3..31996ca6 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py @@ -28,7 +28,6 @@ from oslo_utils import excutils from manila import context from manila import exception from manila.i18n import _, _LE, _LW -from manila.share.drivers.netapp.dataontap.client import api as netapp_api from manila.share.drivers.netapp.dataontap.cluster_mode import lib_base from manila.share.drivers.netapp import utils as na_utils from manila import utils @@ -121,7 +120,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary( self._create_vserver_lifs(vserver_name, vserver_client, network_info) - except netapp_api.NaApiError: + except Exception: with excutils.save_and_reraise_exception(): LOG.error(_LE("Failed to create network interface(s).")) self._client.delete_vserver(vserver_name, vserver_client) @@ -145,7 +144,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary( for node, net_info in node_network_info: net_id = net_info['id'] - port = self._client.get_node_data_port(node) + port = self._get_node_data_port(node) ip = net_info['ip_address'] self._create_lif_if_nonexistent(vserver_name, net_id, @@ -156,6 +155,18 @@ class NetAppCmodeMultiSVMFileStorageLibrary( netmask, vserver_client) + @na_utils.trace + def _get_node_data_port(self, node): + port_names = self._client.list_node_data_ports(node) + pattern = self.configuration.netapp_port_name_search_pattern + matched_port_names = [port_name for port_name in port_names + if re.match(pattern, port_name)] + if not matched_port_names: + raise exception.NetAppException( + _('Could not find eligible network ports on node %s on which ' + 'to create Vserver LIFs.') % node) + return matched_port_names[0] + @na_utils.trace def _create_lif_if_nonexistent(self, vserver_name, allocation_id, vlan, node, port, ip, netmask, vserver_client): diff --git a/manila/share/drivers/netapp/options.py b/manila/share/drivers/netapp/options.py index ff17182c..d857658b 100644 --- a/manila/share/drivers/netapp/options.py +++ b/manila/share/drivers/netapp/options.py @@ -69,6 +69,10 @@ netapp_provisioning_opts = [ cfg.StrOpt('netapp_vserver_name_template', default='os_%s', help='Name template to use for new Vserver.'), + cfg.StrOpt('netapp_port_name_search_pattern', + default='(.*)', + help='Pattern for overriding the selection of network ports ' + 'on which to create Vserver LIFs.'), cfg.StrOpt('netapp_lif_name_template', default='os_%(net_allocation_id)s', help='Logical interface (LIF) name template'), diff --git a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py index 3f6c2b94..beb5aa3a 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py @@ -327,7 +327,7 @@ NET_PORT_GET_ITER_RESPONSE = etree.XML(""" %(node_name)s full none - 1000 + 10 e0a physical data @@ -345,7 +345,7 @@ NET_PORT_GET_ITER_RESPONSE = etree.XML(""" %(node_name)s full none - 1000 + 100 e0b physical data @@ -381,7 +381,7 @@ NET_PORT_GET_ITER_RESPONSE = etree.XML(""" %(node_name)s full none - 1000 + 10000 e0d physical data @@ -391,7 +391,34 @@ NET_PORT_GET_ITER_RESPONSE = etree.XML(""" """ % {'node_name': NODE_NAME}) -PORTS = ('e0a', 'e0b', 'e0c', 'e0d') +SPEED_SORTED_PORTS = ( + {'node': NODE_NAME, 'port': 'e0d', 'speed': '10000'}, + {'node': NODE_NAME, 'port': 'e0c', 'speed': '1000'}, + {'node': NODE_NAME, 'port': 'e0b', 'speed': '100'}, + {'node': NODE_NAME, 'port': 'e0a', 'speed': '10'}, +) +PORT_NAMES = ('e0a', 'e0b', 'e0c', 'e0d') +SPEED_SORTED_PORT_NAMES = ('e0d', 'e0c', 'e0b', 'e0a') + +UNSORTED_PORTS_ALL_SPEEDS = ( + {'node': NODE_NAME, 'port': 'port6', 'speed': 'undef'}, + {'node': NODE_NAME, 'port': 'port3', 'speed': '100'}, + {'node': NODE_NAME, 'port': 'port1', 'speed': '10000'}, + {'node': NODE_NAME, 'port': 'port4', 'speed': '10'}, + {'node': NODE_NAME, 'port': 'port7'}, + {'node': NODE_NAME, 'port': 'port2', 'speed': '1000'}, + {'node': NODE_NAME, 'port': 'port5', 'speed': 'auto'}, +) + +SORTED_PORTS_ALL_SPEEDS = ( + {'node': NODE_NAME, 'port': 'port1', 'speed': '10000'}, + {'node': NODE_NAME, 'port': 'port2', 'speed': '1000'}, + {'node': NODE_NAME, 'port': 'port3', 'speed': '100'}, + {'node': NODE_NAME, 'port': 'port4', 'speed': '10'}, + {'node': NODE_NAME, 'port': 'port5', 'speed': 'auto'}, + {'node': NODE_NAME, 'port': 'port6', 'speed': 'undef'}, + {'node': NODE_NAME, 'port': 'port7'}, +) NET_INTERFACE_GET_ITER_RESPONSE = etree.XML(""" diff --git a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py index 056ed4e9..da4523e8 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py @@ -455,27 +455,65 @@ class NetAppClientCmodeTestCase(test.TestCase): self.assertListEqual([], result) - def test_get_node_data_port(self): + def test_list_node_data_ports(self): + + self.mock_object(self.client, + 'get_node_data_ports', + mock.Mock(return_value=fake.SPEED_SORTED_PORTS)) + + result = self.client.list_node_data_ports(fake.NODE_NAME) + + self.assertSequenceEqual(fake.SPEED_SORTED_PORT_NAMES, result) + + def test_get_node_data_ports(self): api_response = netapp_api.NaElement(fake.NET_PORT_GET_ITER_RESPONSE) self.mock_object(self.client, 'send_request', mock.Mock(return_value=api_response)) - result = self.client.get_node_data_port(fake.NODE_NAME) + result = self.client.get_node_data_ports(fake.NODE_NAME) - self.assertEqual(fake.PORTS[0], result) + net_port_get_iter_args = { + 'query': { + 'net-port-info': { + 'node': fake.NODE_NAME, + 'link-status': 'up', + 'port-type': 'physical|if_group', + 'role': 'data', + }, + }, + 'desired-attributes': { + 'node-details-info': { + 'port': None, + 'node': None, + 'operational-speed': None, + 'ifgrp-port': None, + }, + }, + } - def test_get_node_data_port_not_found(self): + self.assertSequenceEqual(fake.SPEED_SORTED_PORTS, result) + self.client.send_request.assert_has_calls([ + mock.call('net-port-get-iter', net_port_get_iter_args)]) + + def test_get_node_data_ports_not_found(self): api_response = netapp_api.NaElement(fake.NO_RECORDS_RESPONSE) self.mock_object(self.client, 'send_request', mock.Mock(return_value=api_response)) - self.assertRaises(exception.NetAppException, - self.client.get_node_data_port, - fake.NODE_NAME) + result = self.client.get_node_data_ports(fake.NODE_NAME) + + self.assertSequenceEqual([], result) + + def test_sort_data_ports_by_speed(self): + + result = self.client._sort_data_ports_by_speed( + fake.UNSORTED_PORTS_ALL_SPEEDS) + + self.assertSequenceEqual(fake.SORTED_PORTS_ALL_SPEEDS, result) def test_list_aggregates(self): diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py index 4499962e..abec1268 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py @@ -17,6 +17,7 @@ Unit tests for the NetApp Data ONTAP cDOT multi-SVM storage driver library. import copy +import ddt import mock from oslo_log import log @@ -30,6 +31,7 @@ from manila import test from manila.tests.share.drivers.netapp.dataontap import fakes as fake +@ddt.ddt class NetAppFileStorageLibraryTestCase(test.TestCase): def setUp(self): @@ -237,7 +239,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): vserver_id, {'vserver_name': vserver_name}) - def test_create_vserver_if_nonexistent_lif_creation_failure(self): + @ddt.data(netapp_api.NaApiError, exception.NetAppException) + def test_create_vserver_if_nonexistent_lif_creation_failure(self, + lif_exception): vserver_id = fake.NETWORK_INFO['server_id'] vserver_name = fake.VSERVER_NAME_TEMPLATE % vserver_id @@ -257,9 +261,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock.Mock(return_value=fake.AGGREGATES)) self.mock_object(self.library, '_create_vserver_lifs', - mock.Mock(side_effect=netapp_api.NaApiError)) + mock.Mock(side_effect=lif_exception)) - self.assertRaises(netapp_api.NaApiError, + self.assertRaises(lif_exception, self.library._create_vserver_if_nonexistent, fake.NETWORK_INFO) @@ -284,8 +288,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object(self.library._client, 'list_cluster_nodes', mock.Mock(return_value=fake.CLUSTER_NODES)) - self.mock_object(self.library._client, - 'get_node_data_port', + self.mock_object(self.library, + '_get_node_data_port', mock.Mock(return_value=fake.NODE_DATA_PORT)) self.mock_object(self.library, '_create_lif_if_nonexistent') @@ -313,6 +317,30 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): fake.NETWORK_INFO_NETMASK, 'fake_vserver_client')]) + def test_get_node_data_port(self): + + self.mock_object(self.client, + 'list_node_data_ports', + mock.Mock(return_value=fake.NODE_DATA_PORTS)) + self.library.configuration.netapp_port_name_search_pattern = 'e0c' + + result = self.library._get_node_data_port(fake.CLUSTER_NODE) + + self.assertEqual('e0c', result) + self.library._client.list_node_data_ports.assert_has_calls([ + mock.call(fake.CLUSTER_NODE)]) + + def test_get_node_data_port_no_match(self): + + self.mock_object(self.client, + 'list_node_data_ports', + mock.Mock(return_value=fake.NODE_DATA_PORTS)) + self.library.configuration.netapp_port_name_search_pattern = 'ifgroup1' + + self.assertRaises(exception.NetAppException, + self.library._get_node_data_port, + fake.CLUSTER_NODE) + def test_create_lif_if_nonexistent(self): vserver_client = mock.Mock() diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index 58309c49..9609fec7 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -41,8 +41,10 @@ TOTAL_CAPACITY = 20000000000 AGGREGATES = ('manila_aggr_1', 'manila_aggr_2') ROOT_VOLUME_AGGREGATE = 'manila1' ROOT_VOLUME = 'root' +CLUSTER_NODE = 'cluster1_01' CLUSTER_NODES = ('cluster1_01', 'cluster1_02') NODE_DATA_PORT = 'e0c' +NODE_DATA_PORTS = ('e0c', 'e0d') LIF_NAME_TEMPLATE = 'os_%(net_allocation_id)s' SHARE_TYPE_ID = '26e89a5b-960b-46bb-a8cf-0778e653098f' SHARE_TYPE_NAME = 'fake_share_type' diff --git a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py index 5d48241a..f281fd56 100644 --- a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py @@ -15,6 +15,7 @@ Mock unit tests for the NetApp driver protocols CIFS class module. """ +import ddt import mock from oslo_log import log @@ -26,6 +27,7 @@ from manila.tests.share.drivers.netapp.dataontap.protocols \ import fakes as fake +@ddt.ddt class NetAppClusteredCIFSHelperTestCase(test.TestCase): def setUp(self): @@ -169,28 +171,26 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase): target = self.helper.get_target({'export_location': ''}) self.assertEqual('', target) - def test_get_export_location(self): + @ddt.data( + { + 'location': r'\\%s\%s' % (fake.SHARE_ADDRESS_1, fake.SHARE_NAME), + 'ip': fake.SHARE_ADDRESS_1, + 'share_name': fake.SHARE_NAME, + }, { + 'location': r'//%s/%s' % (fake.SHARE_ADDRESS_1, fake.SHARE_NAME), + 'ip': fake.SHARE_ADDRESS_1, + 'share_name': fake.SHARE_NAME, + }, + {'location': '', 'ip': '', 'share_name': ''}, + {'location': 'invalid', 'ip': '', 'share_name': ''}, + ) + @ddt.unpack + def test_get_export_location(self, location, ip, share_name): - host_ip, share_name = self.helper._get_export_location(fake.CIFS_SHARE) - self.assertEqual(fake.SHARE_ADDRESS_1, host_ip) - self.assertEqual(fake.SHARE_NAME, share_name) + share = fake.CIFS_SHARE.copy() + share['export_location'] = location - def test_get_export_location_legacy_forward_slashes(self): + result_ip, result_share_name = self.helper._get_export_location(share) - fake_share = fake.CIFS_SHARE.copy() - fake_share['export_location'] = fake_share['export_location'].replace( - '\\', '/') - - host_ip, share_name = self.helper._get_export_location(fake_share) - self.assertEqual(fake.SHARE_ADDRESS_1, host_ip) - self.assertEqual(fake.SHARE_NAME, share_name) - - def test_get_export_location_missing_location(self): - - fake_share = fake.CIFS_SHARE.copy() - fake_share['export_location'] = '' - - host_ip, share_name = self.helper._get_export_location(fake_share) - - self.assertEqual('', host_ip) - self.assertEqual('', share_name) \ No newline at end of file + self.assertEqual(ip, result_ip) + self.assertEqual(share_name, result_share_name)