Distributed router can not add routes

Centralized router can add routes, but distributed router can not,
the neutron router-update operation fails silently.  This is
because on a distributed router commands need to be run in the
snat-* namespace, and not the qrouter-* namespace as on a
centralized router.

Change-Id: I517effcfc299c67c3413f7dc3352b97515ff69db
Closes-Bug: #1405910
Co-Authored-By: Ryan Moats <rmoats@us.ibm.com>

diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py
index b68af5cdecfbbf350e19a49dac2dafda3ec3f25f..7e9b31d55da20aca5d92c5be9ec44d377e12e354 100644 (file)
--- a/neutron/agent/l3/dvr_edge_router.py
+++ b/neutron/agent/l3/dvr_edge_router.py
@@ -166,3 +166,8 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
 
             self._add_snat_rules(ex_gw_port, self.snat_iptables_manager,
                                  interface_name)
+
+    def update_routing_table(self, operation, route, namespace=None):
+        ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(self.router['id'])
+        super(DvrEdgeRouter, self).update_routing_table(operation, route,
+                                                        namespace=ns_name)

diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py
index 8b25f0a6a33761d97f2921109db74a17f9c72875..6c87befeccb7915237a80f08f27182334d26f3c1 100644 (file)
--- a/neutron/agent/l3/router_info.py
+++ b/neutron/agent/l3/router_info.py
@@ -110,12 +110,17 @@ class RouterInfo(object):
     def get_external_device_interface_name(self, ex_gw_port):
         return self.get_external_device_name(ex_gw_port['id'])
 
-    def _update_routing_table(self, operation, route):
+    def _update_routing_table(self, operation, route, namespace):
         cmd = ['ip', 'route', operation, 'to', route['destination'],
                'via', route['nexthop']]
-        ip_wrapper = ip_lib.IPWrapper(namespace=self.ns_name)
+        ip_wrapper = ip_lib.IPWrapper(namespace=namespace)
         ip_wrapper.netns.execute(cmd, check_exit_code=False)
 
+    def update_routing_table(self, operation, route, namespace=None):
+        if namespace is None:
+            namespace = self.ns_name
+        self._update_routing_table(operation, route, namespace)
+
     def routes_updated(self):
         new_routes = self.router['routes']
 
@@ -129,10 +134,10 @@ class RouterInfo(object):
                 if route['destination'] == del_route['destination']:
                     removes.remove(del_route)
             #replace success even if there is no existing route
-            self._update_routing_table('replace', route)
+            self.update_routing_table('replace', route)
         for route in removes:
             LOG.debug("Removed route entry is '%s'", route)
-            self._update_routing_table('delete', route)
+            self.update_routing_table('delete', route)
         self.routes = new_routes
 
     def get_ex_gw_port(self):

diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index b59c9cc632db933507571868b78cc575890946b9..617735b683d78b178320a75fc1109efd053ab33c 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -1058,6 +1058,28 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
                                        router)
         self.assertEqual(self.send_adv_notif.call_count, 1)
 
+    def test_update_routing_table(self):
+        # Just verify the correct namespace was used in the call
+        router = l3_test_common.prepare_router_data()
+        uuid = router['id']
+        netns = 'snat-' + uuid
+        fake_route1 = {'destination': '135.207.0.0/16',
+                       'nexthop': '1.2.3.4'}
+
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        ri = dvr_router.DvrEdgeRouter(
+            agent,
+            HOSTNAME,
+            uuid,
+            router,
+            **self.ri_kwargs)
+        ri._update_routing_table = mock.Mock()
+
+        ri.update_routing_table('replace', fake_route1)
+        ri._update_routing_table.assert_called_once_with('replace',
+                                                         fake_route1,
+                                                         netns)
+
     def test_process_router_interface_added(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         router = l3_test_common.prepare_router_data()

diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py
index 557a639291aee2bd06f0c8b1657909b4f9999db5..66cafa41f4fa5b5a6e3a1c2d909a20a1fcb5a1a1 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_router_info.py
+++ b/neutron/tests/unit/agent/l3/test_router_info.py
@@ -52,26 +52,41 @@ class TestRouterInfo(base.BaseTestCase):
         fake_route2 = {'destination': '135.207.111.111/32',
                        'nexthop': '1.2.3.4'}
 
-        ri._update_routing_table('replace', fake_route1)
+        ri.update_routing_table('replace', fake_route1)
         expected = [['ip', 'route', 'replace', 'to', '135.207.0.0/16',
                      'via', '1.2.3.4']]
         self._check_agent_method_called(expected)
 
-        ri._update_routing_table('delete', fake_route1)
+        ri.update_routing_table('delete', fake_route1)
         expected = [['ip', 'route', 'delete', 'to', '135.207.0.0/16',
                      'via', '1.2.3.4']]
         self._check_agent_method_called(expected)
 
-        ri._update_routing_table('replace', fake_route2)
+        ri.update_routing_table('replace', fake_route2)
         expected = [['ip', 'route', 'replace', 'to', '135.207.111.111/32',
                      'via', '1.2.3.4']]
         self._check_agent_method_called(expected)
 
-        ri._update_routing_table('delete', fake_route2)
+        ri.update_routing_table('delete', fake_route2)
         expected = [['ip', 'route', 'delete', 'to', '135.207.111.111/32',
                      'via', '1.2.3.4']]
         self._check_agent_method_called(expected)
 
+    def test_update_routing_table(self):
+        # Just verify the correct namespace was used in the call
+        uuid = _uuid()
+        netns = 'qrouter-' + uuid
+        fake_route1 = {'destination': '135.207.0.0/16',
+                       'nexthop': '1.2.3.4'}
+
+        ri = router_info.RouterInfo(uuid, {'id': uuid}, **self.ri_kwargs)
+        ri._update_routing_table = mock.Mock()
+
+        ri.update_routing_table('replace', fake_route1)
+        ri._update_routing_table.assert_called_once_with('replace',
+                                                         fake_route1,
+                                                         netns)
+
     def test_routes_updated(self):
         ri = router_info.RouterInfo(_uuid(), {}, **self.ri_kwargs)
         ri.router = {}