From 8d703bbb5dd974b38427dd2c46c6559c78a2a3ce Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Mon, 29 Sep 2014 20:21:23 -0700 Subject: [PATCH] ML2: move L3 cleanup out of network transaction Move _process_l3_delete out of the delete_network transaction to eliminate the semaphore deadlock that occurs when it tries to delete the ports associated with existing floating IPs. It makes more sense to live outside of the transaction anyway because the operations it performs cannot be rolled back only in the database if the L3 plugin makes external calls for floating IP creation/deletion. e.g. if delete_floatingip is successful, it may have deleted external resources and restoring the DB records would make things inconsistent. If a failure to delete the network does occur, any cleanup done by _process_l3_delete will not be reversed. Closes-Bug: #1374573 Change-Id: I3ae7bb269df9b9dcef94f48f13f1bde1e4106a80 --- neutron/plugins/ml2/plugin.py | 8 +++++++- neutron/tests/unit/ml2/test_ml2_plugin.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 6ef4d107c7..422cbff86e 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -580,9 +580,15 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, # to prevent deadlock waiting to acquire a DB lock # held by another thread in the same process, leading # to 'lock wait timeout' errors. + # + # Process L3 first, since, depending on the L3 plugin, it may + # involve locking the db-access semaphore, sending RPC + # notifications, and/or calling delete_port on this plugin. + # Additionally, a rollback may not be enough to undo the + # deletion of a floating IP with certain L3 backends. + self._process_l3_delete(context, id) with contextlib.nested(lockutils.lock('db-access'), session.begin(subtransactions=True)): - self._process_l3_delete(context, id) # Get ports to auto-delete. ports = (session.query(models_v2.Port). enable_eagerloads(False). diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py index be987827c1..5cb6df44bc 100644 --- a/neutron/tests/unit/ml2/test_ml2_plugin.py +++ b/neutron/tests/unit/ml2/test_ml2_plugin.py @@ -24,6 +24,7 @@ from neutron.common import exceptions as exc from neutron.common import utils from neutron import context from neutron.db import db_base_plugin_v2 as base_plugin +from neutron.extensions import external_net as external_net from neutron.extensions import multiprovidernet as mpnet from neutron.extensions import portbindings from neutron.extensions import providernet as pnet @@ -147,6 +148,22 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): mock.call(_("The port '%s' was deleted"), 'invalid-uuid') ]) + def test_l3_cleanup_on_net_delete(self): + l3plugin = manager.NeutronManager.get_service_plugins().get( + service_constants.L3_ROUTER_NAT) + kwargs = {'arg_list': (external_net.EXTERNAL,), + external_net.EXTERNAL: True} + with self.network(**kwargs) as n: + with self.subnet(network=n, cidr='200.0.0.0/22'): + l3plugin.create_floatingip( + context.get_admin_context(), + {'floatingip': {'floating_network_id': n['network']['id'], + 'tenant_id': n['network']['tenant_id']}} + ) + self._delete('networks', n['network']['id']) + flips = l3plugin.get_floatingips(context.get_admin_context()) + self.assertFalse(flips) + def test_delete_port_no_notify_in_disassociate_floatingips(self): ctx = context.get_admin_context() plugin = manager.NeutronManager.get_plugin()