Fixup conntrackd support

This contains several fixups for which did not make it into the original
change before it got merged:

* Removal of the GPL licensed shell script. This is replaced by Python
  code in neutron-keepalived-state-change. This code was rewritten from
  scratch and is Apache licensed.
* Addition of the conntrackd package to bindep.txt for tests. This
  properly documents this new test requirement.
* Several typo and other small fixes suggested by Ihar Hrachyshka.
* Common pgrep utility function as suggested by lajoskatona.

Closes-Bug: #2092907
Change-Id: Ica1c38b3a635d60a08f48272fcda2034b0323d10

bindep.txt

@@ -21,5 +21,7 @@ mysql-server [platform:dpkg !platform:debian test]
 
 haproxy
 keepalived
+conntrackd [platform:dpkg test conntrackd]
+conntrack-tools [platform:rpm test conntrackd]
 
 iproute-tc [platform:rpm]

neutron/agent/l3/ha_router.py

@@ -207,9 +207,6 @@ class HaRouter(router.RouterInfo):
         interface_name = self.get_ha_device_name()
         subnets = self.ha_port.get('subnets', [])
         ha_port_cidrs = [subnet['cidr'] for subnet in subnets]
-        notify_script = (self.agent_conf.ha_conntrackd_enabled and
-                         self.conntrackd_manager.get_ha_script_path() or
-                         None)
         instance = keepalived.KeepalivedInstance(
             'BACKUP',
             interface_name,
@@ -221,7 +218,6 @@ class HaRouter(router.RouterInfo):
             vrrp_health_check_interval=(
                 self.agent_conf.ha_vrrp_health_check_interval),
             ha_conf_dir=self.keepalived_manager.get_conf_dir(),
-            notify_script=notify_script,
         )
         instance.track_interfaces.append(interface_name)
 
@@ -475,6 +471,13 @@ class HaRouter(router.RouterInfo):
                 '--state_path=%s' % self.agent_conf.state_path,
                 '--user=%s' % os.geteuid(),
                 '--group=%s' % os.getegid()]
+
+            if self.agent_conf.ha_conntrackd_enabled:
+                cmd.append('--enable_conntrackd')
+
+            if self.agent_conf.debug:
+                cmd.append('--debug')
+
             return cmd
 
         return callback

neutron/agent/l3/keepalived_state_change.py

@@ -29,6 +29,7 @@ from neutron.agent.linux import utils as agent_utils
 from neutron.common import config
 from neutron.common import utils as common_utils
 from neutron.conf.agent import common as agent_config
+from neutron.conf.agent.l3 import ha as ha_conf
 from neutron.conf.agent.l3 import keepalived
 from neutron import privileged
 
@@ -49,12 +50,13 @@ class KeepalivedUnixDomainConnection(agent_utils.UnixDomainHTTPConnection):
 
 class MonitorDaemon(daemon.Daemon):
     def __init__(self, pidfile, router_id, user, group, namespace, conf_dir,
-                 interface, cidr):
+                 interface, cidr, ha_conntrackd_enabled):
         self.router_id = router_id
         self.namespace = namespace
         self.conf_dir = conf_dir
         self.interface = interface
         self.cidr = cidr
+        self.ha_conntrackd_enabled = ha_conntrackd_enabled
         self.monitor = None
         self.event_stop = threading.Event()
         self.event_started = threading.Event()
@@ -85,8 +87,6 @@ class MonitorDaemon(daemon.Daemon):
             target=self.read_queue,
             args=(self.queue, self.event_stop, self.event_started))
         self._thread_initial_state.start()
-        self._thread_ip_monitor.start()
-        self._thread_read_queue.start()
 
         # NOTE(ralonsoh): if the initial status is not read in a defined
         # timeout, "backup" state is set.
@@ -95,8 +95,14 @@ class MonitorDaemon(daemon.Daemon):
             LOG.warning('Timeout reading the initial status of router %s, '
                         'state is set to "backup".', self.router_id)
             self.write_state_change('backup')
+            if self.ha_conntrackd_enabled:
+                self.sync_conntrack('backup')
             self.notify_agent('backup')
 
