From cec87e77f6f3999623aeefd1f1bba1b560119e67 Mon Sep 17 00:00:00 2001 From: Assaf Muller Date: Mon, 26 Jan 2015 16:22:16 -0500 Subject: [PATCH] Move extra routes processing to router classes Move legacy and HA routers implementation of the extra routes extension to their respective router classes. Move the unit tests as well. Partially-Implements: blueprint restructure-l3-agent Change-Id: Iec642dde3e1f2ffdfae4cdfb9cbdc777cad52a39 --- neutron/agent/l3/agent.py | 31 +---- neutron/agent/l3/ha_router.py | 6 +- neutron/agent/l3/router_info.py | 31 +++++ neutron/tests/unit/agent/l3/__init__.py | 0 .../tests/unit/agent/l3/test_router_info.py | 110 ++++++++++++++++++ neutron/tests/unit/test_l3_agent.py | 97 +-------------- 6 files changed, 151 insertions(+), 124 deletions(-) create mode 100644 neutron/tests/unit/agent/l3/__init__.py create mode 100644 neutron/tests/unit/agent/l3/test_router_info.py diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index 844911d22..5d5ef5987 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -598,7 +598,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self._process_internal_ports(ri) self._process_external(ri) # Process static routes for router - self.routes_updated(ri) + ri.routes_updated() # Enable or disable keepalived for ha routers self._process_ha_router(ri) @@ -1207,35 +1207,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, # When L3 agent is ready, we immediately do a full sync self.periodic_sync_routers_task(self.context) - def _update_routing_table(self, ri, operation, route): - cmd = ['ip', 'route', operation, 'to', route['destination'], - 'via', route['nexthop']] - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - namespace=ri.ns_name) - ip_wrapper.netns.execute(cmd, check_exit_code=False) - - def routes_updated(self, ri): - new_routes = ri.router['routes'] - if ri.is_ha: - ri._process_virtual_routes(new_routes) - return - - old_routes = ri.routes - adds, removes = common_utils.diff_list_of_dict(old_routes, - new_routes) - for route in adds: - LOG.debug("Added route entry is '%s'", route) - # remove replaced route from deleted route - for del_route in removes: - if route['destination'] == del_route['destination']: - removes.remove(del_route) - #replace success even if there is no existing route - self._update_routing_table(ri, 'replace', route) - for route in removes: - LOG.debug("Removed route entry is '%s'", route) - self._update_routing_table(ri, 'delete', route) - ri.routes = new_routes - class L3NATAgentWithStateReport(L3NATAgent): diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index 01b013b6d..f96c1344b 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -172,7 +172,9 @@ class HaRouter(router.RouterInfo): def _ha_external_gateway_removed(self, interface_name): self._clear_vips(interface_name) - def _process_virtual_routes(self, new_routes): + def routes_updated(self): + new_routes = self.router['routes'] + instance = self._get_keepalived_instance() # Filter out all of the old routes while keeping only the default route @@ -183,6 +185,8 @@ class HaRouter(router.RouterInfo): route['destination'], route['nexthop'])) + self.routes = new_routes + def _add_default_gw_virtual_route(self, ex_gw_port, interface_name): gw_ip = ex_gw_port['subnet']['gateway_ip'] if gw_ip: diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index c76cf76c9..581163c32 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -12,7 +12,12 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron.agent.linux import ip_lib from neutron.agent.linux import iptables_manager +from neutron.common import utils as common_utils +from neutron.openstack.common import log as logging + +LOG = logging.getLogger(__name__) class RouterInfo(object): @@ -73,3 +78,29 @@ class RouterInfo(object): snat_callback(self, self._router.get('gw_port'), *args, action=self._snat_action) self._snat_action = None + + def _update_routing_table(self, operation, route): + cmd = ['ip', 'route', operation, 'to', route['destination'], + 'via', route['nexthop']] + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + namespace=self.ns_name) + ip_wrapper.netns.execute(cmd, check_exit_code=False) + + def routes_updated(self): + new_routes = self.router['routes'] + + old_routes = self.routes + adds, removes = common_utils.diff_list_of_dict(old_routes, + new_routes) + for route in adds: + LOG.debug("Added route entry is '%s'", route) + # remove replaced route from deleted route + for del_route in removes: + 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) + for route in removes: + LOG.debug("Removed route entry is '%s'", route) + self._update_routing_table('delete', route) + self.routes = new_routes diff --git a/neutron/tests/unit/agent/l3/__init__.py b/neutron/tests/unit/agent/l3/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py new file mode 100644 index 000000000..126ea0d79 --- /dev/null +++ b/neutron/tests/unit/agent/l3/test_router_info.py @@ -0,0 +1,110 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock + +from neutron.agent.common import config as agent_config +from neutron.agent.l3 import router_info +from neutron.openstack.common import uuidutils +from neutron.tests import base + + +_uuid = uuidutils.generate_uuid + + +class TestRouterInfo(base.BaseTestCase): + def setUp(self): + super(TestRouterInfo, self).setUp() + + conf = agent_config.setup_conf() + agent_config.register_root_helper(conf) + conf.root_helper = 'sudo' + + self.ip_cls_p = mock.patch('neutron.agent.linux.ip_lib.IPWrapper') + ip_cls = self.ip_cls_p.start() + self.mock_ip = mock.MagicMock() + ip_cls.return_value = self.mock_ip + self.ri_kwargs = {'root_helper': conf.root_helper, + '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 test_routing_table_update(self): + ri = router_info.RouterInfo(_uuid(), {}, ns_name=_uuid(), + **self.ri_kwargs) + ri.router = {} + + fake_route1 = {'destination': '135.207.0.0/16', + 'nexthop': '1.2.3.4'} + fake_route2 = {'destination': '135.207.111.111/32', + '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) + + 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) + 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) + expected = [['ip', 'route', 'delete', 'to', '135.207.111.111/32', + 'via', '1.2.3.4']] + self._check_agent_method_called(expected) + + def test_routes_updated(self): + ri = router_info.RouterInfo(_uuid(), {}, ns_name=_uuid(), + **self.ri_kwargs) + ri.router = {} + + fake_old_routes = [] + fake_new_routes = [{'destination': "110.100.31.0/24", + 'nexthop': "10.100.10.30"}, + {'destination': "110.100.30.0/24", + 'nexthop': "10.100.10.30"}] + ri.routes = fake_old_routes + ri.router['routes'] = fake_new_routes + ri.routes_updated() + + 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) + + fake_new_routes = [{'destination': "110.100.30.0/24", + 'nexthop': "10.100.10.30"}] + ri.router['routes'] = fake_new_routes + ri.routes_updated() + 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() + + expected = [['ip', 'route', 'delete', 'to', '110.100.30.0/24', + 'via', '10.100.10.30']] + self._check_agent_method_called(expected) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index ffffae77c..7206e4b3b 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -173,10 +173,10 @@ def get_ha_interface(ip='169.254.192.1', mac='12:34:56:78:2b:5d'): 'priority': 1} -class TestBasicRouterOperations(base.BaseTestCase): +class BasicRouterOperationsFramework(base.BaseTestCase): def setUp(self): - super(TestBasicRouterOperations, self).setUp() + super(BasicRouterOperationsFramework, self).setUp() self.conf = agent_config.setup_conf() self.conf.register_opts(base_config.core_opts) self.conf.register_cli_opts(log.common_cli_opts) @@ -296,6 +296,8 @@ class TestBasicRouterOperations(base.BaseTestCase): return agent, ri, port + +class TestBasicRouterOperations(BasicRouterOperationsFramework): def test_periodic_sync_routers_task_raise_exception(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) self.plugin_api.get_routers.side_effect = ValueError() @@ -563,97 +565,6 @@ class TestBasicRouterOperations(base.BaseTestCase): router['gw_port_host'] = HOSTNAME self._test_external_gateway_action('remove', router) - def _check_agent_method_called(self, agent, calls, namespace): - self.mock_ip.netns.execute.assert_has_calls( - [mock.call(call, check_exit_code=False) for call in calls], - any_order=True) - - def _test_routing_table_update(self, namespace): - if not namespace: - self.conf.set_override('use_namespaces', False) - - router_id = _uuid() - ri = l3router.RouterInfo(router_id, {}, **self.ri_kwargs) - agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) - - fake_route1 = {'destination': '135.207.0.0/16', - 'nexthop': '1.2.3.4'} - fake_route2 = {'destination': '135.207.111.111/32', - 'nexthop': '1.2.3.4'} - - agent._update_routing_table(ri, 'replace', fake_route1) - expected = [['ip', 'route', 'replace', 'to', '135.207.0.0/16', - 'via', '1.2.3.4']] - self._check_agent_method_called(agent, expected, namespace) - - agent._update_routing_table(ri, 'delete', fake_route1) - expected = [['ip', 'route', 'delete', 'to', '135.207.0.0/16', - 'via', '1.2.3.4']] - self._check_agent_method_called(agent, expected, namespace) - - agent._update_routing_table(ri, 'replace', fake_route2) - expected = [['ip', 'route', 'replace', 'to', '135.207.111.111/32', - 'via', '1.2.3.4']] - self._check_agent_method_called(agent, expected, namespace) - - agent._update_routing_table(ri, 'delete', fake_route2) - expected = [['ip', 'route', 'delete', 'to', '135.207.111.111/32', - 'via', '1.2.3.4']] - self._check_agent_method_called(agent, expected, namespace) - - def test_agent_routing_table_updated(self): - self._test_routing_table_update(namespace=True) - - def test_agent_routing_table_updated_no_namespace(self): - self._test_routing_table_update(namespace=False) - - def test_routes_updated(self): - self._test_routes_updated(namespace=True) - - def test_routes_updated_no_namespace(self): - self._test_routes_updated(namespace=False) - - def _test_routes_updated(self, namespace=True): - if not namespace: - self.conf.set_override('use_namespaces', False) - agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) - router_id = _uuid() - - ri = l3router.RouterInfo(router_id, {}, **self.ri_kwargs) - ri.router = {} - - fake_old_routes = [] - fake_new_routes = [{'destination': "110.100.31.0/24", - 'nexthop': "10.100.10.30"}, - {'destination': "110.100.30.0/24", - 'nexthop': "10.100.10.30"}] - ri.routes = fake_old_routes - ri.router['routes'] = fake_new_routes - agent.routes_updated(ri) - - 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(agent, expected, namespace) - - fake_new_routes = [{'destination': "110.100.30.0/24", - 'nexthop': "10.100.10.30"}] - ri.router['routes'] = fake_new_routes - agent.routes_updated(ri) - expected = [['ip', 'route', 'delete', 'to', '110.100.31.0/24', - 'via', '10.100.10.30']] - - self._check_agent_method_called(agent, expected, namespace) - fake_new_routes = [] - ri.router['routes'] = fake_new_routes - agent.routes_updated(ri) - - expected = [['ip', 'route', 'delete', 'to', '110.100.30.0/24', - 'via', '10.100.10.30']] - self._check_agent_method_called(agent, expected, namespace) - def _verify_snat_rules(self, rules, router, negate=False): interfaces = router[l3_constants.INTERFACE_KEY] source_cidrs = [] -- 2.45.2