From 7abe0ee34c367b4abf84820048b4aed643fc1162 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Tue, 11 Aug 2020 10:47:24 +0200 Subject: [PATCH] Add 'keepalived_use_no_track' config option Patch [1] added option "no_track" to the keepalived's config file which is generated by L3 agent in HA mode. This was added to handle properly keepalived 2.x and interfaces which are in DOWN state in the backup nodes. But this "no_track" option is not compatible with keepalived 1.x series which is available e.g. on Ubuntu 18.04. As there is no easy way to check automatically if keepalived supports or not this config flag, this patch introduces new config option "keepalived_use_no_track". If this config option will be set to False, neutron L3 agent will not add "no_track" to the keepalived's config. As master branch is moving to gate on Ubuntu 20.04 where keepalived 2.x is already available, this new config option default value is set to True. [1] https://review.opendev.org/#/c/721799/ Change-Id: I2dfdb9f56de28d56ca0f240ff34fa7c3a12e339b Closes-Bug: #1890400 --- neutron/agent/linux/keepalived.py | 5 +- neutron/cmd/sanity/checks.py | 2 + neutron/conf/agent/l3/config.py | 6 + .../tests/functional/agent/l3/framework.py | 2 + .../functional/agent/linux/test_keepalived.py | 2 + .../tests/unit/agent/linux/test_keepalived.py | 125 +++++++++++++++--- ..._track-config-option-4fa10304ee2960e6.yaml | 7 + 7 files changed, 132 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/add-keepalived_use_no_track-config-option-4fa10304ee2960e6.yaml diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py index 7afe51a76a5..da757a8416b 100644 --- a/neutron/agent/linux/keepalived.py +++ b/neutron/agent/linux/keepalived.py @@ -105,7 +105,7 @@ class KeepalivedVipAddress(object): result = '%s dev %s' % (self.ip_address, self.interface_name) if self.scope: result += ' scope %s' % self.scope - if not self.track: + if cfg.CONF.keepalived_use_no_track and not self.track: result += ' no_track' return result @@ -128,7 +128,8 @@ class KeepalivedVirtualRoute(object): output += ' dev %s' % self.interface_name if self.scope: output += ' scope %s' % self.scope - output += ' no_track' + if cfg.CONF.keepalived_use_no_track: + output += ' no_track' return output diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py index d45374a7dda..520290db0ff 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -34,6 +34,7 @@ from neutron.agent.linux import ip_lib from neutron.agent.linux import keepalived from neutron.agent.linux import utils as agent_utils from neutron.common import utils as common_utils +from neutron.conf.agent.l3 import config as l3_config from neutron.plugins.ml2.drivers.openvswitch.agent.common \ import constants as ovs_const from neutron.privileged.agent.linux import dhcp as priv_dhcp @@ -259,6 +260,7 @@ def bridge_firewalling_enabled(): class KeepalivedIPv6Test(object): def __init__(self, ha_port, gw_port, gw_vip, default_gw): + l3_config.register_l3_agent_config_opts(l3_config.OPTS, cfg.CONF) self.ha_port = ha_port self.gw_port = gw_port self.gw_vip = gw_vip diff --git a/neutron/conf/agent/l3/config.py b/neutron/conf/agent/l3/config.py index 060423e11a4..e8ee3f1ec9a 100644 --- a/neutron/conf/agent/l3/config.py +++ b/neutron/conf/agent/l3/config.py @@ -106,6 +106,12 @@ OPTS = [ 'the state change monitor. NOTE: Setting to True ' 'could affect the data plane when stopping or ' 'restarting the L3 agent.')), + cfg.BoolOpt('keepalived_use_no_track', + default=True, + help=_('If keepalived without support for "no_track" option ' + 'is used, this should be set to False. ' + 'Support for this option was introduced in keepalived ' + '2.x')) ] diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 00deb8fb020..e28998e6ec1 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -36,6 +36,7 @@ from neutron.agent.linux import keepalived from neutron.agent.metadata import driver as metadata_driver from neutron.common import utils as common_utils from neutron.conf.agent import common as agent_config +from neutron.conf.agent.l3 import config as l3_config from neutron.conf import common as common_config from neutron.tests.common import l3_test_common from neutron.tests.common import net_helpers @@ -93,6 +94,7 @@ class L3AgentTestFramework(base.BaseSudoTestCase): self.mock_plugin_api = mock.patch( 'neutron.agent.l3.agent.L3PluginApi').start().return_value mock.patch('neutron.agent.rpc.PluginReportStateAPI').start() + l3_config.register_l3_agent_config_opts(l3_config.OPTS, cfg.CONF) self.conf = self._configure_agent('agent1') self.agent = neutron_l3_agent.L3NATAgentWithStateReport('agent1', self.conf) diff --git a/neutron/tests/functional/agent/linux/test_keepalived.py b/neutron/tests/functional/agent/linux/test_keepalived.py index cd60200b93d..1451b021710 100644 --- a/neutron/tests/functional/agent/linux/test_keepalived.py +++ b/neutron/tests/functional/agent/linux/test_keepalived.py @@ -22,6 +22,7 @@ from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib from neutron.agent.linux import keepalived from neutron.common import utils as common_utils +from neutron.conf.agent.l3 import config as l3_config from neutron.tests.common import net_helpers from neutron.tests.functional.agent.linux import helpers from neutron.tests.functional import base @@ -33,6 +34,7 @@ class KeepalivedManagerTestCase(base.BaseSudoTestCase, def setUp(self): super(KeepalivedManagerTestCase, self).setUp() + l3_config.register_l3_agent_config_opts(l3_config.OPTS, cfg.CONF) cfg.CONF.set_override('check_child_processes_interval', 1, 'AGENT') self.expected_config = self._get_config() diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py index dbdb37124e0..4a46901dc20 100644 --- a/neutron/tests/unit/agent/linux/test_keepalived.py +++ b/neutron/tests/unit/agent/linux/test_keepalived.py @@ -18,9 +18,11 @@ import textwrap from unittest import mock from neutron_lib import constants as n_consts +from oslo_config import cfg import testtools from neutron.agent.linux import keepalived +from neutron.conf.agent.l3 import config as l3_config from neutron.tests import base # Keepalived user guide: @@ -37,7 +39,14 @@ VRRP_ID = 1 VRRP_INTERVAL = 5 -class KeepalivedGetFreeRangeTestCase(base.BaseTestCase): +class KeepalivedBaseTestCase(base.BaseTestCase): + + def setUp(self): + super(KeepalivedBaseTestCase, self).setUp() + l3_config.register_l3_agent_config_opts(l3_config.OPTS, cfg.CONF) + + +class KeepalivedGetFreeRangeTestCase(KeepalivedBaseTestCase): def test_get_free_range(self): free_range = keepalived.get_free_range( parent_range='169.254.0.0/16', @@ -122,7 +131,7 @@ class KeepalivedConfBaseMixin(object): return config -class KeepalivedConfTestCase(base.BaseTestCase, +class KeepalivedConfTestCase(KeepalivedBaseTestCase, KeepalivedConfBaseMixin): expected = KEEPALIVED_GLOBAL_CONFIG + textwrap.dedent(""" @@ -191,7 +200,62 @@ class KeepalivedConfTestCase(base.BaseTestCase, self.assertEqual(['192.168.2.0/24', '192.168.3.0/24'], current_vips) -class KeepalivedStateExceptionTestCase(base.BaseTestCase): +class KeepalivedConfWithoutNoTrackTestCase(KeepalivedConfTestCase): + + expected = KEEPALIVED_GLOBAL_CONFIG + textwrap.dedent(""" + vrrp_instance VR_1 { + state MASTER + interface eth0 + virtual_router_id 1 + priority 50 + garp_master_delay 60 + advert_int 5 + authentication { + auth_type AH + auth_pass pass123 + } + track_interface { + eth0 + } + virtual_ipaddress { + 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 + } + virtual_routes { + 0.0.0.0/0 via 192.168.1.1 dev eth1 + } + } + vrrp_instance VR_2 { + state MASTER + interface eth4 + virtual_router_id 2 + priority 50 + garp_master_delay 60 + mcast_src_ip 224.0.0.1 + track_interface { + eth4 + } + virtual_ipaddress { + 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 + } + }""") + + def setUp(self): + super(KeepalivedConfWithoutNoTrackTestCase, self).setUp() + cfg.CONF.set_override('keepalived_use_no_track', False) + + +class KeepalivedStateExceptionTestCase(KeepalivedBaseTestCase): def test_state_exception(self): invalid_vrrp_state = 'a seal walks' self.assertRaises(keepalived.InvalidInstanceStateException, @@ -207,7 +271,7 @@ class KeepalivedStateExceptionTestCase(base.BaseTestCase): invalid_auth_type, 'some_password') -class KeepalivedInstanceRoutesTestCase(base.BaseTestCase): +class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase): @classmethod def _get_instance_routes(cls): routes = keepalived.KeepalivedInstanceRoutes() @@ -248,15 +312,27 @@ class KeepalivedInstanceRoutesTestCase(base.BaseTestCase): routes = self._get_instance_routes() self.assertEqual(expected, '\n'.join(routes.build_config())) + def test_build_config_without_no_track_option(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 + }""" + cfg.CONF.set_override('keepalived_use_no_track', False) + routes = self._get_instance_routes() + self.assertEqual(expected, '\n'.join(routes.build_config())) -class KeepalivedInstanceTestCase(base.BaseTestCase, + +class KeepalivedInstanceTestCase(KeepalivedBaseTestCase, KeepalivedConfBaseMixin): def test_get_primary_vip(self): instance = keepalived.KeepalivedInstance('MASTER', 'ha0', 42, ['169.254.192.0/18']) self.assertEqual('169.254.0.42/24', instance.get_primary_vip()) - def test_remove_addresses_by_interface(self): + def _test_remove_addresses_by_interface(self, no_track_value): config = self._get_config() instance = config.get_instance(1) instance.remove_vips_vroutes_by_interface('eth2') @@ -281,10 +357,10 @@ class KeepalivedInstanceTestCase(base.BaseTestCase, 169.254.0.1/24 dev eth0 } virtual_ipaddress_excluded { - 192.168.1.0/24 dev eth1 no_track + 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 + 0.0.0.0/0 via 192.168.1.1 dev eth1%(no_track)s } } vrrp_instance VR_2 { @@ -301,14 +377,21 @@ class KeepalivedInstanceTestCase(base.BaseTestCase, 169.254.0.2/24 dev eth4 } virtual_ipaddress_excluded { - 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 + 192.168.2.0/24 dev eth2%(no_track)s + 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}) self.assertEqual(expected, config.get_config_str()) + def test_remove_addresses_by_interface(self): + self._test_remove_addresses_by_interface(" no_track") + + def test_remove_addresses_by_interface_without_no_track(self): + cfg.CONF.set_override('keepalived_use_no_track', False) + self._test_remove_addresses_by_interface("") + def test_build_config_no_vips(self): expected = textwrap.dedent("""\ vrrp_instance VR_1 { @@ -351,7 +434,7 @@ vrrp_instance VR_1 { self.assertEqual(expected, '\n'.join(instance.build_config())) -class KeepalivedVipAddressTestCase(base.BaseTestCase): +class KeepalivedVipAddressTestCase(KeepalivedBaseTestCase): def test_vip_with_scope(self): vip = keepalived.KeepalivedVipAddress('fe80::3e97:eff:fe26:3bfa/64', 'eth1', @@ -367,20 +450,32 @@ class KeepalivedVipAddressTestCase(base.BaseTestCase): self.assertEqual(1, len(instance.vips)) -class KeepalivedVirtualRouteTestCase(base.BaseTestCase): +class KeepalivedVirtualRouteTestCase(KeepalivedBaseTestCase): 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 no_track', 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', + 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 no_track', 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()) -class KeepalivedTrackScriptTestCase(base.BaseTestCase): + +class KeepalivedTrackScriptTestCase(KeepalivedBaseTestCase): def test_build_config_preamble(self): exp_conf = [ diff --git a/releasenotes/notes/add-keepalived_use_no_track-config-option-4fa10304ee2960e6.yaml b/releasenotes/notes/add-keepalived_use_no_track-config-option-4fa10304ee2960e6.yaml new file mode 100644 index 00000000000..f710212c919 --- /dev/null +++ b/releasenotes/notes/add-keepalived_use_no_track-config-option-4fa10304ee2960e6.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + New config option ``keepalived_use_no_track`` was added. If keepalived + version used on the deployment does not support ``no_track`` flag in its + config file (e.g. keepalived 1.x), this option should be set to ``False``. + Default value of this option is ``True``.