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

2 files changed, 24 insertions, 1 deletions

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()