From: Swaminathan Vasudevan Date: Thu, 1 Oct 2015 18:48:55 +0000 (-0700) Subject: Delete fipnamespace when external net removed on DVR X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=cb465d40f59bfbc109204dc16259c0e4ce1c903a;p=openstack-build%2Fneutron-build.git 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 8191c5a81..13d728d8c 100644 --- 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 703364a41..81aa2172c 100644 --- 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 2bc6c2ccc..328978d3b 100644 --- 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 5cce619cd..3271f24d5 100644 --- 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 1e1e8fa05..2fc399270 100644 --- 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 444110369..80cf11017 100644 --- 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 = {