From d712663b99520e6d26269b0ca193527603178742 Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Mon, 20 Oct 2014 21:48:42 +0000 Subject: [PATCH] Move disabling of metadata and ipv6_ra to _destroy_router_namespace I noticed that disable_ipv6_ra is called from the wrong place and that in some cases it was called with a bogus router_id because the code made an incorrect assumption about the context. In other case, it was never called because _destroy_router_namespace was being called directly. This patch moves the disabling of metadata and ipv6_ra in to _destroy_router_namespace to ensure they get called correctly and avoid duplication. Change-Id: Ia76a5ff4200df072b60481f2ee49286b78ece6c4 Closes-Bug: #1383495 --- neutron/agent/l3_agent.py | 9 ++++----- neutron/tests/unit/services/vpn/test_vpn_agent.py | 5 +++-- neutron/tests/unit/test_l3_agent.py | 5 ----- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 112bf77ad..0784ad73a 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -635,7 +635,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, one attempt will be made to delete them. """ for ns in router_namespaces: - ra.disable_ipv6_ra(ns[len(NS_PREFIX):], ns, self.root_helper) try: self._destroy_namespace(ns) except RuntimeError: @@ -645,8 +644,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, def _destroy_namespace(self, ns): if ns.startswith(NS_PREFIX): - if self.conf.enable_metadata_proxy: - self._destroy_metadata_proxy(ns[len(NS_PREFIX):], ns) self._destroy_router_namespace(ns) elif ns.startswith(FIP_NS_PREFIX): self._destroy_fip_namespace(ns) @@ -695,6 +692,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self.agent_gateway_port = None def _destroy_router_namespace(self, ns): + router_id = ns[len(NS_PREFIX):] + ra.disable_ipv6_ra(router_id, ns, self.root_helper) + if self.conf.enable_metadata_proxy: + self._destroy_metadata_proxy(router_id, ns) ns_ip = ip_lib.IPWrapper(self.root_helper, namespace=ns) for d in ns_ip.get_devices(exclude_loopback=True): if d.name.startswith(INTERNAL_DEV_PREFIX): @@ -798,8 +799,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, for c, r in self.metadata_nat_rules(): ri.iptables_manager.ipv4['nat'].remove_rule(c, r) ri.iptables_manager.apply() - if self.conf.enable_metadata_proxy: - self._destroy_metadata_proxy(ri.router_id, ri.ns_name) del self.router_info[router_id] self._destroy_router_namespace(ri.ns_name) diff --git a/neutron/tests/unit/services/vpn/test_vpn_agent.py b/neutron/tests/unit/services/vpn/test_vpn_agent.py index 987b26cd4..d6ab140fc 100644 --- a/neutron/tests/unit/services/vpn/test_vpn_agent.py +++ b/neutron/tests/unit/services/vpn/test_vpn_agent.py @@ -171,9 +171,10 @@ class TestVPNAgent(base.BaseTestCase): 'neutron.agent.linux.iptables_manager.IptablesManager').start() router_id = _uuid() ri = l3_agent.RouterInfo(router_id, self.conf.root_helper, - self.conf.use_namespaces, {}) + self.conf.use_namespaces, {}, + ns_name="qrouter-%s" % router_id) ri.router = { - 'id': _uuid(), + 'id': router_id, 'admin_state_up': True, 'routes': [], 'external_gateway_info': {}, diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index fb06e6b19..bf9b05de2 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -2016,15 +2016,10 @@ vrrp_instance VR_1 { ns_list = agent._list_namespaces() agent._cleanup_namespaces(ns_list, [r['id'] for r in router_list]) - # Expect process manager to disable one radvd per stale namespace - expected_pm_disables = len(stale_namespace_list) - # Expect process manager to disable metadata proxy per qrouter ns qrouters = [n for n in stale_namespace_list if n.startswith(l3_agent.NS_PREFIX)] - expected_pm_disables += len(qrouters) - self.assertEqual(expected_pm_disables, pm.disable.call_count) self.assertEqual(agent._destroy_router_namespace.call_count, len(qrouters)) self.assertEqual(agent._destroy_snat_namespace.call_count, -- 2.45.2