diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py index 3c32bf5ce0..3993b56ce6 100644 --- a/quantum/agent/dhcp_agent.py +++ b/quantum/agent/dhcp_agent.py @@ -566,7 +566,7 @@ class DeviceManager(object): if namespace is None: device = ip_lib.IPDevice(interface_name, self.root_helper) - device.route.pullup_route() + device.route.pullup_route(interface_name) if self.conf.enable_metadata_network: meta_cidr = netaddr.IPNetwork(METADATA_DEFAULT_IP) diff --git a/quantum/agent/l3_agent.py b/quantum/agent/l3_agent.py index 345df17b47..39ea511fc1 100644 --- a/quantum/agent/l3_agent.py +++ b/quantum/agent/l3_agent.py @@ -416,8 +416,14 @@ class L3NATAgent(manager.Manager): gw_ip = ex_gw_port['subnet']['gateway_ip'] if ex_gw_port['subnet']['gateway_ip']: - ip = ip_lib.IPWrapper(self.root_helper, ri.ns_name()) - ip.route.add_gateway(gw_ip) + cmd = ['route', 'add', 'default', 'gw', gw_ip] + if self.conf.use_namespaces: + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + namespace=ri.ns_name()) + ip_wrapper.netns.execute(cmd, check_exit_code=False) + else: + utils.execute(cmd, check_exit_code=False, + root_helper=self.root_helper) for (c, r) in self.external_gateway_nat_rules(ex_gw_ip, internal_cidrs, @@ -639,9 +645,19 @@ class L3NATAgent(manager.Manager): def after_start(self): LOG.info(_("L3 agent started")) - def routes_updated(self, ri): - ip = ip_lib.IPWrapper(self.conf.root_helper, ri.ns_name()) + def _update_routing_table(self, ri, operation, route): + cmd = ['ip', 'route', operation, 'to', route['destination'], + 'via', route['nexthop']] + #TODO(nati) move this code to iplib + if self.conf.use_namespaces: + ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper, + namespace=ri.ns_name()) + ip_wrapper.netns.execute(cmd, check_exit_code=False) + else: + utils.execute(cmd, check_exit_code=False, + root_helper=self.conf.root_helper) + def routes_updated(self, ri): new_routes = ri.router['routes'] old_routes = ri.routes adds, removes = common_utils.diff_list_of_dict(old_routes, @@ -652,11 +668,11 @@ class L3NATAgent(manager.Manager): for del_route in removes: if route['destination'] == del_route['destination']: removes.remove(del_route) - - ip.route.add(route['destination'], route['nexthop']) + #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) - ip.route.delete(route['destination'], route['nexthop']) + self._update_routing_table(ri, 'delete', route) ri.routes = new_routes diff --git a/quantum/agent/linux/ip_lib.py b/quantum/agent/linux/ip_lib.py index 0d1c046a74..5207c230e7 100644 --- a/quantum/agent/linux/ip_lib.py +++ b/quantum/agent/linux/ip_lib.py @@ -63,7 +63,6 @@ class IPWrapper(SubProcessBase): super(IPWrapper, self).__init__(root_helper=root_helper, namespace=namespace) self.netns = IpNetnsCommand(self) - self.route = IpRouteCommand(self) def device(self, name): return IPDevice(name, self.root_helper, self.namespace) @@ -135,7 +134,7 @@ class IPDevice(SubProcessBase): self.name = name self.link = IpLinkCommand(self) self.addr = IpAddrCommand(self) - self.route = IpRouteDeviceCommand(self) + self.route = IpRouteCommand(self) def __eq__(self, other): return (other is not None and self.name == other.name @@ -301,37 +300,35 @@ class IpAddrCommand(IpDeviceCommandBase): return retval -class IpRouteCommand(IpCommandBase): +class IpRouteCommand(IpDeviceCommandBase): COMMAND = 'route' - def add_gateway(self, gw, metric=None, dev=None): - """Adds a default gateway or replaces an existing one.""" - - args = ['replace', 'default', 'via', gw] + def add_gateway(self, gateway, metric=None): + args = ['replace', 'default', 'via', gateway] if metric: args += ['metric', metric] - if dev: - args += ['dev', dev] - + args += ['dev', self.name] self._as_root(*args) - def delete_gateway(self, gw, dev=None): - args = ['delete', 'default', 'via', gw] - if dev: - args += ['dev', dev] + def delete_gateway(self, gateway): + self._as_root('del', + 'default', + 'via', + gateway, + 'dev', + self.name) - self._as_root(*args) + def get_gateway(self, scope=None, filters=None): + if filters is None: + filters = [] + + retval = None - def get_gateway(self, scope=None, filters=None, dev=None): - args = ['list'] - if dev: - args += ['dev', self.name] if scope: - args += ['scope', scope] - if filters: - args += filters + filters += ['scope', scope] - route_list_lines = self._run(*args).split('\n') + route_list_lines = self._run('list', 'dev', self.name, + *filters).split('\n') default_route_line = next((x.strip() for x in route_list_lines if x.strip().startswith('default')), None) @@ -344,7 +341,7 @@ class IpRouteCommand(IpCommandBase): if parts_has_metric: retval.update(metric=int(parts[metric_index])) - return retval + return retval def pullup_route(self, interface_name): """ @@ -386,49 +383,6 @@ class IpRouteCommand(IpCommandBase): self._as_root('append', subnet, 'proto', 'kernel', 'dev', device) - def add(self, destination, nexthop, dev=None): - """Adds a new route or replaces an existing one.""" - - args = ['replace', 'to', destination, 'via', nexthop] - if dev: - args += ['dev', dev] - - self._as_root(*args) - - def delete(self, destination, nexthop, dev=None): - args = ['delete', 'to', destination, 'via', nexthop] - if dev: - args += ['dev', dev] - - self._as_root(*args) - - -class IpRouteDeviceCommand(IpDeviceCommandBase): - """Wrapper for ip route actions which are bound to a specific device""" - - def __init__(self, parent): - super(IpRouteDeviceCommand, self).__init__(parent) - - self._route = IpRouteCommand(parent) - - def add_gateway(self, gw, metric=None): - return self._route.add_gateway(gw, metric, self.name) - - def delete_gateway(self, gw): - return self._route.delete_gateway(gw, self.name) - - def get_gateway(self, scope=None, filters=None): - return self._route.get_gateway(scope, filters, self.name) - - def pullup_route(self): - return self._route.pullup_route(self.name) - - def add(self, destination, nexthop): - return self._route.add(destination, nexthop, self.name) - - def delete(self, destination, nexthop): - return self._route.delete(destination, nexthop, self.name) - class IpNetnsCommand(IpCommandBase): COMMAND = 'netns' diff --git a/quantum/tests/unit/test_l3_agent.py b/quantum/tests/unit/test_l3_agent.py index 538c342163..0ac24a2c04 100644 --- a/quantum/tests/unit/test_l3_agent.py +++ b/quantum/tests/unit/test_l3_agent.py @@ -66,9 +66,9 @@ class TestBasicRouterOperations(base.BaseTestCase): driver_cls.return_value = self.mock_driver self.ip_cls_p = mock.patch('quantum.agent.linux.ip_lib.IPWrapper') - self.ip_cls = self.ip_cls_p.start() + ip_cls = self.ip_cls_p.start() self.mock_ip = mock.MagicMock() - self.ip_cls.return_value = self.mock_ip + ip_cls.return_value = self.mock_ip self.l3pluginApi_cls_p = mock.patch( 'quantum.agent.l3_agent.L3PluginApi') @@ -210,15 +210,57 @@ class TestBasicRouterOperations(base.BaseTestCase): def testAgentRemoveFloatingIP(self): self._test_floating_ip_action('remove') - def _check_route_calls(self, action, calls, namespace): - mocks = { - 'replace': self.mock_ip.route.add, - 'delete': self.mock_ip.route.delete - } + def _check_agent_method_called(self, agent, calls, namespace): + if namespace: + self.mock_ip.netns.execute.assert_has_calls( + [mock.call(call, check_exit_code=False) for call in calls], + any_order=True) + else: + self.utils_exec.assert_has_calls([ + mock.call(call, root_helper='sudo', + check_exit_code=False) for call in calls], + any_order=True) - m = mocks[action] - m.assert_has_calls([mock.call(*call) 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 = l3_agent.RouterInfo(router_id, self.conf.root_helper, + self.conf.use_namespaces, + None) + 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 testAgentRoutingTableUpdated(self): + self._test_routing_table_update(namespace=True) + + def testAgentRoutingTableUpdatedNoNameSpace(self): + self._test_routing_table_update(namespace=False) def testRoutesUpdated(self): self._test_routes_updated(namespace=True) @@ -246,24 +288,28 @@ class TestBasicRouterOperations(base.BaseTestCase): ri.router['routes'] = fake_new_routes agent.routes_updated(ri) - expected = [['110.100.30.0/24', '10.100.10.30'], - ['110.100.31.0/24', '10.100.10.30']] + 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_route_calls('replace', expected, ri.ns_name()) + 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 = [['110.100.31.0/24', '10.100.10.30']] + expected = [['ip', 'route', 'delete', 'to', '110.100.31.0/24', + 'via', '10.100.10.30']] - self._check_route_calls('delete', expected, ri.ns_name()) + self._check_agent_method_called(agent, expected, namespace) fake_new_routes = [] ri.router['routes'] = fake_new_routes agent.routes_updated(ri) - expected = [['110.100.30.0/24', '10.100.10.30']] - self._check_route_calls('delete', expected, ri.ns_name()) + expected = [['ip', 'route', 'delete', 'to', '110.100.30.0/24', + 'via', '10.100.10.30']] + self._check_agent_method_called(agent, expected, namespace) def testProcessRouter(self): diff --git a/quantum/tests/unit/test_linux_ip_lib.py b/quantum/tests/unit/test_linux_ip_lib.py index 25b0227ec7..47b40631d9 100644 --- a/quantum/tests/unit/test_linux_ip_lib.py +++ b/quantum/tests/unit/test_linux_ip_lib.py @@ -549,54 +549,25 @@ class TestIpAddrCommand(TestIPCmdBase): class TestIpRouteCommand(TestIPCmdBase): def setUp(self): super(TestIpRouteCommand, self).setUp() + self.parent.name = 'eth0' self.command = 'route' self.route_cmd = ip_lib.IpRouteCommand(self.parent) - def test_add(self): - dst = '10.0.1.0/24' - nexthop = '192.168.1.1' - - self.route_cmd.add(dst, nexthop) - self._assert_sudo([], ('replace', 'to', dst, 'via', nexthop)) - - def test_delete(self): - dst = '10.0.1.0/24' - nexthop = '192.168.1.1' - - self.route_cmd.delete(dst, nexthop) - self._assert_sudo([], ('delete', 'to', dst, 'via', nexthop)) - def test_add_gateway(self): - gw = '10.0.1.1' - - self.route_cmd.add_gateway(gw) - self._assert_sudo([], ('replace', 'default', 'via', gw)) - - def test_delete_gateway(self): - gw = '10.0.1.1' - - self.route_cmd.delete_gateway(gw) - self._assert_sudo([], ('delete', 'default', 'via', gw)) - - def test_add_gateway_with_metric_dev(self): gateway = '192.168.45.100' metric = 100 - dev = 'eth0' - - self.route_cmd.add_gateway(gateway, metric, dev) + self.route_cmd.add_gateway(gateway, metric) self._assert_sudo([], ('replace', 'default', 'via', gateway, 'metric', metric, - 'dev', dev)) + 'dev', self.parent.name)) - def test_delete_gateway_with_dev(self): + def test_del_gateway(self): gateway = '192.168.45.100' - dev = 'eth0' - - self.route_cmd.delete_gateway(gateway, dev) + self.route_cmd.delete_gateway(gateway) self._assert_sudo([], - ('delete', 'default', 'via', gateway, - 'dev', dev)) + ('del', 'default', 'via', gateway, + 'dev', self.parent.name)) def test_get_gateway(self): test_cases = [{'sample': GATEWAY_SAMPLE1, @@ -643,59 +614,6 @@ class TestIpRouteCommand(TestIPCmdBase): self.assertEqual(len(self.parent._run.mock_calls), 2) -class TestIpRouteDeviceCommand(TestIPCmdBase): - def setUp(self): - super(TestIpRouteDeviceCommand, self).setUp() - self.parent.name = 'eth0' - self.command = 'route' - self.route_cmd = ip_lib.IpRouteDeviceCommand(self.parent) - - route_p = mock.patch.object(self.route_cmd, '_route') - self._route = route_p.start() - - def test_add_gateway(self): - gw = '10.0.0.1' - metric = 100 - - self.route_cmd.add_gateway(gw, metric) - self._route.add_gateway.assert_called_once_with(gw, metric, - self.route_cmd.name) - - def test_delete_gateway(self): - gw = '10.0.0.1' - - self.route_cmd.delete_gateway(gw) - self._route.delete_gateway.assert_called_once_with(gw, - self.route_cmd.name) - - def test_get_gateway(self): - scope = 'scope' - filters = [] - - self.route_cmd.get_gateway(scope, filters) - self._route.get_gateway.assert_called_once_with(scope, filters, - self.route_cmd.name) - - def test_add(self): - dst = '8.8.8.8' - via = '10.0.1.4' - - self.route_cmd.add(dst, via) - self._route.add.assert_called_once_with(dst, via, self.route_cmd.name) - - def test_delete(self): - dst = '8.8.8.8' - via = '10.0.1.4' - - self.route_cmd.delete(dst, via) - self._route.delete.assert_called_once_with(dst, via, - self.route_cmd.name) - - def test_pullup_route(self): - self.route_cmd.pullup_route() - self._route.pullup_route.assert_called_once_with(self.route_cmd.name) - - class TestIpNetnsCommand(TestIPCmdBase): def setUp(self): super(TestIpNetnsCommand, self).setUp()