From 8ffbcb5a854ac12977e275f96f5f68f6cf41e24f 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/ Conflicts: neutron/conf/agent/l3/config.py neutron/tests/functional/agent/linux/test_keepalived.py Change-Id: I2dfdb9f56de28d56ca0f240ff34fa7c3a12e339b Closes-Bug: #1890400 (cherry picked from commit 7abe0ee34c367b4abf84820048b4aed643fc1162) --- 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 c5534e5bb61..8e9f7f4c9fc 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 1093f9a6e60..081456eea06 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -34,6 +34,7 @@ from neutron.agent.linux import utils as agent_utils from neutron.cmd import runtime_checks from neutron.common import constants 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 @@ -250,6 +251,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 987a29e2bbb..5258752ed1e 100644 --- a/neutron/conf/agent/l3/config.py +++ b/neutron/conf/agent/l3/config.py @@ -109,6 +109,12 @@ OPTS = [ '(by default), the user executing the L3 agent will be ' 'passed. If "root" specified, because radvd is spawned ' 'as root, no "username" parameter will be passed.')), + 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')) ] OPTS += config.EXT_NET_BRIDGE_OPTS diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 7c3cabc9d22..0f35caf0a6d 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.common import constants as n_const 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 5b23f9288fd..c021fbc30b4 100644 --- a/neutron/tests/functional/agent/linux/test_keepalived.py +++ b/neutron/tests/functional/agent/linux/test_keepalived.py @@ -20,6 +20,7 @@ from neutron.agent.linux import external_process from neutron.agent.linux import keepalived from neutron.agent.linux import utils from neutron.common import utils as common_utils +from neutron.conf.agent.l3 import config as l3_config from neutron.tests.functional.agent.linux import helpers from neutron.tests.functional import base from neutron.tests.unit.agent.linux import test_keepalived @@ -30,6 +31,7 @@ class KeepalivedManagerTestCase(base.BaseLoggingTestCase, 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 082db3eb11d..1daef336d4a 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 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``.