From c2a78ee65250a34252aaf34612973e4c152aeb50 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <cyberdemn@gmail.com>
Date: Fri, 5 Jun 2020 09:23:54 +0200
Subject: [PATCH] Bugfix: GET /cluster was showing stale member info in
 zookeeper (#1573)

Zookpeeper implementation heavily relies on cached version of the cluster view in order to minimize the number of requests. Having stale members information is fine for Patroni workflow because it basically relies only on member names and tags.

The `GET /cluster` is a different case. Being exposed outside it might be used for monitoring purposes and therefore we should show the up-to-date members information.
---
 patroni/api.py           | 2 +-
 patroni/dcs/__init__.py  | 7 ++++++-
 patroni/dcs/zookeeper.py | 3 +++
 tests/test_api.py        | 4 ++--
 tests/test_zookeeper.py  | 2 +-
 5 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/patroni/api.py b/patroni/api.py
index d3e60e2f..324a3752 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -134,7 +134,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
         self._write_status_response(200, response)
 
     def do_GET_cluster(self):
-        cluster = self.server.patroni.dcs.cluster or self.server.patroni.dcs.get_cluster()
+        cluster = self.server.patroni.dcs.get_cluster(True)
         self._write_json_response(200, cluster_as_json(cluster))
 
     def do_GET_history(self):
diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py
index 473cbe4a..cda9d43d 100644
--- a/patroni/dcs/__init__.py
+++ b/patroni/dcs/__init__.py
@@ -642,7 +642,12 @@ class AbstractDCS(object):
            If the current node was running as a master and exception raised,
            instance would be demoted."""
 
-    def get_cluster(self):
+    def _bypass_caches(self):
+        """Used only in zookeeper"""
+
+    def get_cluster(self, force=False):
+        if force:
+            self._bypass_caches()
         try:
             cluster = self._load_cluster()
         except Exception:
diff --git a/patroni/dcs/zookeeper.py b/patroni/dcs/zookeeper.py
index 336fca70..5a852547 100644
--- a/patroni/dcs/zookeeper.py
+++ b/patroni/dcs/zookeeper.py
@@ -223,6 +223,9 @@ class ZooKeeper(AbstractDCS):
                 raise ZooKeeperError('ZooKeeper in not responding properly')
         return cluster
 
+    def _bypass_caches(self):
+        self._fetch_cluster = True
+
     def _create(self, path, value, retry=False, ephemeral=False):
         try:
             if retry:
diff --git a/tests/test_api.py b/tests/test_api.py
index 748d3372..0819c6c9 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -203,8 +203,8 @@ class TestRestApiHandler(unittest.TestCase):
 
     @patch.object(MockPatroni, 'dcs')
     def test_do_GET_cluster(self, mock_dcs):
-        mock_dcs.cluster = get_cluster_initialized_without_leader()
-        mock_dcs.cluster.members[1].data['xlog_location'] = 11
+        mock_dcs.get_cluster.return_value = get_cluster_initialized_without_leader()
+        mock_dcs.get_cluster.return_value.members[1].data['xlog_location'] = 11
         self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /cluster'))
 
     @patch.object(MockPatroni, 'dcs')
diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py
index 931aabfb..e121de93 100644
--- a/tests/test_zookeeper.py
+++ b/tests/test_zookeeper.py
@@ -150,7 +150,7 @@ class TestZooKeeper(unittest.TestCase):
 
     def test_get_cluster(self):
         self.assertRaises(ZooKeeperError, self.zk.get_cluster)
-        cluster = self.zk.get_cluster()
+        cluster = self.zk.get_cluster(True)
         self.assertIsInstance(cluster.leader, Leader)
         self.zk.touch_member({'foo': 'foo'})