+        # NOTE(gaudenz): Only starte these threads after the initial status is
+        # set because otherwise the initial status thread sometimes hangs.
+        self._thread_ip_monitor.start()
+        self._thread_read_queue.start()
         self._thread_read_queue.join()
 
     def read_queue(self, _queue, event_stop, event_started):
@@ -115,6 +121,8 @@ class MonitorDaemon(daemon.Daemon):
                 else:
                     new_state = 'backup'
                 self.write_state_change(new_state)
+                if self.ha_conntrackd_enabled:
+                    self.sync_conntrack(new_state)
                 self.notify_agent(new_state)
 
     def handle_initial_state(self):
@@ -132,6 +140,8 @@ class MonitorDaemon(daemon.Daemon):
 
             if not self.initial_state:
                 self.write_state_change(state)
+                if self.ha_conntrackd_enabled:
+                    self.sync_conntrack(state)
                 self.notify_agent(state)
         except Exception:
             if not self.initial_state:
@@ -160,6 +170,53 @@ class MonitorDaemon(daemon.Daemon):
 
         LOG.debug('Notified agent router %s, state %s', self.router_id, state)
 
+    def conntrackd(self, option):
+        execute = ip_lib.IPWrapper(namespace=self.namespace).netns.execute
+        cmd = ['conntrackd', '-C', f'{self.conf_dir}/conntrackd.conf', option]
+
+        LOG.debug('Executing "%s" on router %s', ' '.join(cmd), self.router_id)
+        execute(
+            cmd,
+            run_as_root=True,
+            # Errors are still logged, but should not crash the daemon
+            check_exit_code=False,
+        )
+
+    def sync_conntrack(self, state):
+        if state == 'primary':
+            self.sync_conntrack_primary()
+        elif state == 'backup':
+            self.sync_conntrack_backup()
+        else:
+            LOG.error(f'Unknown state "{state}".')
+
+    def sync_conntrack_primary(self):
+
+        # commit the external cache into the kernel table
+        self.conntrackd('-c')
+
+        # flush the internal and the external caches
+        self.conntrackd('-f')
+
+        # resynchronize my internal cache to the kernel table
+        self.conntrackd('-R')
+
+        # send a bulk update to backups
+        self.conntrackd('-B')
+
+        LOG.debug('Synced connection tracking state on primary for router %s',
+                  self.router_id)
+
+    def sync_conntrack_backup(self):
+        # shorten kernel conntrack timers to remove the zombie entries.
+        self.conntrackd('-t')
+
+        # request resynchronization with primary firewall replica (if any)
+        self.conntrackd('-n')
+
+        LOG.debug('Synced connection tracking state on backup for router %s',
+                  self.router_id)
+
     def handle_sigterm(self, signum, frame):
         self.event_stop.set()
         self._thread_read_queue.join(timeout=5)
@@ -178,6 +235,7 @@ def configure(conf):
 def main():
     keepalived.register_cli_l3_agent_keepalived_opts()
     keepalived.register_l3_agent_keepalived_opts()
+    ha_conf.register_l3_agent_ha_opts()
     agent_config.register_root_helper()
     configure(cfg.CONF)
     MonitorDaemon(cfg.CONF.pid_file,
@@ -187,4 +245,5 @@ def main():
                   cfg.CONF.namespace,
                   cfg.CONF.conf_dir,
                   cfg.CONF.monitor_interface,
-                  cfg.CONF.monitor_cidr).start()
+                  cfg.CONF.monitor_cidr,
+                  cfg.CONF.enable_conntrackd).start()

neutron/agent/linux/conntrackd.py

@@ -15,7 +15,6 @@
 
 import os
 import signal
-import stat
 
 import jinja2
 
@@ -32,6 +31,9 @@ from neutron.common import utils as common_utils
 CONNTRACKD_SERVICE_NAME = 'conntrackd'
 SIGTERM_TIMEOUT = 5
 
+# Unix socket path length is limited to 107 characters in the
+# conntrackd source code. See UNIX_PATH_MAX constant in include/local.h
+UNIX_PATH_MAX = 107
 
 LOG = logging.getLogger(__name__)
 
