Exclude leader from failover candidates in ctl (#2983)

Exclude actual leader (not the passed leader argument) from the
candidates list in the `patronictl failover` prompt.
Abort `patronictl failover` execution if candidate specified is
the same as the current cluster leader
This commit is contained in:
Polina Bungina
2024-01-05 10:12:33 +01:00
committed by Alexander Kukushkin
parent f10e4805db
commit 15b57c5bdc
2 changed files with 25 additions and 19 deletions
+19 -17
View File
@@ -1190,7 +1190,7 @@ def reinit(obj: Dict[str, Any], cluster_name: str, group: Optional[int],
def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: str,
group: Optional[int], leader: Optional[str], candidate: Optional[str],
group: Optional[int], switchover_leader: Optional[str], candidate: Optional[str],
force: bool, scheduled: Optional[str] = None) -> None:
"""Perform a failover or a switchover operation in the cluster.
@@ -1205,7 +1205,7 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
:param cluster_name: name of the Patroni cluster.
:param group: filter Citus group within we should perform a failover or switchover. If ``None``, user will be
prompted for filling it -- unless *force* is ``True``, in which case an exception is raised.
:param leader: name of the current leader member.
:param switchover_leader: name of the leader member passed as switchover option.
:param candidate: name of a standby member to be promoted. Nodes that are tagged with ``nofailover`` cannot be used.
:param force: perform the failover or switchover without asking for confirmations.
:param scheduled: timestamp when the switchover should be scheduled to occur. If ``now`` perform immediately.
@@ -1214,10 +1214,11 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
:class:`PatroniCtlException`: if:
* Patroni is running on a Citus cluster, but no *group* was specified; or
* a switchover was requested by the cluster has no leader; or
* *leader* does not match the current leader of the cluster; or
* *switchover_leader* does not match the current leader of the cluster; or
* cluster has no candidates available for the operation; or
* no *candidate* is given for a failover operation; or
* *leader* and *candidate* are the same; or
* current leader and *candidate* are the same; or
* *candidate* is tagged as nofailover; or
* *candidate* is not a member of the cluster; or
* trying to schedule a switchover in a cluster that is in maintenance mode; or
* user aborts the operation.
@@ -1237,23 +1238,24 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
global_config = get_global_config(cluster)
cluster_leader = cluster.leader and cluster.leader.name
# leader has to be be defined for switchover only
if action == 'switchover':
if cluster.leader is None or not cluster.leader.name:
if not cluster_leader:
raise PatroniCtlException('This cluster has no leader')
if leader is None:
if switchover_leader is None:
if force:
leader = cluster.leader.name
switchover_leader = cluster_leader
else:
prompt = 'Standby Leader' if global_config.is_standby_cluster else 'Primary'
leader = click.prompt(prompt, type=str, default=(cluster.leader and cluster.leader.name))
switchover_leader = click.prompt(prompt, type=str, default=cluster_leader)
if cluster.leader.name != leader:
raise PatroniCtlException(f'Member {leader} is not the leader of cluster {cluster_name}')
if cluster_leader != switchover_leader:
raise PatroniCtlException(f'Member {switchover_leader} is not the leader of cluster {cluster_name}')
# excluding members with nofailover tag
candidate_names = [str(m.name) for m in cluster.members if m.name != leader and not m.nofailover]
candidate_names = [str(m.name) for m in cluster.members if m.name != cluster_leader and not m.nofailover]
# We sort the names for consistent output to the client
candidate_names.sort()
@@ -1266,10 +1268,10 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
if action == 'failover' and not candidate:
raise PatroniCtlException('Failover could be performed only to a specific candidate')
if candidate == leader:
raise PatroniCtlException(action.title() + ' target and source are the same.')
if candidate and candidate not in candidate_names:
if candidate == cluster_leader:
raise PatroniCtlException(
f'Member {candidate} is already the leader of cluster {cluster_name}')
raise PatroniCtlException(
f'Member {candidate} does not exist in cluster {cluster_name} or is tagged as nofailover')
@@ -1298,7 +1300,7 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
failover_value = {'candidate': candidate}
if action == 'switchover':
failover_value['leader'] = leader
failover_value['leader'] = switchover_leader
if scheduled_at_str:
failover_value['scheduled_at'] = scheduled_at_str
@@ -1306,7 +1308,7 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
# By now we have established that the leader exists and the candidate exists
if not force:
demote_msg = f', demoting current leader {cluster.leader.name}' if cluster.leader else ''
demote_msg = f', demoting current leader {cluster_leader}' if cluster_leader else ''
if scheduled_at_str:
# only switchover can be scheduled
if not click.confirm(f'Are you sure you want to schedule switchover of cluster '
@@ -1340,7 +1342,7 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
logging.exception(r)
logging.warning('Failing over to DCS')
click.echo('{0} Could not {1} using Patroni api, falling back to DCS'.format(timestamp(), action))
dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at)
dcs.manual_failover(switchover_leader, candidate, scheduled_at=scheduled_at)
output_members(obj, cluster, cluster_name, group=group)
+6 -2
View File
@@ -156,7 +156,8 @@ class TestCtl(unittest.TestCase):
# Target and source are equal
result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nleader\n\ny')
self.assertEqual(result.exit_code, 1)
self.assertIn('Switchover target and source are the same', result.output)
self.assertIn("Candidate ['other']", result.output)
self.assertIn('Member leader is already the leader of cluster dummy', result.output)
# Candidate is not a member of the cluster
result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nReality\n\ny')
@@ -223,7 +224,10 @@ class TestCtl(unittest.TestCase):
result = self.runner.invoke(ctl, ['failover', 'dummy'], input='0\n')
self.assertIn('Failover could be performed only to a specific candidate', result.output)
cluster = get_cluster_initialized_with_leader(sync=('leader', 'other'))
# Candidate is the same as the leader
result = self.runner.invoke(ctl, ['failover', 'dummy', '--group', '0'], input='leader\n')
self.assertIn("Candidate ['other']", result.output)
self.assertIn('Member leader is already the leader of cluster dummy', result.output)
# Temp test to check a fallback to switchover if leader is specified
with patch('patroni.ctl._do_failover_or_switchover') as failover_func_mock: