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 7abe0ee34c)

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
 
 

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

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

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)

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()

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 = [

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``.