diff --git a/patroni/watchdog/base.py b/patroni/watchdog/base.py index 2c9b7626..9bdb638b 100644 --- a/patroni/watchdog/base.py +++ b/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 diff --git a/tests/test_ha.py b/tests/test_ha.py index 2127dcf1..6dc45491 100644 --- a/tests/test_ha.py +++ b/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 diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 90a2836b..24c9e822 100644 --- a/tests/test_postgresql.py +++ b/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])): diff --git a/tests/test_watchdog.py b/tests/test_watchdog.py index 236a00f2..146f787f 100644 --- a/tests/test_watchdog.py +++ b/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)