From: Assaf Muller Date: Mon, 26 Jan 2015 21:22:16 +0000 (-0500) Subject: Move extra routes processing to router classes X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=cec87e77f6f3999623aeefd1f1bba1b560119e67;p=openstack-build%2Fneutron-build.git 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 --- 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 = []