openstack/neutron
Code Issues Proposed changes

Track all interfaces in Keepalived

Browse Source 
This add track parameter support for the
KeepalivedVirtualRoute class and changes
so that no_track is not included by default
in any of config generation.

Since we have reverted the code that keeps
the standby virtual router's interfaces
down [1] we can now remove no_track by
default and let it track the status of
interfaces since they are always up.

Now that the config option to set no_track
is removed there is no way to override this
for an operator.

[1] https://review.opendev.org/c/openstack/neutron/+/834162

Change-Id: If1a0ac3323a01af3e19f3a5fcdddd59fa6b23598
This commit is contained in:
Tobias Urdin 2023-01-02 17:21:51 +01:00
parent 26da863ca6
commit bee07defac
3 changed files with 75 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
neutron/agent/linux/keepalived.py
View File

@ -126,11 +126,12 @@ class KeepalivedVirtualRoute(object):
"""A virtual route entry of a keepalived configuration.""" """A virtual route entry of a keepalived configuration."""
def __init__(self, destination, nexthop, interface_name=None, def __init__(self, destination, nexthop, interface_name=None,
scope=None): scope=None, track=True):
self.destination = destination self.destination = destination
self.nexthop = nexthop self.nexthop = nexthop
self.interface_name = interface_name self.interface_name = interface_name
self.scope = scope self.scope = scope
self.track = track
def build_config(self): def build_config(self):
output = self.destination output = self.destination
@ -140,7 +141,7 @@ class KeepalivedVirtualRoute(object):
output += ' dev %s' % self.interface_name output += ' dev %s' % self.interface_name
if self.scope: if self.scope:
output += ' scope %s' % self.scope output += ' scope %s' % self.scope
if _is_keepalived_use_no_track_supported(): if not self.track and _is_keepalived_use_no_track_supported():
output += ' no_track' output += ' no_track'
# NOTE(mstinsky): neutron and keepalived are adding the same routes on # NOTE(mstinsky): neutron and keepalived are adding the same routes on
# primary routers. With this we ensure that both are adding the routes # primary routers. With this we ensure that both are adding the routes
@ -223,7 +224,7 @@ class KeepalivedInstance(object):
self.authentication = (auth_type, password) self.authentication = (auth_type, password)
def add_vip(self, ip_cidr, interface_name, scope): def add_vip(self, ip_cidr, interface_name, scope):
track = interface_name in self.track_interfaces track = interface_name not in self.track_interfaces
vip = KeepalivedVipAddress(ip_cidr, interface_name, scope, track=track) vip = KeepalivedVipAddress(ip_cidr, interface_name, scope, track=track)
if vip not in self.vips: if vip not in self.vips:
self.vips.append(vip) self.vips.append(vip)

16
neutron/tests/functional/agent/l3/framework.py
View File

@ -71,16 +71,16 @@ vrrp_instance VR_1 {
169.254.0.1/24 dev %(ha_device_name)s 169.254.0.1/24 dev %(ha_device_name)s
} }
virtual_ipaddress_excluded { virtual_ipaddress_excluded {
%(floating_ip_cidr)s dev %(ex_device_name)s no_track %(floating_ip_cidr)s dev %(ex_device_name)s
%(external_device_cidr)s dev %(ex_device_name)s no_track %(external_device_cidr)s dev %(ex_device_name)s
%(internal_device_cidr)s dev %(internal_device_name)s no_track %(internal_device_cidr)s dev %(internal_device_name)s
%(ex_port_ipv6)s dev %(ex_device_name)s scope link no_track %(ex_port_ipv6)s dev %(ex_device_name)s scope link
%(int_port_ipv6)s dev %(internal_device_name)s scope link no_track %(int_port_ipv6)s dev %(internal_device_name)s scope link
} }
virtual_routes { virtual_routes {
0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s no_track protocol static 0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s protocol static
8.8.8.0/24 via 19.4.4.4 no_track protocol static 8.8.8.0/24 via 19.4.4.4 protocol static
%(extra_subnet_cidr)s dev %(ex_device_name)s scope link no_track protocol static %(extra_subnet_cidr)s dev %(ex_device_name)s scope link protocol static
} }
}""" # noqa: E501 # pylint: disable=line-too-long }""" # noqa: E501 # pylint: disable=line-too-long

