Implement missing tests for watchdog (#487)

and fix one bug

patroni/watchdog/base.py

@@ -53,6 +53,9 @@ class WatchdogConfig(object):
             all(getattr(self, attr) == getattr(other, attr) for attr in
                 ['mode', 'ttl', 'loop_wait', 'safety_margin', 'driver', 'driver_config'])
 
+    def __ne__(self, other):
+        return not self == other
+
     def get_impl(self):
         if self.driver == 'testing':
             from patroni.watchdog.linux import TestingWatchdogDevice

tests/test_ha.py

@@ -1,7 +1,6 @@
 import datetime
 import etcd
 import os
-import time
 import unittest
 
 from mock import Mock, MagicMock, PropertyMock, patch
@@ -15,7 +14,6 @@ from patroni.watchdog import Watchdog
 from patroni.utils import tzutc
 from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get
 from test_postgresql import psycopg2_connect
-from threading import Event
 
 
 def true(*args, **kwargs):
@@ -57,12 +55,12 @@ def get_cluster_initialized_with_only_leader(failover=None):
     return get_cluster(True, l, [l], failover, None)
 
 
-def get_node_status(reachable=True, in_recovery=True, wal_position=10, nofailover=False):
+def get_node_status(reachable=True, in_recovery=True, wal_position=10, nofailover=False, watchdog_failed=False):
     def fetch_node_status(e):
         tags = {}
         if nofailover:
             tags['nofailover'] = True
-        return _MemberStatus(e, reachable, in_recovery, wal_position, tags, False)
+        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)
@@ -328,6 +326,16 @@ class TestHa(unittest.TestCase):
         self.p.is_running = false
         self.assertRaises(PatroniException, self.ha.post_bootstrap)
 
+    def test_bootstrap_release_initialize_key_on_watchdog_failure(self):
+        self.ha.cluster = get_cluster_not_initialized_without_leader()
+        self.e.initialize = true
+        self.ha.bootstrap()
+        self.p.is_running = true
+        self.p.is_leader = true
+        with patch.object(Watchdog, 'activate', Mock(return_value=False)):
+            self.assertEquals(self.ha.post_bootstrap(), 'running post_bootstrap')
+            self.assertRaises(PatroniException, self.ha.post_bootstrap)
+
     @patch('psycopg2.connect', psycopg2_connect)
     def test_reinitialize(self):
         self.assertIsNotNone(self.ha.reinitialize())
@@ -390,6 +398,8 @@ class TestHa(unittest.TestCase):
         self.assertEquals(self.ha.run_cycle(), 'manual failover: demoting myself')
         self.ha.fetch_node_status = get_node_status(nofailover=True)
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
+        self.ha.fetch_node_status = get_node_status(watchdog_failed=True)
+        self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
         self.ha.fetch_node_status = get_node_status(wal_position=1)
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
         # manual failover from the previous leader to us won't happen if we hold the nofailover flag
@@ -470,6 +480,8 @@ class TestHa(unittest.TestCase):
         self.ha.patroni.nofailover = False
         self.ha.fetch_node_status = get_node_status()
         self.assertTrue(self.ha.is_healthiest_node())
+        with patch.object(Watchdog, 'is_healthy', PropertyMock(return_value=False)):
+            self.assertFalse(self.ha.is_healthiest_node())
         with patch('patroni.postgresql.Postgresql.is_starting', return_value=True):
             self.assertFalse(self.ha.is_healthiest_node())
         self.ha.is_paused = true

tests/test_postgresql.py

@@ -1,7 +1,6 @@
 import errno
 import mock  # for the mock.call method, importing it without a namespace breaks python3
 import os
-import psutil
 import psycopg2
 import shutil
 import subprocess
@@ -273,16 +272,21 @@ class TestPostgresql(unittest.TestCase):
     @patch.object(Postgresql, 'is_running')
     @patch.object(Postgresql, 'get_pid')
     def test_stop(self, mock_get_pid, mock_is_running):
+        mock_callback = Mock()
+        mock_is_running.return_value = False
+        self.assertTrue(self.p.stop(on_safepoint=mock_callback))
+        mock_callback.assert_called()
         mock_is_running.return_value = True
         mock_get_pid.return_value = 0
-        mock_callback = Mock()
+        mock_callback.reset_mock()
         self.assertTrue(self.p.stop(on_safepoint=mock_callback))
         mock_callback.assert_called()
         mock_get_pid.return_value = -1
         self.assertFalse(self.p.stop())
         mock_get_pid.return_value = 123
-        with patch('os.kill', Mock(side_effect=[OSError(errno.ESRCH, ''), OSError, None])),\
-                patch('psutil.Process', Mock(side_effect=psutil.NoSuchProcess(123))):
+        with patch('os.kill', Mock(side_effect=[OSError(errno.ESRCH, ''), OSError, None])):
+            self.assertTrue(self.p.stop())
+            self.assertFalse(self.p.stop())
             self.assertTrue(self.p.stop())
         with patch.object(Postgresql, '_signal_postmaster_stop', Mock(return_value=(123, None))):
             with patch.object(Postgresql, 'is_pid_running', Mock(side_effect=[True, False, False])):

tests/test_watchdog.py

@@ -85,7 +85,14 @@ class TestWatchdog(unittest.TestCase):
     @patch('platform.system', Mock(return_value='Linux'))
     @patch.object(Watchdog, 'is_running', PropertyMock(return_value=False))
     def test_watchdog_not_activated(self):
-        self.assertEquals(Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required'}}).activate(), False)
+        self.assertFalse(Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required'}}).activate())
+
+    @patch('platform.system', Mock(return_value='Linux'))
+    @patch.object(LinuxWatchdogDevice, 'is_running', PropertyMock(return_value=False))
+    def test_watchdog_activate(self):
+        with patch.object(LinuxWatchdogDevice, 'open', Mock(side_effect=WatchdogError(''))):
+            self.assertTrue(Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'auto'}}).activate())
+        self.assertFalse(Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required'}}).activate())
 
     @patch('platform.system', Mock(return_value='Linux'))
     def test_basic_operation(self):
@@ -119,7 +126,8 @@ class TestWatchdog(unittest.TestCase):
 
     @patch('platform.system', Mock(return_value='Unknown'))
     def test_unsupported_platform(self):
-        self.assertRaises(SystemExit, Watchdog, {'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'required', 'driver': 'bad'}})
+        self.assertRaises(SystemExit, Watchdog, {'ttl': 30, 'loop_wait': 10,
+                                                 'watchdog': {'mode': 'required', 'driver': 'bad'}})
 
     def test_exceptions(self):
         wd = Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'bad'}})
@@ -153,6 +161,7 @@ class TestWatchdog(unittest.TestCase):
         watchdog.reload_config({'ttl': 60, 'loop_wait': 15, 'watchdog': {'mode': 'required'}})
         watchdog.keepalive()
 
+
 class TestNullWatchdog(unittest.TestCase):
 
     def test_basics(self):
@@ -188,3 +197,10 @@ class TestLinuxWatchdogDevice(unittest.TestCase):
         self.assertRaises(WatchdogError, self.impl.get_support)
         self.impl.open()
         self.assertRaises(IOError, self.impl.get_support)
+
+    def test_is_healthy(self):
+        self.assertFalse(self.impl.is_healthy)
+
+    @patch('os.open', Mock(side_effect=OSError))
+    def test_open(self):
+        self.assertRaises(WatchdogError, self.impl.open)