Merge "Avoid notifying while inside transaction opened in delete_port()" into stable/icehouse2014.1.2

13 files changed, 111 insertions, 22 deletions

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index c8f26b5b2d..7d47a29b4a 100644
--- a/neutron/db/l3_db.py
+++ b/neutron/db/l3_db.py
@@ -747,12 +747,14 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                           {'port_id': port_db['id'],
                            'port_owner': port_db['device_owner']})
 
-    def disassociate_floatingips(self, context, port_id):
+    def disassociate_floatingips(self, context, port_id, do_notify=True):
+        router_ids = []
+
         with context.session.begin(subtransactions=True):
             try:
                 fip_qry = context.session.query(FloatingIP)
                 floating_ip = fip_qry.filter_by(fixed_port_id=port_id).one()
-                router_id = floating_ip['router_id']
+                router_ids.append(floating_ip['router_id'])
                 floating_ip.update({'fixed_port_id': None,
                                     'fixed_ip_address': None,
                                     'router_id': None})
@@ -762,9 +764,18 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                 # should never happen
                 raise Exception(_('Multiple floating IPs found for port %s')
                                 % port_id)
-        if router_id:
+        if do_notify:
+            self.notify_routers_updated(context, router_ids)
+            # since caller assumes that we handled notifications on its
+            # behalf, return nothing
+            return
+
+        return router_ids
+
+    def notify_routers_updated(self, context, router_ids):
+        if router_ids:
             self.l3_rpc_notifier.routers_updated(
-                context, [router_id])
+                context, router_ids)
 
     def _build_routers_list(self, routers, gw_ports):
         gw_port_id_gw_port_dict = dict((gw_port['id'], gw_port)
diff --git a/neutron/plugins/bigswitch/plugin.py b/neutron/plugins/bigswitch/plugin.py
index c84f381400..64eb09fde7 100644
--- a/neutron/plugins/bigswitch/plugin.py
+++ b/neutron/plugins/bigswitch/plugin.py
@@ -817,7 +817,8 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
         if l3_port_check:
             self.prevent_l3_port_deletion(context, port_id)
         with context.session.begin(subtransactions=True):
-            self.disassociate_floatingips(context, port_id)
+            router_ids = self.disassociate_floatingips(
+                context, port_id, do_notify=False)
             self._delete_port_security_group_bindings(context, port_id)
             port = super(NeutronRestProxyV2, self).get_port(context, port_id)
             # Tenant ID must come from network in case the network is shared
@@ -825,6 +826,9 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
             self._delete_port(context, port_id)
             self.servers.rest_delete_port(tenid, port['network_id'], port_id)
 
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
+
     @put_context_in_serverpool
     def create_subnet(self, context, subnet):
         LOG.debug(_("NeutronRestProxyV2: create_subnet() called"))
@@ -1097,11 +1101,12 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
                 self._send_floatingip_update(context)
 
     @put_context_in_serverpool
-    def disassociate_floatingips(self, context, port_id):
+    def disassociate_floatingips(self, context, port_id, do_notify=True):
         LOG.debug(_("NeutronRestProxyV2: diassociate_floatingips() called"))
-        super(NeutronRestProxyV2, self).disassociate_floatingips(context,
-                                                                 port_id)
+        router_ids = super(NeutronRestProxyV2, self).disassociate_floatingips(
+            context, port_id, do_notify=do_notify)
         self._send_floatingip_update(context)
+        return router_ids
 
     def _send_floatingip_update(self, context):
         try:
diff --git a/neutron/plugins/cisco/n1kv/n1kv_neutron_plugin.py b/neutron/plugins/cisco/n1kv/n1kv_neutron_plugin.py
index 02c492f772..a4e2961d38 100644
--- a/neutron/plugins/cisco/n1kv/n1kv_neutron_plugin.py
+++ b/neutron/plugins/cisco/n1kv/n1kv_neutron_plugin.py
@@ -1229,8 +1229,13 @@ class N1kvNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                 n1kv_db_v2.delete_vm_network(context.session,
                                              port[n1kv.PROFILE_ID],
                                              port['network_id'])
-            self.disassociate_floatingips(context, id)
+            router_ids = self.disassociate_floatingips(
+                context, id, do_notify=False)
             super(N1kvNeutronPluginV2, self).delete_port(context, id)
+
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
+
         self._send_delete_port_request(context, port, vm_network)
 
     def get_port(self, context, id, fields=None):
diff --git a/neutron/plugins/embrane/base_plugin.py b/neutron/plugins/embrane/base_plugin.py
index 33d2138886..024797ff9e 100644
--- a/neutron/plugins/embrane/base_plugin.py
+++ b/neutron/plugins/embrane/base_plugin.py
@@ -353,14 +353,15 @@ class EmbranePlugin(object):
                 args=(nat_info,))
         return result
 
