From 7559a2ea827b6f36ece8029cb3fceff0308fee03 Mon Sep 17 00:00:00 2001 From: Maximilian Stinsky Date: Thu, 24 Nov 2022 11:40:04 +0100 Subject: [PATCH] Fix duplicated routes exceptions Since the train release neutron adds routes with protocol static. Keepalived also adds the same routes with different protocols depending on the keepalived version. This can result in duplicated routes inside network namespaces. On l3 agent restarts those duplicate routes then prevent the l3 agent from updating its router state because it runs into 'RTNETLINK answers: File exists expections' when it tries to execute 'ip route' commands. This patch adds the same protocol static to each virtual route of keepalived's configuration so network namespaces do not run into duplicated routes anymore. Closes-Bug: #1956846 Change-Id: Ic35b5d4b9110b832c10345c45ec62c0923237cfd (cherry picked from commit c813b658d0e5e0c00093f90f849fcf67ddca16cf) --- neutron/agent/linux/keepalived.py | 5 +++ .../tests/functional/agent/l3/framework.py | 8 ++-- .../tests/unit/agent/linux/test_keepalived.py | 44 ++++++++++--------- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py index 910d0be96e2..a42e42ad1f7 100644 --- a/neutron/agent/linux/keepalived.py +++ b/neutron/agent/linux/keepalived.py @@ -152,6 +152,11 @@ class KeepalivedVirtualRoute(object): LOG.warning("keepalived_use_no_track cfg option is True but " "keepalived on host seems to not support this " "option") + # NOTE(mstinsky): neutron and keepalived are adding the same routes on + # primary routers. With this we ensure that both are adding the routes + # with the same procotol and prevent duplicated routes which result in + # neutron exception for ip route commands. + output += ' protocol static' return output diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 0bcc9374770..c2fea9a9d24 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -78,11 +78,11 @@ vrrp_instance VR_1 { %(int_port_ipv6)s dev %(internal_device_name)s scope link no_track } virtual_routes { - 0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s no_track - 8.8.8.0/24 via 19.4.4.4 no_track - %(extra_subnet_cidr)s dev %(ex_device_name)s scope link no_track + 0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s no_track protocol static + 8.8.8.0/24 via 19.4.4.4 no_track protocol static + %(extra_subnet_cidr)s dev %(ex_device_name)s scope link no_track protocol static } -}""" +}""" # noqa: E501 # pylint: disable=line-too-long def get_ovs_bridge(br_name): diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py index 28474f9dc89..169f075ff95 100644 --- a/neutron/tests/unit/agent/linux/test_keepalived.py +++ b/neutron/tests/unit/agent/linux/test_keepalived.py @@ -164,7 +164,7 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase, 192.168.55.0/24 dev eth10 no_track } virtual_routes { - 0.0.0.0/0 via 192.168.1.1 dev eth1 no_track + 0.0.0.0/0 via 192.168.1.1 dev eth1 no_track protocol static } } vrrp_instance VR_2 { @@ -247,7 +247,7 @@ class KeepalivedConfWithoutNoTrackTestCase(KeepalivedConfTestCase): 192.168.55.0/24 dev eth10 } virtual_routes { - 0.0.0.0/0 via 192.168.1.1 dev eth1 + 0.0.0.0/0 via 192.168.1.1 dev eth1 protocol static } } vrrp_instance VR_2 { @@ -323,11 +323,11 @@ class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase): def test_build_config(self): expected = """ virtual_routes { - 0.0.0.0/0 via 1.0.0.254 dev eth0 no_track - ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1 no_track - 10.0.0.0/8 via 1.0.0.1 no_track - 20.0.0.0/8 via 2.0.0.2 no_track - 30.0.0.0/8 dev eth0 scope link no_track + 0.0.0.0/0 via 1.0.0.254 dev eth0 no_track protocol static + ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1 no_track protocol static + 10.0.0.0/8 via 1.0.0.1 no_track protocol static + 20.0.0.0/8 via 2.0.0.2 no_track protocol static + 30.0.0.0/8 dev eth0 scope link no_track protocol static }""" with mock.patch.object( keepalived, '_is_keepalived_use_no_track_supported', @@ -337,11 +337,11 @@ class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase): def _get_no_track_less_expected_config(self): expected = """ virtual_routes { - 0.0.0.0/0 via 1.0.0.254 dev eth0 - ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1 - 10.0.0.0/8 via 1.0.0.1 - 20.0.0.0/8 via 2.0.0.2 - 30.0.0.0/8 dev eth0 scope link + 0.0.0.0/0 via 1.0.0.254 dev eth0 protocol static + ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1 protocol static + 10.0.0.0/8 via 1.0.0.1 protocol static + 20.0.0.0/8 via 2.0.0.2 protocol static + 30.0.0.0/8 dev eth0 scope link protocol static }""" return expected @@ -395,7 +395,7 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase, 192.168.1.0/24 dev eth1%(no_track)s } virtual_routes { - 0.0.0.0/0 via 192.168.1.1 dev eth1%(no_track)s + 0.0.0.0/0 via 192.168.1.1 dev eth1%(no_track)s protocol static } } vrrp_instance VR_2 { @@ -416,7 +416,7 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase, 192.168.3.0/24 dev eth6%(no_track)s 192.168.55.0/24 dev eth10%(no_track)s } - }""" % {'no_track': no_track_value}) + }""" % {'no_track': no_track_value}) # noqa: E501 # pylint: disable=line-too-long self.assertEqual(expected, config.get_config_str()) @@ -501,8 +501,9 @@ class KeepalivedVirtualRouteTestCase(KeepalivedBaseTestCase): return_value=True): route = keepalived.KeepalivedVirtualRoute( n_consts.IPv4_ANY, '1.2.3.4', 'eth0') - self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0 no_track', - route.build_config()) + self.assertEqual( + '0.0.0.0/0 via 1.2.3.4 dev eth0 no_track protocol static', + route.build_config()) def test_virtual_route_with_dev_no_track_not_supported(self): with mock.patch.object( @@ -510,14 +511,14 @@ class KeepalivedVirtualRouteTestCase(KeepalivedBaseTestCase): return_value=False): route = keepalived.KeepalivedVirtualRoute( n_consts.IPv4_ANY, '1.2.3.4', 'eth0') - self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0', + self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0 protocol static', route.build_config()) def test_virtual_route_with_dev_without_no_track(self): cfg.CONF.set_override('keepalived_use_no_track', False) route = keepalived.KeepalivedVirtualRoute(n_consts.IPv4_ANY, '1.2.3.4', 'eth0') - self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0', + self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0 protocol static', route.build_config()) def test_virtual_route_without_dev(self): @@ -525,7 +526,7 @@ class KeepalivedVirtualRouteTestCase(KeepalivedBaseTestCase): keepalived, '_is_keepalived_use_no_track_supported', return_value=True): route = keepalived.KeepalivedVirtualRoute('50.0.0.0/8', '1.2.3.4') - self.assertEqual('50.0.0.0/8 via 1.2.3.4 no_track', + self.assertEqual('50.0.0.0/8 via 1.2.3.4 no_track protocol static', route.build_config()) def test_virtual_route_without_dev_no_track_not_supported(self): @@ -533,13 +534,14 @@ class KeepalivedVirtualRouteTestCase(KeepalivedBaseTestCase): keepalived, '_is_keepalived_use_no_track_supported', return_value=False): route = keepalived.KeepalivedVirtualRoute('50.0.0.0/8', '1.2.3.4') - self.assertEqual('50.0.0.0/8 via 1.2.3.4', + self.assertEqual('50.0.0.0/8 via 1.2.3.4 protocol static', route.build_config()) def test_virtual_route_without_dev_without_no_track(self): cfg.CONF.set_override('keepalived_use_no_track', False) route = keepalived.KeepalivedVirtualRoute('50.0.0.0/8', '1.2.3.4') - self.assertEqual('50.0.0.0/8 via 1.2.3.4', route.build_config()) + self.assertEqual('50.0.0.0/8 via 1.2.3.4 protocol static', + route.build_config()) class KeepalivedTrackScriptTestCase(KeepalivedBaseTestCase):