diff --git a/patroni/ha.py b/patroni/ha.py index 3cea58d4..b0f2f520 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -207,6 +207,17 @@ class Ha(object): """ return self.is_synchronous_mode() and not self.cluster.sync.is_empty + def _get_failover_action_name(self) -> str: + """Return the currently requested manual failover action name or the default ``failover``. + + :returns: :class:`str` representing the manually requested action (``manual failover`` if no leader + is specified in the ``/failover`` in DCS, ``switchover`` otherwise) or ``failover`` if + ``/failover`` is empty. + """ + if not self.cluster.failover: + return 'failover' + return 'switchover' if self.cluster.failover.leader else 'manual failover' + def load_cluster_from_dcs(self) -> None: cluster = self.dcs.get_cluster() @@ -921,13 +932,12 @@ class Ha(object): """ candidates = self.get_failover_candidates(exclude_failover_candidate) - action = 'switchover' if self.cluster.failover and self.cluster.failover.leader else 'failover' + action = self._get_failover_action_name() if self.is_synchronous_mode() and self.cluster.failover and self.cluster.failover.candidate and not candidates: logger.warning('%s candidate=%s does not match with sync_standbys=%s', action.title(), self.cluster.failover.candidate, self.cluster.sync.sync_standby) elif not candidates: - logger.warning('%s%s: candidates list is empty', '' if not self.cluster.failover else 'manual ', action) - logger.warning('manual failover: candidates list is empty') + logger.warning('%s: candidates list is empty', action) ret = False cluster_timeline = self.cluster.timeline @@ -955,7 +965,7 @@ class Ha(object): if TYPE_CHECKING: # pragma: no cover assert failover is not None - action = 'switchover' if failover.leader else 'failover' + action = self._get_failover_action_name() if failover.candidate: # manual failover/switchover to specific member if failover.candidate == self.state_handler.name: # manual failover/switchover to me @@ -965,7 +975,7 @@ class Ha(object): # In order to avoid attempts to delete this key from all nodes only the primary is allowed to do it. if not self.cluster.get_member(failover.candidate, fallback_to_leader=False)\ and self.state_handler.is_leader(): - logger.warning("manual %s: removing failover key because failover candidate is not running", action) + logger.warning("%s: removing failover key because failover candidate is not running", action) self.dcs.manual_failover('', '', version=failover.version) return None return False @@ -981,10 +991,10 @@ class Ha(object): st = self.fetch_node_status(member) not_allowed_reason = st.failover_limitation() if not_allowed_reason is None: # node is healthy - logger.info('manual %s: to %s, i am %s', action, st.member.name, self.state_handler.name) + logger.info('%s: to %s, i am %s', action, st.member.name, self.state_handler.name) return False # we wanted to failover/switchover to specific member but it is not healthy - logger.warning('manual %s: member %s is %s', action, st.member.name, not_allowed_reason) + logger.warning('%s: member %s is %s', action, st.member.name, not_allowed_reason) # at this point we should consider all members as a candidates for failover/switchover # i.e. we assume that failover.candidate is None @@ -1217,7 +1227,7 @@ class Ha(object): if not failover or (self.is_paused() and not self.state_handler.is_leader()): return - action = 'failover' if not failover.leader else 'switchover' + action = self._get_failover_action_name() # it is not the time for the the scheduled failover yet, do nothing if (failover.scheduled_at and not @@ -1230,15 +1240,15 @@ class Ha(object): if not failover.candidate and self.is_paused(): logger.warning('%s is possible only to a specific candidate in a paused state', action.title()) elif self.is_failover_possible(): - ret = self._async_executor.try_run_async(f'manual {action}: demote', self.demote, ('graceful',)) - return ret or f'manual {action}: demoting myself' + ret = self._async_executor.try_run_async(f'{action}: demote', self.demote, ('graceful',)) + return ret or f'{action}: demoting myself' else: - logger.warning('manual %s: no healthy members found, %s is not possible', action, action) + logger.warning('%s: no healthy members found, %s is not possible', + action, action.removeprefix('manual ')) else: - logger.warning('manual %s: I am already the leader, no need to %s', action, action) + logger.warning('%s: I am already the leader, no need to %s', action, action.removeprefix('manual ')) else: - logger.warning('manual %s: leader name does not match: %s != %s', - action, failover.leader, self.state_handler.name) + logger.warning('%s: leader name does not match: %s != %s', action, failover.leader, self.state_handler.name) logger.info('Cleaning up failover key') self.dcs.manual_failover('', '', version=failover.version) diff --git a/tests/test_ha.py b/tests/test_ha.py index b6b21640..cd5aa3f1 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -693,14 +693,14 @@ class TestHa(PostgresInit): with patch('patroni.ha.logger.warning') as mock_warning: self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', self.p.name, None)) self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock') - mock_warning.assert_called_with('manual %s: I am already the leader, no need to %s', 'failover', 'failover') + mock_warning.assert_called_with('%s: I am already the leader, no need to %s', 'manual failover', 'failover') # to a non-existent candidate with patch('patroni.ha.logger.warning') as mock_warning: self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'blabla', None)) self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock') mock_warning.assert_called_with( - 'manual %s: no healthy members found, %s is not possible', 'failover', 'failover') + '%s: no healthy members found, %s is not possible', 'manual failover', 'failover') # to an existent candidate self.ha.fetch_node_status = get_node_status() @@ -735,14 +735,14 @@ class TestHa(PostgresInit): self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '', None)) self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock') mock_warning.assert_called_with( - 'manual %s: leader name does not match: %s != %s', 'switchover', 'blabla', 'postgresql0') + '%s: leader name does not match: %s != %s', 'switchover', 'blabla', 'postgresql0') # no candidate self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, '', None)) - self.assertEqual(self.ha.run_cycle(), 'manual switchover: demoting myself') + self.assertEqual(self.ha.run_cycle(), 'switchover: demoting myself') self.ha._rewind.rewind_or_reinitialize_needed_and_possible = true - self.assertEqual(self.ha.run_cycle(), 'manual switchover: demoting myself') + self.assertEqual(self.ha.run_cycle(), 'switchover: demoting myself') # other members with failover_limitation_s with patch('patroni.ha.logger.info') as mock_info: @@ -782,7 +782,7 @@ class TestHa(PostgresInit): scheduled = datetime.datetime.utcnow().replace(tzinfo=tzutc) self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'b', scheduled)) self.ha.cluster.members.append(Member(0, 'b', 28, {'api_url': 'http://127.0.0.1:8011/patroni'})) - self.assertEqual('manual switchover: demoting myself', self.ha.run_cycle()) + self.assertEqual('switchover: demoting myself', self.ha.run_cycle()) # scheduled in the future with patch('patroni.ha.logger.info') as mock_info: @@ -857,7 +857,7 @@ class TestHa(PostgresInit): self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'a', None), sync=(self.p.name, 'a')) self.ha.cluster.members.append(Member(0, 'a', 28, {'api_url': 'http://127.0.0.1:8011/patroni'})) - self.assertEqual('manual switchover: demoting myself', self.ha.run_cycle()) + self.assertEqual('switchover: demoting myself', self.ha.run_cycle()) # the candidate is in sync members but is not healthy with patch('patroni.ha.logger.info') as mock_info: @@ -874,7 +874,7 @@ class TestHa(PostgresInit): self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader', None)) self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') self.assertEqual(mock_warning.call_args_list[1][0], - ('manual %s: member %s is %s', 'failover', 'leader', 'not reachable')) + ('%s: member %s is %s', 'manual failover', 'leader', 'not reachable')) # failover to another member, candidate is accessible, in_recovery self.p.set_role('replica') @@ -901,14 +901,14 @@ class TestHa(PostgresInit): self.ha.cluster.members.append(Member(0, 'b', 28, {'api_url': 'http://127.0.0.1:8011/patroni'})) self.ha.fetch_node_status = get_node_status(timeline=1) self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') - mock_info.assert_called_with('manual %s: to %s, i am %s', 'failover', 'b', 'postgresql0') + mock_info.assert_called_with('%s: to %s, i am %s', 'manual failover', 'b', 'postgresql0') # failover to another member lagging behind the cluster_lsn (only failover_limitation() is checked) with patch('patroni.ha.logger.info') as mock_info: self.ha.cluster.config.data.update({'maximum_lag_on_failover': 5}) self.ha.fetch_node_status = get_node_status(wal_position=1) self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node') - mock_info.assert_called_with('manual %s: to %s, i am %s', 'failover', 'b', 'postgresql0') + mock_info.assert_called_with('%s: to %s, i am %s', 'manual failover', 'b', 'postgresql0') def test_manual_switchover_process_no_leader(self): self.p.is_leader = false @@ -945,7 +945,7 @@ class TestHa(PostgresInit): self.ha.dcs.write_sync_state = Mock(return_value=SyncState.empty()) self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock') self.assertEqual(mock_warning.call_args_list[0][0], - ('manual %s: member %s is %s', 'failover', 'other', 'not allowed to promote')) + ('%s: member %s is %s', 'manual failover', 'other', 'not allowed to promote')) # manual failover to our node (postgresql0), # which name is not in sync nodes list (some sync nodes are available) @@ -1001,7 +1001,7 @@ class TestHa(PostgresInit): self.assertEqual('PAUSE: acquired session lock as a leader', self.ha.run_cycle()) self.assertEqual( mock_warning.call_args_list[0][0], - ('manual %s: removing failover key because failover candidate is not running', 'switchover')) + ('%s: removing failover key because failover candidate is not running', 'switchover')) # switchover to me, I am not leader self.p.is_leader = false @@ -1253,7 +1253,7 @@ class TestHa(PostgresInit): f = Failover(0, self.p.name, '', None) self.ha.cluster = get_cluster_initialized_with_leader(f) self.ha.fetch_node_status = get_node_status() # accessible, in_recovery - self.assertEqual(self.ha.run_cycle(), 'manual switchover: demoting myself') + self.assertEqual(self.ha.run_cycle(), 'switchover: demoting myself') @patch('patroni.ha.Ha.demote') def test_failover_immediately_on_zero_primary_start_timeout(self, demote):