Different bugfixes around pause state, mostly related to watchdog (#507)

* Do not send keepalives if watchdog is not active
* Avoid activating watchdog in a pause mode
* Set correct postgres state in pause mode
* Don't try to run queries from API if postgres is stopped

features/watchdog.feature

@@ -1,16 +1,26 @@
 Feature: watchdog
   Verify that watchdog gets pinged and triggered under appropriate circumstances.
 
-  Scenario: watchdog is opened, pinged and closed
+  Scenario: watchdog is opened and pinged
     Given I start postgres0 with watchdog
     Then postgres0 is a leader after 10 seconds
     And postgres0 role is the primary after 10 seconds
     And postgres0 watchdog has been pinged after 10 seconds
-    When I shut down postgres0
+
+  Scenario: watchdog is disabled during pause
+    Given I run patronictl.py pause batman
+    Then I receive a response returncode 0
+    When I sleep for 2 seconds
     Then postgres0 watchdog has been closed
 
-  #TODO: test watchdog is disabled during pause
-  #TODO: test watchdog is disabled properly when shutting down
+  Scenario: watchdog is opened and pinged after resume
+    Given I run patronictl.py resume batman
+    Then I receive a response returncode 0
+    And postgres0 watchdog has been pinged after 10 seconds
+
+  Scenario: watchdog is disabled when shutting down
+    Given I shut down postgres0
+    Then postgres0 watchdog has been closed
 
   Scenario: watchdog is triggered if patroni stops responding
     Given I start postgres0 with watchdog

patroni/api.py

@@ -376,6 +376,8 @@ class RestApiHandler(BaseHTTPRequestHandler):
 
     def get_postgresql_status(self, retry=False):
         try:
+            if self.server.patroni.postgresql.state not in ('running', 'restarting', 'starting'):
+                raise RetryFailedError('')
             row = self.query("""WITH replication_info AS (
                                     SELECT usename, application_name, client_addr, state, sync_state, sync_priority
                                       FROM pg_stat_replication

patroni/ha.py

@@ -341,14 +341,13 @@ class Ha(object):
                 self._disable_sync -= 1
 
     def enforce_master_role(self, message, promote_message):
-        if not self.watchdog.is_running:
-            if not self.watchdog.activate():
-                if self.state_handler.is_leader():
-                    self.demote('immediate')
-                    return 'Demoting self because watchdog could not be activated'
-                else:
-                    self.release_leader_key_voluntarily()
-                    return 'Not promoting self because watchdog could not be actived'
+        if not self.is_paused() and not self.watchdog.is_running and not self.watchdog.activate():
+            if self.state_handler.is_leader():
+                self.demote('immediate')
+                return 'Demoting self because watchdog could not be activated'
+            else:
+                self.release_leader_key_voluntarily()
+                return 'Not promoting self because watchdog could not be activated'
 
         if self.state_handler.is_leader() or self.state_handler.role == 'master':
             # Inform the state handler about its master role.
@@ -915,6 +914,8 @@ class Ha(object):
         # Check if we are in startup, when paused defer to main loop for manual failovers.
         if not self.state_handler.check_for_startup() or self.is_paused():
             self.set_start_timeout(None)
+            if self.is_paused():
+                self.state_handler.set_state(self.state_handler.is_running() and 'running' or 'stopped')
             return None
 
         # state_handler.state == 'starting' here

patroni/watchdog/base.py

@@ -202,7 +202,8 @@ class Watchdog(object):
     @synchronized
     def keepalive(self):
         try:
-            self.impl.keepalive()
+            if self.active:
+                self.impl.keepalive()
             # In case there are any pending configuration changes apply them now.
             if self.active and self.config != self.active_config:
                 if self.config.mode != MODE_OFF and self.active_config.mode == MODE_OFF:

tests/test_api.py

@@ -3,7 +3,7 @@ import json
 import psycopg2
 import unittest
 
-from mock import Mock, patch
+from mock import Mock, PropertyMock, patch
 from patroni.api import RestApiHandler, RestApiServer
 from patroni.dcs import ClusterConfig, Member
 from patroni.ha import _MemberStatus
@@ -152,6 +152,7 @@ class TestRestApiHandler(unittest.TestCase):
     def test_do_OPTIONS(self):
         self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'OPTIONS / HTTP/1.0'))
 
+    @patch.object(MockPostgresql, 'state', PropertyMock(return_value='stopped'))
     def test_do_GET_patroni(self):
         self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /patroni'))
 

tests/test_ha.py

@@ -63,6 +63,7 @@ def get_node_status(reachable=True, in_recovery=True, wal_position=10, nofailove
         return _MemberStatus(e, reachable, in_recovery, wal_position, tags, watchdog_failed)
     return fetch_node_status
 
+
 future_restart_time = datetime.datetime.now(tzutc) + datetime.timedelta(days=5)
 postmaster_start_time = datetime.datetime.now(tzutc)
 
@@ -246,7 +247,7 @@ class TestHa(unittest.TestCase):
         with patch.object(Watchdog, 'activate', Mock(return_value=False)):
             self.assertEquals(self.ha.run_cycle(), 'Demoting self because watchdog could not be activated')
             self.p.is_leader = false
-            self.assertEquals(self.ha.run_cycle(), 'Not promoting self because watchdog could not be actived')
+            self.assertEquals(self.ha.run_cycle(), 'Not promoting self because watchdog could not be activated')
 
     def test_leader_with_lock(self):
         self.ha.cluster.is_unlocked = false

tests/test_watchdog.py

@@ -132,8 +132,9 @@ class TestWatchdog(unittest.TestCase):
     def test_exceptions(self):
         wd = Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'bad'}})
         wd.impl.close = wd.impl.keepalive = Mock(side_effect=WatchdogError(''))
-        self.assertIsNone(wd.disable())
+        self.assertTrue(wd.activate())
         self.assertIsNone(wd.keepalive())
+        self.assertIsNone(wd.disable())
 
     @patch('platform.system', Mock(return_value='Linux'))
     def test_config_reload(self):