Keep HA ports info for HA router during entire lifecycle

Once HA port is set, it must remain this value no matter
what the server return. Because there is race condition
between l3-agent side sync router info for processing
and server side router deleting.

This patch adds a helper function for every ha_port set
action. If the ha_port is not None, it will always stay
with original value.

Closes-Bug: #1826726
Change-Id: I96a088d25048be02a9c5b12c1d087df075b36fc4

neutron/agent/l3/dvr_edge_ha_router.py

@@ -62,7 +62,7 @@ class DvrEdgeHaRouter(dvr_edge_router.DvrEdgeRouter,
                 self.get_ex_gw_port())
         self._add_vip(fip_cidr, interface_name)
 
-        self.ha_port = self.router.get(constants.HA_INTERFACE_KEY)
+        self.set_ha_port()
         if (self.is_router_master() and self.ha_port and
                 self.ha_port['status'] == constants.PORT_STATUS_ACTIVE):
             return super(DvrEdgeHaRouter, self).add_centralized_floatingip(

neutron/agent/l3/ha_router.py

@@ -125,7 +125,7 @@ class HaRouter(router.RouterInfo):
             raise Exception(msg)
         super(HaRouter, self).initialize(process_monitor)
 
-        self.ha_port = ha_port
+        self.set_ha_port()
         self._init_keepalived_manager(process_monitor)
         self.ha_network_added()
         self.update_initial_state(self.state_change_callback)
@@ -466,10 +466,23 @@ class HaRouter(router.RouterInfo):
         self.ha_network_removed()
         super(HaRouter, self).delete()
 
+    def set_ha_port(self):
+        ha_port = self.router.get(n_consts.HA_INTERFACE_KEY)
+        if not ha_port:
+            return
+        # NOTE: once HA port is set, it MUST remain this value no matter what
+        # the server return. Because there is race condition between l3-agent
+        # side sync router info for processing and server side router deleting.
+        # TODO(liuyulong): make sure router HA ports never change.
+        if not self.ha_port or (self.ha_port and
+                                self.ha_port['status'] != ha_port['status']):
+            self.ha_port = ha_port
+
     def process(self):
         super(HaRouter, self).process()
 
-        self.ha_port = self.router.get(n_consts.HA_INTERFACE_KEY)
+        self.set_ha_port()
+        LOG.debug("Processing HA router with HA port: %s", self.ha_port)
         if (self.ha_port and
                 self.ha_port['status'] == n_consts.PORT_STATUS_ACTIVE):
             self.enable_keepalived()

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

@@ -16,6 +16,7 @@ import copy
 import signal
 
 import mock
+from neutron_lib import constants as n_consts
 from oslo_utils import uuidutils
 
 from neutron.agent.l3 import ha_router
@@ -170,3 +171,31 @@ class TestBasicRouterOperations(base.BaseTestCase):
         new_gw_port['fixed_ips'].remove(fixed_ip)
         new_gw_port['qos_policy_id'] = _uuid()
         self.assertFalse(ri._gateway_ports_equal(old_gw_port, new_gw_port))
+
+    def test_set_ha_port(self):
+        ri = self._create_router()
+        self.assertIsNone(ri.ha_port)
+
+        ri.router = {}
+        ri.set_ha_port()
+        self.assertIsNone(ri.ha_port)
+
+        # HA_INTERFACE_KEY from None to some value
+        ri.router = {n_consts.HA_INTERFACE_KEY: {"id": _uuid(),
+                                                 "status": "DOWN"}}
+        ri.set_ha_port()
+        self.assertIsNotNone(ri.ha_port)
+        self.assertEqual('DOWN', ri.ha_port["status"])
+
+        # HA port state change
+        ri.router = {n_consts.HA_INTERFACE_KEY: {"id": _uuid(),
+                                                 "status": "ACTIVE"}}
+        ri.set_ha_port()
+        self.assertIsNotNone(ri.ha_port)
+        self.assertEqual('ACTIVE', ri.ha_port["status"])
+
+        ri.router = {}
+        ri.set_ha_port()
+        # neutron server return empty HA_INTERFACE_KEY, but
+        # agent side router info should remain the original value.
+        self.assertIsNotNone(ri.ha_port)