From e106f2e16f63cfd0ced0e07284777aa848ccc0a8 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)
(cherry picked from commit 13cb3cd34c26b6e6668732d664b640a69cec4dec)
---
 neutron/agent/l3/ha_router.py                 | 17 +++++++++--
 neutron/tests/unit/agent/l3/test_ha_router.py | 29 +++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index 2bf4aa5bf9b..0feb64b2787 100644
--- a/neutron/agent/l3/ha_router.py
+++ b/neutron/agent/l3/ha_router.py
@@ -113,7 +113,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)
@@ -443,10 +443,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 20a5bce1b04..2b6bf3207c1 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
@@ -117,3 +118,31 @@ class TestBasicRouterOperations(base.BaseTestCase):
         calls = ["sig='str(%d)'" % signal.SIGTERM,
                  "sig='str(%d)'" % signal.SIGKILL]
         mock_pm.disable.has_calls(calls)
+
+    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)