From 40f1c8d691f2738246c8c5a427e5600d508f5e95 Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Tue, 11 Aug 2015 15:21:25 +0000 Subject: [PATCH] Refactor IpRouteCommand to allow using it without a device Routing tables are not tied to devices (though routes are), so it makes sense to be able to use IpRouteCommand without a device. Change-Id: I95806eb5823820092d91ec53d9e65c8d7f56f5f5 Partially-Implements: blueprint address-scopes --- neutron/agent/linux/ip_lib.py | 57 ++++++++++++------- neutron/tests/unit/agent/linux/test_ip_lib.py | 57 +++++++++++++++++-- 2 files changed, 87 insertions(+), 27 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index cadbd019f..88597f70f 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -497,30 +497,29 @@ class IpRouteCommand(IpDeviceCommandBase): """Return an instance of IpRouteCommand which works on given table""" return IpRouteCommand(self._parent, table) - def _table_args(self): + def _table_args(self, override=None): + if override: + return ['table', override] return ['table', self._table] if self._table else [] + def _dev_args(self): + return ['dev', self.name] if self.name else [] + def add_gateway(self, gateway, metric=None, table=None): ip_version = get_ip_version(gateway) args = ['replace', 'default', 'via', gateway] if metric: args += ['metric', metric] - args += ['dev', self.name] - if table: - args += ['table', table] - else: - args += self._table_args() + args += self._dev_args() + args += self._table_args(table) self._as_root([ip_version], tuple(args)) def delete_gateway(self, gateway, table=None): ip_version = get_ip_version(gateway) args = ['del', 'default', - 'via', gateway, - 'dev', self.name] - if table: - args += ['table', table] - else: - args += self._table_args() + 'via', gateway] + args += self._dev_args() + args += self._table_args(table) try: self._as_root([ip_version], tuple(args)) except RuntimeError as rte: @@ -532,7 +531,9 @@ class IpRouteCommand(IpDeviceCommandBase): def list_onlink_routes(self, ip_version): def iterate_routes(): - args = ['list', 'dev', self.name, 'scope', 'link'] + args = ['list'] + args += self._dev_args() + args += ['scope', 'link'] args += self._table_args() output = self._run([ip_version], tuple(args)) for line in output.split('\n'): @@ -544,20 +545,25 @@ class IpRouteCommand(IpDeviceCommandBase): def add_onlink_route(self, cidr): ip_version = get_ip_version(cidr) - args = ['replace', cidr, 'dev', self.name, 'scope', 'link'] + args = ['replace', cidr] + args += self._dev_args() + args += ['scope', 'link'] args += self._table_args() self._as_root([ip_version], tuple(args)) def delete_onlink_route(self, cidr): ip_version = get_ip_version(cidr) - args = ['del', cidr, 'dev', self.name, 'scope', 'link'] + args = ['del', cidr] + args += self._dev_args() + args += ['scope', 'link'] args += self._table_args() self._as_root([ip_version], tuple(args)) def get_gateway(self, scope=None, filters=None, ip_version=None): options = [ip_version] if ip_version else [] - args = ['list', 'dev', self.name] + args = ['list'] + args += self._dev_args() args += self._table_args() if filters: args += filters @@ -635,19 +641,26 @@ class IpRouteCommand(IpDeviceCommandBase): def add_route(self, cidr, ip, table=None): ip_version = get_ip_version(cidr) - args = ['replace', cidr, 'via', ip, 'dev', self.name] - if table: - args += ['table', table] + args = ['replace', cidr, 'via', ip] + args += self._dev_args() + args += self._table_args(table) self._as_root([ip_version], tuple(args)) def delete_route(self, cidr, ip, table=None): ip_version = get_ip_version(cidr) - args = ['del', cidr, 'via', ip, 'dev', self.name] - if table: - args += ['table', table] + args = ['del', cidr, 'via', ip] + args += self._dev_args() + args += self._table_args(table) self._as_root([ip_version], tuple(args)) +class IPRoute(SubProcessBase): + def __init__(self, namespace=None, table=None): + super(IPRoute, self).__init__(namespace=namespace) + self.name = None + self.route = IpRouteCommand(self, table=table) + + class IpNeighCommand(IpDeviceCommandBase): COMMAND = 'neigh' diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 87a2a8227..4d7ef190f 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -533,13 +533,13 @@ class TestIPCmdBase(base.BaseTestCase): self.parent.name = 'eth0' def _assert_call(self, options, args): - self.parent.assert_has_calls([ - mock.call._run(options, self.command, args)]) + self.parent._run.assert_has_calls([ + mock.call(options, self.command, args)]) def _assert_sudo(self, options, args, use_root_namespace=False): - self.parent.assert_has_calls( - [mock.call._as_root(options, self.command, args, - use_root_namespace=use_root_namespace)]) + self.parent._as_root.assert_has_calls( + [mock.call(options, self.command, args, + use_root_namespace=use_root_namespace)]) class TestIpRuleCommand(TestIPCmdBase): @@ -967,6 +967,53 @@ class TestIPv6IpRouteCommand(TestIpRouteCommand): 'metric': 1024}}] +class TestIPRoute(TestIpRouteCommand): + """Leverage existing tests for IpRouteCommand for IPRoute + + This test leverages the tests written for IpRouteCommand. The difference + is that the 'dev' argument should not be passed for each of the commands. + So, this test removes the dev argument from the expected arguments in each + assert. + """ + def setUp(self): + super(TestIPRoute, self).setUp() + self.parent = ip_lib.IPRoute() + self.parent._run = mock.Mock() + self.parent._as_root = mock.Mock() + self.route_cmd = self.parent.route + self.check_dev_args = False + + def _remove_dev_args(self, args): + def args_without_dev(): + previous = None + for arg in args: + if 'dev' not in (arg, previous): + yield arg + previous = arg + + return tuple(arg for arg in args_without_dev()) + + def _assert_call(self, options, args): + if not self.check_dev_args: + args = self._remove_dev_args(args) + super(TestIPRoute, self)._assert_call(options, args) + + def _assert_sudo(self, options, args, use_root_namespace=False): + if not self.check_dev_args: + args = self._remove_dev_args(args) + super(TestIPRoute, self)._assert_sudo(options, args) + + def test_pullup_route(self): + # This method gets the interface name passed to it as an argument. So, + # don't remove it from the expected arguments. + self.check_dev_args = True + super(TestIPRoute, self).test_pullup_route() + + def test_del_gateway_cannot_find_device(self): + # This test doesn't make sense for this case since dev won't be passed + pass + + class TestIpNetnsCommand(TestIPCmdBase): def setUp(self): super(TestIpNetnsCommand, self).setUp() -- 2.45.2