openstack/neutron
Code Issues Proposed changes

Partially revert "Do not link up HA router gateway in backup node"

Browse Source 
This partially reverts commit c52029c39aa824a67095fbbf9e59eff769d92587.

We revert everything except one minor addition to
neutron/agent/l3/ha_router.py which ensures that ha_confs path is
created when the keepalived manager is initialised.

Closes-Bug: #1965297
Change-Id: I14ad015c4344b32f7210c924902dac4e6ad1ae88
This commit is contained in:
Edward Hope-Morley 2022-03-17 12:56:21 +00:00 committed by Damian Dabrowski
parent 9ca5c1cc04
commit 36bf1df46d
6 changed files with 24 additions and 76 deletions

4
neutron/agent/l3/dvr_edge_ha_router.py

@ -134,9 +134,7 @@ class DvrEdgeHaRouter(dvr_edge_router.DvrEdgeRouter,
def _external_gateway_added(self, ex_gw_port, interface_name,
ns_name, preserve_ips):
link_up = self.external_gateway_link_up()
self._plug_external_gateway(ex_gw_port, interface_name, ns_name,
link_up=link_up)
self._plug_external_gateway(ex_gw_port, interface_name, ns_name)
def _is_this_snat_host(self):
return self.agent_conf.agent_mode == constants.L3_AGENT_MODE_DVR_SNAT

9
neutron/agent/l3/ha.py

@ -189,15 +189,6 @@ class AgentMixin(object):
'agent %(host)s; NDP proxy enabled: %(enable_ndp_proxy)s',
state_change_data)
# Set external gateway port link up or down according to state
if state == 'primary':
ri.set_external_gw_port_link_status(link_up=True, set_gw=True)
elif state == 'backup':
ri.set_external_gw_port_link_status(link_up=False)
else:
LOG.warning('Router %s has status %s, '
'no action to router gateway device.',
router_id, state)
# TODO(dalvarez): Fix bug 1677279 by moving the IPv6 parameters
# configuration to keepalived-state-change in order to remove the
# dependency that currently exists on l3-agent running for the IPv6

32
neutron/agent/l3/ha_router.py

@ -170,6 +170,10 @@ class HaRouter(router.RouterInfo):
throttle_restart_value=(
self.agent_conf.ha_vrrp_advert_int * THROTTLER_MULTIPLIER))
# The following call is required to ensure that if the state path does
# not exist it gets created.
self.keepalived_manager.get_full_config_file_path('test')
config = self.keepalived_manager.config
interface_name = self.get_ha_device_name()
@ -469,9 +473,7 @@ class HaRouter(router.RouterInfo):
return port1_filtered == port2_filtered
def external_gateway_added(self, ex_gw_port, interface_name):
link_up = self.external_gateway_link_up()
self._plug_external_gateway(ex_gw_port, interface_name,
self.ns_name, link_up=link_up)
self._plug_external_gateway(ex_gw_port, interface_name, self.ns_name)
self._add_gateway_vip(ex_gw_port, interface_name)
self._disable_ipv6_addressing_on_interface(interface_name)
@ -535,27 +537,3 @@ class HaRouter(router.RouterInfo):
if (self.keepalived_manager.get_process().active and
self.ha_state == 'primary'):
super(HaRouter, self).enable_radvd(internal_ports)
def external_gateway_link_up(self):
# Check HA router ha_state for its gateway port link state.
# 'backup' instance will not link up the gateway port.
return self.ha_state == 'primary'
def set_external_gw_port_link_status(self, link_up, set_gw=False):
link_state = "up" if link_up else "down"
LOG.info('Set router %s gateway device link state to %s.',
self.router_id, link_state)
ex_gw_port = self.get_ex_gw_port()
ex_gw_port_id = (ex_gw_port and ex_gw_port['id'] or
self.ex_gw_port and self.ex_gw_port['id'])
if ex_gw_port_id:
interface_name = self.get_external_device_name(ex_gw_port_id)
ns_name = self.get_gw_ns_name()
self.driver.set_link_status(interface_name, ns_name,
link_up=link_up)
if link_up and set_gw:
preserve_ips = self.get_router_preserve_ips()
self._external_gateway_settings(ex_gw_port, interface_name,
ns_name, preserve_ips)
self.routes_updated([], self.routes)

20
neutron/agent/l3/router_info.py

