Merge "Do not update static routes in snat-ns for dvr router with ha"

neutron/agent/l3/dvr_edge_ha_router.py

@@ -154,3 +154,7 @@ class DvrEdgeHaRouter(dvr_edge_router.DvrEdgeRouter,
                          namespace=self.snat_namespace.name,
                          prefix=constants.SNAT_INT_DEV_PREFIX,
                          mtu=port.get('mtu'))
+
+    def _should_update_snat_routing_table(self):
+        # NOTE: keepalived is responsible for routes updating
+        return False

neutron/agent/l3/dvr_edge_router.py

@@ -244,18 +244,29 @@ 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):
+    def _should_update_snat_routing_table(self):
         if self.get_ex_gw_port() and self._is_this_snat_host():
-            ns_name = self.snat_namespace.name
             # NOTE: For now let us apply the static routes both in SNAT
             # namespace and Router Namespace, to reduce the complexity.
             if self.snat_namespace.exists():
-                self._update_routing_table(operation, route, ns_name)
+                return True
             else:
                 LOG.error("The SNAT namespace %s does not exist for "
-                          "the router.", ns_name)
+                          "the router.", self.snat_namespace.name)
+        return False
+
+    def update_routing_table(self, operation, route):
+        if self._should_update_snat_routing_table():
+            ns_name = self.snat_namespace.name
+            self._update_routing_table(operation, route, ns_name)
         super(DvrEdgeRouter, self).update_routing_table(operation, route)
 
+    def update_routing_table_ecmp(self, route_list):
+        if self._should_update_snat_routing_table():
+            ns_name = self.snat_namespace.name
+            self._update_routing_table_ecmp(route_list, ns_name)
+        super(DvrEdgeRouter, self).update_routing_table_ecmp(route_list)
+
     def delete(self):
         super(DvrEdgeRouter, self).delete()
         if self.snat_namespace.exists():

neutron/agent/l3/router_info.py

@@ -186,15 +186,18 @@ class RouterInfo(BaseRouterInfo):
     def update_routing_table(self, operation, route):
         self._update_routing_table(operation, route, self.ns_name)
 
-    def update_routing_table_ecmp(self, route_list):
+    def _update_routing_table_ecmp(self, route_list, namespace):
         multipath = [dict(via=route['nexthop'])
                      for route in route_list]
         try:
-            ip_lib.add_ip_route(self.ns_name, route_list[0]['destination'],
+            ip_lib.add_ip_route(namespace, route_list[0]['destination'],
                                 via=multipath)
         except (RuntimeError, OSError, pyroute2_exc.NetlinkError):
             pass
 
+    def update_routing_table_ecmp(self, route_list):
+        self._update_routing_table_ecmp(route_list, self.ns_name)
+
     def check_and_remove_ecmp_route(self, old_routes, remove_route):
         route_list = []
         for route in old_routes:

neutron/tests/unit/agent/l3/test_agent.py

@@ -1848,8 +1848,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
     def test_process_router_snat_enabled_random_fully_false(self):
         self._test_process_router_snat_enabled(False)
 
-    def _test_update_routing_table(self, is_snat_host=True):
+    def _test_update_routing_table(self, is_snat_host=True, enable_ha=False):
-        router = l3_test_common.prepare_router_data()
+        router = l3_test_common.prepare_router_data(enable_ha=enable_ha)
         uuid = router['id']
         s_netns = 'snat-' + uuid
         q_netns = 'qrouter-' + uuid
@@ -1858,16 +1858,18 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         calls = [mock.call('replace', fake_route1, q_netns)]
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         self._set_ri_kwargs(agent, uuid, router)
-        ri = dvr_router.DvrEdgeRouter(HOSTNAME, **self.ri_kwargs)
+        router_cls = (dvr_edge_ha_router.DvrEdgeHaRouter if enable_ha else
+                      dvr_router.DvrEdgeRouter)
+        ri = router_cls(HOSTNAME, **self.ri_kwargs)
         ri._update_routing_table = mock.Mock()
 
         with mock.patch.object(ri, '_is_this_snat_host') as snat_host:
             snat_host.return_value = is_snat_host
             ri.update_routing_table('replace', fake_route1)
-            if is_snat_host:
+            if is_snat_host and not enable_ha:
-                ri._update_routing_table('replace', fake_route1, s_netns)
                 calls += [mock.call('replace', fake_route1, s_netns)]
             ri._update_routing_table.assert_has_calls(calls, any_order=True)
+            self.assertEqual(len(calls), ri._update_routing_table.call_count)
 
     def test_process_update_snat_routing_table(self):
         self._test_update_routing_table()
@@ -1875,6 +1877,46 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
     def test_process_not_update_snat_routing_table(self):
         self._test_update_routing_table(is_snat_host=False)
 
+    def test_process_not_update_ha_routing_table(self):
+        self._test_update_routing_table(enable_ha=True)
+
+    def _test_update_routing_table_ecmp(self, is_snat_host=True,
+                                        enable_ha=False):
+        router = l3_test_common.prepare_router_data()
+        uuid = router['id']
+        s_netns = 'snat-' + uuid
+        q_netns = 'qrouter-' + uuid
+        fake_route_list = [{'destination': '135.207.0.0/16',
+                            'nexthop': '19.4.4.200'},
+                           {'destination': '135.207.0.0/16',
+                            'nexthop': '19.4.4.201'}]
+        calls = [mock.call(fake_route_list, q_netns)]
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        self._set_ri_kwargs(agent, uuid, router)
+        router_cls = (dvr_edge_ha_router.DvrEdgeHaRouter if enable_ha else
+                      dvr_router.DvrEdgeRouter)
+        ri = router_cls(HOSTNAME, **self.ri_kwargs)
+        ri._update_routing_table_ecmp = mock.Mock()
+
+        with mock.patch.object(ri, '_is_this_snat_host') as snat_host:
+            snat_host.return_value = is_snat_host
+            ri.update_routing_table_ecmp(fake_route_list)
+            if is_snat_host and not enable_ha:
+                calls += [mock.call(fake_route_list, s_netns)]
+            ri._update_routing_table_ecmp.assert_has_calls(calls,
+                                                           any_order=True)
+            self.assertEqual(
+                len(calls), ri._update_routing_table_ecmp.call_count)
+
+    def test_process_update_snat_routing_table_ecmp(self):
+        self._test_update_routing_table_ecmp()
+
+    def test_process_not_update_snat_routing_table_ecmp(self):
+        self._test_update_routing_table_ecmp(is_snat_host=False)
+
+    def test_process_not_update_ha_routing_table_ecmp(self):
+        self._test_update_routing_table_ecmp(enable_ha=True)
+
     def test_process_router_interface_added(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         router = l3_test_common.prepare_router_data()