From 998b22b38353c6296529e7abf9bb1184aa838868 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 8 Jun 2020 18:09:29 +0000 Subject: [PATCH] Implement "RouterInfo.update_routing_table" with Pyroute2 Change-Id: I715172e6bd64fcdd34ce10956fd4fdc20887a70a Related-Bug: #1492714 --- neutron/agent/l3/dvr_edge_router.py | 3 +- neutron/agent/l3/router_info.py | 11 ++-- .../tests/unit/agent/l3/test_router_info.py | 55 +++++++++---------- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index b1ca09ecf7a..de27f9bb8fa 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -223,8 +223,7 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): # 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(): - super(DvrEdgeRouter, self)._update_routing_table( - operation, route, namespace=ns_name) + self._update_routing_table(operation, route, ns_name) else: LOG.error("The SNAT namespace %s does not exist for " "the router.", ns_name) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 96f219095f3..2f418b7dc1e 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -20,6 +20,7 @@ from neutron_lib import constants as lib_constants from neutron_lib.exceptions import l3 as l3_exc from neutron_lib.utils import helpers from oslo_log import log as logging +from pyroute2.netlink import exceptions as pyroute2_exc from neutron._i18n import _ from neutron.agent.l3 import namespaces @@ -173,10 +174,12 @@ class RouterInfo(BaseRouterInfo): return True def _update_routing_table(self, operation, route, namespace): - cmd = ['ip', 'route', operation, 'to', route['destination'], - 'via', route['nexthop']] - ip_wrapper = ip_lib.IPWrapper(namespace=namespace) - ip_wrapper.netns.execute(cmd, check_exit_code=False) + method = (ip_lib.add_ip_route if operation == 'replace' else + ip_lib.delete_ip_route) + try: + method(namespace, route['destination'], via=route['nexthop']) + except (RuntimeError, OSError, pyroute2_exc.NetlinkError): + pass def update_routing_table(self, operation, route): self._update_routing_table(operation, route, self.ns_name) diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py index 6837e7d0341..34f68913e0d 100644 --- a/neutron/tests/unit/agent/l3/test_router_info.py +++ b/neutron/tests/unit/agent/l3/test_router_info.py @@ -37,13 +37,20 @@ class TestRouterInfo(base.BaseTestCase): ip_cls = self.ip_cls_p.start() self.mock_ip = mock.MagicMock() ip_cls.return_value = self.mock_ip + self.mock_add_ip_route = mock.patch.object( + ip_lib, 'add_ip_route').start() + self.mock_delete_ip_route = mock.patch.object( + ip_lib, 'delete_ip_route').start() self.ri_kwargs = {'agent_conf': conf, 'interface_driver': mock.sentinel.interface_driver} - def _check_agent_method_called(self, calls): - self.mock_ip.netns.execute.assert_has_calls( - [mock.call(call, check_exit_code=False) for call in calls], - any_order=True) + def _check_agent_method_called(self, router, action_calls): + for action, calls in action_calls.items(): + mock_calls = [mock.call(router.ns_name, c[0], via=c[1]) + for c in calls] + mock_method = (self.mock_add_ip_route if action == 'replace' else + self.mock_delete_ip_route) + mock_method.assert_has_calls(mock_calls, any_order=True) def test_routing_table_update(self): ri = router_info.RouterInfo(mock.Mock(), _uuid(), {}, **self.ri_kwargs) @@ -55,24 +62,20 @@ class TestRouterInfo(base.BaseTestCase): 'nexthop': '1.2.3.4'} 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) + expected = {'replace': [('135.207.0.0/16', '1.2.3.4')]} + self._check_agent_method_called(ri, expected) 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) + expected = {'delete': [('135.207.0.0/16', '1.2.3.4')]} + self._check_agent_method_called(ri, expected) 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) + expected = {'replace': [('135.207.111.111/32', '1.2.3.4')]} + self._check_agent_method_called(ri, expected) 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) + expected = {'delete': [('135.207.111.111/32', '1.2.3.4')]} + self._check_agent_method_called(ri, expected) def test_update_routing_table(self): # Just verify the correct namespace was used in the call @@ -103,28 +106,22 @@ class TestRouterInfo(base.BaseTestCase): ri.router['routes'] = fake_new_routes ri.routes_updated(fake_old_routes, fake_new_routes) - expected = [['ip', 'route', 'replace', 'to', '110.100.30.0/24', - 'via', '10.100.10.30'], - ['ip', 'route', 'replace', 'to', '110.100.31.0/24', - 'via', '10.100.10.30']] - - self._check_agent_method_called(expected) + expected = {'replace': [('110.100.30.0/24', '10.100.10.30'), + ('110.100.31.0/24', '10.100.10.30')]} + self._check_agent_method_called(ri, 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, fake_new_routes) - expected = [['ip', 'route', 'delete', 'to', '110.100.31.0/24', - 'via', '10.100.10.30']] - - self._check_agent_method_called(expected) + expected = {'delete': [('110.100.31.0/24', '10.100.10.30')]} + self._check_agent_method_called(ri, expected) fake_new_routes = [] ri.router['routes'] = fake_new_routes ri.routes_updated(ri.routes, fake_new_routes) - expected = [['ip', 'route', 'delete', 'to', '110.100.30.0/24', - 'via', '10.100.10.30']] - self._check_agent_method_called(expected) + expected = {'delete': [('110.100.30.0/24', '10.100.10.30')]} + self._check_agent_method_called(ri, expected) def test__process_pd_iptables_rules(self): subnet_id = _uuid()