From 2d4e30bd2141cf4d6494f34471685db1da76fe77 Mon Sep 17 00:00:00 2001
From: Clinton Knight <cknight@netapp.com>
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 <gouthamr@netapp.com>
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.