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

This partially reverts commit c52029c.

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
(cherry picked from commit 36bf1df)

Author: dosaboy

neutron/agent/l3/dvr_edge_ha_router.py

@@ -134,9 +134,7 @@ def external_gateway_updated(self, ex_gw_port, interface_name):
 
     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

neutron/agent/l3/ha.py

@@ -188,15 +188,6 @@ def _enqueue_state_change(self, router_id, state):
                  'agent %(host)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

neutron/agent/l3/ha_router.py

@@ -170,6 +170,10 @@ def _init_keepalived_manager(self, process_monitor):
             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 @@ def _get_filtered_dict(d, ignore):
         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 @@ def enable_radvd(self, internal_ports=None):
         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)

neutron/agent/l3/router_info.py

@@ -750,16 +750,14 @@ def _list_floating_ip_cidrs(self):
         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 @@ def _external_gateway_added(self, ex_gw_port, interface_name,
         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 @@ def is_v6_gateway_set(self, gateway_ips):
         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)
 

neutron/agent/linux/interface.py

@@ -256,12 +256,17 @@ def configure_ipv6_forwarding(namespace, dev_name, enabled):
                {'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 @@ def set_mtu(self, device_name, mtu, namespace=None, prefix=None):
             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 @@ def _add_device_to_namespace(self, ip_wrapper, device, namespace):
             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 @@ def plug_new(self, network_id, port_id, device_name, mac_address,
         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 @@ def plug_new(self, network_id, port_id, device_name, mac_address,
             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."""

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

@@ -983,7 +983,6 @@ def test_initialize_dvr_ha_router_snat_ns_once(self):
         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())