85
neutron/tests/unit/agent/linux/test_keepalived.py
View File

@ -87,7 +87,7 @@ class KeepalivedGetFreeRangeTestCase(KeepalivedBaseTestCase):
class KeepalivedConfBaseMixin(object): class KeepalivedConfBaseMixin(object):
def _get_config(self): def _get_config(self, track=True):
config = keepalived.KeepalivedConf() config = keepalived.KeepalivedConf()
instance1 = keepalived.KeepalivedInstance('MASTER', 'eth0', 1, instance1 = keepalived.KeepalivedInstance('MASTER', 'eth0', 1,
@ -97,16 +97,16 @@ class KeepalivedConfBaseMixin(object):
instance1.track_interfaces.append("eth0") instance1.track_interfaces.append("eth0")
vip_address1 = keepalived.KeepalivedVipAddress('192.168.1.0/24', vip_address1 = keepalived.KeepalivedVipAddress('192.168.1.0/24',
'eth1', track=False) 'eth1', track=track)
vip_address2 = keepalived.KeepalivedVipAddress('192.168.2.0/24', vip_address2 = keepalived.KeepalivedVipAddress('192.168.2.0/24',
'eth2', track=False) 'eth2', track=track)
vip_address3 = keepalived.KeepalivedVipAddress('192.168.3.0/24', vip_address3 = keepalived.KeepalivedVipAddress('192.168.3.0/24',
'eth2', track=False) 'eth2', track=track)
vip_address_ex = keepalived.KeepalivedVipAddress('192.168.55.0/24', vip_address_ex = keepalived.KeepalivedVipAddress('192.168.55.0/24',
'eth10', track=False) 'eth10', track=track)
instance1.vips.append(vip_address1) instance1.vips.append(vip_address1)
instance1.vips.append(vip_address2) instance1.vips.append(vip_address2)
@ -115,7 +115,7 @@ class KeepalivedConfBaseMixin(object):
virtual_route = keepalived.KeepalivedVirtualRoute(n_consts.IPv4_ANY, virtual_route = keepalived.KeepalivedVirtualRoute(n_consts.IPv4_ANY,
"192.168.1.1", "192.168.1.1",
"eth1") "eth1", track=track)
instance1.virtual_routes.gateway_routes = [virtual_route] instance1.virtual_routes.gateway_routes = [virtual_route]
instance2 = keepalived.KeepalivedInstance('MASTER', 'eth4', 2, instance2 = keepalived.KeepalivedInstance('MASTER', 'eth4', 2,
@ -124,7 +124,7 @@ class KeepalivedConfBaseMixin(object):
instance2.track_interfaces.append("eth4") instance2.track_interfaces.append("eth4")
vip_address1 = keepalived.KeepalivedVipAddress('192.168.3.0/24', vip_address1 = keepalived.KeepalivedVipAddress('192.168.3.0/24',
'eth6', track=False) 'eth6', track=track)
instance2.vips.append(vip_address1) instance2.vips.append(vip_address1)
instance2.vips.append(vip_address2) instance2.vips.append(vip_address2)
@ -192,7 +192,8 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase,
keepalived, '_is_keepalived_use_no_track_supported', keepalived, '_is_keepalived_use_no_track_supported',
return_value=True): return_value=True):
config = self._get_config() config = self._get_config()
self.assertEqual(self.expected, config.get_config_str()) self.assertEqual(self.expected.replace(' no_track', ''),
config.get_config_str())
def test_config_generation_no_track_not_supported(self): def test_config_generation_no_track_not_supported(self):
self._mock_no_track_supported.start().return_value = False self._mock_no_track_supported.start().return_value = False
@ -207,7 +208,7 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase,
with mock.patch.object( with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported', keepalived, '_is_keepalived_use_no_track_supported',
return_value=True): return_value=True):
config = self._get_config() config = self._get_config(track=False)
self.assertEqual(self.expected, config.get_config_str()) self.assertEqual(self.expected, config.get_config_str())
config.reset() config.reset()
@ -238,20 +239,24 @@ class KeepalivedStateExceptionTestCase(KeepalivedBaseTestCase):
class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase): class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase):
@classmethod @classmethod
def _get_instance_routes(cls): def _get_instance_routes(cls, track=True):
routes = keepalived.KeepalivedInstanceRoutes() routes = keepalived.KeepalivedInstanceRoutes()
default_gw_eth0 = keepalived.KeepalivedVirtualRoute( default_gw_eth0 = keepalived.KeepalivedVirtualRoute(
'0.0.0.0/0', '1.0.0.254', 'eth0') '0.0.0.0/0', '1.0.0.254', 'eth0', track=track)
default_gw_eth1 = keepalived.KeepalivedVirtualRoute( default_gw_eth1 = keepalived.KeepalivedVirtualRoute(
'::/0', 'fe80::3e97:eff:fe26:3bfa/64', 'eth1') '::/0', 'fe80::3e97:eff:fe26:3bfa/64', 'eth1',
track=track)
routes.gateway_routes = [default_gw_eth0, default_gw_eth1] routes.gateway_routes = [default_gw_eth0, default_gw_eth1]
extra_routes = [ extra_routes = [
keepalived.KeepalivedVirtualRoute('10.0.0.0/8', '1.0.0.1'), keepalived.KeepalivedVirtualRoute(
keepalived.KeepalivedVirtualRoute('20.0.0.0/8', '2.0.0.2')] '10.0.0.0/8', '1.0.0.1', track=track),
keepalived.KeepalivedVirtualRoute(
'20.0.0.0/8', '2.0.0.2', track=track)]
routes.extra_routes = extra_routes routes.extra_routes = extra_routes
extra_subnets = [ extra_subnets = [
keepalived.KeepalivedVirtualRoute( keepalived.KeepalivedVirtualRoute(
'30.0.0.0/8', None, 'eth0', scope='link')] '30.0.0.0/8', None, 'eth0', scope='link',
track=track)]
routes.extra_subnets = extra_subnets routes.extra_subnets = extra_subnets
return routes return routes
@ -277,7 +282,7 @@ class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase):
with mock.patch.object( with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported', keepalived, '_is_keepalived_use_no_track_supported',
return_value=True): return_value=True):
routes = self._get_instance_routes() routes = self._get_instance_routes(track=False)
self.assertEqual(expected, '\n'.join(routes.build_config())) self.assertEqual(expected, '\n'.join(routes.build_config()))
def _get_no_track_less_expected_config(self): def _get_no_track_less_expected_config(self):
@ -290,11 +295,19 @@ class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase):
}""" }"""
return expected return expected
def test_build_config_without_no_track(self):
with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported',
return_value=True):
routes = self._get_instance_routes()
self.assertEqual(self._get_no_track_less_expected_config(),
'\n'.join(routes.build_config()))
def test_build_config_no_track_not_supported(self): def test_build_config_no_track_not_supported(self):
with mock.patch.object( with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported', keepalived, '_is_keepalived_use_no_track_supported',
return_value=False): return_value=False):
routes = self._get_instance_routes() routes = self._get_instance_routes(track=False)
self.assertEqual(self._get_no_track_less_expected_config(), self.assertEqual(self._get_no_track_less_expected_config(),
'\n'.join(routes.build_config())) '\n'.join(routes.build_config()))
@ -306,12 +319,14 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase,
['169.254.192.0/18']) ['169.254.192.0/18'])
self.assertEqual('169.254.0.42/24', instance.get_primary_vip()) self.assertEqual('169.254.0.42/24', instance.get_primary_vip())
def _test_remove_addresses_by_interface(self, no_track_value): def _test_remove_addresses_by_interface(self, track=True):
config = self._get_config() config = self._get_config(track=track)
instance = config.get_instance(1) instance = config.get_instance(1)
instance.remove_vips_vroutes_by_interface('eth2') instance.remove_vips_vroutes_by_interface('eth2')
instance.remove_vips_vroutes_by_interface('eth10') instance.remove_vips_vroutes_by_interface('eth10')
no_track_value = ' no_track' if not track else ''
expected = KEEPALIVED_GLOBAL_CONFIG + textwrap.dedent(""" expected = KEEPALIVED_GLOBAL_CONFIG + textwrap.dedent("""
vrrp_instance VR_1 { vrrp_instance VR_1 {
state MASTER state MASTER
@ -363,13 +378,19 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase,
with mock.patch.object( with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported', keepalived, '_is_keepalived_use_no_track_supported',
return_value=True): return_value=True):
self._test_remove_addresses_by_interface(" no_track") self._test_remove_addresses_by_interface()
def test_remove_addresses_by_interface_with_no_track(self):
with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported',
return_value=True):
self._test_remove_addresses_by_interface(track=False)
def test_remove_address_by_interface_no_track_not_supported(self): def test_remove_address_by_interface_no_track_not_supported(self):
with mock.patch.object( with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported', keepalived, '_is_keepalived_use_no_track_supported',
return_value=False): return_value=False):
self._test_remove_addresses_by_interface("") self._test_remove_addresses_by_interface()
def test_build_config_no_vips(self): def test_build_config_no_vips(self):
expected = textwrap.dedent("""\ expected = textwrap.dedent("""\
@ -436,6 +457,16 @@ class KeepalivedVirtualRouteTestCase(KeepalivedBaseTestCase):
return_value=True): return_value=True):
route = keepalived.KeepalivedVirtualRoute( route = keepalived.KeepalivedVirtualRoute(
n_consts.IPv4_ANY, '1.2.3.4', 'eth0') n_consts.IPv4_ANY, '1.2.3.4', '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_supported_no_track(self):
with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported',
return_value=True):
route = keepalived.KeepalivedVirtualRoute(
n_consts.IPv4_ANY, '1.2.3.4', 'eth0', track=False)
self.assertEqual( self.assertEqual(
'0.0.0.0/0 via 1.2.3.4 dev eth0 no_track protocol static', '0.0.0.0/0 via 1.2.3.4 dev eth0 no_track protocol static',
route.build_config()) route.build_config())
@ -449,11 +480,21 @@ class KeepalivedVirtualRouteTestCase(KeepalivedBaseTestCase):
self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0 protocol static', self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0 protocol static',
route.build_config()) route.build_config())
def test_virtual_route_with_dev_no_track_not_supported_not_track(self):
with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported',
return_value=False):
route = keepalived.KeepalivedVirtualRoute(
n_consts.IPv4_ANY, '1.2.3.4', 'eth0', track=False)
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): def test_virtual_route_without_dev(self):
with mock.patch.object( with mock.patch.object(
keepalived, '_is_keepalived_use_no_track_supported', keepalived, '_is_keepalived_use_no_track_supported',
return_value=True): return_value=True):
route = keepalived.KeepalivedVirtualRoute('50.0.0.0/8', '1.2.3.4') route = keepalived.KeepalivedVirtualRoute(
'50.0.0.0/8', '1.2.3.4', track=False)
self.assertEqual('50.0.0.0/8 via 1.2.3.4 no_track protocol static', self.assertEqual('50.0.0.0/8 via 1.2.3.4 no_track protocol static',
route.build_config()) route.build_config())