@@ -77,135 +79,6 @@ Sync {
 }
 """)
 
-HA_SCRIPT_TEMPLATE = jinja2.Template(
-    """#!/usr/bin/env sh
-#
-# (C) 2006-2011 by Pablo Neira Ayuso <pablo@netfilter.org>
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# Description:
-#
-# This is the script for primary-backup setups for keepalived
-# (http://www.keepalived.org). You may adapt it to make it work with other
-# high-availability managers.
-#
-# Do not forget to include the required modifications to your keepalived.conf
-# file to invoke this script during keepalived's state transitions.
-#
-# Contributions to improve this script are welcome :).
-#
-
-CONNTRACKD_BIN=/usr/sbin/conntrackd
-CONNTRACKD_LOCK={{ lock }}
-CONNTRACKD_CONFIG={{ config }}
-
-case "$1" in
-  primary)
-    #
-    # commit the external cache into the kernel table
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -c
-    if [ $? -eq 1 ]
-    then
-      logger "ERROR: failed to invoke conntrackd -c"
-    fi
-
-    #
-    # flush the internal and the external caches
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -f
-    if [ $? -eq 1 ]
-    then
-      logger "ERROR: failed to invoke conntrackd -f"
-    fi
-
-    #
-    # resynchronize my internal cache to the kernel table
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -R
-    if [ $? -eq 1 ]
-    then
-      logger "ERROR: failed to invoke conntrackd -R"
-    fi
-
-    #
-    # send a bulk update to backups
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -B
-    if [ $? -eq 1 ]
-    then
-      logger "ERROR: failed to invoke conntrackd -B"
-    fi
-    ;;
-  backup)
-    #
-    # is conntrackd running? request some statistics to check it
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -s
-    if [ $? -eq 1 ]
-    then
-      #
-      # something's wrong, do we have a lock file?
-      #
-      if [ -f $CONNTRACKD_LOCK ]
-      then
-        logger "WARNING: conntrackd was not cleanly stopped."
-        logger "If you suspect that it has crashed:"
-        logger "1) Enable coredumps"
-        logger "2) Try to reproduce the problem"
-        logger "3) Post the coredump to netfilter-devel@vger.kernel.org"
-        rm -f $CONNTRACKD_LOCK
-      fi
-      $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -d
-      if [ $? -eq 1 ]
-      then
-        logger "ERROR: cannot launch conntrackd"
-        exit 1
-      fi
-    fi
-    #
-    # shorten kernel conntrack timers to remove the zombie entries.
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -t
-    if [ $? -eq 1 ]
-    then
-      logger "ERROR: failed to invoke conntrackd -t"
-    fi
-
-    #
-    # request resynchronization with master firewall replica (if any)
-    # Note: this does nothing in the alarm approach.
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -n
-    if [ $? -eq 1 ]
-    then
-      logger "ERROR: failed to invoke conntrackd -n"
-    fi
-    ;;
-  fault)
-    #
-    # shorten kernel conntrack timers to remove the zombie entries.
-    #
-    $CONNTRACKD_BIN -C $CONNTRACKD_CONFIG -t
-    if [ $? -eq 1 ]
-    then
-      logger "ERROR: failed to invoke conntrackd -t"
-    fi
-    ;;
-  *)
-    logger "ERROR: unknown state transition"
-    echo "Usage: primary-backup.sh {primary|backup|fault}"
-    exit 1
-    ;;
-esac
-
-exit 0
-""")
-
 
 class ConntrackdManager:
     """Wrapper for conntrackd.
@@ -225,18 +98,8 @@ class ConntrackdManager:
         self.ha_iface = ha_iface
         self.namespace = namespace
 
