From 4a29caa9d38de77aa28c5843d9c2614353fa1f75 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 27 Feb 2020 12:22:44 +0100 Subject: [PATCH] On role change callback didn't fire on failed primary (#1420) Bug was introduced in https://github.com/zalando/patroni/pull/703 Close https://github.com/zalando/patroni/issues/1418 --- patroni/ha.py | 10 ++++++---- patroni/postgresql/__init__.py | 6 +++++- tests/test_ha.py | 1 + tests/test_postgresql.py | 3 +++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index d38be332..02025fec 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -1143,7 +1143,8 @@ class Ha(object): if not self.state_handler.is_running(): self.watchdog.disable() if self.has_lock(): - self.state_handler.set_role('demoted') + if self.state_handler.role in ('master', 'standby_leader'): + self.state_handler.set_role('demoted') self._delete_leader() return 'removed leader key after trying and failing to start postgres' return 'failed to start postgres' @@ -1172,10 +1173,11 @@ class Ha(object): return ret or 'running post_bootstrap' self.state_handler.bootstrapping = False - self.dcs.set_config_value(json.dumps(self.patroni.config.dynamic_configuration, separators=(',', ':'))) if not self.watchdog.activate(): logger.error('Cancelling bootstrap because watchdog activation failed') self.cancel_initialization() + self.dcs.initialize(create_new=(self.cluster.initialize is None), sysid=self.state_handler.sysid) + self.dcs.set_config_value(json.dumps(self.patroni.config.dynamic_configuration, separators=(',', ':'))) self.state_handler.slots_handler.sync_replication_slots(self.cluster) self.dcs.take_leader() self.set_is_leader(True) @@ -1290,8 +1292,8 @@ class Ha(object): data_sysid = self.state_handler.sysid if not self.sysid_valid(data_sysid): # data directory is not empty, but no valid sysid, cluster must be broken, suggest reinit - return ("data dir for the cluster is not empty, but system ID is invalid; consider doing" - "reinitialize") + return ("data dir for the cluster is not empty, " + "but system ID is invalid; consider doing reinitialize") if self.sysid_valid(self.cluster.initialize): if self.cluster.initialize != data_sysid: diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py index d4b262d6..d31b77d7 100644 --- a/patroni/postgresql/__init__.py +++ b/patroni/postgresql/__init__.py @@ -413,7 +413,11 @@ class Postgresql(object): self.set_state('starting') self._pending_restart = False - configuration = self.config.effective_configuration + try: + configuration = self.config.effective_configuration + except Exception: + return None + self.config.check_directories() self.config.write_postgresql_conf(configuration) self.config.resolve_connection_addresses() diff --git a/tests/test_ha.py b/tests/test_ha.py index 2ade4526..b054cfc2 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -621,6 +621,7 @@ class TestHa(PostgresInit): def test_post_recover(self): self.p.is_running = false self.ha.has_lock = true + self.p.set_role('master') self.assertEqual(self.ha.post_recover(), 'removed leader key after trying and failing to start postgres') self.ha.has_lock = false self.assertEqual(self.ha.post_recover(), 'failed to start postgres') diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 00adb8f8..8a81ddc5 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -134,6 +134,9 @@ class TestPostgresql(BaseTestPostgresql): self.p.cancellable.cancel() self.assertFalse(self.p.start()) + with patch('patroni.postgresql.config.ConfigHandler.effective_configuration', + PropertyMock(side_effect=Exception)): + self.assertIsNone(self.p.start()) @patch.object(Postgresql, 'pg_isready') @patch('patroni.postgresql.polling_loop', Mock(return_value=range(1)))