From 2d4e30bd2141cf4d6494f34471685db1da76fe77 Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Tue, 26 Jul 2016 20:56:41 -0400 Subject: [PATCH] NetApp cDOT: Apply network MTU to VLAN ports Through change I9b4efae620ec9f6790547c8fffc58872d43277f5 we added support in manila to request the Maximum Transmission Unit (MTU) from the network provider (neutron, standalone, etc.). To honor this MTU, the NetApp cDOT driver needs to apply it to the VLAN ports (via the broadcast domain) that the LIFs of the newly created share servers are assigned to. Co-Authored-By: Goutham Pacha Ravi Depends-On: I9b4efae620ec9f6790547c8fffc58872d43277f5 Change-Id: Ia01d982c7328feff292d54588d33a16df705f81e Implements: blueprint netapp-cdot-honor-mtu-size --- .../netapp/dataontap/client/client_cmode.py | 26 ++++-- .../dataontap/cluster_mode/lib_multi_svm.py | 5 +- .../dataontap/client/test_client_cmode.py | 80 ++++++++++++------- .../cluster_mode/test_lib_multi_svm.py | 18 ++++- .../share/drivers/netapp/dataontap/fakes.py | 5 ++ ...rom-network-provider-d12179a2374cdda0.yaml | 6 ++ 6 files changed, 103 insertions(+), 37 deletions(-) create mode 100644 releasenotes/notes/netapp-cdot-apply-mtu-from-network-provider-d12179a2374cdda0.yaml diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 42631c2426..b7e5800cd2 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -473,7 +473,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): @na_utils.trace def create_network_interface(self, ip, netmask, vlan, node, port, - vserver_name, lif_name, ipspace_name): + vserver_name, lif_name, ipspace_name, mtu): """Creates LIF on VLAN port.""" home_port_name = port @@ -482,8 +482,8 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): home_port_name = '%(port)s-%(tag)s' % {'port': port, 'tag': vlan} if self.features.BROADCAST_DOMAINS: - self._ensure_broadcast_domain_for_port(node, home_port_name, - ipspace=ipspace_name) + self._ensure_broadcast_domain_for_port( + node, home_port_name, mtu, ipspace=ipspace_name) LOG.debug('Creating LIF %(lif)s for Vserver %(vserver)s ', {'lif': lif_name, 'vserver': vserver_name}) @@ -554,7 +554,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): raise exception.NetAppException(msg % msg_args) @na_utils.trace - def _ensure_broadcast_domain_for_port(self, node, port, + def _ensure_broadcast_domain_for_port(self, node, port, mtu, domain=DEFAULT_BROADCAST_DOMAIN, ipspace=DEFAULT_IPSPACE): """Ensure a port is in a broadcast domain. Create one if necessary. @@ -572,6 +572,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): # Port already in desired ipspace and broadcast domain. if (port_info['ipspace'] == ipspace and port_info['broadcast-domain'] == domain): + self._modify_broadcast_domain(domain, ipspace, mtu) return # If in another broadcast domain, remove port from it. @@ -582,7 +583,9 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): # If desired broadcast domain doesn't exist, create it. if not self._broadcast_domain_exists(domain, ipspace): - self._create_broadcast_domain(domain, ipspace) + self._create_broadcast_domain(domain, ipspace, mtu) + else: + self._modify_broadcast_domain(domain, ipspace, mtu) # Move the port into the broadcast domain where it is needed. self._add_port_to_broadcast_domain(node, port, domain, ipspace) @@ -640,7 +643,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): return self._has_records(result) @na_utils.trace - def _create_broadcast_domain(self, domain, ipspace, mtu=1500): + def _create_broadcast_domain(self, domain, ipspace, mtu): """Create a broadcast domain.""" api_args = { 'ipspace': ipspace, @@ -649,6 +652,16 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): } self.send_request('net-port-broadcast-domain-create', api_args) + @na_utils.trace + def _modify_broadcast_domain(self, domain, ipspace, mtu): + """Modify a broadcast domain.""" + api_args = { + 'ipspace': ipspace, + 'broadcast-domain': domain, + 'mtu': mtu, + } + self.send_request('net-port-broadcast-domain-modify', api_args) + @na_utils.trace def _delete_broadcast_domain(self, domain, ipspace): """Delete a broadcast domain.""" @@ -658,6 +671,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): } self.send_request('net-port-broadcast-domain-destroy', api_args) + @na_utils.trace def _delete_broadcast_domains_for_ipspace(self, ipspace_name): """Deletes all broadcast domains in an IPspace.""" ipspaces = self.get_ipspaces(ipspace_name=ipspace_name) 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 d266ac132c..80e7daafd3 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 @@ -36,6 +36,7 @@ from manila import utils LOG = log.getLogger(__name__) SUPPORTED_NETWORK_TYPES = (None, 'flat', 'vlan') SEGMENTED_NETWORK_TYPES = ('vlan',) +DEFAULT_MTU = 1500 class NetAppCmodeMultiSVMFileStorageLibrary( @@ -273,13 +274,15 @@ class NetAppCmodeMultiSVMFileStorageLibrary( ip_address = network_allocation['ip_address'] netmask = utils.cidr_to_netmask(network_allocation['cidr']) vlan = network_allocation['segmentation_id'] + network_mtu = network_allocation.get('mtu') + mtu = network_mtu or DEFAULT_MTU if not vserver_client.network_interface_exists( vserver_name, node_name, port, ip_address, netmask, vlan): self._client.create_network_interface( ip_address, netmask, vlan, node_name, port, vserver_name, - lif_name, ipspace_name) + lif_name, ipspace_name, mtu) @na_utils.trace def get_network_allocations_number(self): 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 61dc515b44..aeeb400608 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 @@ -869,7 +869,8 @@ class NetAppClientCmodeTestCase(test.TestCase): fake.PORT, fake.VSERVER_NAME, fake.LIF_NAME, - fake.IPSPACE_NAME) + fake.IPSPACE_NAME, + fake.MTU) if use_vlans: self.client._create_vlan.assert_called_with( @@ -880,7 +881,7 @@ class NetAppClientCmodeTestCase(test.TestCase): if broadcast_domains_supported: self.client._ensure_broadcast_domain_for_port.assert_called_with( fake.NODE_NAME, fake.VLAN_PORT if use_vlans else fake.PORT, - ipspace=fake.IPSPACE_NAME) + fake.MTU, ipspace=fake.IPSPACE_NAME) else: self.assertFalse( self.client._ensure_broadcast_domain_for_port.called) @@ -993,14 +994,17 @@ class NetAppClientCmodeTestCase(test.TestCase): '_broadcast_domain_exists', mock.Mock(return_value=True)) self.mock_object(self.client, '_create_broadcast_domain') + self.mock_object(self.client, '_modify_broadcast_domain') self.mock_object(self.client, '_add_port_to_broadcast_domain') self.client._ensure_broadcast_domain_for_port( - fake.NODE_NAME, fake.PORT, domain=fake.BROADCAST_DOMAIN, + fake.NODE_NAME, fake.PORT, fake.MTU, domain=fake.BROADCAST_DOMAIN, ipspace=fake.IPSPACE_NAME) - self.client._get_broadcast_domain_for_port.assert_has_calls([ - mock.call(fake.NODE_NAME, fake.PORT)]) + self.client._get_broadcast_domain_for_port.assert_called_once_with( + fake.NODE_NAME, fake.PORT) + self.client._modify_broadcast_domain.assert_called_once_with( + fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME, fake.MTU) self.assertFalse(self.client._broadcast_domain_exists.called) self.assertFalse(self.client._create_broadcast_domain.called) self.assertFalse(self.client._add_port_to_broadcast_domain.called) @@ -1018,24 +1022,26 @@ class NetAppClientCmodeTestCase(test.TestCase): '_broadcast_domain_exists', mock.Mock(return_value=True)) self.mock_object(self.client, '_create_broadcast_domain') + self.mock_object(self.client, '_modify_broadcast_domain') self.mock_object(self.client, '_remove_port_from_broadcast_domain') self.mock_object(self.client, '_add_port_to_broadcast_domain') self.client._ensure_broadcast_domain_for_port( fake.NODE_NAME, fake.PORT, domain=fake.BROADCAST_DOMAIN, - ipspace=fake.IPSPACE_NAME) + ipspace=fake.IPSPACE_NAME, mtu=fake.MTU) - self.client._get_broadcast_domain_for_port.assert_has_calls([ - mock.call(fake.NODE_NAME, fake.PORT)]) - self.client._remove_port_from_broadcast_domain.assert_has_calls([ - mock.call(fake.NODE_NAME, fake.PORT, 'other_domain', - fake.IPSPACE_NAME)]) - self.client._broadcast_domain_exists.assert_has_calls([ - mock.call(fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME)]) + self.client._get_broadcast_domain_for_port.assert_called_once_with( + fake.NODE_NAME, fake.PORT) + self.client._remove_port_from_broadcast_domain.assert_called_once_with( + fake.NODE_NAME, fake.PORT, 'other_domain', fake.IPSPACE_NAME) + self.client._broadcast_domain_exists.assert_called_once_with( + fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME) self.assertFalse(self.client._create_broadcast_domain.called) - self.client._add_port_to_broadcast_domain.assert_has_calls([ - mock.call(fake.NODE_NAME, fake.PORT, fake.BROADCAST_DOMAIN, - fake.IPSPACE_NAME)]) + self.client._modify_broadcast_domain.assert_called_once_with( + fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME, fake.MTU) + self.client._add_port_to_broadcast_domain.assert_called_once_with( + fake.NODE_NAME, fake.PORT, fake.BROADCAST_DOMAIN, + fake.IPSPACE_NAME) def test_ensure_broadcast_domain_for_port_no_domain(self): @@ -1050,23 +1056,25 @@ class NetAppClientCmodeTestCase(test.TestCase): '_broadcast_domain_exists', mock.Mock(return_value=False)) self.mock_object(self.client, '_create_broadcast_domain') + self.mock_object(self.client, '_modify_broadcast_domain') self.mock_object(self.client, '_remove_port_from_broadcast_domain') self.mock_object(self.client, '_add_port_to_broadcast_domain') self.client._ensure_broadcast_domain_for_port( fake.NODE_NAME, fake.PORT, domain=fake.BROADCAST_DOMAIN, - ipspace=fake.IPSPACE_NAME) + ipspace=fake.IPSPACE_NAME, mtu=fake.MTU) - self.client._get_broadcast_domain_for_port.assert_has_calls([ - mock.call(fake.NODE_NAME, fake.PORT)]) + self.client._get_broadcast_domain_for_port.assert_called_once_with( + fake.NODE_NAME, fake.PORT) self.assertFalse(self.client._remove_port_from_broadcast_domain.called) - self.client._broadcast_domain_exists.assert_has_calls([ - mock.call(fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME)]) - self.client._create_broadcast_domain.assert_has_calls([ - mock.call(fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME)]) - self.client._add_port_to_broadcast_domain.assert_has_calls([ - mock.call(fake.NODE_NAME, fake.PORT, fake.BROADCAST_DOMAIN, - fake.IPSPACE_NAME)]) + self.client._broadcast_domain_exists.assert_called_once_with( + fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME) + self.client._create_broadcast_domain.assert_called_once_with( + fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME, fake.MTU) + self.assertFalse(self.client._modify_broadcast_domain.called) + self.client._add_port_to_broadcast_domain.assert_called_once_with( + fake.NODE_NAME, fake.PORT, fake.BROADCAST_DOMAIN, + fake.IPSPACE_NAME) def test_get_broadcast_domain_for_port(self): @@ -1177,7 +1185,7 @@ class NetAppClientCmodeTestCase(test.TestCase): result = self.client._create_broadcast_domain(fake.BROADCAST_DOMAIN, fake.IPSPACE_NAME, - mtu=fake.MTU) + fake.MTU) net_port_broadcast_domain_create_args = { 'ipspace': fake.IPSPACE_NAME, @@ -1189,6 +1197,24 @@ class NetAppClientCmodeTestCase(test.TestCase): mock.call('net-port-broadcast-domain-create', net_port_broadcast_domain_create_args)]) + def test_modify_broadcast_domain(self): + + self.mock_object(self.client, 'send_request') + + result = self.client._modify_broadcast_domain(fake.BROADCAST_DOMAIN, + fake.IPSPACE_NAME, + fake.MTU) + + net_port_broadcast_domain_modify_args = { + 'ipspace': fake.IPSPACE_NAME, + 'broadcast-domain': fake.BROADCAST_DOMAIN, + 'mtu': fake.MTU, + } + self.assertIsNone(result) + self.client.send_request.assert_called_once_with( + 'net-port-broadcast-domain-modify', + net_port_broadcast_domain_modify_args) + def test_delete_broadcast_domain(self): self.mock_object(self.client, 'send_request') 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 ea61bbb7ef..2cddbd5a44 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 @@ -538,7 +538,19 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.assertEqual('os_132dbb10-9a36-46f2-8d89-3d909830c356', result) - def test_create_lif(self): + @ddt.data(fake.MTU, None, 'not-present') + def test_create_lif(self, mtu): + """Tests cases where MTU is a valid value, None or not present.""" + + expected_mtu = (mtu if mtu not in (None, 'not-present') else + fake.DEFAULT_MTU) + + network_allocations = copy.deepcopy( + fake.NETWORK_INFO['network_allocations'][0]) + network_allocations['mtu'] = mtu + + if mtu == 'not-present': + network_allocations.pop('mtu') vserver_client = mock.Mock() vserver_client.network_interface_exists = mock.Mock( @@ -552,12 +564,12 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 'fake_ipspace', 'fake_node', 'fake_lif', - fake.NETWORK_INFO['network_allocations'][0]) + network_allocations) self.library._client.create_network_interface.assert_has_calls([ mock.call('10.10.10.10', '255.255.255.0', '1000', 'fake_node', 'fake_port', 'fake_vserver', 'fake_lif', - 'fake_ipspace')]) + 'fake_ipspace', expected_mtu)]) def test_create_lif_if_nonexistent_already_present(self): diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index faf76acb1f..39b11b4a21 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -65,6 +65,8 @@ SHARE_TYPE_ID = '26e89a5b-960b-46bb-a8cf-0778e653098f' SHARE_TYPE_NAME = 'fake_share_type' IPSPACE = 'fake_ipspace' IPSPACE_ID = '27d38c27-3e8b-4d7d-9d91-fcf295e3ac8f' +MTU = 1234 +DEFAULT_MTU = 1500 CLIENT_KWARGS = { 'username': 'admin', @@ -219,6 +221,7 @@ USER_NETWORK_ALLOCATIONS = [ 'segmentation_id': '1000', 'network_type': 'vlan', 'label': 'user', + 'mtu': MTU, }, { 'id': '7eabdeed-bad2-46ea-bd0f-a33884c869e0', @@ -227,6 +230,7 @@ USER_NETWORK_ALLOCATIONS = [ 'segmentation_id': '1000', 'network_type': 'vlan', 'label': 'user', + 'mtu': MTU, } ] @@ -238,6 +242,7 @@ ADMIN_NETWORK_ALLOCATIONS = [ 'segmentation_id': None, 'network_type': 'flat', 'label': 'admin', + 'mtu': MTU, }, ] diff --git a/releasenotes/notes/netapp-cdot-apply-mtu-from-network-provider-d12179a2374cdda0.yaml b/releasenotes/notes/netapp-cdot-apply-mtu-from-network-provider-d12179a2374cdda0.yaml new file mode 100644 index 0000000000..c89888705f --- /dev/null +++ b/releasenotes/notes/netapp-cdot-apply-mtu-from-network-provider-d12179a2374cdda0.yaml @@ -0,0 +1,6 @@ +--- +features: + - The NetApp cDOT driver operating in + ``driver_handles_share_servers = True`` mode applies the Maximum + Transmission Unit (MTU) from the network provider where available when + creating Logical Interfaces (LIFs) for newly created share servers.