From e24bdb3e9dea261232f5e94a919387e2d4515b64 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Wed, 22 Apr 2020 09:56:40 +0000 Subject: [PATCH] [L3 HA] Add "no_track" option to VIPs in keepalived config Patch [1] introduced new mechanism which only brings UP interfaces on master node of HA router. It works fine with keepalived 1.x but it is broken when keepalived 2.x was used (e.g. on Centos 8) as in this new version of keepalived by default all interfaces of VIPs and routes are tracked, and if one of them is DOWN, keepalived is going to FAULT state. Because of that router will never be transitioned to MASTER on any node. This patch fixes it by adding "no_track" option to all VIPs and routes in keepalived's config file. This "no_track" option isn't added to ha interface so this one is still tracked by keepalived. [1] https://review.opendev.org/#/c/707406/ Closes-bug: #1874211 Change-Id: Ic16cf83fe1d1576d91047adb2d4f9e07d57185b6 (cherry picked from commit dc9084a8ec9db08d7ea947e0b73581b748be5819) --- neutron/agent/linux/keepalived.py | 16 +++-- .../tests/functional/agent/l3/framework.py | 67 ++++++++++--------- .../tests/unit/agent/linux/test_keepalived.py | 51 +++++++------- 3 files changed, 71 insertions(+), 63 deletions(-) diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py index e199352fee4..c5534e5bb61 100644 --- a/neutron/agent/linux/keepalived.py +++ b/neutron/agent/linux/keepalived.py @@ -85,24 +85,28 @@ class InvalidAuthenticationTypeException(exceptions.NeutronException): class KeepalivedVipAddress(object): """A virtual address entry of a keepalived configuration.""" - def __init__(self, ip_address, interface_name, scope=None): + def __init__(self, ip_address, interface_name, scope=None, track=True): self.ip_address = ip_address self.interface_name = interface_name self.scope = scope + self.track = track def __eq__(self, other): return (isinstance(other, KeepalivedVipAddress) and self.ip_address == other.ip_address) def __str__(self): - return '[%s, %s, %s]' % (self.ip_address, - self.interface_name, - self.scope) + return '[%s, %s, %s, %s]' % (self.ip_address, + self.interface_name, + self.scope, + self.track) def build_config(self): result = '%s dev %s' % (self.ip_address, self.interface_name) if self.scope: result += ' scope %s' % self.scope + if not self.track: + result += ' no_track' return result @@ -124,6 +128,7 @@ class KeepalivedVirtualRoute(object): output += ' dev %s' % self.interface_name if self.scope: output += ' scope %s' % self.scope + output += ' no_track' return output @@ -200,7 +205,8 @@ class KeepalivedInstance(object): self.authentication = (auth_type, password) def add_vip(self, ip_cidr, interface_name, scope): - vip = KeepalivedVipAddress(ip_cidr, interface_name, scope) + track = interface_name in self.track_interfaces + vip = KeepalivedVipAddress(ip_cidr, interface_name, scope, track=track) if vip not in self.vips: self.vips.append(vip) else: diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index ab235ceb876..7c3cabc9d22 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -15,7 +15,6 @@ import copy import functools -import textwrap import mock import netaddr @@ -47,6 +46,39 @@ _uuid = uuidutils.generate_uuid OVS_INTERFACE_DRIVER = 'neutron.agent.linux.interface.OVSInterfaceDriver' +KEEPALIVED_CONFIG = """\ +global_defs { + notification_email_from %(email_from)s + router_id %(router_id)s +} +vrrp_instance VR_1 { + state BACKUP + interface %(ha_device_name)s + virtual_router_id 1 + priority 50 + garp_master_delay 60 + nopreempt + advert_int 2 + track_interface { + %(ha_device_name)s + } + virtual_ipaddress { + 169.254.0.1/24 dev %(ha_device_name)s + } + virtual_ipaddress_excluded { + %(floating_ip_cidr)s dev %(ex_device_name)s no_track + %(external_device_cidr)s dev %(ex_device_name)s no_track + %(internal_device_cidr)s dev %(internal_device_name)s no_track + %(ex_port_ipv6)s dev %(ex_device_name)s scope link no_track + %(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 + } +}""" + def get_ovs_bridge(br_name): return ovs_lib.OVSBridge(br_name) @@ -432,38 +464,7 @@ class L3AgentTestFramework(base.BaseSudoTestCase): router.get_floating_ips()[0]['floating_ip_address']) default_gateway_ip = external_port['subnets'][0].get('gateway_ip') extra_subnet_cidr = external_port['extra_subnets'][0].get('cidr') - return textwrap.dedent("""\ - global_defs { - notification_email_from %(email_from)s - router_id %(router_id)s - } - vrrp_instance VR_1 { - state BACKUP - interface %(ha_device_name)s - virtual_router_id 1 - priority 50 - garp_master_delay 60 - nopreempt - advert_int 2 - track_interface { - %(ha_device_name)s - } - virtual_ipaddress { - 169.254.0.1/24 dev %(ha_device_name)s - } - virtual_ipaddress_excluded { - %(floating_ip_cidr)s dev %(ex_device_name)s - %(external_device_cidr)s dev %(ex_device_name)s - %(internal_device_cidr)s dev %(internal_device_name)s - %(ex_port_ipv6)s dev %(ex_device_name)s scope link - %(int_port_ipv6)s dev %(internal_device_name)s scope link - } - virtual_routes { - 0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s - 8.8.8.0/24 via 19.4.4.4 - %(extra_subnet_cidr)s dev %(ex_device_name)s scope link - } - }""") % { + return KEEPALIVED_CONFIG % { 'email_from': keepalived.KEEPALIVED_EMAIL_FROM, 'router_id': keepalived.KEEPALIVED_ROUTER_ID, 'ha_device_name': ha_device_name, diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py index 8ec6b4fa472..082db3eb11d 100644 --- a/neutron/tests/unit/agent/linux/test_keepalived.py +++ b/neutron/tests/unit/agent/linux/test_keepalived.py @@ -83,16 +83,16 @@ class KeepalivedConfBaseMixin(object): instance1.track_interfaces.append("eth0") vip_address1 = keepalived.KeepalivedVipAddress('192.168.1.0/24', - 'eth1') + 'eth1', track=False) vip_address2 = keepalived.KeepalivedVipAddress('192.168.2.0/24', - 'eth2') + 'eth2', track=False) vip_address3 = keepalived.KeepalivedVipAddress('192.168.3.0/24', - 'eth2') + 'eth2', track=False) vip_address_ex = keepalived.KeepalivedVipAddress('192.168.55.0/24', - 'eth10') + 'eth10', track=False) instance1.vips.append(vip_address1) instance1.vips.append(vip_address2) @@ -110,7 +110,7 @@ class KeepalivedConfBaseMixin(object): instance2.track_interfaces.append("eth4") vip_address1 = keepalived.KeepalivedVipAddress('192.168.3.0/24', - 'eth6') + 'eth6', track=False) instance2.vips.append(vip_address1) instance2.vips.append(vip_address2) @@ -144,13 +144,13 @@ class KeepalivedConfTestCase(base.BaseTestCase, 169.254.0.1/24 dev eth0 } virtual_ipaddress_excluded { - 192.168.1.0/24 dev eth1 - 192.168.2.0/24 dev eth2 - 192.168.3.0/24 dev eth2 - 192.168.55.0/24 dev eth10 + 192.168.1.0/24 dev eth1 no_track + 192.168.2.0/24 dev eth2 no_track + 192.168.3.0/24 dev eth2 no_track + 192.168.55.0/24 dev eth10 no_track } 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 no_track } } vrrp_instance VR_2 { @@ -167,9 +167,9 @@ class KeepalivedConfTestCase(base.BaseTestCase, 169.254.0.2/24 dev eth4 } virtual_ipaddress_excluded { - 192.168.2.0/24 dev eth2 - 192.168.3.0/24 dev eth6 - 192.168.55.0/24 dev eth10 + 192.168.2.0/24 dev eth2 no_track + 192.168.3.0/24 dev eth6 no_track + 192.168.55.0/24 dev eth10 no_track } }""") @@ -239,11 +239,11 @@ class KeepalivedInstanceRoutesTestCase(base.BaseTestCase): def test_build_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 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 }""" routes = self._get_instance_routes() self.assertEqual(expected, '\n'.join(routes.build_config())) @@ -281,10 +281,10 @@ class KeepalivedInstanceTestCase(base.BaseTestCase, 169.254.0.1/24 dev eth0 } virtual_ipaddress_excluded { - 192.168.1.0/24 dev eth1 + 192.168.1.0/24 dev eth1 no_track } 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 no_track } } vrrp_instance VR_2 { @@ -301,9 +301,9 @@ class KeepalivedInstanceTestCase(base.BaseTestCase, 169.254.0.2/24 dev eth4 } virtual_ipaddress_excluded { - 192.168.2.0/24 dev eth2 - 192.168.3.0/24 dev eth6 - 192.168.55.0/24 dev eth10 + 192.168.2.0/24 dev eth2 no_track + 192.168.3.0/24 dev eth6 no_track + 192.168.55.0/24 dev eth10 no_track } }""") @@ -371,12 +371,13 @@ class KeepalivedVirtualRouteTestCase(base.BaseTestCase): def test_virtual_route_with_dev(self): 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 no_track', route.build_config()) def test_virtual_route_without_dev(self): 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 no_track', + route.build_config()) class KeepalivedTrackScriptTestCase(base.BaseTestCase):