-    def build_ha_script(self):
-        ha_script_content = HA_SCRIPT_TEMPLATE.render(
-            lock=self.get_lockfile_path(),
-            config=self.get_conffile_path(),
-        )
-        ha_script_path = self.get_ha_script_path()
-
-        file_utils.replace_file(ha_script_path, ha_script_content)
-        os.chmod(ha_script_path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
-
     def get_full_config_file_path(self, filename, maxlen=255):
-        # Maximum PATH lenght for most paths in conntrackd is limited to
+        # Maximum PATH length for most paths in conntrackd is limited to
         # 255 characters.
         conf_dir = self.get_conf_dir()
         ensure_tree(conf_dir, 0o755)
@@ -254,11 +117,7 @@ class ConntrackdManager:
         confs_dir = os.path.abspath(
             os.path.normpath(self.agent_conf.ha_confs_path))
 
-        conf_dir = os.path.join(confs_dir, self.resource_id)
+        return os.path.join(confs_dir, self.resource_id)
-        return conf_dir
-
-    def get_ha_script_path(self):
-        return self.get_full_config_file_path('primary-backup.sh')
 
     def get_pid_file_path(self):
         return self.get_full_config_file_path('conntrackd.pid')
@@ -267,9 +126,10 @@ class ConntrackdManager:
         return self.get_full_config_file_path('conntrackd.lock')
 
     def get_ctlfile_path(self):
-        # Unix socket path length is limited to 107 characters in the
+        return self.get_full_config_file_path(
-        # conntrackd source code. See UNIX_PATH_MAX constant in include/local.h
+            'conntrackd.ctl',
-        return self.get_full_config_file_path('conntrackd.ctl', maxlen=107)
+            maxlen=UNIX_PATH_MAX,
+        )
 
     def get_conffile_path(self):
         return self.get_full_config_file_path('conntrackd.conf')
@@ -279,13 +139,15 @@ class ConntrackdManager:
         pid_file = self.get_pid_file_path()
 
         cmd = 'conntrackd -d -C %s' % config_path
-        pid = utils.find_pid_by_cmd(cmd)
+        pid = utils.pgrep(cmd)
+
+        if not pid:
+            raise RuntimeError(_('No process for "%s" found.' % cmd))
 
         file_utils.replace_file(pid_file, pid)
 
     def spawn(self):
         config_path = self.output_config_file()
-        self.build_ha_script()
 
         def callback(pidfile):
             cmd = ['conntrackd', '-d',
@@ -335,7 +197,7 @@ class ConntrackdManager:
         if not pm.active:
             return
 
-        # First try to stop conntrackd by using it's own control command
+        # First try to stop conntrackd by using its own control command
         config_path = self.get_conffile_path()
         cmd = ['conntrackd', '-C', config_path, '-k']
         utils.execute(cmd, run_as_root=True)

neutron/agent/linux/external_process.py

@@ -70,7 +70,6 @@ class ProcessManager(MonitoredProcess):
         self.default_cmd_callback = default_cmd_callback
         self.default_pre_cmd_callback = default_pre_cmd_callback
         self.default_post_cmd_callback = default_post_cmd_callback
-        self.cmd_addl_env = cmd_addl_env
         self.pids_path = pids_path or self.conf.external_pids
         self.pid_file = pid_file
         self.run_as_root = run_as_root or self.namespace is not None

neutron/agent/linux/keepalived.py

@@ -189,7 +189,7 @@ class KeepalivedInstance:
                  advert_int=None, mcast_src_ip=None, nopreempt=False,
                  garp_primary_delay=GARP_PRIMARY_DELAY,
                  vrrp_health_check_interval=0,
-                 ha_conf_dir=None, notify_script=None):
+                 ha_conf_dir=None):
         self.name = 'VR_%s' % vrouter_id
 
         if state not in VALID_STATES:
@@ -201,7 +201,6 @@ class KeepalivedInstance:
         self.priority = priority
         self.nopreempt = nopreempt
         self.advert_int = advert_int
-        self.notify_script = notify_script
         self.mcast_src_ip = mcast_src_ip
         self.garp_primary_delay = garp_primary_delay
         self.track_interfaces = []
@@ -318,13 +317,6 @@ class KeepalivedInstance:
                        '    priority %s' % self.priority,
                        '    garp_master_delay %s' % self.garp_primary_delay])
 
-        if self.notify_script:
-            config.extend([
-                       '    notify_master "%s primary"' % self.notify_script,
-                       '    notify_backup "%s backup"' % self.notify_script,
-                       '    notify_fault "%s fault"' % self.notify_script,
-            ])
-
         if self.nopreempt:
             config.append('    nopreempt')
 

