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

diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py
index cadbd019fb1d4c989168493685050f627b99f86a..88597f70ff3a5e40ecc2ccf8f27c777135bb101e 100644 (file)
--- 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 87a2a82274c2cc5accc69de17d6b47e21f2373c3..4d7ef190f4429b34f1fac279fdd803da29a0e4ef 100644 (file)
--- 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()