Fix leak of ports on share server deletion

- Manila share server deletion happens asynchronously. If deletion of
share server fails in between, share server network ports remained as
it is. So we should better delete share network ports first and then
continue share server deletion.
- It may be possible, that there are ports existing without a
corresponding manila network allocation entry in the manila db, because
port create request may have been successfully sent to neutron, but not
stored in db. So query(and delete) those from neutron after db entries
are deleted.

Closes-bug: #2067266
Change-Id: Id86dade1194494e599aea9adad06e4ca6cb119b6

manila/network/__init__.py

@@ -107,7 +107,8 @@ class NetworkBaseAPI(db_base.Base, metaclass=abc.ABCMeta):
         pass
 
     @abc.abstractmethod
-    def deallocate_network(self, context, share_server_id):
+    def deallocate_network(self, context, share_server_id, share_network=None,
+                           share_network_subnet=None):
         pass
 
     @abc.abstractmethod

manila/network/neutron/neutron_network_plugin.py

@@ -296,7 +296,8 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI):
                                                             ip_version}
         raise exception.NetworkBadConfigurationException(reason=msg)
 
-    def deallocate_network(self, context, share_server_id):
+    def deallocate_network(self, context, share_server_id,
+                           share_network=None, share_network_subnet=None):
         """Deallocate neutron network resources for the given share server.
 
         Delete previously allocated neutron ports, delete manila db
@@ -304,6 +305,8 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI):
 
         :param context: RequestContext object
         :param share_server_id: id of share server
+        :param share_network: share network data
+        :param share_network_subnet: share network subnet data
         :rtype: None
         """
         ports = self.db.network_allocations_get_for_share_server(
@@ -312,6 +315,24 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI):
         for port in ports:
             self._delete_port(context, port)
 
+        # It may be possible that there are ports existing without a
+        # corresponding manila network allocation entry in the manila db,
+        # because port create request may have been successfully sent to
+        # neutron, but the response, the created port could not be stored
+        # in manila due to unreachable db
+        if share_network_subnet:
+            ports = []
+            try:
+                ports = self.neutron_api.list_ports(
+                    network_id=share_network_subnet['neutron_net_id'],
+                    device_owner='manila:share',
+                    device_id=share_server_id)
+            except exception.NetworkException:
+                LOG.warning("Failed to list ports using neutron API during "
+                            "deallocate_network.")
+            for port in ports:
+                self._delete_port(context, port, ignore_db=True)
+
     def _get_port_create_args(self, share_server, share_network_subnet,
                               device_owner, count=0):
         return {
@@ -354,7 +375,7 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI):
 
         return self.db.network_allocation_create(context, port_dict)
 
-    def _delete_port(self, context, port):
+    def _delete_port(self, context, port, ignore_db=False):
         try:
             self.neutron_api.delete_port(port['id'])
         except exception.NetworkException:
@@ -362,7 +383,8 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI):
                 context, port['id'], {'status': constants.STATUS_ERROR})
             raise
         else:
-            self.db.network_allocation_delete(context, port['id'])
+            if not ignore_db:
+                self.db.network_allocation_delete(context, port['id'])
 
     def _has_provider_network_extension(self):
         extensions = self.neutron_api.list_extensions()

manila/network/standalone_network_plugin.py

@@ -314,7 +314,8 @@ class StandaloneNetworkPlugin(network.NetworkBaseAPI):
                 self.db.network_allocation_create(context, data))
         return allocations
 
-    def deallocate_network(self, context, share_server_id):
+    def deallocate_network(self, context, share_server_id,
+                           share_network=None, share_network_subnet=None):
         """Deallocate network resources for share server."""
         allocations = self.db.network_allocations_get_for_share_server(
             context, share_server_id)

manila/share/driver.py

@@ -957,10 +957,13 @@ class ShareDriver(object):
             self.admin_network_api.allocate_network(
                 context, share_server, **kwargs)
 
-    def deallocate_network(self, context, share_server_id):
+    def deallocate_network(self, context, share_server_id, share_network=None,
+                           share_network_subnet=None):
         """Deallocate network resources for the given share server."""
         if self.get_network_allocations_number():
-            self.network_api.deallocate_network(context, share_server_id)
+            self.network_api.deallocate_network(
+                context, share_server_id, share_network=share_network,
+                share_network_subnet=share_network_subnet)
 
     def choose_share_server_compatible_with_share(self, context, share_servers,
                                                   share, snapshot=None,

manila/share/manager.py

@@ -4612,6 +4612,22 @@ class ShareManager(manager.SchedulerDependentManager):
             self.db.share_server_update(context, server_id,
                                         {'status': constants.STATUS_DELETING})
             try:
+                LOG.debug("Deleting network of share server '%s'", server_id)
+                share_net = None
+                share_net_subnet = None
+                if subnet_id:
+                    try:
+                        share_net_subnet = self.db.share_network_subnet_get(
+                            context, subnet_id)
+                        share_net = self.db.share_network_get(
+                            context, share_net_subnet['share_network_id'])
+                    except Exception:
+                        LOG.warning('Share network subnet not found during '
+                                    'deletion of share server.')
+                self.driver.deallocate_network(context, share_server['id'],
+                                               share_net,
+                                               share_net_subnet)
+
                 LOG.debug("Deleting share server '%s'", server_id)
                 security_services = []
                 for ss_name in constants.SECURITY_SERVICES_ALLOWED_TYPES:
@@ -4636,7 +4652,6 @@ class ShareManager(manager.SchedulerDependentManager):
         LOG.info(
             "Share server '%s' has been deleted successfully.",
             share_server['id'])
-        self.driver.deallocate_network(context, share_server['id'])
 
     @add_hooks
     @utils.require_driver_initialized

releasenotes/notes/bug-2067266-Fix-leak-of-Manila-ports-on-share-server-deletion-b6faf19725727988.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    Share server deletion happens asynchronously and failure during this delete
+    results in leakage of neutron ports. This is fixed in two steps, first by
+    trying to delete ports before share server deletion. Second, after ports
+    from Manila db entries are deleted, query is made to neutron to get ports
+    which are allocated for share server and missing in db. And then try to
+    delete those ports. For more details please check
+    Launchpad `bug 2067266 <https://bugs.launchpad.net/manila/+bug/2067266>`_