Move members failover status check to a separate func

patroni/ha.py

@@ -912,6 +912,27 @@ class Ha(object):
         lag = (self.cluster.last_lsn or 0) - wal_position
         return lag > self.global_config.maximum_lag_on_failover
 
+    def has_members_eligible_to_promote(self, members: List[Member], reference_lsn: int = 0,
+                                        fast_path: bool = False) -> bool:
+        ret = False
+        cluster_timeline = self.cluster.timeline
+
+        for st in self.fetch_nodes_statuses(members):
+            not_allowed_reason = st.failover_limitation()
+            if not_allowed_reason:
+                logger.info('Member %s is %s', st.member.name, not_allowed_reason)
+            elif fast_path:
+                return True
+            elif reference_lsn and st.wal_position < reference_lsn or \
+                    not reference_lsn and self.is_lagging(st.wal_position):
+                logger.info('Member %s exceeds maximum replication lag', st.member.name)
+            elif self.check_timeline() and (not st.timeline or st.timeline < cluster_timeline):
+                logger.info('Timeline %s of member %s is behind the cluster timeline %s',
+                            st.timeline, st.member.name, cluster_timeline)
+            else:
+                ret = True
+        return ret
+
     def _is_healthiest_node(self, members: Collection[Member], check_replication_lag: bool = True) -> bool:
         """This method tries to determine whether I am healthy enough to became a new leader candidate or not."""
 
@@ -964,21 +985,7 @@ class Ha(object):
         elif not candidates:
             logger.warning('%s: candidates list is empty', action)
 
-        ret = False
-        cluster_timeline = self.cluster.timeline
-        for st in self.fetch_nodes_statuses(candidates):
-            not_allowed_reason = st.failover_limitation()
-            if not_allowed_reason:
-                logger.info('Member %s is %s', st.member.name, not_allowed_reason)
-            elif cluster_lsn and st.wal_position < cluster_lsn or \
-                    not cluster_lsn and self.is_lagging(st.wal_position):
-                logger.info('Member %s exceeds maximum replication lag', st.member.name)
-            elif self.check_timeline() and (not st.timeline or st.timeline < cluster_timeline):
-                logger.info('Timeline %s of member %s is behind the cluster timeline %s',
-                            st.timeline, st.member.name, cluster_timeline)
-            else:
-                ret = True
-        return ret
+        return self.has_members_eligible_to_promote(candidates, cluster_lsn)
 
     def manual_failover_process_no_leader(self) -> Optional[bool]:
         """Handles manual failover/switchover when the old leader already stepped down.

patroni/manual_failover.py

@@ -87,11 +87,7 @@ class ManualFailover(object):
             if not members:
                 return ManualFailoverPrecheckStatus.ONLY_LEADER
 
-        if self.patroni:
-            for st in self.patroni.ha.fetch_nodes_statuses(members):
-                if st.failover_limitation() is None:
-                    break
-            else:
-                return ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES
+        if self.patroni and not self.patroni.ha.has_members_eligible_to_promote(members, fast_path=True):
+            return ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES
 
         return ManualFailoverPrecheckStatus.CHECK_PASSED

tests/test_api.py

@@ -122,6 +122,9 @@ class MockHa(object):
     def is_paused():
         return True
 
+    def has_members_eligible_to_promote(*args, **kwargs):
+        return True
+
 
 class MockLogger(object):
 
@@ -599,7 +602,7 @@ class TestRestApiHandler(unittest.TestCase):
         dcs.manual_failover.return_value = True
 
         # Candidate is not healthy to be promoted
-        with patch.object(MockHa, 'fetch_nodes_statuses', Mock(return_value=[])), \
+        with patch.object(MockHa, 'has_members_eligible_to_promote', Mock(return_value=False)), \
              patch.object(RestApiHandler, 'write_response') as response_mock:
             MockRestApiServer(RestApiHandler, request)
             response_mock.assert_called_with(412, 'switchover is not possible: no good candidates have been found')

tests/test_ha.py

@@ -1626,3 +1626,13 @@ class TestHa(PostgresInit):
             self.assertEqual(self.ha.patroni.request.call_args[1]['timeout'], 2)
             mock_logger.assert_called()
             self.assertTrue(mock_logger.call_args[0][0].startswith('Request to Citus coordinator'))
+
+    def test_has_members_eligible_to_promote(self):
+        self.ha.fetch_node_status = get_node_status()
+        members = [
+            Member(0, 'test', 1, {'api_url': 'http://127.0.0.1:8011/patroni', 'conn_url': 'postgres://127.0.0.1:5432/postgres'}),
+            Member(0, 'test2', 1, {'api_url': 'http://127.0.0.1:8011/patroni', 'conn_url': 'postgres://127.0.0.1:5432/postgres'}),
+        ]
+        with patch('patroni.ha.logger.info') as mock_logger:
+            self.assertTrue(self.ha.has_members_eligible_to_promote(members, fast_path=True))
+            mock_logger.assert_not_called()