From 19622387dc54ed2458bc514854e742160954318c Mon Sep 17 00:00:00 2001 From: Kiran Pawar Date: Mon, 27 May 2024 12:43:01 +0000 Subject: [PATCH] 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 (cherry picked from commit 76d032969848fa409f3b7da499cc2da3de67f6be) --- manila/network/__init__.py | 3 +- .../network/neutron/neutron_network_plugin.py | 28 +++++++++++++++++-- manila/network/standalone_network_plugin.py | 3 +- manila/share/driver.py | 7 +++-- manila/share/manager.py | 17 ++++++++++- ...hare-server-deletion-b6faf19725727988.yaml | 10 +++++++ 6 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/bug-2067266-Fix-leak-of-Manila-ports-on-share-server-deletion-b6faf19725727988.yaml diff --git a/manila/network/__init__.py b/manila/network/__init__.py index 1fb2682a63..2bcf9b486c 100644 --- a/manila/network/__init__.py +++ b/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 diff --git a/manila/network/neutron/neutron_network_plugin.py b/manila/network/neutron/neutron_network_plugin.py index 727d58edcf..edd735ea08 100644 --- a/manila/network/neutron/neutron_network_plugin.py +++ b/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() diff --git a/manila/network/standalone_network_plugin.py b/manila/network/standalone_network_plugin.py index 100473c33f..796a2be151 100644 --- a/manila/network/standalone_network_plugin.py +++ b/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) diff --git a/manila/share/driver.py b/manila/share/driver.py index bf27a2934c..b5e97f83fe 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -960,10 +960,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, diff --git a/manila/share/manager.py b/manila/share/manager.py index b4a89cfa15..26fc8d3647 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -4605,6 +4605,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: @@ -4629,7 +4645,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 diff --git a/releasenotes/notes/bug-2067266-Fix-leak-of-Manila-ports-on-share-server-deletion-b6faf19725727988.yaml b/releasenotes/notes/bug-2067266-Fix-leak-of-Manila-ports-on-share-server-deletion-b6faf19725727988.yaml new file mode 100644 index 0000000000..1aec666803 --- /dev/null +++ b/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 `_