neutron/agent/linux/utils.py

@@ -22,6 +22,7 @@ import shlex
 import socket
 import threading
 import time
+import typing
 
 import eventlet
 from eventlet.green import subprocess
@@ -35,7 +36,6 @@ from oslo_utils import excutils
 from oslo_utils import fileutils
 import psutil
 
-from neutron._i18n import _
 from neutron.api import wsgi
 from neutron.common import utils
 from neutron.conf.agent import common as config
@@ -182,18 +182,20 @@ def find_child_pids(pid, recursive=False):
     return child_pids
 
 
-def find_pid_by_cmd(cmd):
+def pgrep(
-    """Retrieve a list of the pids by their cmd."""
+        command: str,
-    pids = execute(['pgrep', '-f', cmd], log_fail_as_error=False).split()
+        entire_command_line: bool = True
+) -> typing.Optional[str]:
+    cmd = ['pgrep']
+    if entire_command_line:
+        cmd += ['-f']
+    cmd += [command]
+    try:
+        result = execute(cmd).strip()
+    except exceptions.ProcessExecutionError:
+        return None
 
-    if len(pids) > 1:
+    return result if result else None
-        raise RuntimeError(
-            _('%i processes for "%s" found.' % (len(pids), cmd))
-        )
-    if pids == []:
-        raise RuntimeError(_('No process for "%s" found.' % cmd))
-
-    return pids[0]
 
 
 def find_parent_pid(pid):

neutron/conf/agent/l3/keepalived.py

