diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index d2424096d13..4e45bebde7c 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -14,6 +14,7 @@ from oslo_log import log as logging +from neutron._i18n import _LE from neutron.agent.l3 import dvr_local_router from neutron.agent.l3 import dvr_snat_ns from neutron.agent.l3 import router_info as router @@ -35,6 +36,11 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): ex_gw_port, interface_name) if self._is_this_snat_host(): self._create_dvr_gateway(ex_gw_port, interface_name) + # NOTE: When a router is created without a gateway the routes get + # added to the router namespace, but if we wanted to populate + # the same routes to the snat namespace after the gateway port + # is added, we need to call routes_updated here. + self.routes_updated([], self.router['routes']) def external_gateway_updated(self, ex_gw_port, interface_name): if not self._is_this_snat_host(): @@ -182,10 +188,20 @@ 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) + def update_routing_table(self, operation, route): + if self.get_ex_gw_port() and self._is_this_snat_host(): + ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name( + self.router['id']) + # NOTE: For now let us apply the static routes both in SNAT + # namespace and Router Namespace, to reduce the complexity. + ip_wrapper = ip_lib.IPWrapper(namespace=ns_name) + if ip_wrapper.netns.exists(ns_name): + super(DvrEdgeRouter, self)._update_routing_table( + operation, route, namespace=ns_name) + else: + LOG.error(_LE("The SNAT namespace %s does not exist for " + "the router."), ns_name) + super(DvrEdgeRouter, self).update_routing_table(operation, route) def delete(self, agent): super(DvrEdgeRouter, self).delete(agent) diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index d1d1a1f395c..6d54ddfab38 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -179,15 +179,12 @@ class HaRouter(router.RouterInfo): def get_router_cidrs(self, device): return set(self._get_cidrs_from_keepalived(device.name)) - def routes_updated(self): - new_routes = self.router['routes'] - + def routes_updated(self, old_routes, new_routes): instance = self._get_keepalived_instance() instance.virtual_routes.extra_routes = [ keepalived.KeepalivedVirtualRoute( route['destination'], route['nexthop']) for route in new_routes] - self.routes = new_routes def _add_default_gw_virtual_route(self, ex_gw_port, interface_name): gateway_ips = self._get_external_gw_ips(ex_gw_port) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 968bd9d3632..4a4917a0cc7 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -107,15 +107,10 @@ class RouterInfo(object): 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 update_routing_table(self, operation, route): + self._update_routing_table(operation, route, self.ns_name) - def routes_updated(self): - new_routes = self.router['routes'] - - old_routes = self.routes + def routes_updated(self, old_routes, new_routes): adds, removes = common_utils.diff_list_of_dict(old_routes, new_routes) for route in adds: @@ -129,7 +124,6 @@ class RouterInfo(object): for route in removes: LOG.debug("Removed route entry is '%s'", route) self.update_routing_table('delete', route) - self.routes = new_routes def get_ex_gw_port(self): return self.router.get('gw_port') @@ -707,7 +701,8 @@ class RouterInfo(object): agent.pd.sync_router(self.router['id']) self.process_external(agent, delete) # Process static routes for router - self.routes_updated() + self.routes_updated(self.routes, self.router['routes']) + self.routes = self.router['routes'] # Update ex_gw_port and enable_snat on the router info cache self.ex_gw_port = self.get_ex_gw_port() diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 465338a9083..797dac7c7f2 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -423,8 +423,10 @@ class L3AgentTestFramework(base.BaseSudoTestCase): self.assertTrue(self.device_exists_with_ips_and_mac( device, router.get_internal_device_name, router.ns_name)) - def _assert_extra_routes(self, router): - routes = ip_lib.get_routing_table(4, namespace=router.ns_name) + def _assert_extra_routes(self, router, namespace=None): + if namespace is None: + namespace = router.ns_name + routes = ip_lib.get_routing_table(4, namespace=namespace) routes = [{'nexthop': route['nexthop'], 'destination': route['destination']} for route in routes] diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index 831d670a695..482c3f2847f 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -115,7 +115,7 @@ class TestDvrRouter(framework.L3AgentTestFramework): # We get the router info particular to a dvr router router_info = self.generate_dvr_router_info( - enable_ha, enable_snat) + enable_ha, enable_snat, extra_routes=True) # We need to mock the get_agent_gateway_port return value # because the whole L3PluginApi is mocked and we need the port @@ -164,7 +164,6 @@ class TestDvrRouter(framework.L3AgentTestFramework): self._assert_snat_chains(router) self._assert_floating_ip_chains(router) self._assert_metadata_chains(router) - self._assert_extra_routes(router) self._assert_rfp_fpr_mtu(router, custom_mtu) if enable_snat: ip_versions = [4, 6] if (ip_version == 6 or dual_stack) else [4] @@ -172,7 +171,7 @@ class TestDvrRouter(framework.L3AgentTestFramework): router.router_id) self._assert_onlink_subnet_routes( router, ip_versions, snat_ns_name) - + self._assert_extra_routes(router, namespace=snat_ns_name) self._delete_router(self.agent, router.router_id) self._assert_fip_namespace_deleted(ext_gateway_port) self._assert_router_does_not_exist(router) @@ -182,6 +181,7 @@ class TestDvrRouter(framework.L3AgentTestFramework): enable_ha=False, enable_snat=False, agent=None, + extra_routes=False, **kwargs): if not agent: agent = self.agent @@ -189,6 +189,8 @@ class TestDvrRouter(framework.L3AgentTestFramework): enable_snat=enable_snat, enable_floating_ip=True, enable_ha=enable_ha, + extra_routes=extra_routes, + num_internal_ports=2, **kwargs) internal_ports = router.get(l3_constants.INTERFACE_KEY, []) router['distributed'] = True @@ -644,6 +646,24 @@ class TestDvrRouter(framework.L3AgentTestFramework): self._assert_ip_addresses_in_dvr_ha_snat_namespace(router2) self._assert_no_ip_addresses_in_dvr_ha_snat_namespace(router1) + def test_dvr_router_static_routes(self): + """Test to validate the extra routes on dvr routers.""" + self.agent.conf.agent_mode = 'dvr_snat' + router_info = self.generate_dvr_router_info(enable_snat=True) + router1 = self.manage_router(self.agent, router_info) + self.assertTrue(self._namespace_exists(router1.ns_name)) + self._assert_snat_namespace_exists(router1) + snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name( + router1.router_id) + # Now try to add routes that are suitable for both the + # router namespace and the snat namespace. + router1.router['routes'] = [{'destination': '8.8.4.0/24', + 'nexthop': '35.4.0.20'}] + self.agent._process_updated_router(router1.router) + router_updated = self.agent.router_info[router_info['id']] + self._assert_extra_routes(router_updated, namespace=snat_ns_name) + self._assert_extra_routes(router_updated) + def _assert_fip_namespace_deleted(self, ext_gateway_port): ext_net_id = ext_gateway_port['network_id'] self.agent.fipnamespace_delete_on_ext_net( diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index f187f1e41d1..2953125e5e3 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -1096,14 +1096,14 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): router) self.assertEqual(1, self.send_adv_notif.call_count) - def test_update_routing_table(self): - # Just verify the correct namespace was used in the call + def _test_update_routing_table(self, is_snat_host=True): router = l3_test_common.prepare_router_data() uuid = router['id'] - netns = 'snat-' + uuid + s_netns = 'snat-' + uuid + q_netns = 'qrouter-' + uuid fake_route1 = {'destination': '135.207.0.0/16', - 'nexthop': '1.2.3.4'} - + 'nexthop': '19.4.4.200'} + calls = [mock.call('replace', fake_route1, q_netns)] agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) ri = dvr_router.DvrEdgeRouter( agent, @@ -1113,10 +1113,19 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): **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) + 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: + 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) + + def test_process_update_snat_routing_table(self): + self._test_update_routing_table() + + def test_process_not_update_snat_routing_table(self): + self._test_update_routing_table(is_snat_host=False) def test_process_router_interface_added(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py index f1378af2d33..4921d151965 100644 --- a/neutron/tests/unit/agent/l3/test_router_info.py +++ b/neutron/tests/unit/agent/l3/test_router_info.py @@ -97,7 +97,7 @@ class TestRouterInfo(base.BaseTestCase): 'nexthop': "10.100.10.30"}] ri.routes = fake_old_routes ri.router['routes'] = fake_new_routes - ri.routes_updated() + ri.routes_updated(fake_old_routes, fake_new_routes) expected = [['ip', 'route', 'replace', 'to', '110.100.30.0/24', 'via', '10.100.10.30'], @@ -105,18 +105,18 @@ class TestRouterInfo(base.BaseTestCase): 'via', '10.100.10.30']] self._check_agent_method_called(expected) - + ri.routes = fake_new_routes fake_new_routes = [{'destination': "110.100.30.0/24", 'nexthop': "10.100.10.30"}] ri.router['routes'] = fake_new_routes - ri.routes_updated() + ri.routes_updated(ri.routes, fake_new_routes) expected = [['ip', 'route', 'delete', 'to', '110.100.31.0/24', 'via', '10.100.10.30']] self._check_agent_method_called(expected) fake_new_routes = [] ri.router['routes'] = fake_new_routes - ri.routes_updated() + ri.routes_updated(ri.routes, fake_new_routes) expected = [['ip', 'route', 'delete', 'to', '110.100.30.0/24', 'via', '10.100.10.30']]