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

diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index 112bf77ad27184c876b865f2aec329e2ce0052fe..0784ad73a3e3fd9bcd5fdd2bd9831a5e0b798005 100644 (file)
--- 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 987b26cd4d115237ca32e12be6b1d7aad57f8b06..d6ab140fc304f6e6d025e414542ac046e3d52cf0 100644 (file)
--- 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 fb06e6b19cd7155039d7c4b06e5b83a6df839933..bf9b05de29744f0f32167349806b4df3c063815f 100644 (file)
--- 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,