Merge "Avoid notifying while inside transaction opened in delete_port()"

neutron/db/l3_db.py

@@ -877,7 +877,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):
+        """Disassociate all floating IPs linked to specific port.
+
+        @param port_id: ID of the port to disassociate floating IPs.
+        @param do_notify: whether we should notify routers right away.
+        @return: set of router-ids that require notification updates
+                 if do_notify is False, otherwise None.
+        """
         router_ids = set()
 
         with context.session.begin(subtransactions=True):
@@ -888,7 +895,15 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                 floating_ip.update({'fixed_port_id': None,
                                     'fixed_ip_address': None,
                                     'router_id': None})
+        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, list(router_ids),

neutron/plugins/bigswitch/plugin.py

@@ -810,7 +810,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
@@ -818,6 +819,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"))
@@ -1090,11 +1094,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,
+        router_ids = super(NeutronRestProxyV2, self).disassociate_floatingips(
-                                                                 port_id)
+            context, port_id, do_notify=do_notify)
         self._send_floatingip_update(context)
+        return router_ids
 
     # overriding method from l3_db as original method calls
     # self.delete_floatingip() which in turn calls self.delete_port() which

neutron/plugins/cisco/n1kv/n1kv_neutron_plugin.py

@@ -1228,8 +1228,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):

neutron/plugins/embrane/base_plugin.py

@@ -351,14 +351,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"]
@@ -371,3 +372,4 @@ class EmbranePlugin(object):
                     p_con.Events.RESET_NAT_RULE, neutron_router, context,
                     state_change),
                 args=(fip_id,))
+        return router_ids

neutron/plugins/linuxbridge/lb_neutron_plugin.py

@@ -526,11 +526,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):

neutron/plugins/ml2/plugin.py

@@ -744,10 +744,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:

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)

neutron/plugins/nec/nec_plugin.py

@@ -651,9 +651,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)
 
 

neutron/plugins/nuage/plugin.py

@@ -935,8 +935,10 @@ class NuagePlugin(db_base_plugin_v2.NeutronDbPluginV2,
                         context, neutron_fip['id'])
             return neutron_fip
 
-    def disassociate_floatingips(self, context, port_id):
+    def disassociate_floatingips(self, context, port_id, do_notify=True):
-        super(NuagePlugin, self).disassociate_floatingips(context, port_id)
+        router_ids = super(NuagePlugin, self).disassociate_floatingips(
+            context, port_id, do_notify=do_notify)
+
         port_mapping = nuagedb.get_port_mapping_by_id(context.session,
                                                       port_id)
         if port_mapping:
@@ -946,10 +948,13 @@ class NuagePlugin(db_base_plugin_v2.NeutronDbPluginV2,
             }
             self.nuageclient.update_nuage_vm_vport(params)
 
+        return router_ids
+
     def update_floatingip(self, context, id, floatingip):
         fip = floatingip['floatingip']
         orig_fip = self._get_floatingip(context, id)
         port_id = orig_fip['fixed_port_id']
+        router_ids = []
         with context.session.begin(subtransactions=True):
             neutron_fip = super(NuagePlugin, self).update_floatingip(
                 context, id, floatingip)
@@ -965,9 +970,9 @@ class NuagePlugin(db_base_plugin_v2.NeutronDbPluginV2,
                                                    fip['port_id'])
                 except nuage_exc.OperationNotSupported:
                     with excutils.save_and_reraise_exception():
-                        super(NuagePlugin,
+                        router_ids = super(
-                              self).disassociate_floatingips(context,
+                            NuagePlugin, self).disassociate_floatingips(
-                                                             fip['port_id'])
+                                context, fip['port_id'], do_notify=False)
                 except n_exc.BadRequest:
                     with excutils.save_and_reraise_exception():
                         super(NuagePlugin, self).delete_floatingip(context,
@@ -981,7 +986,11 @@ class NuagePlugin(db_base_plugin_v2.NeutronDbPluginV2,
                         'nuage_fip_id': None
                     }
                     self.nuageclient.update_nuage_vm_vport(params)
-            return neutron_fip
+
+        # now that we've left db transaction, we are safe to notify
+        self.notify_routers_updated(context, router_ids)
+
+        return neutron_fip
 
     def delete_floatingip(self, context, id):
         fip = self._get_floatingip(context, id)

neutron/plugins/oneconvergence/plugin.py

@@ -351,7 +351,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)
 
@@ -360,6 +361,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):

neutron/plugins/openvswitch/ovs_neutron_plugin.py

@@ -626,9 +626,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)

neutron/plugins/plumgrid/plumgrid_plugin/plumgrid_plugin.py

@@ -245,7 +245,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:
@@ -260,6 +261,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,
@@ -531,7 +535,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"))
 
@@ -549,8 +553,9 @@ class NeutronPluginPLUMgridV2(db_base_plugin_v2.NeutronDbPluginV2,
         except Exception as err_message:
             raise plum_excep.PLUMgridException(err_msg=err_message)
 
-        super(NeutronPluginPLUMgridV2,
+        return super(NeutronPluginPLUMgridV2,
-              self).disassociate_floatingips(context, port_id)
+                     self).disassociate_floatingips(
+                             context, port_id, do_notify=do_notify)
 
     """
     Internal PLUMgrid Fuctions

neutron/plugins/ryu/ryu_neutron_plugin.py

@@ -231,11 +231,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):

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 uuid
@@ -24,6 +25,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 driver_api
@@ -133,6 +135,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):