-    def disassociate_floatingips(self, context, port_id):
+    def disassociate_floatingips(self, context, port_id, do_notify=True):
         try:
             fip_qry = context.session.query(l3_db.FloatingIP)
             floating_ip = fip_qry.filter_by(fixed_port_id=port_id).one()
             router_id = floating_ip["router_id"]
         except exc.NoResultFound:
             return
-        self._l3super.disassociate_floatingips(self, context, port_id)
+        router_ids = self._l3super.disassociate_floatingips(
+            self, context, port_id, do_notify=do_notify)
         if router_id:
             neutron_router = self._get_router(context, router_id)
             fip_id = floating_ip["id"]
@@ -373,3 +374,4 @@ class EmbranePlugin(object):
                     p_con.Events.RESET_NAT_RULE, neutron_router, context,
                     state_change),
                 args=(fip_id,))
+        return router_ids
diff --git a/neutron/plugins/linuxbridge/lb_neutron_plugin.py b/neutron/plugins/linuxbridge/lb_neutron_plugin.py
index 52fbdcaddd..8100917ade 100644
--- a/neutron/plugins/linuxbridge/lb_neutron_plugin.py
+++ b/neutron/plugins/linuxbridge/lb_neutron_plugin.py
@@ -524,11 +524,14 @@ class LinuxBridgePluginV2(db_base_plugin_v2.NeutronDbPluginV2,
 
         session = context.session
         with session.begin(subtransactions=True):
-            self.disassociate_floatingips(context, id)
+            router_ids = self.disassociate_floatingips(
+                context, id, do_notify=False)
             port = self.get_port(context, id)
             self._delete_port_security_group_bindings(context, id)
             super(LinuxBridgePluginV2, self).delete_port(context, id)
 
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
         self.notify_security_groups_member_updated(context, port)
 
     def _notify_port_updated(self, context, port):
diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index a72c5128ab..a127407ebf 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -736,10 +736,15 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             self._delete_port_security_group_bindings(context, id)
             LOG.debug(_("Calling base delete_port"))
             if l3plugin:
-                l3plugin.disassociate_floatingips(context, id)
+                router_ids = l3plugin.disassociate_floatingips(
+                    context, id, do_notify=False)
 
             super(Ml2Plugin, self).delete_port(context, id)
 
+        # now that we've left db transaction, we are safe to notify
+        if l3plugin:
+            l3plugin.notify_routers_updated(context, router_ids)
+
         try:
             self.mechanism_manager.delete_port_postcommit(mech_context)
         except ml2_exc.MechanismDriverError:
diff --git a/neutron/plugins/mlnx/mlnx_plugin.py b/neutron/plugins/mlnx/mlnx_plugin.py
index 920038f5bc..280541a2b7 100644
--- a/neutron/plugins/mlnx/mlnx_plugin.py
+++ b/neutron/plugins/mlnx/mlnx_plugin.py
@@ -502,9 +502,12 @@ class MellanoxEswitchPlugin(db_base_plugin_v2.NeutronDbPluginV2,
 
         session = context.session
         with session.begin(subtransactions=True):
-            self.disassociate_floatingips(context, port_id)
+            router_ids = self.disassociate_floatingips(
+                context, port_id, do_notify=False)
             port = self.get_port(context, port_id)
             self._delete_port_security_group_bindings(context, port_id)
             super(MellanoxEswitchPlugin, self).delete_port(context, port_id)
 
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
         self.notify_security_groups_member_updated(context, port)
diff --git a/neutron/plugins/nec/nec_plugin.py b/neutron/plugins/nec/nec_plugin.py
index 8b9f5b3d57..e4a7c6ffcb 100644
--- a/neutron/plugins/nec/nec_plugin.py
+++ b/neutron/plugins/nec/nec_plugin.py
@@ -652,9 +652,13 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         if l3_port_check:
             self.prevent_l3_port_deletion(context, id)
         with context.session.begin(subtransactions=True):
-            self.disassociate_floatingips(context, id)
+            router_ids = self.disassociate_floatingips(
+                context, id, do_notify=False)
             self._delete_port_security_group_bindings(context, id)
             super(NECPluginV2, self).delete_port(context, id)
+
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
         self.notify_security_groups_member_updated(context, port)
 
 
diff --git a/neutron/plugins/oneconvergence/plugin.py b/neutron/plugins/oneconvergence/plugin.py
index e4aea6f90a..49bda6fe26 100644
--- a/neutron/plugins/oneconvergence/plugin.py
+++ b/neutron/plugins/oneconvergence/plugin.py
@@ -356,7 +356,8 @@ class OneConvergencePluginV2(db_base_plugin_v2.NeutronDbPluginV2,
 
             self._delete_port_security_group_bindings(context, port_id)
 
-            self.disassociate_floatingips(context, port_id)
+            router_ids = self.disassociate_floatingips(
+                context, port_id, do_notify=False)
 
             super(OneConvergencePluginV2, self).delete_port(context, port_id)
 
@@ -365,6 +366,8 @@ class OneConvergencePluginV2(db_base_plugin_v2.NeutronDbPluginV2,
 
             self.nvsdlib.delete_port(port_id, neutron_port)
 
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
         self.notify_security_groups_member_updated(context, neutron_port)
 
     def create_floatingip(self, context, floatingip):
diff --git a/neutron/plugins/openvswitch/ovs_neutron_plugin.py b/neutron/plugins/openvswitch/ovs_neutron_plugin.py
index 2cfe2674b6..7f66dfdac0 100644
--- a/neutron/plugins/openvswitch/ovs_neutron_plugin.py
+++ b/neutron/plugins/openvswitch/ovs_neutron_plugin.py
@@ -627,9 +627,12 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
 
         session = context.session
         with session.begin(subtransactions=True):
-            self.disassociate_floatingips(context, id)
+            router_ids = self.disassociate_floatingips(
+                context, id, do_notify=False)
             port = self.get_port(context, id)
             self._delete_port_security_group_bindings(context, id)
             super(OVSNeutronPluginV2, self).delete_port(context, id)
 
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
         self.notify_security_groups_member_updated(context, port)
diff --git a/neutron/plugins/plumgrid/plumgrid_plugin/plumgrid_plugin.py b/neutron/plugins/plumgrid/plumgrid_plugin/plumgrid_plugin.py
index 17c05c771b..42df5c9f4d 100644
--- a/neutron/plugins/plumgrid/plumgrid_plugin/plumgrid_plugin.py
+++ b/neutron/plugins/plumgrid/plumgrid_plugin/plumgrid_plugin.py
@@ -242,7 +242,8 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
             # Plugin DB - Port Create and Return port
             port_db = super(NeutronPluginPLUMgridV2,
                             self).get_port(context, port_id)
-            self.disassociate_floatingips(context, port_id)
+            router_ids = self.disassociate_floatingips(
+                context, port_id, do_notify=False)
             super(NeutronPluginPLUMgridV2, self).delete_port(context, port_id)
 
             if port_db["device_owner"] == constants.DEVICE_OWNER_ROUTER_GW:
@@ -257,6 +258,9 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
             except Exception as err_message:
                 raise plum_excep.PLUMgridException(err_msg=err_message)
 
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
+
     def get_port(self, context, id, fields=None):
         with context.session.begin(subtransactions=True):
             port_db = super(NeutronPluginPLUMgridV2,
@@ -528,7 +532,7 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
             except Exception as err_message:
                 raise plum_excep.PLUMgridException(err_msg=err_message)
 
-    def disassociate_floatingips(self, context, port_id):
+    def disassociate_floatingips(self, context, port_id, do_notify=True):
         LOG.debug(_("Neutron PLUMgrid Director: disassociate_floatingips() "
                     "called"))
 
@@ -546,8 +550,8 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
         except Exception as err_message:
             raise plum_excep.PLUMgridException(err_msg=err_message)
 
-        super(NeutronPluginPLUMgridV2,
-              self).disassociate_floatingips(context, port_id)
+        return super(NeutronPluginPLUMgridV2, self).disassociate_floatingips(
+            context, port_id, do_notify=do_notify)
 
     """
     Internal PLUMgrid Fuctions
diff --git a/neutron/plugins/ryu/ryu_neutron_plugin.py b/neutron/plugins/ryu/ryu_neutron_plugin.py
index 9de98979d2..5f65c139f4 100644
--- a/neutron/plugins/ryu/ryu_neutron_plugin.py
+++ b/neutron/plugins/ryu/ryu_neutron_plugin.py
@@ -235,11 +235,14 @@ class RyuNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
             self.prevent_l3_port_deletion(context, id)
 
         with context.session.begin(subtransactions=True):
-            self.disassociate_floatingips(context, id)
+            router_ids = self.disassociate_floatingips(
+                context, id, do_notify=False)
             port = self.get_port(context, id)
             self._delete_port_security_group_bindings(context, id)
             super(RyuNeutronPluginV2, self).delete_port(context, id)
 
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
         self.notify_security_groups_member_updated(context, port)
 
     def update_port(self, context, id, port):
diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py
index 53bc1d8e5b..4aab353536 100644
--- a/neutron/tests/unit/ml2/test_ml2_plugin.py
+++ b/neutron/tests/unit/ml2/test_ml2_plugin.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import contextlib
 import mock
 import testtools
 import webob
@@ -23,6 +24,7 @@ from neutron.extensions import multiprovidernet as mpnet
 from neutron.extensions import portbindings
 from neutron.extensions import providernet as pnet
 from neutron import manager
+from neutron.plugins.common import constants as service_constants
 from neutron.plugins.ml2.common import exceptions as ml2_exc
 from neutron.plugins.ml2 import config
 from neutron.plugins.ml2 import plugin as ml2_plugin
@@ -119,6 +121,42 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
                 mock.call(_("The port '%s' was deleted"), 'invalid-uuid')
             ])
 
+    def test_delete_port_no_notify_in_disassociate_floatingips(self):
+        ctx = context.get_admin_context()
+        plugin = manager.NeutronManager.get_plugin()
+        l3plugin = manager.NeutronManager.get_service_plugins().get(
+            service_constants.L3_ROUTER_NAT)
+        with contextlib.nested(
+            self.port(no_delete=True),
+            mock.patch.object(l3plugin, 'disassociate_floatingips'),
+            mock.patch.object(l3plugin, 'notify_routers_updated')
+        ) as (port, disassociate_floatingips, notify):
+
+            port_id = port['port']['id']
+            plugin.delete_port(ctx, port_id)
+
+            # check that no notification was requested while under
+            # transaction
+            disassociate_floatingips.assert_has_calls([
+                mock.call(ctx, port_id, do_notify=False)
+            ])
+
+            # check that notifier was still triggered
+            notify.assert_has_calls([
+                mock.call(ctx, disassociate_floatingips.return_value)
+            ])
+
+    def test_disassociate_floatingips_do_notify_returns_nothing(self):
+        ctx = context.get_admin_context()
+        l3plugin = manager.NeutronManager.get_service_plugins().get(
+            service_constants.L3_ROUTER_NAT)
+        with self.port() as port:
+
+            port_id = port['port']['id']
+            # check that nothing is returned when notifications are handled
+            # by the called method
+            self.assertIsNone(l3plugin.disassociate_floatingips(ctx, port_id))
+
 
 class TestMl2PortBinding(Ml2PluginV2TestCase,
                          test_bindings.PortBindingsTestCase):