Delete fipnamespace when external net removed on DVR

The fipnamespace is associated with an external network
on a given node. In the case of DVR there is just one
single FIP namespace for a given node.

We have seen some race conditions in the agent for creation
and deletion of the fip namespace. See the bug report for
details on the failure.

So in order to address this race condition and make the
code more stable, we will be cleaning up the fip namespace
only when an external network is removed.

The server will be sending a rpc notification message to
the agent to cleanup the fip namespace when the external
net is removed.

This patch address the above mentioned issue by not constantly
deleting and creating the fip namespace.

Closes-Bug: #1501873
Change-Id: I86869f66d4afffad7db09942578b1a456a9bd418

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index 8191c5a814b176a68d2b15cf43ac721f8d1faa3b..13d728d8cfdb8f27bd73682520b0fb4c811039ee 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -164,9 +164,11 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         1.2 - DVR support: new L3 agent methods added.
               - add_arp_entry
               - del_arp_entry
+        1.3 - fipnamespace_delete_on_ext_net - to delete fipnamespace
+              after the external network is removed
               Needed by the L3 service when dealing with DVR
     """
-    target = oslo_messaging.Target(version='1.2')
+    target = oslo_messaging.Target(version='1.3')
 
     def __init__(self, host, conf=None):
         if conf:

diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py
index 703364a410281a57ad2e1b2f51b25927e6bea21a..81aa2172cd4e0f7684ecaafdcb1cf74298ff76e9 100644 (file)
--- a/neutron/agent/l3/dvr.py
+++ b/neutron/agent/l3/dvr.py
@@ -77,3 +77,9 @@ class AgentMixin(object):
         mac = arp_table['mac_address']
         subnet_id = arp_table['subnet_id']
         ri._update_arp_entry(ip, mac, subnet_id, 'delete')
+
+    def fipnamespace_delete_on_ext_net(self, context, ext_net_id):
+        """Delete fip namespace after external network removed."""
+        fip_ns = self.get_fip_ns(ext_net_id)
+        if fip_ns.agent_gateway_port and not fip_ns.destroyed:
+            fip_ns.delete()

diff --git a/neutron/api/rpc/agentnotifiers/l3_rpc_agent_api.py b/neutron/api/rpc/agentnotifiers/l3_rpc_agent_api.py
index 2bc6c2ccc79fc025e5a95f4e4928568016219d6a..328978d3b5319306ca43c3a0e5e917ac54d87784 100644 (file)
--- a/neutron/api/rpc/agentnotifiers/l3_rpc_agent_api.py
+++ b/neutron/api/rpc/agentnotifiers/l3_rpc_agent_api.py
@@ -120,15 +120,28 @@ class L3AgentNotifyAPI(object):
             cctxt = self.client.prepare(fanout=True)
             cctxt.cast(context, method, routers=router_ids)
 
-    def _notification_fanout(self, context, method, router_id):
-        """Fanout the deleted router to all L3 agents."""
-        LOG.debug('Fanout notify agent at %(topic)s the message '
-                  '%(method)s on router %(router_id)s',
-                  {'topic': topics.L3_AGENT,
-                   'method': method,
-                   'router_id': router_id})
+    def _notification_fanout(self, context, method, router_id=None, **kwargs):
+        """Fanout the information to all L3 agents.
+
+        This function will fanout the router_id or ext_net_id
+        to the L3 Agents.
+        """
+        ext_net_id = kwargs.get('ext_net_id')
+        if router_id:
+            kwargs['router_id'] = router_id
+            LOG.debug('Fanout notify agent at %(topic)s the message '
+                      '%(method)s on router %(router_id)s',
+                      {'topic': topics.L3_AGENT,
+                       'method': method,
+                       'router_id': router_id})
+        if ext_net_id:
+            LOG.debug('Fanout notify agent at %(topic)s the message '
+                      '%(method)s for external_network  %(ext_net_id)s',
+                      {'topic': topics.L3_AGENT,
+                       'method': method,
+                       'ext_net_id': ext_net_id})
         cctxt = self.client.prepare(fanout=True)
-        cctxt.cast(context, method, router_id=router_id)
+        cctxt.cast(context, method, **kwargs)
 
     def agent_updated(self, context, admin_state_up, host):
         self._notification_host(context, 'agent_updated', host,
@@ -151,6 +164,11 @@ class L3AgentNotifyAPI(object):
         self._agent_notification_arp(context, 'del_arp_entry', router_id,
                                      operation, arp_table)
 
+    def delete_fipnamespace_for_ext_net(self, context, ext_net_id):
+        self._notification_fanout(
+            context, 'fipnamespace_delete_on_ext_net',
+            ext_net_id=ext_net_id)
+
     def router_removed_from_agent(self, context, router_id, host):
         self._notification_host(context, 'router_removed_from_agent', host,
                                 payload={'router_id': router_id})

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index 5cce619cd27aa404fa6d086522a853f68a1648f1..3271f24d55e5dba94de9ce8aed99d82e1f94ada2 100644 (file)
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -161,7 +161,7 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
               self)._delete_current_gw_port(context, router_id,
                                             router, new_network)
         if (is_distributed_router(router) and
-            gw_ext_net_id != new_network):
+            gw_ext_net_id != new_network and gw_ext_net_id is not None):
             self.delete_csnat_router_interface_ports(
                 context.elevated(), router)
             # NOTE(Swami): Delete the Floatingip agent gateway port
@@ -174,6 +174,10 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
             if not ext_net_gw_ports:
                 self.delete_floatingip_agent_gateway_port(
                     context.elevated(), None, gw_ext_net_id)
+                # Send the information to all the L3 Agent hosts
+                # to clean up the fip namespace as it is no longer required.
+                self.l3_rpc_notifier.delete_fipnamespace_for_ext_net(
+                    context, gw_ext_net_id)
 
     def _create_gw_port(self, context, router_id, router, new_network,
                         ext_ips):

diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py
index 1e1e8fa05496f044d097a17c1f1e9d839f900d63..2fc399270c6fe97f5ac89547fafe31356e28e77e 100644 (file)
--- a/neutron/tests/functional/agent/test_l3_agent.py
+++ b/neutron/tests/functional/agent/test_l3_agent.py
@@ -1146,6 +1146,7 @@ class TestDvrRouter(L3AgentTestFramework):
                 router.ns_name)
             utils.wait_until_true(device_exists)
 
+        ext_gateway_port = router_info['gw_port']
         self.assertTrue(self._namespace_exists(router.ns_name))
         utils.wait_until_true(
             lambda: self._metadata_proxy_exists(self.agent.conf, router))
@@ -1166,6 +1167,7 @@ class TestDvrRouter(L3AgentTestFramework):
                 router, ip_versions, snat_ns_name)
 
         self._delete_router(self.agent, router.router_id)
+        self._assert_fip_namespace_deleted(ext_gateway_port)
         self._assert_router_does_not_exist(router)
         self._assert_snat_namespace_does_not_exist(router)
 
@@ -1632,3 +1634,9 @@ class TestDvrRouter(L3AgentTestFramework):
 
         self._assert_ip_addresses_in_dvr_ha_snat_namespace(router2)
         self._assert_no_ip_addresses_in_dvr_ha_snat_namespace(router1)
+
+    def _assert_fip_namespace_deleted(self, ext_gateway_port):
+        ext_net_id = ext_gateway_port['network_id']
+        self.agent.fipnamespace_delete_on_ext_net(
+            self.agent.context, ext_net_id)
+        self._assert_interfaces_deleted_from_ovs()

diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py
index 444110369626501a956f7b89e95dd574b946e293..80cf11017c11aff6ebb19e27582c9fc941a83e6c 100644 (file)
--- a/neutron/tests/unit/db/test_l3_dvr_db.py
+++ b/neutron/tests/unit/db/test_l3_dvr_db.py
@@ -301,16 +301,19 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
             'foo_host')
 
     def _setup_delete_current_gw_port_deletes_fip_agent_gw_port(
-        self, port=None):
-        gw_port_db = {
-            'id': 'my_gw_id',
-            'network_id': 'ext_net_id',
-            'device_owner': l3_const.DEVICE_OWNER_ROUTER_GW
-        }
+        self, port=None, gw_port=True):
         router = mock.MagicMock()
         router.extra_attributes.distributed = True
-        router['gw_port_id'] = gw_port_db['id']
-        router.gw_port = gw_port_db
+        if gw_port:
+            gw_port_db = {
+                'id': 'my_gw_id',
+                'network_id': 'ext_net_id',
+                'device_owner': l3_const.DEVICE_OWNER_ROUTER_GW
+            }
+            router.gw_port = gw_port_db
+        else:
+            router.gw_port = None
+
         with mock.patch.object(manager.NeutronManager, 'get_plugin') as gp,\
             mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin,
                               '_delete_current_gw_port'),\
@@ -322,24 +325,28 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
                 'delete_csnat_router_interface_ports') as del_csnat_port,\
             mock.patch.object(
                 self.mixin,
-                'delete_floatingip_agent_gateway_port') as del_agent_gw_port:
+                'delete_floatingip_agent_gateway_port') as del_agent_gw_port,\
+            mock.patch.object(
+                self.mixin.l3_rpc_notifier,
+                'delete_fipnamespace_for_ext_net') as del_fip:
             plugin = mock.Mock()
             gp.return_value = plugin
             plugin.get_ports.return_value = port
             grtr.return_value = router
             self.mixin._delete_current_gw_port(
                 self.ctx, router['id'], router, 'ext_network_id')
-            return router, plugin, del_csnat_port, del_agent_gw_port
+            return router, plugin, del_csnat_port, del_agent_gw_port, del_fip
 
-    def test_delete_current_gw_port_deletes_fip_agent_gw_port(self):
-        rtr, plugin, d_csnat_port, d_agent_gw_port = (
+    def test_delete_current_gw_port_deletes_fip_agent_gw_port_and_fipnamespace(
+            self):
+        rtr, plugin, d_csnat_port, d_agent_gw_port, del_fip = (
             self._setup_delete_current_gw_port_deletes_fip_agent_gw_port())
         self.assertTrue(d_csnat_port.called)
         self.assertTrue(d_agent_gw_port.called)
         d_csnat_port.assert_called_once_with(
             mock.ANY, rtr)
-        d_agent_gw_port.assert_called_once_with(
-            mock.ANY, None, 'ext_net_id')
+        d_agent_gw_port.assert_called_once_with(mock.ANY, None, 'ext_net_id')
+        del_fip.assert_called_once_with(mock.ANY, 'ext_net_id')
 
     def test_delete_current_gw_port_never_calls_delete_fip_agent_gw_port(self):
         port = [{
@@ -352,14 +359,23 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
             'network_id': 'ext_net_id',
             'device_owner': l3_const.DEVICE_OWNER_ROUTER_GW
         }]
-        rtr, plugin, d_csnat_port, d_agent_gw_port = (
+        rtr, plugin, d_csnat_port, d_agent_gw_port, del_fip = (
             self._setup_delete_current_gw_port_deletes_fip_agent_gw_port(
                 port=port))
         self.assertTrue(d_csnat_port.called)
         self.assertFalse(d_agent_gw_port.called)
+        self.assertFalse(del_fip.called)
         d_csnat_port.assert_called_once_with(
             mock.ANY, rtr)
 
+    def test_delete_current_gw_port_never_calls_delete_fipnamespace(self):
+        rtr, plugin, d_csnat_port, d_agent_gw_port, del_fip = (
+            self._setup_delete_current_gw_port_deletes_fip_agent_gw_port(
+                gw_port=False))
+        self.assertFalse(d_csnat_port.called)
+        self.assertFalse(d_agent_gw_port.called)
+        self.assertFalse(del_fip.called)
+
     def _floatingip_on_port_test_setup(self, hostid):
         router = {'id': 'foo_router_id', 'distributed': True}
         floatingip = {