Check if routing rule exists before adding

Since there is no equivalent to 'ip route replace...' with
'ip rule...', we need to check the existence first before
adding, as otherwise we'll end up with multiple identical
routing rules.

Change-Id: I8f1f2424dd854fad8a72478463fa8fadd6a5c168
Closes-Bug: #1398865

diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py
index 122bc20f10060a3fffddb36212c5d4bd038b0060..57f529339d6f6fdb7b763aa2f69ae622e195f737 100644 (file)
--- a/neutron/agent/linux/ip_lib.py
+++ b/neutron/agent/linux/ip_lib.py
@@ -191,17 +191,30 @@ class IPWrapper(SubProcessBase):
 
 
 class IpRule(IPWrapper):
+    def _exists(self, ip, ip_version, table, rule_pr):
+        # Typical rule from 'ip rule show':
+        # 4030201:  from 1.2.3.4/24 lookup 10203040
+
+        rule_pr = str(rule_pr) + ":"
+        for line in self._as_root([ip_version], 'rule', ['show']).splitlines():
+            parts = line.split()
+            if parts and (parts[0] == rule_pr and
+                          parts[2] == str(ip) and
+                          parts[-1] == str(table)):
+                return True
+
+        return False
+
     def add(self, ip, table, rule_pr):
         ip_version = netaddr.IPNetwork(ip).version
-        args = ['add', 'from', ip, 'table', table, 'priority', rule_pr]
-        ip = self._as_root([ip_version], 'rule', tuple(args))
-        return ip
+        if not self._exists(ip, ip_version, table, rule_pr):
+            args = ['add', 'from', ip, 'table', table, 'priority', rule_pr]
+            self._as_root([ip_version], 'rule', tuple(args))
 
     def delete(self, ip, table, rule_pr):
         ip_version = netaddr.IPNetwork(ip).version
         args = ['del', 'table', table, 'priority', rule_pr]
-        ip = self._as_root([ip_version], 'rule', tuple(args))
-        return ip
+        self._as_root([ip_version], 'rule', tuple(args))
 
 
 class IPDevice(SubProcessBase):

diff --git a/neutron/tests/unit/test_linux_ip_lib.py b/neutron/tests/unit/test_linux_ip_lib.py
index 6f7254d05b2d9847c41294403c2486199a49d0f2..5fece244273335e5a10f88dc2b510a897f59d07b 100644 (file)
--- a/neutron/tests/unit/test_linux_ip_lib.py
+++ b/neutron/tests/unit/test_linux_ip_lib.py
@@ -141,6 +141,20 @@ SUBNET_SAMPLE1 = ("10.0.0.0/24 dev qr-23380d11-d2  scope link  src 10.0.0.1\n"
 SUBNET_SAMPLE2 = ("10.0.0.0/24 dev tap1d7888a7-10  scope link  src 10.0.0.2\n"
                   "10.0.0.0/24 dev qr-23380d11-d2  scope link  src 10.0.0.1")
 
+RULE_V4_SAMPLE = ("""
+0:      from all lookup local
+32766:  from all lookup main
+32767:  from all lookup default
+101:    from 192.168.45.100 lookup 2
+""")
+
+RULE_V6_SAMPLE = ("""
+0:      from all lookup local
+32766:  from all lookup main
+32767:  from all lookup default
+201:    from 2001:db8::1 lookup 3
+""")
+
 
 class TestSubProcessBase(base.BaseTestCase):
     def setUp(self):
@@ -425,12 +439,27 @@ class TestIpRule(base.BaseTestCase):
         self.execute = self.execute_p.start()
 
     def _test_add_rule(self, ip, table, priority):
+        ip_version = netaddr.IPNetwork(ip).version
+        ip_lib.IpRule('sudo').add(ip, table, priority)
+        call_1 = mock.call([ip_version], 'rule', ['show'],
+                           run_as_root=True, namespace=None,
+                           log_fail_as_error=True)
+        call_2 = mock.call().splitlines()
+        # This is for call().splitlines().__iter__(), which can't be mocked
+        call_3 = mock.ANY
+        call_4 = mock.call([ip_version], 'rule',
+                           ('add', 'from', ip,
+                            'table', table, 'priority', priority),
+                           run_as_root=True, namespace=None,
+                           log_fail_as_error=True)
+        self.execute.assert_has_calls([call_1, call_2, call_3, call_4])
+
+    def _test_add_rule_exists(self, ip, table, priority, output):
+        self.execute.return_value = output
         ip_version = netaddr.IPNetwork(ip).version
         ip_lib.IpRule('sudo').add(ip, table, priority)
         self.execute.assert_called_once_with([ip_version], 'rule',
-                                             ('add', 'from', ip,
-                                              'table', table,
-                                              'priority', priority),
+                                             ['show'],
                                              run_as_root=True, namespace=None,
                                              log_fail_as_error=True)
 
@@ -446,9 +475,15 @@ class TestIpRule(base.BaseTestCase):
     def test_add_rule_v4(self):
         self._test_add_rule('192.168.45.100', 2, 100)
 
+    def test_add_rule_v4_exists(self):
+        self._test_add_rule_exists('192.168.45.100', 2, 101, RULE_V4_SAMPLE)
+
     def test_add_rule_v6(self):
         self._test_add_rule('2001:db8::1', 3, 200)
 
+    def test_add_rule_v6_exists(self):
+        self._test_add_rule_exists('2001:db8::1', 3, 201, RULE_V6_SAMPLE)
+
     def test_delete_rule_v4(self):
         self._test_delete_rule('192.168.45.100', 2, 100)