Unify all follow the leader calls from eventloop.

Call normal follow the leader method from HA even during recovery.
This provides a single place that changes recovery.conf, making
it easier to plug in a cascading replica in the future.

Remove an obsolete demote function from PostreSQL module, modified
the tests.

patroni/ha.py

@@ -19,6 +19,7 @@ class Ha:
         self.dcs = patroni.dcs
         self.cluster = None
         self.old_cluster = None
+        self.recovering = False
         self._async_executor = AsyncExecutor()
 
     def load_cluster_from_dcs(self):
@@ -94,33 +95,21 @@ class Ha:
             return 'waiting for leader to bootstrap'
 
     def recover(self):
-        has_lock = self.has_lock()
-
         # try to see if we are the former master that crashed. If so - we likely need to run pg_rewind
         # in order to join the former standby being promoted.
         pg_controldata = self.state_handler.controldata()
-        if not has_lock and pg_controldata and\
+        if (self.state_handler.role == 'master') and pg_controldata and\
                 pg_controldata.get('Database cluster state', '') == 'in production':  # crashed master
             self.state_handler.require_rewind()
+        self.recovering = True
+        return self.follow_the_leader("started as readonly because i had the session lock",
+                                      "started as a secondary",
+                                      refresh=True, recovery=True)
 
-        # XXX: follow the leader calls stop, which might take quite some time.
-        # perhaps we should run sync asynchronously
-        # (we still need the exit code from follow_the_leader)
-        ret = self.state_handler.follow_the_leader(None if has_lock else self.cluster.leader, recovery=True)
-        if not ret:
-            if not has_lock:
-                return 'failed to start postgres'
-            self.dcs.delete_leader()
-            self.dcs.reset_cluster()
-            return 'removed leader key after trying and failing to start postgres'
-        if not has_lock:
-            return 'started as a secondary'
-        logger.info('started as readonly because i had the session lock')
-        self.load_cluster_from_dcs()
-
-    def follow_the_leader(self, demote_reason, follow_reason, refresh=True):
+    def follow_the_leader(self, demote_reason, follow_reason, refresh=True, recovery=False):
         refresh and self.load_cluster_from_dcs()
-        ret = demote_reason if self.state_handler.is_leader() else follow_reason
+        ret = demote_reason if (not recovery and self.state_handler.is_leader()
+                                or recovery and self.state_handler.role == 'master') else follow_reason
         leader = self.cluster.leader
         leader = None if (leader and leader.name) == self.state_handler.name else leader
         if not self.state_handler.check_recovery_conf(leader):
@@ -382,6 +371,15 @@ class Ha:
         # so even 1 << 32 would have 10 digits.
         return str(sysid) and len(str(sysid)) >= 10 and str(sysid).isdigit()
 
+    def post_recover(self):
+        if not self.state_handler.is_running():
+            if self.has_lock():
+                self.dcs.delete_leader()
+                self.dcs.reset_cluster()
+                return 'removed leader key after trying and failing to start postgres'
+            return 'failed to start postgres'
+        return None
+
     def _run_cycle(self):
         try:
             self.load_cluster_from_dcs()
@@ -395,6 +393,13 @@ class Ha:
             if self._async_executor.busy:
                 return self.handle_long_action_in_progress()
 
+            # we've go here, so async action has finished. Check if we tried to recover and failed
+            if self.recovering:
+                self.recovering = False
+                msg = self.post_recover()
+                if msg is not None:
+                    return msg
+
             # currently it can trigger only reinitialize
             msg = self.process_scheduled_action()
             if msg is not None:

patroni/postgresql.py

@@ -596,9 +596,6 @@ recovery_target_timeline = 'latest'
             self.call_nowait(ACTION_ON_ROLE_CHANGE)
         return ret
 
-    def demote(self):
-        self.follow_the_leader(None)
-
     def create_or_update_role(self, name, password, options):
         self.query("""DO $$
 BEGIN

tests/test_ha.py

@@ -127,13 +127,18 @@ class TestHa(unittest.TestCase):
     def test_recover_replica_failed(self):
         self.p.controldata = lambda: {'Database cluster state': 'in production'}
         self.p.is_healthy = false
+        self.p.is_running = false
         self.p.follow_the_leader = false
+        self.assertEquals(self.ha.run_cycle(), 'started as a secondary')
         self.assertEquals(self.ha.run_cycle(), 'failed to start postgres')
 
     def test_recover_master_failed(self):
         self.p.follow_the_leader = false
         self.p.is_healthy = false
+        self.p.is_running = false
         self.ha.has_lock = true
+        self.p.role = 'master'
+        self.assertEquals(self.ha.run_cycle(), 'started as readonly because i had the session lock')
         self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres')
 
     @patch('sys.exit', return_value=1)
@@ -144,7 +149,8 @@ class TestHa(unittest.TestCase):
 
     @patch.object(Cluster, 'is_unlocked', Mock(return_value=False))
     def test_start_as_readonly(self):
-        self.p.is_leader = self.p.is_healthy = false
+        self.p.is_leader = false
+        self.p.is_healthy = true
         self.ha.has_lock = true
         self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock')
 

tests/test_postgresql.py

@@ -243,9 +243,7 @@ class TestPostgresql(unittest.TestCase):
     @patch('patroni.postgresql.Postgresql.single_user_mode', MagicMock(return_value=1))
     @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
     def test_follow_the_leader(self, mock_pg_rewind):
-        self.p.demote()
         self.p.follow_the_leader(None)
-        self.p.demote()
         self.p.follow_the_leader(self.leader)
         self.p.follow_the_leader(Leader(-1, 28, self.other))
         self.p.rewind = mock_pg_rewind