@ -750,16 +750,14 @@ class RouterInfo(BaseRouterInfo):
return [common_utils.ip_to_cidr(ip['floating_ip_address'])
for ip in floating_ips]
def _plug_external_gateway(self, ex_gw_port, interface_name, ns_name,
link_up=True):
def _plug_external_gateway(self, ex_gw_port, interface_name, ns_name):
self.driver.plug(ex_gw_port['network_id'],
ex_gw_port['id'],
interface_name,
ex_gw_port['mac_address'],
namespace=ns_name,
prefix=EXTERNAL_DEV_PREFIX,
mtu=ex_gw_port.get('mtu'),
link_up=link_up)
mtu=ex_gw_port.get('mtu'))
def _get_external_gw_ips(self, ex_gw_port):
gateway_ips = []
@ -819,11 +817,7 @@ class RouterInfo(BaseRouterInfo):
LOG.debug("External gateway added: port(%s), interface(%s), ns(%s)",
ex_gw_port, interface_name, ns_name)
self._plug_external_gateway(ex_gw_port, interface_name, ns_name)
self._external_gateway_settings(ex_gw_port, interface_name,
ns_name, preserve_ips)
def _external_gateway_settings(self, ex_gw_port, interface_name,
ns_name, preserve_ips):
# Build up the interface and gateway IP addresses that
# will be added to the interface.
ip_cidrs = common_utils.fixed_ip_cidrs(ex_gw_port['fixed_ips'])
@ -868,19 +862,17 @@ class RouterInfo(BaseRouterInfo):
return any(netaddr.IPAddress(gw_ip).version == 6
for gw_ip in gateway_ips)
def get_router_preserve_ips(self):
def external_gateway_added(self, ex_gw_port, interface_name):
preserve_ips = self._list_floating_ip_cidrs() + list(
self.centralized_port_forwarding_fip_set)
preserve_ips.extend(self.agent.pd.get_preserve_ips(self.router_id))
return preserve_ips
def external_gateway_added(self, ex_gw_port, interface_name):
preserve_ips = self.get_router_preserve_ips()
self._external_gateway_added(
ex_gw_port, interface_name, self.ns_name, preserve_ips)
def external_gateway_updated(self, ex_gw_port, interface_name):
preserve_ips = self.get_router_preserve_ips()
preserve_ips = self._list_floating_ip_cidrs() + list(
self.centralized_port_forwarding_fip_set)
preserve_ips.extend(self.agent.pd.get_preserve_ips(self.router_id))
self._external_gateway_added(
ex_gw_port, interface_name, self.ns_name, preserve_ips)

34
neutron/agent/linux/interface.py

@ -256,12 +256,17 @@ class LinuxInterfaceDriver(interface.LinuxInterfaceDriver,
{'dev': dev_name, 'enabled': int(enabled)}]
ip_lib.sysctl(cmd, namespace=namespace)
@abc.abstractmethod
def plug_new(self, network_id, port_id, device_name, mac_address,
bridge=None, namespace=None, prefix=None, mtu=None):
"""Plug in the interface only for new devices that don't exist yet."""
def plug(self, network_id, port_id, device_name, mac_address,
bridge=None, namespace=None, prefix=None, mtu=None, link_up=True):
bridge=None, namespace=None, prefix=None, mtu=None):
if not ip_lib.device_exists(device_name,
namespace=namespace):
self.plug_new(network_id, port_id, device_name, mac_address,
bridge, namespace, prefix, mtu, link_up)
bridge, namespace, prefix, mtu)
else:
LOG.info("Device %s already exists", device_name)
if mtu:
@ -293,21 +298,10 @@ class LinuxInterfaceDriver(interface.LinuxInterfaceDriver,
LOG.warning("Interface driver cannot update MTU for ports")
self._mtu_update_warn_logged = True
def set_link_status(self, device_name, namespace=None, link_up=True):
ns_dev = ip_lib.IPWrapper(namespace=namespace).device(device_name)
if not ns_dev.exists():
LOG.debug("Device %s may concurrently be deleted.", device_name)
return
if link_up:
ns_dev.link.set_up()
else:
ns_dev.link.set_down()
class NullDriver(LinuxInterfaceDriver):
def plug_new(self, network_id, port_id, device_name, mac_address,
bridge=None, namespace=None, prefix=None, mtu=None,
link_up=True):
bridge=None, namespace=None, prefix=None, mtu=None):
pass
def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
@ -385,8 +379,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
namespace_obj.add_device_to_namespace(device)
def plug_new(self, network_id, port_id, device_name, mac_address,
bridge=None, namespace=None, prefix=None, mtu=None,
link_up=True):
bridge=None, namespace=None, prefix=None, mtu=None):
"""Plug in the interface."""
if not bridge:
bridge = self.conf.OVS.integration_bridge
@ -442,8 +435,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
else:
LOG.warning("No MTU configured for port %s", port_id)
if link_up:
ns_dev.link.set_up()
ns_dev.link.set_up()
if self.conf.ovs_use_veth:
# ovs-dpdk does not do checksum calculations for veth interface
# (bug 1832021)
@ -488,8 +480,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
DEV_NAME_PREFIX = 'ns-'
def plug_new(self, network_id, port_id, device_name, mac_address,
bridge=None, namespace=None, prefix=None, mtu=None,
link_up=True):
bridge=None, namespace=None, prefix=None, mtu=None):
"""Plugin the interface."""
ip = ip_lib.IPWrapper()
@ -508,8 +499,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
LOG.warning("No MTU configured for port %s", port_id)
root_veth.link.set_up()
if link_up:
ns_veth.link.set_up()
ns_veth.link.set_up()
def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
"""Unplug the interface."""

1
neutron/tests/unit/agent/l3/test_dvr_local_router.py

@ -983,7 +983,6 @@ class TestDvrRouterOperations(base.BaseTestCase):
self.mock_driver.unplug.reset_mock()
self._set_ri_kwargs(agent, router['id'], router)
ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri._ha_state_path = self.get_temp_file_path('router_ha_state')
ri._create_snat_namespace = mock.Mock()
ri._plug_external_gateway = mock.Mock()
ri.initialize(mock.Mock())