From 9a0814c760777e53c9499bb3a891f436d34f3bee Mon Sep 17 00:00:00 2001 From: Gaudenz Steinlin Date: Thu, 9 Jan 2025 18:09:27 +0100 Subject: [PATCH] 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 | 2 + neutron/agent/l3/ha_router.py | 11 +- neutron/agent/l3/keepalived_state_change.py | 67 ++++++- neutron/agent/linux/conntrackd.py | 166 ++---------------- neutron/agent/linux/external_process.py | 1 - neutron/agent/linux/keepalived.py | 10 +- neutron/agent/linux/utils.py | 26 +-- neutron/conf/agent/l3/keepalived.py | 3 + neutron/tests/common/helpers.py | 19 -- neutron/tests/fullstack/resources/config.py | 2 +- .../tests/functional/agent/l3/framework.py | 8 - .../agent/l3/test_keepalived_state_change.py | 42 ++++- neutron/tests/functional/resources/process.py | 3 +- .../tests/unit/agent/linux/test_keepalived.py | 55 +----- ...r-conntrackd-support-6bc8b039fc3ac207.yaml | 2 +- 15 files changed, 149 insertions(+), 268 deletions(-) diff --git a/bindep.txt b/bindep.txt index c7f75f31dfd..71ad4d7141c 100644 --- a/bindep.txt +++ b/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] diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index 84b9acc9ef2..76b71107e01 100644 --- a/neutron/agent/l3/ha_router.py +++ b/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 diff --git a/neutron/agent/l3/keepalived_state_change.py b/neutron/agent/l3/keepalived_state_change.py index de32c13dede..226f6426ae8 100644 --- a/neutron/agent/l3/keepalived_state_change.py +++ b/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() diff --git a/neutron/agent/linux/conntrackd.py b/neutron/agent/linux/conntrackd.py index 364c5a9cb6c..41e5e7227dc 100644 --- a/neutron/agent/linux/conntrackd.py +++ b/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 -# -# 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 conf_dir - - def get_ha_script_path(self): - return self.get_full_config_file_path('primary-backup.sh') + return os.path.join(confs_dir, self.resource_id) 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 - # conntrackd source code. See UNIX_PATH_MAX constant in include/local.h - return self.get_full_config_file_path('conntrackd.ctl', maxlen=107) + return self.get_full_config_file_path( + 'conntrackd.ctl', + 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) diff --git a/neutron/agent/linux/external_process.py b/neutron/agent/linux/external_process.py index a668e81c5e2..116e3370876 100644 --- a/neutron/agent/linux/external_process.py +++ b/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 diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py index db001eeb3f3..16e00b81eeb 100644 --- a/neutron/agent/linux/keepalived.py +++ b/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') diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py index e8001a3322f..5d52e214367 100644 --- a/neutron/agent/linux/utils.py +++ b/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): - """Retrieve a list of the pids by their cmd.""" - pids = execute(['pgrep', '-f', cmd], log_fail_as_error=False).split() +def pgrep( + command: str, + 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: - raise RuntimeError( - _('%i processes for "%s" found.' % (len(pids), cmd)) - ) - if pids == []: - raise RuntimeError(_('No process for "%s" found.' % cmd)) - - return pids[0] + return result if result else None def find_parent_pid(pid): diff --git a/neutron/conf/agent/l3/keepalived.py b/neutron/conf/agent/l3/keepalived.py index bd46c723fc9..f82bd07635b 100644 --- a/neutron/conf/agent/l3/keepalived.py +++ b/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 ' diff --git a/neutron/tests/common/helpers.py b/neutron/tests/common/helpers.py index ec1b2e69ec8..e4a8d9b7835 100644 --- a/neutron/tests/common/helpers.py +++ b/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 diff --git a/neutron/tests/fullstack/resources/config.py b/neutron/tests/fullstack/resources/config.py index 23cca8f3bda..091318aebe7 100644 --- a/neutron/tests/fullstack/resources/config.py +++ b/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, diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index f95cab6af1d..94600ea9895 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/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, diff --git a/neutron/tests/functional/agent/l3/test_keepalived_state_change.py b/neutron/tests/functional/agent/l3/test_keepalived_state_change.py index 3feeb27b9b8..2804e47ed7f 100644 --- a/neutron/tests/functional/agent/l3/test_keepalived_state_change.py +++ b/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()) diff --git a/neutron/tests/functional/resources/process.py b/neutron/tests/functional/resources/process.py index ec5c5ad54b0..c3d5ee998d0 100644 --- a/neutron/tests/functional/resources/process.py +++ b/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) diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py index f5bfae39756..c0942de4e39 100644 --- a/neutron/tests/unit/agent/linux/test_keepalived.py +++ b/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'], - None) + ['169.254.192.0/18']) 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)) diff --git a/releasenotes/notes/ha-router-conntrackd-support-6bc8b039fc3ac207.yaml b/releasenotes/notes/ha-router-conntrackd-support-6bc8b039fc3ac207.yaml index 1c43684b4e9..4e6aa15eb87 100644 --- a/releasenotes/notes/ha-router-conntrackd-support-6bc8b039fc3ac207.yaml +++ b/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.