From 13cb3cd34c26b6e6668732d664b640a69cec4dec Mon Sep 17 00:00:00 2001
From: LIU Yulong <i@liuyulong.me>
Date: Thu, 25 Apr 2019 15:25:34 +0800
Subject: [PATCH] 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.

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

Closes-Bug: #1826726
Change-Id: I96a088d25048be02a9c5b12c1d087df075b36fc4
(cherry picked from commit 45957f12c897f6dcdf3676649a28dac866450afd)
---
 neutron/agent/l3/dvr_edge_ha_router.py        |  2 +-
 neutron/agent/l3/ha_router.py                 | 17 +++++++++--
 neutron/tests/unit/agent/l3/test_ha_router.py | 29 +++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/neutron/agent/l3/dvr_edge_ha_router.py b/neutron/agent/l3/dvr_edge_ha_router.py
index f49093919e2..553863e392b 100644
--- a/neutron/agent/l3/dvr_edge_ha_router.py
+++ b/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(
diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index c1294bb6aef..58a4501eb28 100644
--- a/neutron/agent/l3/ha_router.py
+++ b/neutron/agent/l3/ha_router.py
@@ -124,7 +124,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)
@@ -462,10 +462,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()
diff --git a/neutron/tests/unit/agent/l3/test_ha_router.py b/neutron/tests/unit/agent/l3/test_ha_router.py
index 33ec7bc902c..a44745267b4 100644
--- a/neutron/tests/unit/agent/l3/test_ha_router.py
+++ b/neutron/tests/unit/agent/l3/test_ha_router.py
@@ -15,6 +15,7 @@
 import signal
 
 import mock
+from neutron_lib import constants as n_consts
 from oslo_utils import uuidutils
 
 from neutron.agent.l3 import ha_router
@@ -141,3 +142,31 @@ class TestBasicRouterOperations(base.BaseTestCase):
             'ha_state')
         self.mock_open = IOError
         self.assertEqual('unknown', ri.ha_state)
+
+    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)