From 8e06d1d1eb57e35e6a51ecfe5a76ffa18b5e8c62 Mon Sep 17 00:00:00 2001 From: Nate Johnston Date: Wed, 16 Sep 2020 22:47:18 -0400 Subject: [PATCH] Remove extraneous garp emission Previously, to work around a bug in keepalived, the neutron-keepalived-state-change-monitor process was altered to emit garps under certain circumstances. That issue with keepalived issue has been resolved, so remove the extraneous garp. Two tests that tested the removed functionality have also been removed. This is a partial revert of https://review.opendev.org/393886/ Change-Id: I9fc5868067e890321ad880cc134fb034b5401f47 Co-Authored-By: Rodolfo Alonso Hernandez --- neutron/agent/l3/keepalived_state_change.py | 23 ------- .../agent/l3/test_keepalived_state_change.py | 61 ------------------- 2 files changed, 84 deletions(-) diff --git a/neutron/agent/l3/keepalived_state_change.py b/neutron/agent/l3/keepalived_state_change.py index fc46cfbeb07..f3c85ae8aeb 100644 --- a/neutron/agent/l3/keepalived_state_change.py +++ b/neutron/agent/l3/keepalived_state_change.py @@ -18,7 +18,6 @@ import sys import threading import httplib2 -import netaddr from oslo_config import cfg from oslo_log import log as logging @@ -90,15 +89,6 @@ class MonitorDaemon(daemon.Daemon): new_state = 'backup' self.write_state_change(new_state) self.notify_agent(new_state) - elif event['name'] != self.interface and event['event'] == 'added': - # Send GARPs for all new router interfaces. - # REVISIT(jlibosva): keepalived versions 1.2.19 and below - # contain bug where gratuitous ARPs are not sent on receiving - # SIGHUP signal. This is a workaround to this bug. keepalived - # has this issue fixed since 1.2.20 but the version is not - # packaged in some distributions (RHEL/CentOS/Ubuntu Xenial). - # Remove this code once new keepalived versions are available. - self.send_garp(event) def handle_initial_state(self): try: @@ -137,19 +127,6 @@ class MonitorDaemon(daemon.Daemon): LOG.debug('Notified agent router %s, state %s', self.router_id, state) - def send_garp(self, event): - """Send gratuitous ARP for given event.""" - ip_address = str(netaddr.IPNetwork(event['cidr']).ip) - ip_lib.send_ip_addr_adv_notif( - self.namespace, - event['name'], - ip_address, - log_exception=False, - use_eventlet=False - ) - LOG.debug('Sent GARP to %(ip_address)s from %(device_name)s', - {'ip_address': ip_address, 'device_name': event['name']}) - def handle_sigterm(self, signum, frame): self.event_stop.set() self._thread_read_queue.join(timeout=5) 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 dbdc4387ed6..150b66a6132 100644 --- a/neutron/tests/functional/agent/l3/test_keepalived_state_change.py +++ b/neutron/tests/functional/agent/l3/test_keepalived_state_change.py @@ -12,12 +12,10 @@ # License for the specific language governing permissions and limitations # under the License. -import functools import os from unittest import mock import eventlet -import netaddr from oslo_utils import uuidutils from neutron.agent.l3 import ha @@ -110,49 +108,6 @@ class TestMonitorDaemon(base.BaseLoggingTestCase): {'file_name': file_name, 'text': text, 'devices': devices, 'file_content': open(file_name).read()}) - def test_new_fip_sends_garp(self): - ns_ip_wrapper = ip_lib.IPWrapper(self.router.namespace) - new_interface = ns_ip_wrapper.add_dummy('new_interface') - new_interface_cidr = '169.254.152.1/24' - new_interface.link.set_up() - new_interface.addr.add(new_interface_cidr) - self._generate_cmd_opts(monitor_interface='new_interface', - cidr=new_interface_cidr) - - self._run_monitor() - - next_ip_cidr = net_helpers.increment_ip_cidr(self.machines.ip_cidr, 2) - expected_ip = str(netaddr.IPNetwork(next_ip_cidr).ip) - # Create incomplete ARP entry - self.peer.assert_no_ping(expected_ip) - # Wait for ping expiration - eventlet.sleep(1) - has_entry = has_expected_arp_entry( - self.peer.port.name, - self.peer.namespace, - expected_ip, - self.router.port.link.address) - self.assertFalse(has_entry) - self.router.port.addr.add(next_ip_cidr) - has_arp_entry_predicate = functools.partial( - has_expected_arp_entry, - self.peer.port.name, - self.peer.namespace, - expected_ip, - self.router.port.link.address, - ) - exc = RuntimeError( - "No ARP entry in %s namespace containing IP address %s and MAC " - "address %s" % ( - self.peer.namespace, - expected_ip, - self.router.port.link.address)) - utils.wait_until_true(has_arp_entry_predicate, timeout=15, - exception=exc) - msg = ('Sent GARP to %(cidr)s from %(device)s' % - {'cidr': expected_ip, 'device': self.router.port.name}) - self._search_in_file(self.log_file, msg) - def test_read_queue_change_state(self): self._run_monitor() msg = 'Wrote router %s state %s' @@ -161,22 +116,6 @@ class TestMonitorDaemon(base.BaseLoggingTestCase): self.router.port.addr.delete(self.cidr) self._search_in_file(self.log_file, msg % (self.router_id, 'backup')) - def test_read_queue_send_garp(self): - self._run_monitor() - dev_dummy = 'dev_dummy' - ip_wrapper = ip_lib.IPWrapper(namespace=self.router.namespace) - ip_wrapper.add_dummy(dev_dummy) - ip_device = ip_lib.IPDevice(dev_dummy, namespace=self.router.namespace) - ip_device.link.set_up() - msg = 'Sent GARP to %(ip_address)s from %(device_name)s' - for idx in range(2, 20): - next_cidr = net_helpers.increment_ip_cidr(self.cidr, idx) - ip_device.addr.add(next_cidr) - msg_args = {'ip_address': str(netaddr.IPNetwork(next_cidr).ip), - 'device_name': dev_dummy} - self._search_in_file(self.log_file, msg % msg_args) - ip_device.addr.delete(next_cidr) - def test_handle_initial_state_backup(self): # No tracked IP (self.cidr) is configured in the monitored interface # (self.router.port)