Delete VLAN on delete_vserver in Netapp cmode

Currently when DHSS=true with a Netapp cmode backend
during vserver creation, lifs and VLANs get created.
But on cleanup everything gets removed except the VLANs.
This patch removes the vlans at the end of vserver deletion.
Netapp prevents deleting VLANs with existing lifs on it.

Change-Id: Id0b56bbce8e5e9b8707f22d401d99c7867372a50
Closes-Bug: #1580163

manila/share/drivers/netapp/dataontap/client/client_cmode.py

@@ -17,6 +17,7 @@
 
 import copy
 import hashlib
+import re
 import time
 
 from oslo_log import log
@@ -522,6 +523,34 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
                 msg_args = {'vlan': vlan, 'port': port, 'err_msg': e.message}
                 raise exception.NetAppException(msg % msg_args)
 
+    @na_utils.trace
+    def delete_vlan(self, node, port, vlan):
+        try:
+            api_args = {
+                'vlan-info': {
+                    'parent-interface': port,
+                    'node': node,
+                    'vlanid': vlan,
+                },
+            }
+            self.send_request('net-vlan-delete', api_args)
+        except netapp_api.NaApiError as e:
+            p = re.compile('port already has a lif bound.*', re.IGNORECASE)
+            if (e.code == netapp_api.EAPIERROR and re.match(p, e.message)):
+                LOG.debug('VLAN %(vlan)s on port %(port)s node %(node)s '
+                          'still used by LIF and cannot be deleted.',
+                          {'vlan': vlan, 'port': port, 'node': node})
+            else:
+                msg = _('Failed to delete VLAN %(vlan)s on '
+                        'port %(port)s node %(node)s: %(err_msg)s')
+                msg_args = {
+                    'vlan': vlan,
+                    'port': port,
+                    'node': node,
+                    'err_msg': e.message
+                }
+                raise exception.NetAppException(msg % msg_args)
+
     @na_utils.trace
     def _ensure_broadcast_domain_for_port(self, node, port,
                                           domain=DEFAULT_BROADCAST_DOMAIN,

manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py

@@ -107,19 +107,26 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
     @na_utils.trace
     def setup_server(self, network_info, metadata=None):
         """Creates and configures new Vserver."""
-        LOG.debug('Creating server %s', network_info['server_id'])
-        self._validate_network_type(network_info)
 
-        vserver_name = self._get_vserver_name(network_info['server_id'])
+        vlan = network_info['segmentation_id']
-        server_details = {'vserver_name': vserver_name}
 
-        try:
+        @utils.synchronized('netapp-VLAN-%s' % vlan, external=True)
-            self._create_vserver(vserver_name, network_info)
+        def setup_server_with_lock():
-        except Exception as e:
+            LOG.debug('Creating server %s', network_info['server_id'])
-            e.detail_data = {'server_details': server_details}
+            self._validate_network_type(network_info)
-            raise
 
-        return server_details
+            vserver_name = self._get_vserver_name(network_info['server_id'])
+            server_details = {'vserver_name': vserver_name}
+
+            try:
+                self._create_vserver(vserver_name, network_info)
+            except Exception as e:
+                e.detail_data = {'server_details': server_details}
+                raise
+
+            return server_details
+
+        return setup_server_with_lock()
 
     @na_utils.trace
     def _validate_network_type(self, network_info):
@@ -311,10 +318,34 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
         ipspace_name = self._client.get_vserver_ipspace(vserver)
 
         vserver_client = self._get_api_client(vserver=vserver)
-        self._client.delete_vserver(vserver,
+        network_interfaces = vserver_client.get_network_interfaces()
-                                    vserver_client,
-                                    security_services=security_services)
 
-        if ipspace_name and not self._client.ipspace_has_data_vservers(
+        home_port = network_interfaces[0]['home-port']
-                ipspace_name):
+        vlan = home_port.split('-')[1]
-            self._client.delete_ipspace(ipspace_name)
+
+        @utils.synchronized('netapp-VLAN-%s' % vlan, external=True)
+        def _delete_vserver_with_lock():
+            self._client.delete_vserver(vserver,
+                                        vserver_client,
+                                        security_services=security_services)
+
+            if ipspace_name and not self._client.ipspace_has_data_vservers(
+                    ipspace_name):
+                self._client.delete_ipspace(ipspace_name)
+
+            self._delete_vserver_vlan(network_interfaces)
+
+        return _delete_vserver_with_lock()
+
+    @na_utils.trace
+    def _delete_vserver_vlan(self, vserver_network_interfaces):
+        """Delete Vserver's VLAN configuration from ports"""
+
+        for interface in vserver_network_interfaces:
+            try:
+                home_port = interface['home-port']
+                port, vlan = home_port.split('-')
+                node = interface['home-node']
+                self._client.delete_vlan(node, port, vlan)
+            except exception.NetAppException:
+                LOG.exception(_LE("Deleting Vserver VLAN failed."))

manila/tests/share/drivers/netapp/dataontap/client/fakes.py

@@ -82,6 +82,15 @@ SM_SOURCE_VOLUME = 'fake_source_volume'
 SM_DEST_VSERVER = 'fake_destination_vserver'
 SM_DEST_VOLUME = 'fake_destination_volume'
 
+NETWORK_INTERFACES = [{
+    'interface_name': 'fake_interface',
+    'address': IP_ADDRESS,
+    'vserver': VSERVER_NAME,
+    'netmask': NETMASK,
+    'role': 'data',
+    'home-node': NODE_NAME,
+    'home-port': VLAN_PORT
+}]
 
 IPSPACES = [{
     'uuid': 'fake_uuid',

manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py

@@ -61,8 +61,9 @@ class NetAppClientCmodeTestCase(test.TestCase):
         self.vserver_client.set_vserver(fake.VSERVER_NAME)
         self.vserver_client.connection = mock.MagicMock()
 
-    def _mock_api_error(self, code='fake'):
+    def _mock_api_error(self, code='fake', message='fake'):
-        return mock.Mock(side_effect=netapp_api.NaApiError(code=code))
+        return mock.Mock(side_effect=netapp_api.NaApiError(code=code,
+                                                           message=message))
 
     def test_init_features_ontapi_1_21(self):
 
@@ -932,6 +933,53 @@ class NetAppClientCmodeTestCase(test.TestCase):
                           fake.PORT,
                           fake.VLAN)
 
+    def test_delete_vlan(self):
+
+        self.mock_object(self.client, 'send_request')
+
+        vlan_delete_args = {
+            'vlan-info': {
+                'parent-interface': fake.PORT,
+                'node': fake.NODE_NAME,
+                'vlanid': fake.VLAN
+            }
+        }
+        self.client.delete_vlan(fake.NODE_NAME, fake.PORT, fake.VLAN)
+
+        self.client.send_request.assert_has_calls([
+            mock.call('net-vlan-delete', vlan_delete_args)])
+
+    def test_delete_vlan_still_used(self):
+
+        self.mock_object(self.client,
+                         'send_request',
+                         self._mock_api_error(code=netapp_api.EAPIERROR,
+                                              message='Port already has a '
+                                              'lif bound. '))
+
+        vlan_delete_args = {
+            'vlan-info': {
+                'parent-interface': fake.PORT,
+                'node': fake.NODE_NAME,
+                'vlanid': fake.VLAN
+            }
+        }
+        self.client.delete_vlan(fake.NODE_NAME, fake.PORT, fake.VLAN)
+
+        self.client.send_request.assert_has_calls([
+            mock.call('net-vlan-delete', vlan_delete_args)])
+        self.assertEqual(1, client_cmode.LOG.debug.call_count)
+
+    def test_delete_vlan_api_error(self):
+
+        self.mock_object(self.client, 'send_request', self._mock_api_error())
+
+        self.assertRaises(exception.NetAppException,
+                          self.client.delete_vlan,
+                          fake.NODE_NAME,
+                          fake.PORT,
+                          fake.VLAN)
+
     def test_ensure_broadcast_domain_for_port_domain_match(self):
 
         port_info = {

manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py

@@ -28,6 +28,7 @@ from manila.share.drivers.netapp.dataontap.cluster_mode import lib_base
 from manila.share.drivers.netapp.dataontap.cluster_mode import lib_multi_svm
 from manila.share.drivers.netapp import utils as na_utils
 from manila import test
+from manila.tests.share.drivers.netapp.dataontap.client import fakes as c_fake
 from manila.tests.share.drivers.netapp.dataontap import fakes as fake
 
 
@@ -649,6 +650,11 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.mock_object(self.library,
                          '_get_api_client',
                          mock.Mock(return_value=vserver_client))
+        mock_delete_vserver_vlan = self.mock_object(self.library,
+                                                    '_delete_vserver_vlan')
+        self.mock_object(vserver_client,
+                         'get_network_interfaces',
+                         mock.Mock(return_value=c_fake.NETWORK_INTERFACES))
         security_services = fake.NETWORK_INFO['security_services']
 
         self.library._delete_vserver(fake.VSERVER1,
@@ -659,6 +665,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.library._client.delete_vserver.assert_called_once_with(
             fake.VSERVER1, vserver_client, security_services=security_services)
         self.assertFalse(self.library._client.delete_ipspace.called)
+        mock_delete_vserver_vlan.assert_called_once_with(
+            c_fake.NETWORK_INTERFACES)
 
     def test_delete_vserver_ipspace_has_data_vservers(self):
 
@@ -672,6 +680,11 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.mock_object(self.library._client,
                          'ipspace_has_data_vservers',
                          mock.Mock(return_value=True))
+        mock_delete_vserver_vlan = self.mock_object(self.library,
+                                                    '_delete_vserver_vlan')
+        self.mock_object(vserver_client,
+                         'get_network_interfaces',
+                         mock.Mock(return_value=c_fake.NETWORK_INTERFACES))
         security_services = fake.NETWORK_INFO['security_services']
 
         self.library._delete_vserver(fake.VSERVER1,
@@ -682,6 +695,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.library._client.delete_vserver.assert_called_once_with(
             fake.VSERVER1, vserver_client, security_services=security_services)
         self.assertFalse(self.library._client.delete_ipspace.called)
+        mock_delete_vserver_vlan.assert_called_once_with(
+            c_fake.NETWORK_INTERFACES)
 
     def test_delete_vserver_with_ipspace(self):
 
@@ -695,6 +710,12 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.mock_object(self.library._client,
                          'ipspace_has_data_vservers',
                          mock.Mock(return_value=False))
+        mock_delete_vserver_vlan = self.mock_object(self.library,
+                                                    '_delete_vserver_vlan')
+        self.mock_object(vserver_client,
+                         'get_network_interfaces',
+                         mock.Mock(return_value=c_fake.NETWORK_INTERFACES))
+
         security_services = fake.NETWORK_INFO['security_services']
 
         self.library._delete_vserver(fake.VSERVER1,
@@ -706,3 +727,32 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
             fake.VSERVER1, vserver_client, security_services=security_services)
         self.library._client.delete_ipspace.assert_called_once_with(
             fake.IPSPACE)
+        mock_delete_vserver_vlan.assert_called_once_with(
+            c_fake.NETWORK_INTERFACES)
+
+    def test_delete_vserver_vlan(self):
+
+        self.library._delete_vserver_vlan(c_fake.NETWORK_INTERFACES)
+        for interface in c_fake.NETWORK_INTERFACES:
+            home_port = interface['home-port']
+            port, vlan = home_port.split('-')
+            node = interface['home-node']
+            self.library._client.delete_vlan.assert_called_once_with(
+                node, port, vlan)
+
+    def test_delete_vserver_vlan_client_error(self):
+
+        mock_exception_log = self.mock_object(lib_multi_svm.LOG, 'exception')
+        self.mock_object(
+            self.library._client,
+            'delete_vlan',
+            mock.Mock(side_effect=exception.NetAppException("fake error")))
+
+        self.library._delete_vserver_vlan(c_fake.NETWORK_INTERFACES)
+        for interface in c_fake.NETWORK_INTERFACES:
+            home_port = interface['home-port']
+            port, vlan = home_port.split('-')
+            node = interface['home-node']
+            self.library._client.delete_vlan.assert_called_once_with(
+                node, port, vlan)
+            self.assertEqual(1, mock_exception_log.call_count)

manila/tests/share/drivers/netapp/dataontap/fakes.py

@@ -247,6 +247,7 @@ NETWORK_INFO = {
     'network_allocations': USER_NETWORK_ALLOCATIONS,
     'admin_network_allocations': ADMIN_NETWORK_ALLOCATIONS,
     'neutron_subnet_id': '62bf1c2c-18eb-421b-8983-48a6d39aafe0',
+    'segmentation_id': '1000',
 }
 NETWORK_INFO_NETMASK = '255.255.255.0'
 

releasenotes/notes/delete_vlan_on_vserver_delete-a7acd145c0b8236d.yaml

@@ -0,0 +1,3 @@
+---
+features:
+  - NetApp cMode driver - configured VLAN will be deleted on Vserver removal