openstack/neutron
Code Issues Proposed changes

Fix duplicated routes exceptions

Browse Source 
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
This commit is contained in:
Maximilian Stinsky 2022-11-24 11:40:04 +01:00
parent 23d79219da
commit c813b658d0
No known key found for this signature in database
GPG Key ID: 46D3BABABE00D5E6
3 changed files with 32 additions and 25 deletions
neutron
agent/linux
keepalived.py
tests
functional/agent/l3
framework.py
unit/agent/linux
test_keepalived.py

5
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

8
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):

44
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):