From: Carl Baldwin <carl.baldwin@hp.com>
Date: Thu, 10 Jul 2014 19:41:40 +0000 (+0000)
Subject: Implement namespace cleanup for new DVR namespaces
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=163e8edc29646110490831b4590f3464703e6a1a;p=openstack-build%2Fneutron-build.git

Implement namespace cleanup for new DVR namespaces

DVR introduces a new namespace type called snat-<router_id>.  These
namespaces are not properly cleaned up when found stale after an agent
restart.  This patch fixes that.

Change-Id: I0d8b83e0b7838957742e72aa2f42b15e0ca67f04
Partially-implements: blueprint neutron-ovs-dvr
---

diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index dc308ca35..b33e380bc 100644
--- a/neutron/agent/l3_agent.py
+++ b/neutron/agent/l3_agent.py
@@ -468,7 +468,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
 
             host_namespaces = root_ip.get_namespaces(self.root_helper)
             return set(ns for ns in host_namespaces
-                       if ns.startswith(NS_PREFIX))
+                       if (ns.startswith(NS_PREFIX)
+                           or ns.startswith(SNAT_NS_PREFIX)))
         except RuntimeError:
             LOG.exception(_('RuntimeError in obtaining router list '
                             'for namespace cleanup.'))
@@ -483,6 +484,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
         The argument router_ids is the list of ids for known routers.
         """
         ns_to_ignore = set(NS_PREFIX + id for id in router_ids)
+        ns_to_ignore.update(SNAT_NS_PREFIX + id for id in router_ids)
         ns_to_destroy = router_namespaces - ns_to_ignore
         self._destroy_stale_router_namespaces(ns_to_destroy)
 
@@ -542,7 +544,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
                 ns_ip.del_veth(d.name)
             elif d.name.startswith(FIP_EXT_DEV_PREFIX):
                 # single port from FIP NS to br-ext
-                # TODO(mrsmith): remove br-ext interface
+                # TODO(carl) Where does the port get deleted?
                 LOG.debug('DVR: unplug: %s', d.name)
                 self.driver.unplug(d.name,
                                    bridge=self.conf.external_network_bridge,
diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index a1854cff8..b08a690f2 100644
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -1691,6 +1691,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         good_namespace_list = [l3_agent.NS_PREFIX + r['id']
                                for r in router_list]
+        good_namespace_list += [l3_agent.SNAT_NS_PREFIX + r['id']
+                                for r in router_list]
         self.mock_ip.get_namespaces.return_value = (stale_namespace_list +
                                                     good_namespace_list +
                                                     other_namespaces)
@@ -1703,16 +1705,24 @@ class TestBasicRouterOperations(base.BaseTestCase):
         pm.reset_mock()
 
         agent._destroy_router_namespace = mock.MagicMock()
+        agent._destroy_snat_namespace = mock.MagicMock()
         ns_list = agent._list_namespaces()
         agent._cleanup_namespaces(ns_list, [r['id'] for r in router_list])
 
-        # Expect process manager to disable two processes (metadata_proxy
-        # and radvd) per stale namespace.
-        expected_pm_disables = 2 * len(stale_namespace_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(stale_namespace_list))
-        expected_args = [mock.call(ns) for ns in stale_namespace_list]
+                         len(qrouters))
+        self.assertEqual(agent._destroy_snat_namespace.call_count,
+                         len(stale_namespace_list) - len(qrouters))
+        expected_args = [mock.call(ns) for ns in qrouters]
         agent._destroy_router_namespace.assert_has_calls(expected_args,
                                                          any_order=True)
         self.assertFalse(agent._clean_stale_namespaces)
@@ -1720,7 +1730,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
     def test_cleanup_namespace(self):
         self.conf.set_override('router_id', None)
         stale_namespaces = [l3_agent.NS_PREFIX + 'foo',
-                            l3_agent.NS_PREFIX + 'bar']
+                            l3_agent.NS_PREFIX + 'bar',
+                            l3_agent.SNAT_NS_PREFIX + 'foo']
         other_namespaces = ['unknown']
 
         self._cleanup_namespace_test(stale_namespaces,
@@ -1730,7 +1741,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
     def test_cleanup_namespace_with_registered_router_ids(self):
         self.conf.set_override('router_id', None)
         stale_namespaces = [l3_agent.NS_PREFIX + 'cccc',
-                            l3_agent.NS_PREFIX + 'eeeee']
+                            l3_agent.NS_PREFIX + 'eeeee',
+                            l3_agent.SNAT_NS_PREFIX + 'fffff']
         router_list = [{'id': 'foo', 'distributed': False},
                        {'id': 'aaaa', 'distributed': False}]
         other_namespaces = ['qdhcp-aabbcc', 'unknown']