From: Kevin Benton <blak111@gmail.com>
Date: Sat, 11 Oct 2014 10:42:47 +0000 (-0700)
Subject: Call DVR VMARP notify outside of transaction
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4c2b42e21744be56cbf32aeac6f4b4f1c87de24e;p=openstack-build%2Fneutron-build.git

Call DVR VMARP notify outside of transaction

The dvr vmarp table update notification was being called inside
of the delete_port transaction in ML2, which can cause a yield
and lead to the glorious mysql/eventlet deadlock.

This patch moves it outside the transaction and adjusts it to
use an existing port dictionary rather than re-looking it up since
the port is now gone from the DB by the time it is called.

Closes-Bug: #1377241
Change-Id: I0b4dac61e49b2a926353f8478e421cd1a70be038
---

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index b6e826b8c..1b39d5bed 100644
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -538,13 +538,12 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
             self._populate_subnet_for_ports(context, port_list)
         return port_list
 
-    def dvr_vmarp_table_update(self, context, port_id, action):
+    def dvr_vmarp_table_update(self, context, port_dict, action):
         """Notify the L3 agent of VM ARP table changes.
 
         Provide the details of the VM ARP to the L3 agent when
         a Nova instance gets created or deleted.
         """
-        port_dict = self._core_plugin._get_port(context, port_id)
         # Check this is a valid VM port
         if ("compute:" not in port_dict['device_owner'] or
             not port_dict['fixed_ips']):
diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index d744f19fe..b271af83f 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -1009,8 +1009,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             if l3plugin:
                 router_ids = l3plugin.disassociate_floatingips(
                     context, id, do_notify=False)
-                if is_dvr_enabled:
-                    l3plugin.dvr_vmarp_table_update(context, id, "del")
 
             LOG.debug("Calling delete_port for %(port_id)s owned by %(owner)s"
                       % {"port_id": id, "owner": device_owner})
@@ -1018,6 +1016,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
 
         # now that we've left db transaction, we are safe to notify
         if l3plugin:
+            if is_dvr_enabled:
+                l3plugin.dvr_vmarp_table_update(context, port, "del")
             l3plugin.notify_routers_updated(context, router_ids)
             for router in removed_routers:
                 try:
diff --git a/neutron/plugins/ml2/rpc.py b/neutron/plugins/ml2/rpc.py
index d9cb07efd..d962c1d22 100644
--- a/neutron/plugins/ml2/rpc.py
+++ b/neutron/plugins/ml2/rpc.py
@@ -165,7 +165,8 @@ class RpcCallbacks(n_rpc.RpcCallback,
             utils.is_extension_supported(l3plugin,
                                          q_const.L3_DISTRIBUTED_EXT_ALIAS)):
             try:
-                l3plugin.dvr_vmarp_table_update(rpc_context, port_id, "add")
+                port = plugin._get_port(rpc_context, port_id)
+                l3plugin.dvr_vmarp_table_update(rpc_context, port, "add")
             except exceptions.PortNotFound:
                 LOG.debug('Port %s not found during ARP update', port_id)
 
diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py
index 0d43145d2..9bd8ccd9f 100644
--- a/neutron/tests/unit/ml2/test_ml2_plugin.py
+++ b/neutron/tests/unit/ml2/test_ml2_plugin.py
@@ -988,9 +988,9 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase):
                     self._delete('ports', port['port']['id'])
 
 
-class TestMl2PluginCreateUpdatePort(base.BaseTestCase):
+class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase):
     def setUp(self):
-        super(TestMl2PluginCreateUpdatePort, self).setUp()
+        super(TestMl2PluginCreateUpdateDeletePort, self).setUp()
         self.context = mock.MagicMock()
 
     def _ensure_transaction_is_closed(self):
@@ -1043,3 +1043,24 @@ class TestMl2PluginCreateUpdatePort(base.BaseTestCase):
 
             plugin._notify_l3_agent_new_port.assert_called_once_with(
                 self.context, new_host_port)
+
+    def test_vmarp_table_update_outside_of_delete_transaction(self):
+        l3plugin = mock.Mock()
+        l3plugin.dvr_vmarp_table_update = (
+            lambda *args, **kwargs: self._ensure_transaction_is_closed())
+        l3plugin.dvr_deletens_if_no_port.return_value = []
+        l3plugin.supported_extension_aliases = [
+            'router', constants.L3_AGENT_SCHEDULER_EXT_ALIAS,
+            constants.L3_DISTRIBUTED_EXT_ALIAS
+        ]
+        with contextlib.nested(
+            mock.patch.object(ml2_plugin.Ml2Plugin, '__init__',
+                              return_value=None),
+            mock.patch.object(manager.NeutronManager,
+                              'get_service_plugins',
+                              return_value={'L3_ROUTER_NAT': l3plugin}),
+        ):
+            plugin = self._create_plugin_for_create_update_port(mock.Mock())
+            # deleting the port will call dvr_vmarp_table_update, which will
+            # run the transaction balancing function defined in this test
+            plugin.delete_port(self.context, 'fake_id')