Merge "Use wrappers instead of direct calls to ip route."
This commit is contained in:
commit
bea61bf94e
@ -566,7 +566,7 @@ class DeviceManager(object):
|
||||
if namespace is None:
|
||||
device = ip_lib.IPDevice(interface_name,
|
||||
self.root_helper)
|
||||
device.route.pullup_route(interface_name)
|
||||
device.route.pullup_route()
|
||||
|
||||
if self.conf.enable_metadata_network:
|
||||
meta_cidr = netaddr.IPNetwork(METADATA_DEFAULT_IP)
|
||||
|
@ -416,14 +416,8 @@ class L3NATAgent(manager.Manager):
|
||||
|
||||
gw_ip = ex_gw_port['subnet']['gateway_ip']
|
||||
if ex_gw_port['subnet']['gateway_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)
|
||||
ip = ip_lib.IPWrapper(self.root_helper, ri.ns_name())
|
||||
ip.route.add_gateway(gw_ip)
|
||||
|
||||
for (c, r) in self.external_gateway_nat_rules(ex_gw_ip,
|
||||
internal_cidrs,
|
||||
@ -645,19 +639,9 @@ class L3NATAgent(manager.Manager):
|
||||
def after_start(self):
|
||||
LOG.info(_("L3 agent started"))
|
||||
|
||||
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):
|
||||
ip = ip_lib.IPWrapper(self.conf.root_helper, ri.ns_name())
|
||||
|
||||
new_routes = ri.router['routes']
|
||||
old_routes = ri.routes
|
||||
adds, removes = common_utils.diff_list_of_dict(old_routes,
|
||||
@ -668,11 +652,11 @@ class L3NATAgent(manager.Manager):
|
||||
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)
|
||||
|
||||
ip.route.add(route['destination'], route['nexthop'])
|
||||
for route in removes:
|
||||
LOG.debug(_("Removed route entry is '%s'"), route)
|
||||
self._update_routing_table(ri, 'delete', route)
|
||||
ip.route.delete(route['destination'], route['nexthop'])
|
||||
ri.routes = new_routes
|
||||
|
||||
|
||||
|
@ -63,6 +63,7 @@ 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)
|
||||
@ -134,7 +135,7 @@ class IPDevice(SubProcessBase):
|
||||
self.name = name
|
||||
self.link = IpLinkCommand(self)
|
||||
self.addr = IpAddrCommand(self)
|
||||
self.route = IpRouteCommand(self)
|
||||
self.route = IpRouteDeviceCommand(self)
|
||||
|
||||
def __eq__(self, other):
|
||||
return (other is not None and self.name == other.name
|
||||
@ -300,35 +301,37 @@ class IpAddrCommand(IpDeviceCommandBase):
|
||||
return retval
|
||||
|
||||
|
||||
class IpRouteCommand(IpDeviceCommandBase):
|
||||
class IpRouteCommand(IpCommandBase):
|
||||
COMMAND = 'route'
|
||||
|
||||
def add_gateway(self, gateway, metric=None):
|
||||
args = ['replace', 'default', 'via', gateway]
|
||||
def add_gateway(self, gw, metric=None, dev=None):
|
||||
"""Adds a default gateway or replaces an existing one."""
|
||||
|
||||
args = ['replace', 'default', 'via', gw]
|
||||
if metric:
|
||||
args += ['metric', metric]
|
||||
args += ['dev', self.name]
|
||||
if dev:
|
||||
args += ['dev', dev]
|
||||
|
||||
self._as_root(*args)
|
||||
|
||||
def delete_gateway(self, gateway):
|
||||
self._as_root('del',
|
||||
'default',
|
||||
'via',
|
||||
gateway,
|
||||
'dev',
|
||||
self.name)
|
||||
def delete_gateway(self, gw, dev=None):
|
||||
args = ['delete', 'default', 'via', gw]
|
||||
if dev:
|
||||
args += ['dev', dev]
|
||||
|
||||
def get_gateway(self, scope=None, filters=None):
|
||||
if filters is None:
|
||||
filters = []
|
||||
|
||||
retval = None
|
||||
self._as_root(*args)
|
||||
|
||||
def get_gateway(self, scope=None, filters=None, dev=None):
|
||||
args = ['list']
|
||||
if dev:
|
||||
args += ['dev', self.name]
|
||||
if scope:
|
||||
filters += ['scope', scope]
|
||||
args += ['scope', scope]
|
||||
if filters:
|
||||
args += filters
|
||||
|
||||
route_list_lines = self._run('list', 'dev', self.name,
|
||||
*filters).split('\n')
|
||||
route_list_lines = self._run(*args).split('\n')
|
||||
default_route_line = next((x.strip() for x in
|
||||
route_list_lines if
|
||||
x.strip().startswith('default')), None)
|
||||
@ -383,6 +386,49 @@ class IpRouteCommand(IpDeviceCommandBase):
|
||||
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'
|
||||
|
@ -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')
|
||||
ip_cls = self.ip_cls_p.start()
|
||||
self.ip_cls = self.ip_cls_p.start()
|
||||
self.mock_ip = mock.MagicMock()
|
||||
ip_cls.return_value = self.mock_ip
|
||||
self.ip_cls.return_value = self.mock_ip
|
||||
|
||||
self.l3pluginApi_cls_p = mock.patch(
|
||||
'quantum.agent.l3_agent.L3PluginApi')
|
||||
@ -210,57 +210,15 @@ class TestBasicRouterOperations(base.BaseTestCase):
|
||||
def testAgentRemoveFloatingIP(self):
|
||||
self._test_floating_ip_action('remove')
|
||||
|
||||
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],
|
||||
def _check_route_calls(self, action, calls, namespace):
|
||||
mocks = {
|
||||
'replace': self.mock_ip.route.add,
|
||||
'delete': self.mock_ip.route.delete
|
||||
}
|
||||
|
||||
m = mocks[action]
|
||||
m.assert_has_calls([mock.call(*call) 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)
|
||||
|
||||
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)
|
||||
@ -288,28 +246,24 @@ class TestBasicRouterOperations(base.BaseTestCase):
|
||||
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']]
|
||||
expected = [['110.100.30.0/24', '10.100.10.30'],
|
||||
['110.100.31.0/24', '10.100.10.30']]
|
||||
|
||||
self._check_agent_method_called(agent, expected, namespace)
|
||||
self._check_route_calls('replace', expected, ri.ns_name())
|
||||
|
||||
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']]
|
||||
expected = [['110.100.31.0/24', '10.100.10.30']]
|
||||
|
||||
self._check_agent_method_called(agent, expected, namespace)
|
||||
self._check_route_calls('delete', expected, ri.ns_name())
|
||||
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)
|
||||
expected = [['110.100.30.0/24', '10.100.10.30']]
|
||||
self._check_route_calls('delete', expected, ri.ns_name())
|
||||
|
||||
def testProcessRouter(self):
|
||||
|
||||
|
@ -549,25 +549,54 @@ 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
|
||||
self.route_cmd.add_gateway(gateway, metric)
|
||||
dev = 'eth0'
|
||||
|
||||
self.route_cmd.add_gateway(gateway, metric, dev)
|
||||
self._assert_sudo([],
|
||||
('replace', 'default', 'via', gateway,
|
||||
'metric', metric,
|
||||
'dev', self.parent.name))
|
||||
'dev', dev))
|
||||
|
||||
def test_del_gateway(self):
|
||||
def test_delete_gateway_with_dev(self):
|
||||
gateway = '192.168.45.100'
|
||||
self.route_cmd.delete_gateway(gateway)
|
||||
dev = 'eth0'
|
||||
|
||||
self.route_cmd.delete_gateway(gateway, dev)
|
||||
self._assert_sudo([],
|
||||
('del', 'default', 'via', gateway,
|
||||
'dev', self.parent.name))
|
||||
('delete', 'default', 'via', gateway,
|
||||
'dev', dev))
|
||||
|
||||
def test_get_gateway(self):
|
||||
test_cases = [{'sample': GATEWAY_SAMPLE1,
|
||||
@ -614,6 +643,59 @@ 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()
|
||||
|
Loading…
Reference in New Issue
Block a user