@@ -25,6 +25,9 @@ CLI_OPTS = [
     cfg.StrOpt('monitor_interface', help=_('Interface to monitor')),
     cfg.StrOpt('monitor_cidr', help=_('CIDR to monitor')),
     cfg.StrOpt('pid_file', help=_('Path to PID file for this process')),
+    cfg.BoolOpt('enable_conntrackd',
+                help=_('Enable conntrackd support'),
+                default=False),
     cfg.StrOpt('user', help=_('User (uid or name) running this process '
                               'after its initialization')),
     cfg.StrOpt('group', help=_('Group (gid or name) running this process '

neutron/tests/common/helpers.py

@@ -17,16 +17,13 @@ import math
 import os
 import random
 import signal
-import typing
 
 from neutron_lib.agent import topics
 from neutron_lib import constants
 from neutron_lib import context
-from neutron_lib import exceptions
 from oslo_utils import timeutils
 
 import neutron
-from neutron.agent.linux import utils as linux_utils
 from neutron.db import agents_db
 
 HOST = 'localhost'
@@ -262,19 +259,3 @@ class TestTimer:
 
         if self._alarm_fn:
             self._alarm_fn(timeout)
-
-
-def pgrep(
-        command: str,
-        entire_command_line: bool = True
-) -> typing.Optional[str]:
-    cmd = ['pgrep']
-    if entire_command_line:
-        cmd += ['-f']
-    cmd += [command]
-    try:
-        result = linux_utils.execute(cmd)
-    except exceptions.ProcessExecutionError:
-        return
-
-    return result[0] if result else None

neutron/tests/fullstack/resources/config.py

@@ -71,7 +71,7 @@ class NeutronConfigFixture(ConfigFixture):
                 'host': self._generate_host(),
                 # Enable conntrackd for tests to get full test coverage
                 'ha_conntrackd_enabled': 'True',
-                # Conntrackd only supports 107 characters for it's control
+                # Conntrackd only supports 107 characters for its control
                 # socket path. Thus the "state_path" should not be nested in
                 # a temporary directory to avoid the final path being too long.
                 'state_path': self.temp_dir,

neutron/tests/functional/agent/l3/framework.py

@@ -62,9 +62,6 @@ vrrp_instance VR_1 {
     virtual_router_id 1
     priority 50
     garp_master_delay 60
-    notify_master "%(conf_dir)s/primary-backup.sh primary"
-    notify_backup "%(conf_dir)s/primary-backup.sh backup"
-    notify_fault "%(conf_dir)s/primary-backup.sh fault"
     nopreempt
     advert_int 2
     track_interface {
@@ -480,10 +477,6 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
 
     def get_expected_keepalive_configuration(self, router):
         ha_device_name = router.get_ha_device_name()
-        conf_dir = os.path.join(
-            self.agent.conf.ha_confs_path,
-            router.router_id,
-        )
         external_port = router.get_ex_gw_port()
         ex_port_ipv6 = ip_lib.get_ipv6_lladdr(external_port['mac_address'])
         ex_device_name = router.get_external_device_name(
@@ -502,7 +495,6 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
             'email_from': keepalived.KEEPALIVED_EMAIL_FROM,
             'router_id': keepalived.KEEPALIVED_ROUTER_ID,
             'ha_device_name': ha_device_name,
-            'conf_dir': conf_dir,
             'ex_device_name': ex_device_name,
             'external_device_cidr': external_device_cidr,
             'internal_device_name': internal_device_name,

neutron/tests/functional/agent/l3/test_keepalived_state_change.py

@@ -71,7 +71,8 @@ class TestMonitorDaemon(base.BaseLoggingTestCase):
     def _callback(self, *args):
         return self.cmd_opts
 
-    def _generate_cmd_opts(self, monitor_interface=None, cidr=None):
+    def _generate_cmd_opts(self, monitor_interface=None, cidr=None,
+                           conntrackd=True):
         monitor_interface = monitor_interface or self.router.port.name
         cidr = cidr or self.cidr
         self.cmd_opts = [
@@ -89,6 +90,9 @@ class TestMonitorDaemon(base.BaseLoggingTestCase):
             '--debug',
         ]
 
+        if conntrackd:
+            self.cmd_opts.append('--enable_conntrackd')
+
     def _search_in_file(self, file_name, text):
         def text_in_file():
             try:
@@ -113,27 +117,46 @@ class TestMonitorDaemon(base.BaseLoggingTestCase):
 
     def test_read_queue_change_state(self):
         self._run_monitor()
-        msg = 'Wrote router %s state %s'
+        msg_state = 'Wrote router %s state %s'
+        msg_conntrack = 'Synced connection tracking state on %s for router %s'
+
         self.router.port.addr.add(self.cidr)
-        self._search_in_file(self.log_file, msg % (self.router_id, 'primary'))
+        self._search_in_file(self.log_file,
+                             msg_state % (self.router_id, 'primary'))
+        self._search_in_file(self.log_file,
+                             msg_conntrack % ('primary', self.router_id))
+
         self.router.port.addr.delete(self.cidr)
-        self._search_in_file(self.log_file, msg % (self.router_id, 'backup'))
+        self._search_in_file(self.log_file,
+                             msg_state % (self.router_id, 'backup'))
+        self._search_in_file(self.log_file,
+                             msg_conntrack % ('backup', self.router_id))
 
     def test_handle_initial_state_backup(self):
         # No tracked IP (self.cidr) is configured in the monitored interface
         # (self.router.port)
         self._run_monitor()
+
         msg = 'Initial status of router {} is {}'.format(
             self.router_id, 'backup')
         self._search_in_file(self.log_file, msg)
 
+        msg = ('Synced connection tracking state on backup for router %s' %
+               self.router_id)
+        self._search_in_file(self.log_file, msg)
+
     def test_handle_initial_state_primary(self):
         self.router.port.addr.add(self.cidr)
         self._run_monitor()
+
         msg = 'Initial status of router {} is {}'.format(
             self.router_id, 'primary')
         self._search_in_file(self.log_file, msg)
 
+        msg = ('Synced connection tracking state on primary for router %s' %
+               self.router_id)
+        self._search_in_file(self.log_file, msg)
+
     def test_handle_initial_state_backup_error_reading_initial_status(self):
         # By passing this wrong IP address, the thread "_thread_initial_state"
         # will fail generating an exception (caught inside the called method).
@@ -153,3 +176,14 @@ class TestMonitorDaemon(base.BaseLoggingTestCase):
         msg = 'Initial status of router {} is {}'.format(
             self.router_id, 'backup')
         self._search_in_file(self.log_file, msg)
+
+    def test_conntrackd_disabled(self):
+        self._generate_cmd_opts(conntrackd=False)
+        self._run_monitor()
+
+        # Wait for initial notification. This ensures the first update cycle is
+        # completed
+        self._search_in_file(self.log_file, 'Notified agent router')
+
+        # Check there are no conntrack messages
+        self.assertNotIn('conntrack', open(self.log_file).read())

neutron/tests/functional/resources/process.py

@@ -22,11 +22,10 @@ import psutil
 import tenacity
 
 from neutron.agent.linux import utils
-from neutron.tests.common import helpers
 
 
 def _kill_process_if_exists(command: str) -> None:
-    _pid = helpers.pgrep(command)
+    _pid = utils.pgrep(command)
     if _pid:
         utils.kill_process(_pid, signal.SIGKILL)
 

neutron/tests/unit/agent/linux/test_keepalived.py

@@ -92,22 +92,10 @@ class KeepalivedGetFreeRangeTestCase(KeepalivedBaseTestCase):
 
 class KeepalivedConfBaseMixin:
 
-    def _get_conntrackd_manager(self):
-        conntrackd_manager = mock.Mock()
-        conntrackd_manager.get_ha_script_path.return_value = '/tmp/ha.sh'
-        return conntrackd_manager
-
     def _get_config(self, track=True):
         config = keepalived.KeepalivedConf()
-        conntrackd_manager = self._get_conntrackd_manager()
-
-        notify_script = (cfg.CONF.ha_conntrackd_enabled and
-                         conntrackd_manager.get_ha_script_path() or
-                         None)
-
         instance1 = keepalived.KeepalivedInstance(
             'MASTER', 'eth0', 1, ['169.254.192.0/18'],
-            notify_script=notify_script,
             advert_int=5)
         instance1.set_authentication('AH', 'pass123')
         instance1.track_interfaces.append("eth0")
@@ -136,7 +124,6 @@ class KeepalivedConfBaseMixin:
 
         instance2 = keepalived.KeepalivedInstance(
             'MASTER', 'eth4', 2, ['169.254.192.0/18'],
-            notify_script=notify_script,
             mcast_src_ip='224.0.0.1')
         instance2.track_interfaces.append("eth4")
 
@@ -163,9 +150,6 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase,
             virtual_router_id 1
             priority 50
             garp_master_delay 60
-            notify_master "/tmp/ha.sh primary"
-            notify_backup "/tmp/ha.sh backup"
-            notify_fault "/tmp/ha.sh fault"
             advert_int 5
             authentication {
                 auth_type AH
@@ -193,9 +177,6 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase,
             virtual_router_id 2
             priority 50
             garp_master_delay 60
-            notify_master "/tmp/ha.sh primary"
-            notify_backup "/tmp/ha.sh backup"
-            notify_fault "/tmp/ha.sh fault"
             mcast_src_ip 224.0.0.1
             track_interface {
                 eth4
@@ -243,17 +224,6 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase,
         current_vips = sorted(instance.get_existing_vip_ip_addresses('eth2'))
         self.assertEqual(['192.168.2.0/24', '192.168.3.0/24'], current_vips)
 
-    def test_config_generation_no_conntrackd(self):
-        # Disable conntrackd support
-        self.config(ha_conntrackd_enabled=False)
-
-        config = self._get_config()
-
-        # Assert no notification scripts are configured
-        self.assertNotIn('notify_master', config.get_config_str())
-        self.assertNotIn('notify_backup', config.get_config_str())
-        self.assertNotIn('notify_fault', config.get_config_str())
-
 
 class KeepalivedStateExceptionTestCase(KeepalivedBaseTestCase):
     def test_state_exception(self):
@@ -261,11 +231,11 @@ class KeepalivedStateExceptionTestCase(KeepalivedBaseTestCase):
         self.assertRaises(keepalived.InvalidInstanceStateException,
                           keepalived.KeepalivedInstance,
                           invalid_vrrp_state, 'eth0', 33,
-                          ['169.254.192.0/18'], None)
+                          ['169.254.192.0/18'])
 
         invalid_auth_type = 'into a club'
         instance = keepalived.KeepalivedInstance('MASTER', 'eth0', 1,
-                                                 ['169.254.192.0/18'], None)
+                                                 ['169.254.192.0/18'])
         self.assertRaises(keepalived.InvalidAuthenticationTypeException,
                           instance.set_authentication,
                           invalid_auth_type, 'some_password')
@@ -350,8 +320,7 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase,
                                  KeepalivedConfBaseMixin):
     def test_get_primary_vip(self):
         instance = keepalived.KeepalivedInstance('MASTER', 'ha0', 42,
-                                                 ['169.254.192.0/18'],
+                                                 ['169.254.192.0/18'])
-                                                 None)
         self.assertEqual('169.254.0.42/24', instance.get_primary_vip())
 
     def _test_remove_addresses_by_interface(self, track=True):
@@ -369,9 +338,6 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase,
                 virtual_router_id 1
                 priority 50
                 garp_master_delay 60
-                notify_master "/tmp/ha.sh primary"
-                notify_backup "/tmp/ha.sh backup"
-                notify_fault "/tmp/ha.sh fault"
                 advert_int 5
                 authentication {{
                     auth_type AH
@@ -396,9 +362,6 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase,
                 virtual_router_id 2
                 priority 50
                 garp_master_delay 60
-                notify_master "/tmp/ha.sh primary"
-                notify_backup "/tmp/ha.sh backup"
-                notify_fault "/tmp/ha.sh fault"
                 mcast_src_ip 224.0.0.1
                 track_interface {{
                     eth4
@@ -441,18 +404,13 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase,
                 virtual_router_id 1
                 priority 50
                 garp_master_delay 60
-                notify_master "/tmp/ha.sh primary"
-                notify_backup "/tmp/ha.sh backup"
-                notify_fault "/tmp/ha.sh fault"
                 virtual_ipaddress {
                     169.254.0.1/24 dev eth0
                 }
             }""")
 
-        conntrackd_manager = self._get_conntrackd_manager()
         instance = keepalived.KeepalivedInstance(
             'MASTER', 'eth0', VRRP_ID, ['169.254.192.0/18'],
-            notify_script=conntrackd_manager.get_ha_script_path(),
         )
         self.assertEqual(expected, os.linesep.join(instance.build_config()))
 
@@ -472,18 +430,13 @@ class KeepalivedInstanceTestCase(KeepalivedBaseTestCase,
                 virtual_router_id 1
                 priority 50
                 garp_master_delay 60
-                notify_master "/tmp/ha.sh primary"
-                notify_backup "/tmp/ha.sh backup"
-                notify_fault "/tmp/ha.sh fault"
                 virtual_ipaddress {
                     169.254.0.1/24 dev eth0
                 }
             }""")
 
-        conntrackd_manager = self._get_conntrackd_manager()
         instance = keepalived.KeepalivedInstance(
             'MASTER', 'eth0', VRRP_ID, ['169.254.192.0/18'],
-            notify_script=conntrackd_manager.get_ha_script_path(),
         )
         instance.track_script = keepalived.KeepalivedTrackScript(
             VRRP_INTERVAL, '/etc/ha_confs/qrouter-x', VRRP_ID)
@@ -500,7 +453,7 @@ class KeepalivedVipAddressTestCase(KeepalivedBaseTestCase):
 
     def test_add_vip_idempotent(self):
         instance = keepalived.KeepalivedInstance('MASTER', 'eth0', 1,
-                                                 ['169.254.192.0/18'], None)
+                                                 ['169.254.192.0/18'])
         instance.add_vip('192.168.222.1/32', 'eth11', None)
         instance.add_vip('192.168.222.1/32', 'eth12', 'link')
         self.assertEqual(1, len(instance.vips))

releasenotes/notes/ha-router-conntrackd-support-6bc8b039fc3ac207.yaml

@@ -2,7 +2,7 @@
 features:
   - |
     HA routers can now run "conntrackd" in addition to "keepalived" to
-    synchronize connection tracking states accross router instances. This
+    synchronize connection tracking states across router instances. This
     ensures that established connections survive a HA router failover. L3 agent
     hosts must have the "conntrackd" binary installed.