From faa9a6fb46f1c32ae7cf79dab55501c585eda7aa Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 30 Jan 2015 13:22:17 -0500 Subject: [PATCH] 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 --- neutron/agent/linux/ip_lib.py | 23 +++++++++++--- neutron/tests/unit/test_linux_ip_lib.py | 41 +++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 122bc20f1..57f529339 100644 --- 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 6f7254d05..5fece2442 100644 --- 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) -- 2.45.2