Merge branch 'feature/failover-switchover-definition' into refactor/failover-limitations-checks

Co-authored-by: Matt Baker <[email protected]>

docs/pause.rst

@@ -19,7 +19,7 @@ When Patroni runs in a paused mode, it does not change the state of PostgreSQL,
 
 - For the Postgres primary with the leader lock Patroni updates the lock. If the node with the leader lock stops being the primary (i.e. is demoted manually), Patroni will release the lock instead of promoting the node back.
 
-- Manual unscheduled restart, reinitialize and manual failover are allowed. Manual failover is only allowed if the node to failover to is specified. In the paused mode, manual failover does not require a running primary node.
+- Manual unscheduled restart, manual unscheduled failover/switchover and reinitialize are allowed. No scheduled action is allowed. Manual switchover is only allowed if the node to switchover to is specified.
 
 - If 'parallel' primaries are detected by Patroni, it emits a warning, but does not demote the primary without the leader lock.
 

docs/rest_api.rst

@@ -554,39 +554,111 @@ The above call removes ``postgresql.parameters.max_connections`` from the dynami
 Switchover and failover endpoints
 ---------------------------------
 
-``POST /switchover`` or ``POST /failover``. These endpoints are very similar to each other. There are a couple of minor differences though:
+.. _switchover_api:
 
-1. The failover endpoint allows to perform a manual failover when there are no healthy nodes, but at the same time it will not allow you to schedule a switchover.
+Switchover
+^^^^^^^^^^
 
-2. The switchover endpoint is the opposite. It works only when the cluster is healthy (there is a leader) and allows to schedule a switchover at a given time.
+``/switchover`` endpoint only works when cluster is healthy (there is a leader). It allows to schedule a switchover at a given time.
 
+When calling ``/switchover`` endpoint candidate can be specified but is not required, in contrast to ``/failover`` endpoint. If candidate is not provided, all the healthy nodes that are allowed to failover participate in the leader race.
 
-In the JSON body of the ``POST`` request you must specify at least the ``leader`` or ``candidate`` fields and optionally the ``scheduled_at`` field if you want to schedule a switchover at a specific time.
+In the JSON body of the ``POST`` request, you must specify at least the ``leader`` field and, optionally, the ``candidate`` and ``scheduled_at`` field if you want to schedule a switchover at a specific time.
 
+Depending on the situation, requests might return different HTTP status codes and bodies. Status code **200** is returned when the switchover or failover successfully completed. If the switchover was successfully scheduled, Patroni will return HTTP status code **202**. In case something went wrong, the error status code (one of **400**, **412**, or **503**) will be returned with some details in the response body.
 
-Example: perform a failover to the specific node:
+``DELETE /switchover`` can be used to delete the currently scheduled switchover.
+
+**Example:** perform a switchover to any healthy standby
 
 .. code-block:: bash
 
-    $ curl -s http://localhost:8009/failover -XPOST -d '{"candidate":"postgresql1"}'
+	$ curl -s http://localhost:8008/switchover -XPOST -d '{"leader":"postgresql1"}'
-    Successfully failed over to "postgresql1"
+	Successfully switched over to "postgresql2"
 
 
-Example: schedule a switchover from the leader to any other healthy replica in the cluster at a specific time:
+**Example:** perform a switchover to a specific node
 
 .. code-block:: bash
 
-    $ curl -s http://localhost:8008/switchover -XPOST -d \
+	$ curl -s http://localhost:8008/switchover -XPOST -d \
-	    '{"leader":"postgresql0","scheduled_at":"2019-09-24T12:00+00"}'
+		'{"leader":"postgresql1","candidate":"postgresql2"}'
-    Switchover scheduled
+	Successfully switched over to "postgresql2"
 
 
-Depending on the situation the request might finish with a different HTTP status code and body. The status code **200** is returned when the switchover or failover successfully completed. If the switchover was successfully scheduled, Patroni will return HTTP status code **202**. In case something went wrong, the error status code (one of **400**, **412** or **503**) will be returned with some details in the response body. For more information please check the source code of ``patroni/api.py:do_POST_failover()`` method.
+**Example:** schedule a switchover from the leader to any other healthy standby in the cluster at a specific time.
 
-- ``DELETE /switchover``: delete the scheduled switchover
+.. code-block:: bash
 
-The ``POST /switchover`` and ``POST failover`` endpoints are used by ``patronictl switchover`` and ``patronictl failover``, respectively.
+	$ curl -s http://localhost:8008/switchover -XPOST -d \
-The ``DELETE /switchover`` is used by ``patronictl flush <cluster-name> switchover``.
+		'{"leader":"postgresql0","scheduled_at":"2019-09-24T12:00+00"}'
+	Switchover scheduled
+
+
+Failover
+^^^^^^^^
+
+``/failover`` endpoint can be used to perform a manual failover when there are no healthy nodes (e.g. to an asynchronous standby if all synchronous standbys are not healthy enough to promote). However there is no requirement for a cluster not to have leader - failover can also be run on a healthy cluster.
+
+In the JSON body of the ``POST`` request you must specify ``candidate`` field. If ``leader`` field is specified, switchover is triggered. 
+
+**Example:**
+
+.. code-block:: bash
+
+	$ curl -s http://localhost:8008/failover -XPOST -d '{"candidate":"postgresql1"}'
+	Successfully failed over to "postgresql1"
+
+.. warning::
+	:ref:`Be very careful <failover_healthcheck>` using this endpoint, as this can cause data loss in certain situations. In most cases, :ref:`the switchover endpoint <switchover_api>` satisfies the administrator's needs. 
+
+
+``POST /switchover`` and ``POST /failover`` endpoints are used by ``patronictl switchover`` and ``patronictl failover``, respectively.
+
+``DELETE /switchover`` is used by ``patronictl flush <cluster-name> switchover``.
+
+.. list-table:: Failover/Switchover comparison
+   :widths: 25 25 25
+   :header-rows: 1
+
+   * -
+     - Failover
+     - Switchover
+   * - Requires leader specified
+     - no
+     - yes
+   * - Requires candidate specified
+     - yes
+     - no
+   * - Can be run in pause
+     - yes
+     - yes (only to a specific candidate)
+   * - Can be scheduled
+     - no
+     - yes (if not in pause)
+
+.. _failover_healthcheck:
+
+Healthy standby
+^^^^^^^^^^^^^^^
+
+There are a couple of checks that a member of a cluster should pass to be able to participate in the leader race during a switchover or to become a leader as a failover/switchover candidate:
+
+- be reachable via Patroni API,
+- not have ``nofailover`` tag set to ``true``,
+- have watchdog fully functional (if required by the configuration),
+- in case of a switchover or a failover in a healthy cluster, not exceed maximum replication lag (``maximum_lag_on_failover`` :ref:`configuration parameter <dynamic_configuration>`),
+- in case of a switchover or a failover in a healthy cluster, not have a timeline number smaller than the cluster timeline,
+- in :ref:`synchronous mode <synchronous_mode>`:
+
+  - In case of a switchover (both with and without a candidate): be listed in the ``/sync`` key members.
+  - For a failover in both healthy and unhealthy clusters, this check is omitted.
+
+.. warning::
+    In case of a failover in a cluster without a leader, a candidate will be allowed to promote even if:
+	- it is not in the ``/sync`` key members when synchronous mode is enabled,
+	- its lag exceeds the maximum replication lag allowed,
+	- it has the timeline number smaller than the cluster timeline.
 
 
 Restart endpoint

patroni/api.py

@@ -12,7 +12,6 @@ import json
 import logging
 import time
 import traceback
-import dateutil.parser
 import datetime
 import os
 import socket
@@ -28,11 +27,11 @@ from typing import Any, Callable, Dict, Iterator, List, Optional, Tuple, TYPE_CH
 
 from . import psycopg
 from .__main__ import Patroni
-from .dcs import Cluster
 from .exceptions import PostgresConnectionException, PostgresException
+from .manual_failover import ManualFailover
 from .postgresql.misc import postgres_version_to_int
 from .utils import deep_compare, enable_keepalive, parse_bool, patch_config, Retry, \
-    RetryFailedError, parse_int, split_host_port, tzutc, uri, cluster_as_json
+    RetryFailedError, parse_int, parse_schedule, split_host_port, tzutc, uri, cluster_as_json
 
 logger = logging.getLogger(__name__)
 
@@ -770,44 +769,6 @@ class RestApiHandler(BaseHTTPRequestHandler):
             self.server.patroni.api_sigterm()
         self.write_response(202, 'shutdown scheduled')
 
-    @staticmethod
-    def parse_schedule(schedule: str,
-                       action: str) -> Tuple[Union[int, None], Union[str, None], Union[datetime.datetime, None]]:
-        """Parse the given *schedule* and validate it.
-
-        :param schedule: a string representing a timestamp, e.g. ``2023-04-14T20:27:00+00:00``.
-        :param action: the action to be scheduled (``restart``, ``switchover``, or ``failover``).
-
-        :returns: a tuple composed of 3 items:
-
-            * Suggested HTTP status code for a response:
-
-                * ``None``: if no issue was faced while parsing, leaving it up to the caller to decide the status; or
-                * ``400``: if no timezone information could be found in *schedule*; or
-                * ``422``: if *schedule* is invalid -- in the past or not parsable.
-
-            * An error message, if any error is faced, otherwise ``None``;
-            * Parsed *schedule*, if able to parse, otherwise ``None``.
-
-        """
-        error = None
-        scheduled_at = None
-        try:
-            scheduled_at = dateutil.parser.parse(schedule)
-            if scheduled_at.tzinfo is None:
-                error = 'Timezone information is mandatory for the scheduled {0}'.format(action)
-                status_code = 400
-            elif scheduled_at < datetime.datetime.now(tzutc):
-                error = 'Cannot schedule {0} in the past'.format(action)
-                status_code = 422
-            else:
-                status_code = None
-        except (ValueError, TypeError):
-            logger.exception('Invalid scheduled %s time: %s', action, schedule)
-            error = 'Unable to parse scheduled timestamp. It should be in an unambiguous format, e.g. ISO 8601'
-            status_code = 422
-        return status_code, error, scheduled_at
-
     @check_access
     def do_POST_restart(self) -> None:
         """Handle a ``POST`` request to ``/restart`` path.
@@ -863,9 +824,9 @@ class RestApiHandler(BaseHTTPRequestHandler):
 
         for k in request:
             if k == 'schedule':
-                (_, data, request[k]) = self.parse_schedule(request[k], "restart")
+                parse_result, request[k] = parse_schedule(request[k])
-                if _:
+                if parse_result:
-                    status_code = _
+                    data, status_code = parse_result.value[0], parse_result.value[1]
                     break
             elif k == 'role':
                 if request[k] not in ('master', 'primary', 'replica'):
@@ -1015,39 +976,6 @@ class RestApiHandler(BaseHTTPRequestHandler):
                 logger.debug('Exception occurred during polling %s result: %s', action, e)
         return 503, action.title() + ' status unknown'
 
-    def is_failover_possible(self, cluster: Cluster, leader: Optional[str], candidate: Optional[str],
-                             action: str) -> Optional[str]:
-        """Checks whether there are nodes that could take over after demoting the primary.
-
-        :param cluster: the Patroni cluster.
-        :param leader: name of the current Patroni leader.
-        :param candidate: name of the Patroni node to be promoted.
-        :param action: the action to be performed (``switchover`` or ``failover``).
-
-        :returns: a string with the error message or ``None`` if good nodes are found.
-        """
-        is_synchronous_mode = self.server.patroni.config.get_global_config(cluster).is_synchronous_mode
-        if leader and (not cluster.leader or cluster.leader.name != leader):
-            return 'leader name does not match'
-        if candidate:
-            if action == 'switchover' and is_synchronous_mode and not cluster.sync.matches(candidate):
-                return 'candidate name does not match with sync_standby'
-            members = [m for m in cluster.members if m.name == candidate]
-            if not members:
-                return 'candidate does not exists'
-        elif is_synchronous_mode:
-            members = [m for m in cluster.members if cluster.sync.matches(m.name)]
-            if not members:
-                return action + ' is not possible: can not find sync_standby'
-        else:
-            members = [m for m in cluster.members if not cluster.leader or m.name != cluster.leader.name and m.api_url]
-            if not members:
-                return action + ' is not possible: cluster does not have members except leader'
-        for st in self.server.patroni.ha.fetch_nodes_statuses(members):
-            if st.failover_limitation() is None:
-                return None
-        return action + ' is not possible: no good candidates have been found'
-
     @check_access
     def do_POST_failover(self, action: str = 'failover') -> None:
         """Handle a ``POST`` request to ``/failover`` path.
@@ -1075,7 +1003,6 @@ class RestApiHandler(BaseHTTPRequestHandler):
         :param action: the action to be performed (``switchover`` or ``failover``).
         """
         request = self._read_json_content()
-        (status_code, data) = (400, '')
         if not request:
             return
 
@@ -1088,26 +1015,15 @@ class RestApiHandler(BaseHTTPRequestHandler):
         logger.info("received %s request with leader=%s candidate=%s scheduled_at=%s",
                     action, leader, candidate, scheduled_at)
 
-        if action == 'failover' and not candidate:
+        manual_failover = ManualFailover(action, cluster, leader, candidate, scheduled_at,
-            data = 'Failover could be performed only to a specific candidate'
+                                         global_config.is_paused, global_config.is_synchronous_mode,
-        elif action == 'switchover' and not leader:
+                                         self.server.patroni)
-            data = 'Switchover could be performed only from a specific leader'
+        data, status_code = manual_failover.run_precheck().value
 
         if not data and scheduled_at:
-            if not leader:
+            parse_result, scheduled_at = manual_failover.parse_scheduled()
-                data = 'Scheduled {0} is possible only from a specific leader'.format(action)
+            if parse_result:
-            if not data and global_config.is_paused:
+                data, status_code = parse_result.value[0], parse_result.value[1]
-                data = "Can't schedule {0} in the paused state".format(action)
-            if not data:
-                (status_code, data, scheduled_at) = self.parse_schedule(scheduled_at, action)
-
-        if not data and global_config.is_paused and not candidate:
-            data = action.title() + ' is possible only to a specific candidate in a paused state'
-
-        if not data and not scheduled_at:
-            data = self.is_failover_possible(cluster, leader, candidate, action)
-            if data:
-                status_code = 412
 
         if not data:
             if self.server.patroni.dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at):
@@ -1119,14 +1035,12 @@ class RestApiHandler(BaseHTTPRequestHandler):
                     status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name,
                                                                   candidate, action)
             else:
-                data = 'failed to write {0} key into DCS'.format(action)
+                data = 'failed to write failover key into DCS'
                 status_code = 503
-        # pyright thinks ``status_code`` can be ``None`` because ``parse_schedule`` call may return ``None``. However,
+
-        # if that's the case, ``status_code`` will be overwritten somewhere between ``parse_schedule`` and
+        status_code = status_code or 400
-        # ``write_response`` calls.
+        self.write_response(status_code, data.format(action=action, leader=leader, candidate=candidate,
-        if TYPE_CHECKING:  # pragma: no cover
+                                                     cluster_name=self.server.patroni.postgresql.scope))
-            assert isinstance(status_code, int)
-        self.write_response(status_code, data)
 
     def do_POST_switchover(self) -> None:
         """Handle a ``POST`` request to ``/switchover`` path.

patroni/ctl.py

@@ -16,8 +16,6 @@ import click
 import codecs
 import copy
 import datetime
-import dateutil.parser
-import dateutil.tz
 import difflib
 import io
 import json
@@ -46,10 +44,12 @@ try:
 except ImportError:  # pragma: no cover
     from cdiff import markup_to_pager, PatchStream  # pyright: ignore [reportMissingModuleSource]
 
+from .config import Config, get_global_config
 from .dcs import get_dcs as _get_dcs, AbstractDCS, Cluster, Member
 from .exceptions import PatroniException
+from .manual_failover import ManualFailover
 from .postgresql.misc import postgres_version_to_int
-from .utils import cluster_as_json, patch_config, polling_loop
+from .utils import cluster_as_json, parse_schedule, patch_config, polling_loop
 from .request import PatroniRequest
 from .version import __version__
 
@@ -225,8 +225,6 @@ def load_config(path: str, dcs_url: Optional[str]) -> Dict[str, Any]:
     :raises:
         :class:`PatroniCtlException`: if *path* does not exist or is not readable.
     """
-    from patroni.config import Config
-
     if not (os.path.exists(path) and os.access(path, os.R_OK)):
         if path != CONFIG_FILE_PATH:    # bail if non-default config location specified but file not found / readable
             raise PatroniCtlException('Provided config file {0} not existing or no read rights.'
@@ -644,7 +642,8 @@ def get_members(obj: Dict[str, Any], cluster: Cluster, cluster_name: str, member
     if member_names:
         member_names = list(set(member_names) & candidates)
         if not member_names:
-            raise PatroniCtlException('No {0} among provided members'.format(role))
+            raise PatroniCtlException(
+                'No{0} among provided members'.format('t a single cluster member' if role == 'any' else ' ' + role))
     elif action != 'reinitialize':
         member_names = list(candidates)
 
@@ -944,43 +943,6 @@ def check_response(response: urllib3.response.HTTPResponse, member_name: str,
     return True
 
 
-def parse_scheduled(scheduled: Optional[str]) -> Optional[datetime.datetime]:
-    """Parse a string *scheduled* timestamp as a :class:`~datetime.datetime` object.
-
-    :param scheduled: string representation of the timestamp. May also be ``now``.
-
-    :returns: the corresponding :class:`~datetime.datetime` object, if *scheduled* is not ``now``, otherwise ``None``.
-
-    :raises:
-        :class:`PatroniCtlException`: if unable to parse *scheduled* from :class:`str` to :class:`~datetime.datetime`.
-
-    :Example:
-
-        >>> parse_scheduled(None) is None
-        True
-
-        >>> parse_scheduled('now') is None
-        True
-
-        >>> parse_scheduled('2023-05-29T04:32:31')
-        datetime.datetime(2023, 5, 29, 4, 32, 31, tzinfo=tzlocal())
-
-        >>> parse_scheduled('2023-05-29T04:32:31-3')
-        datetime.datetime(2023, 5, 29, 4, 32, 31, tzinfo=tzoffset(None, -10800))
-    """
-    if scheduled is not None and (scheduled or 'now') != 'now':
-        try:
-            scheduled_at = dateutil.parser.parse(scheduled)
-            if scheduled_at.tzinfo is None:
-                scheduled_at = scheduled_at.replace(tzinfo=dateutil.tz.tzlocal())
-        except (ValueError, TypeError):
-            message = 'Unable to parse scheduled timestamp ({0}). It should be in an unambiguous format (e.g. ISO 8601)'
-            raise PatroniCtlException(message.format(scheduled))
-        return scheduled_at
-
-    return None
-
-
 @ctl.command('reload', help='Reload cluster member configuration')
 @click.argument('cluster_name')
 @click.argument('member_names', nargs=-1)
@@ -1011,7 +973,6 @@ def reload(obj: Dict[str, Any], cluster_name: str, member_names: List[str],
         if r.status == 200:
             click.echo('No changes to apply on member {0}'.format(member.name))
         elif r.status == 202:
-            from patroni.config import get_global_config
             config = get_global_config(cluster)
             click.echo('Reload request received for member {0} and will be processed within {1} seconds'.format(
                 member.name, config.get('loop_wait') or dcs.loop_wait)
@@ -1061,16 +1022,20 @@ def restart(obj: Dict[str, Any], cluster_name: str, group: Optional[int], member
             * *version* could not be parsed; or
             * a restart is attempted against a cluster that is in maintenance mode.
     """
+    action = 'restart'
     cluster = get_dcs(obj, cluster_name, group).get_cluster()
 
-    members = get_members(obj, cluster, cluster_name, member_names, role, force, 'restart', False, group=group)
+    members = get_members(obj, cluster, cluster_name, member_names, role, force, action, False, group=group)
     if scheduled is None and not force:
-        next_hour = (datetime.datetime.now() + datetime.timedelta(hours=1)).strftime('%Y-%m-%dT%H:%M')
+        next_hour = (datetime.datetime.now() + datetime.timedelta(hours=1)).strftime('%Y-%m-%dT%H:%M+00')
         scheduled = click.prompt('When should the restart take place (e.g. ' + next_hour + ') ',
                                  type=str, default='now')
+        scheduled = scheduled if scheduled != 'now' else None
 
-    scheduled_at = parse_scheduled(scheduled)
+    parse_result, scheduled_at = parse_schedule(scheduled)
-    confirm_members_action(members, force, 'restart', scheduled_at)
+    if parse_result:
+        raise PatroniCtlException(parse_result.value[0].format(action=action))
+    confirm_members_action(members, force, action, scheduled_at)
 
     if p_any:
         random.shuffle(members)
@@ -1093,7 +1058,6 @@ def restart(obj: Dict[str, Any], cluster_name: str, group: Optional[int], member
         content['postgres_version'] = version
 
     if scheduled_at:
-        from patroni.config import get_global_config
         if get_global_config(cluster).is_paused:
             raise PatroniCtlException("Can't schedule restart in the paused state")
         content['schedule'] = scheduled_at.isoformat()
@@ -1176,8 +1140,8 @@ 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], candidate: Optional[str], force: bool,
-                               force: bool, scheduled: Optional[str] = None) -> None:
+                               leader: Optional[str] = None, scheduled: Optional[str] = None) -> None:
     """Perform a failover or a switchover operation in the cluster.
 
     Informational messages are printed in the console during the operation, as well as the list of members before and
@@ -1191,9 +1155,9 @@ 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 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 leader: name of the leader passed to the switchover command if any.
     :param scheduled: timestamp when the switchover should be scheduled to occur. If ``now`` perform immediately.
 
     :raises:
@@ -1213,6 +1177,9 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
     click.echo('Current cluster topology')
     output_members(obj, cluster, cluster_name, group=group)
 
+    # Define everything missing via interactive input or available cluster info (if force mode)
+
+    # Require Citus group
     if obj.get('citus') and group is None:
         if force:
             raise PatroniCtlException('For Citus clusters the --group must me specified')
@@ -1221,72 +1188,81 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
             dcs = get_dcs(obj, cluster_name, group)
             cluster = dcs.get_cluster()
 
-    if action == 'switchover' and (cluster.leader is None or not cluster.leader.name):
+    global_config = get_global_config(cluster)
-        raise PatroniCtlException('This cluster has no leader')
 
-    if leader is None:
+    # Leader is required for switchover only
-        if force or action == 'failover':
+    if action == 'switchover' and leader is None:
-            leader = cluster.leader and cluster.leader.name
+        if cluster.leader is None or not cluster.leader.name:
+            raise PatroniCtlException('This cluster has no leader')
+        if force:
+            leader = cluster.leader.name
         else:
-            from patroni.config import get_global_config
+            prompt = 'Standby Leader' if global_config.is_standby_cluster else 'Primary'
-            prompt = 'Standby Leader' if get_global_config(cluster).is_standby_cluster else 'Primary'
+            leader = click.prompt(prompt, type=str, default=(cluster.leader and cluster.leader.name))
-            leader = click.prompt(prompt, type=str, default=(cluster.leader and cluster.leader.member.name))
-
-    if leader is not None and cluster.leader and cluster.leader.member.name != leader:
-        raise PatroniCtlException('Member {0} is not the leader of cluster {1}'.format(leader, 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]
-    # We sort the names for consistent output to the client
-    candidate_names.sort()
-
-    if not candidate_names:
-        raise PatroniCtlException('No candidates found to {0} to'.format(action))
 
     if candidate is None and not force:
+        # Check if there are any candidates available at all
+        candidate_names = [str(m.name) for m in cluster.members if m.name != leader and not m.nofailover]
+        if not candidate_names:
+            raise PatroniCtlException('No candidates found to {0} to'.format(action))
+        candidate_names.sort()   # we sort the names for consistent output to the client
         candidate = click.prompt('Candidate ' + str(candidate_names), type=str, default='')
 
-    if action == 'failover' and not candidate:
+    # We allow manual failover to an aync node in the sync mode, so we better ask for the confirmation
-        raise PatroniCtlException('Failover could be performed only to a specific candidate')
+    if all((not force,
+            action == 'failover',
+            global_config.is_synchronous_mode,
+            not cluster.sync.is_empty,
+            not cluster.sync.matches(candidate, True))):
+        if click.confirm(f'Are you sure you want to failover to the asynchronous node {candidate}'):
+            raise PatroniCtlException('Aborting ' + action)
 
-    if candidate == leader:
+    if action == 'switchover' and scheduled is None and not force:
-        raise PatroniCtlException(action.title() + ' target and source are the same.')
+        next_hour = (datetime.datetime.now() + datetime.timedelta(hours=1)).strftime('%Y-%m-%dT%H:%M+00')
+        scheduled = click.prompt('When should the switchover take place (e.g. ' + next_hour + ') ',
+                                 type=str, default='now')
+        scheduled = scheduled if scheduled != 'now' else None
 
-    if candidate and candidate not in candidate_names:
+    # Now, when we collected all the possible info, run checks
-        raise PatroniCtlException('Member {0} does not exist in cluster {1}'.format(candidate, cluster_name))
+    manual_failover = ManualFailover(action, cluster, leader, candidate, scheduled,
+                                     global_config.is_paused, global_config.is_synchronous_mode)
+
+    result_text, _ = manual_failover.run_precheck().value
+    if result_text:
+        raise PatroniCtlException(result_text.format(action=action, leader=leader, candidate=candidate,
+                                                     cluster_name=cluster_name))
 
     scheduled_at_str = None
     scheduled_at = None
-
     if action == 'switchover':
-        if scheduled is None and not force:
+        parse_result, scheduled_at = manual_failover.parse_scheduled()
-            next_hour = (datetime.datetime.now() + datetime.timedelta(hours=1)).strftime('%Y-%m-%dT%H:%M')
+        if parse_result:
-            scheduled = click.prompt('When should the switchover take place (e.g. ' + next_hour + ' ) ',
+            raise PatroniCtlException(parse_result.value[0].format(action=action))
-                                     type=str, default='now')
-
-        scheduled_at = parse_scheduled(scheduled)
         if scheduled_at:
-            from patroni.config import get_global_config
-            if get_global_config(cluster).is_paused:
-                raise PatroniCtlException("Can't schedule switchover in the paused state")
             scheduled_at_str = scheduled_at.isoformat()
 
-    failover_value = {'leader': leader, 'candidate': candidate, 'scheduled_at': scheduled_at_str}
+    # By now we have established that the leader exists and the candidate exists,
-
+    # so confirm the action that is about to be run
-    logging.debug(failover_value)
-
-    # By now we have established that the leader exists and the candidate exists
     if not force:
-        demote_msg = ', demoting current leader ' + leader if leader else ''
+        demote_msg = f', demoting current leader {cluster.leader.name}' if cluster.leader else ''
         if scheduled_at_str:
-            if not click.confirm('Are you sure you want to schedule {0} of cluster {1} at {2}{3}?'
+            # only switchover can be scheduled
-                                 .format(action, cluster_name, scheduled_at_str, demote_msg)):
+            if not click.confirm(f'Are you sure you want to schedule switchover of cluster'
+                                 f'{cluster_name} at {scheduled_at_str}{demote_msg}?'):
                 raise PatroniCtlException('Aborting scheduled ' + action)
         else:
-            if not click.confirm('Are you sure you want to {0} cluster {1}{2}?'
+            if not click.confirm(f'Are you sure you want to {action} cluster {cluster_name}{demote_msg}?'):
-                                 .format(action, cluster_name, demote_msg)):
                 raise PatroniCtlException('Aborting ' + action)
 
+    # And finally the actual work
+    failover_value = {'candidate': candidate}
+    if action == 'switchover':
+        failover_value['leader'] = leader
+    if scheduled_at_str:
+        failover_value['scheduled_at'] = scheduled_at_str
+
+    logging.debug(failover_value)
+
     r = None
     try:
         member = cluster.leader.member if cluster.leader else candidate and cluster.get_member(candidate, False)
@@ -1318,19 +1294,15 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
 @ctl.command('failover', help='Failover to a replica')
 @arg_cluster_name
 @option_citus_group
-@click.option('--leader', '--primary', '--master', 'leader', help='The name of the current leader', default=None)
 @click.option('--candidate', help='The name of the candidate', default=None)
 @option_force
 @click.pass_obj
 def failover(obj: Dict[str, Any], cluster_name: str, group: Optional[int],
-             leader: Optional[str], candidate: Optional[str], force: bool) -> None:
+             candidate: Optional[str], force: bool) -> None:
     """Process ``failover`` command of ``patronictl`` utility.
 
     Perform a failover operation immediately in the cluster.
 
-    .. note::
-        If *leader* is given perform a switchover instead of a failover.
-
     .. seealso::
         Refer to :func:`_do_failover_or_switchover` for details.
 
@@ -1339,12 +1311,10 @@ def failover(obj: Dict[str, Any], cluster_name: str, group: Optional[int],
     :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 by
         :func:`_do_failover_or_switchover`.
-    :param leader: name of the current leader member.
     :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.
     """
-    action = 'switchover' if leader else 'failover'
+    _do_failover_or_switchover(obj, 'failover', cluster_name, group, candidate, force)
-    _do_failover_or_switchover(obj, action, cluster_name, group, leader, candidate, force)
 
 
 @ctl.command('switchover', help='Switchover to a replica')
@@ -1375,7 +1345,7 @@ def switchover(obj: Dict[str, Any], cluster_name: str, group: Optional[int],
     :param force: perform the switchover without asking for confirmations.
     :param scheduled: timestamp when the switchover should be scheduled to occur. If ``now`` perform immediately.
     """
-    _do_failover_or_switchover(obj, 'switchover', cluster_name, group, leader, candidate, force, scheduled)
+    _do_failover_or_switchover(obj, 'switchover', cluster_name, group, candidate, force, leader, scheduled)
 
 
 def generate_topology(level: int, member: Dict[str, Any],
@@ -1714,7 +1684,6 @@ def wait_until_pause_is_applied(dcs: AbstractDCS, paused: bool, old_cluster: Clu
     :param old_cluster: original cluster information before pause or unpause has been requested. Used to report which
         nodes are still pending to have ``pause`` equal *paused* at a given point in time.
     """
-    from patroni.config import get_global_config
     config = get_global_config(old_cluster)
 
     click.echo("'{0}' request sent, waiting until it is recognized by all nodes".format(paused and 'pause' or 'resume'))
@@ -1752,7 +1721,6 @@ def toggle_pause(config: Dict[str, Any], cluster_name: str, group: Optional[int]
             * ``pause`` state is already *paused*; or
             * cluster contains no accessible members.
     """
-    from patroni.config import get_global_config
     dcs = get_dcs(config, cluster_name, group)
     cluster = dcs.get_cluster()
     if get_global_config(cluster).is_paused == paused:

patroni/ha.py

@@ -215,6 +215,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()
 
@@ -901,6 +912,27 @@ class Ha(object):
         lag = (self.cluster.last_lsn or 0) - wal_position
         return lag > self.global_config.maximum_lag_on_failover
 
+    def has_members_eligible_to_promote(self, members: List[Member], reference_lsn: int = 0,
+                                        fast_path: bool = False) -> bool:
+        ret = False
+        cluster_timeline = self.cluster.timeline
+
+        for st in self.fetch_nodes_statuses(members):
+            not_allowed_reason = st.failover_limitation()
+            if not_allowed_reason:
+                logger.info('Member %s is %s', st.member.name, not_allowed_reason)
+            elif fast_path:
+                return True
+            elif reference_lsn and st.wal_position < reference_lsn or \
+                    not reference_lsn and self.is_lagging(st.wal_position):
+                logger.info('Member %s exceeds maximum replication lag', st.member.name)
+            elif self.check_timeline() and (not st.timeline or st.timeline < cluster_timeline):
+                logger.info('Timeline %s of member %s is behind the cluster timeline %s',
+                            st.timeline, st.member.name, cluster_timeline)
+            else:
+                ret = True
+        return ret
+
     def _is_healthiest_node(self, members: Collection[Member], check_replication_lag: bool = True) -> bool:
         """This method tries to determine whether I am healthy enough to became a new leader candidate or not."""
 
@@ -946,27 +978,14 @@ class Ha(object):
         """
         candidates = self.get_failover_candidates(exclude_failover_candidate)
 
+        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('Failover candidate=%s does not match with sync_standbys=%s',
+            logger.warning('%s candidate=%s does not match with sync_standbys=%s',
-                           self.cluster.failover.candidate, self.cluster.sync.sync_standby)
+                           action.title(), self.cluster.failover.candidate, self.cluster.sync.sync_standby)
         elif not candidates:
-            logger.warning('manual failover: candidates list is empty')
+            logger.warning('%s: candidates list is empty', action)
 
-        ret = False
+        return self.has_members_eligible_to_promote(candidates, cluster_lsn)
-        cluster_timeline = self.cluster.timeline
-        for st in self.fetch_nodes_statuses(candidates):
-            not_allowed_reason = st.failover_limitation()
-            if not_allowed_reason:
-                logger.info('Member %s is %s', st.member.name, not_allowed_reason)
-            elif cluster_lsn and st.wal_position < cluster_lsn or \
-                    not cluster_lsn and self.is_lagging(st.wal_position):
-                logger.info('Member %s exceeds maximum replication lag', st.member.name)
-            elif self.check_timeline() and (not st.timeline or st.timeline < cluster_timeline):
-                logger.info('Timeline %s of member %s is behind the cluster timeline %s',
-                            st.timeline, st.member.name, cluster_timeline)
-            else:
-                ret = True
-        return ret
 
     def manual_failover_process_no_leader(self) -> Optional[bool]:
         """Handles manual failover/switchover when the old leader already stepped down.
@@ -977,15 +996,18 @@ class Ha(object):
         failover = self.cluster.failover
         if TYPE_CHECKING:  # pragma: no cover
             assert failover is not None
-        if failover.candidate:  # manual failover to specific member
+
-            if failover.candidate == self.state_handler.name:  # manual failover to me
+        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
                 return True
             elif self.is_paused():
                 # Remove failover key if the node to failover has terminated to avoid waiting for it indefinitely
                 # 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_primary():
-                    logger.warning("manual failover: removing failover key because failover candidate is not running")
+                    logger.warning("%s: removing failover key because failover candidate is not running", action)
                     self.dcs.manual_failover('', '', version=failover.version)
                     return None
                 return False
@@ -1001,17 +1023,17 @@ 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 failover: to %s, i am %s', 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 to specific member but it is not healthy
+                # we wanted to failover/switchover to specific member but it is not healthy
-                logger.warning('manual failover: member %s is %s', 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
+            # at this point we should consider all members as a candidates for failover/switchover
             # i.e. we assume that failover.candidate is None
         elif self.is_paused():
             return False
 
-        # try to pick some other members to failover and check that they are healthy
+        # try to pick some other members to switchover and check that they are healthy
         if failover.leader:
             if self.state_handler.name == failover.leader:  # I was the leader
                 # exclude desired member which is unhealthy if it was specified
@@ -1069,8 +1091,8 @@ class Ha(object):
 
         if self.cluster.failover:
             # When doing a switchover in synchronous mode only synchronous nodes and former leader are allowed to race
-            if self.sync_mode_is_active() and not self.cluster.sync.matches(self.state_handler.name, True) and \
+            if self.cluster.failover.leader and self.sync_mode_is_active() \
-               self.cluster.failover.leader:
+                    and not self.cluster.sync.matches(self.state_handler.name, True):
                 return False
             return self.manual_failover_process_no_leader() or False
 
@@ -1234,28 +1256,35 @@ class Ha(object):
 
         :returns: action message if demote was initiated, None if no action was taken"""
         failover = self.cluster.failover
+        # if there is no failover key or
+        # I am holding the lock but am not primary = I am the standby leader,
+        # then do nothing
         if not failover or (self.is_paused() and not self.state_handler.is_primary()):
             return
 
+        action = self._get_failover_action_name()
+        bare_action = action.replace('manual ', '')
+
+        # it is not the time for the the scheduled failover yet, do nothing
         if (failover.scheduled_at and not
-            self.should_run_scheduled_action("failover", failover.scheduled_at, lambda:
+            self.should_run_scheduled_action(bare_action, failover.scheduled_at, lambda:
                                              self.dcs.manual_failover('', '', version=failover.version))):
             return
 
         if not failover.leader or failover.leader == self.state_handler.name:
             if not failover.candidate or failover.candidate != self.state_handler.name:
                 if not failover.candidate and self.is_paused():
-                    logger.warning('Failover is possible only to a specific candidate in a paused state')
+                    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('manual failover: demote', self.demote, ('graceful',))
+                    ret = self._async_executor.try_run_async(f'{action}: demote', self.demote, ('graceful',))
-                    return ret or 'manual failover: demoting myself'
+                    return ret or f'{action}: demoting myself'
                 else:
-                    logger.warning('manual failover: no healthy members found, failover is not possible')
+                    logger.warning('%s: no healthy members found, %s is not possible',
+                                   action, bare_action)
             else:
-                logger.warning('manual failover: I am already the leader, no need to failover')
+                logger.warning('%s: I am already the leader, no need to %s', action, bare_action)
         else:
-            logger.warning('manual failover: leader name does not match: %s != %s',
+            logger.warning('%s: leader name does not match: %s != %s', action, failover.leader, self.state_handler.name)
-                           failover.leader, self.state_handler.name)
 
         logger.info('Cleaning up failover key')
         self.dcs.manual_failover('', '', version=failover.version)
@@ -1312,6 +1341,7 @@ class Ha(object):
                     self._delete_leader()
                     return 'removed leader lock because postgres is not running as primary'
 
+            # update lock to avoid split-brain
             if self.update_lock(True):
                 msg = self.process_manual_failover_from_leader()
                 if msg is not None:
@@ -1980,8 +2010,9 @@ class Ha(object):
         exclude = [self.state_handler.name] + ([failover.candidate] if failover and exclude_failover_candidate else [])
 
         def is_eligible(node: Member) -> bool:
-            # TODO: allow manual failover (=no leader specified) to async node
+            # in synchronous mode we allow failover (not switchover!) to async node
-            if self.sync_mode_is_active() and not self.cluster.sync.matches(node.name):
+            if self.sync_mode_is_active() and not self.cluster.sync.matches(node.name)\
+                    and not (failover and not failover.leader):
                 return False
             # Don't spend time on "nofailover" nodes checking.
             # We also don't need nodes which we can't query with the api in the list.

patroni/manual_failover.py

@@ -0,0 +1,93 @@
+from enum import Enum
+from typing import Optional, Tuple, TYPE_CHECKING
+
+if TYPE_CHECKING:  # pragma: no cover
+    import datetime
+
+    from .dcs import Cluster
+    from .ha import Patroni
+    from .utils import ParseScheduleErrors
+
+from .utils import parse_schedule
+
+
+class ManualFailoverPrecheckStatus(Enum):
+    FAILOVER_NO_CANDIDATE = ('Failover could be performed only to a specific candidate', 400)
+    SWITCHOVER_NO_LEADER = ('Switchover could be performed only from a specific leader', 400)
+    SCHEDULED_FAILOVER = ("Failover can't be scheduled", 400)
+    SCHEDULED_SWITCHOVER_PAUSE = ("Can't schedule switchover in the paused state", 400)
+    SWITCHOVER_PAUSE_NO_CANDIDATE = ('Switchover is possible only to a specific candidate in a paused state', 400)
+    SWITCHOVER_TO_LEADER = ('Switchover target and source are the same', 400)
+
+    CLUSTER_NO_LEADER = ('Cluster {cluster_name} has no leader', 412)
+    LEADER_NOT_MEMBER = ('Member {leader} is not the leader of cluster {cluster_name}', 412)
+    CANDIDATE_NOT_SYNC_STANDBY = ('candidate name does not match with sync_standby', 412)
+    NO_SYNC_CANDIDATE = ('{action} is not possible: can not find sync_standby', 412)
+    ONLY_LEADER = ('{action} is not possible: cluster does not have members except leader', 412)
+    CANDIDATE_NOT_MEMEBER = ('Member {candidate} does not exist in cluster {cluster_name} or is tagged as nofailover',
+                             412)
+    NO_GOOD_CANDIDATES = ('{action} is not possible: no good candidates have been found', 412)
+
+    CHECK_PASSED = ('', None)
+
+
+class ManualFailover(object):
+
+    def __init__(self, action: str, cluster: 'Cluster',
+                 leader: Optional[str], candidate: Optional[str], scheduled: Optional[str],
+                 paused: bool = False, sync_mode: bool = False, patroni_obj: Optional['Patroni'] = None) -> None:
+        self.action = action
+        self.cluster = cluster
+        self.leader = leader
+        self.candidate = candidate
+        self.scheduled = scheduled
+        self.paused = paused
+        self.sync_mode = sync_mode
+        self.patroni = patroni_obj
+
+    def parse_scheduled(self) -> Tuple[Optional['ParseScheduleErrors'], Optional['datetime.datetime']]:
+        return parse_schedule(self.scheduled)
+
+    def run_precheck(self) -> ManualFailoverPrecheckStatus:
+        if self.action == 'failover' and not self.candidate:
+            return ManualFailoverPrecheckStatus.FAILOVER_NO_CANDIDATE
+        elif self.action == 'switchover' and not self.leader:
+            return ManualFailoverPrecheckStatus.SWITCHOVER_NO_LEADER
+
+        if self.scheduled:
+            if self.action == 'failover':
+                return ManualFailoverPrecheckStatus.SCHEDULED_FAILOVER
+            elif self.paused:
+                return ManualFailoverPrecheckStatus.SCHEDULED_SWITCHOVER_PAUSE
+
+        if self.paused and not self.candidate:
+            return ManualFailoverPrecheckStatus.SWITCHOVER_PAUSE_NO_CANDIDATE
+
+        if self.leader == self.candidate:
+            return ManualFailoverPrecheckStatus.SWITCHOVER_TO_LEADER
+
+        if self.action == 'switchover':
+            if self.cluster.leader is None or not self.cluster.leader.name:
+                return ManualFailoverPrecheckStatus.CLUSTER_NO_LEADER
+            if self.cluster.leader.name != self.leader:
+                return ManualFailoverPrecheckStatus.LEADER_NOT_MEMBER
+
+        if self.candidate:
+            if self.action == 'switchover' and self.sync_mode and not self.cluster.sync.matches(self.candidate):
+                return ManualFailoverPrecheckStatus.CANDIDATE_NOT_SYNC_STANDBY
+            members = [m for m in self.cluster.members if m.name == self.candidate]
+            if not members:
+                return ManualFailoverPrecheckStatus.CANDIDATE_NOT_MEMEBER
+        elif self.sync_mode:
+            members = [m for m in self.cluster.members if self.cluster.sync.matches(m.name)]
+            if not members:
+                return ManualFailoverPrecheckStatus.NO_SYNC_CANDIDATE
+        else:
+            members = [m for m in self.cluster.members if not self.cluster.leader or m.name != self.cluster.leader.name and m.api_url]
+            if not members:
+                return ManualFailoverPrecheckStatus.ONLY_LEADER
+
+        if self.patroni and not self.patroni.ha.has_members_eligible_to_promote(members, fast_path=True):
+            return ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES
+
+        return ManualFailoverPrecheckStatus.CHECK_PASSED

patroni/utils.py

@@ -9,6 +9,8 @@
 :var DBL_RE: regular expression to match double precision numbers, signed or unsigned. Matches scientific notation too.
 :var WHITESPACE_RE: regular expression to match whitespace characters
 """
+import datetime
+import dateutil.parser
 import errno
 import logging
 import os
@@ -20,6 +22,7 @@ import subprocess
 import sys
 import tempfile
 import time
+from enum import Enum
 from shlex import split
 
 from typing import Any, Callable, Dict, Iterator, List, Optional, Union, Tuple, Type, TYPE_CHECKING
@@ -1061,3 +1064,23 @@ def get_major_version(bin_dir: Optional[str] = None, bin_name: str = 'postgres')
     if TYPE_CHECKING:  # pragma: no cover
         assert version is not None
     return '.'.join([version.group(1), version.group(3)]) if int(version.group(1)) < 10 else version.group(1)
+
+
+class ParseScheduleErrors(Enum):
+    NO_TIMEZONE = ('Timezone information is mandatory for the scheduled {action}', 400)
+    SCHEDULED_IN_PAST = ('Cannot schedule {action} in the past', 422)
+    PARSING_ERROR = ('Unable to parse scheduled timestamp. It should be in an unambiguous format, e.g. ISO 8601', 422)
+
+
+def parse_schedule(schedule: Optional[str]) -> Tuple[Optional[ParseScheduleErrors], Optional[datetime.datetime]]:
+    scheduled_at = None
+    if schedule is not None:
+        try:
+            scheduled_at = dateutil.parser.parse(schedule)
+            if scheduled_at.tzinfo is None:
+                return ParseScheduleErrors.NO_TIMEZONE, scheduled_at
+            elif scheduled_at < datetime.datetime.now(tzutc):
+                return ParseScheduleErrors.SCHEDULED_IN_PAST, scheduled_at
+        except (ValueError, TypeError):
+            return ParseScheduleErrors.PARSING_ERROR, scheduled_at
+    return None, scheduled_at

tests/test_api.py

@@ -12,9 +12,10 @@ from patroni.api import RestApiHandler, RestApiServer
 from patroni.config import GlobalConfig
 from patroni.dcs import ClusterConfig, Member
 from patroni.ha import _MemberStatus
-from patroni.utils import RetryFailedError, tzutc
+from patroni.manual_failover import ManualFailoverPrecheckStatus
+from patroni.utils import ParseScheduleErrors, RetryFailedError, tzutc
 
-from .test_ha import get_cluster_initialized_without_leader
+from .test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader
 
 
 future_restart_time = datetime.datetime.now(tzutc) + datetime.timedelta(days=5)
@@ -122,6 +123,9 @@ class MockHa(object):
     def is_paused():
         return True
 
+    def has_members_eligible_to_promote(*args, **kwargs):
+        return True
+
 
 class MockLogger(object):
 
@@ -498,86 +502,185 @@ class TestRestApiHandler(unittest.TestCase):
 
         post = 'POST /switchover HTTP/1.0' + self._authorization + '\nContent-Length: '
 
-        MockRestApiServer(RestApiHandler, post + '7\n\n{"1":2}')
+        # Invalid content
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            MockRestApiServer(RestApiHandler, post + '7\n\n{"1":2}')
+            response_mock.assert_called_with(*ManualFailoverPrecheckStatus.SWITCHOVER_NO_LEADER.value[::-1])
 
+        # Empty content
         request = post + '0\n\n'
         MockRestApiServer(RestApiHandler, request)
 
-        cluster.leader.name = 'postgresql1'
+        # [Switchover without a candidate]
-        MockRestApiServer(RestApiHandler, request)
 
+        cluster.leader.name = 'postgresql1'
         request = post + '25\n\n{"leader": "postgresql1"}'
 
-        with patch.object(GlobalConfig, 'is_paused', PropertyMock(return_value=True)):
+        # No candidate in pause mode
+        with patch.object(RestApiHandler, 'write_response') as response_mock, \
+             patch.object(GlobalConfig, 'is_paused', PropertyMock(return_value=True)):
             MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(*ManualFailoverPrecheckStatus.SWITCHOVER_PAUSE_NO_CANDIDATE.value[::-1])
 
-        for is_synchronous_mode in (True, False):
+        # No healthy nodes to promote in both sync and async mode
-            with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=is_synchronous_mode)):
+        for is_synchronous_mode, response in (
+                (True, ManualFailoverPrecheckStatus.NO_SYNC_CANDIDATE.value[0].format(action='switchover')),
+                (False, ManualFailoverPrecheckStatus.ONLY_LEADER.value[0].format(action='switchover'))):
+            with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=is_synchronous_mode)), \
+                 patch.object(RestApiHandler, 'write_response') as response_mock:
                 MockRestApiServer(RestApiHandler, request)
+                response_mock.assert_called_with(412, response)
 
-        cluster.leader.name = 'postgresql2'
+        # [Switchover to the candidate specified]
-        request = post + '53\n\n{"leader": "postgresql1", "candidate": "postgresql2"}'
-        MockRestApiServer(RestApiHandler, request)
 
+        # Candidate to promote is the same as the leader specified
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            request = post + '53\n\n{"leader": "postgresql2", "candidate": "postgresql2"}'
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(*ManualFailoverPrecheckStatus.SWITCHOVER_TO_LEADER.value[::-1])
+
+        # Current leader is different from the one specified
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            cluster.leader.name = 'postgresql2'
+            request = post + '53\n\n{"leader": "postgresql1", "candidate": "postgresql2"}'
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(
+                ManualFailoverPrecheckStatus.LEADER_NOT_MEMBER.value[1],
+                ManualFailoverPrecheckStatus.LEADER_NOT_MEMBER.value[0].format(leader='postgresql1',
+                                                                               cluster_name='dummy'))
+
+        # Candidate to promote is not a sync standby/a member of the cluster
         cluster.leader.name = 'postgresql1'
         cluster.sync.matches.return_value = False
-        for is_synchronous_mode in (True, False):
+        for is_synchronous_mode, response in (
-            with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=is_synchronous_mode)):
+                (True, ManualFailoverPrecheckStatus.CANDIDATE_NOT_SYNC_STANDBY.value[0]),
+                (False, ManualFailoverPrecheckStatus.CANDIDATE_NOT_MEMEBER.value[0].format(candidate="postgresql2",
+                                                                                           cluster_name='dummy'))):
+            with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=is_synchronous_mode)), \
+                 patch.object(RestApiHandler, 'write_response') as response_mock:
                 MockRestApiServer(RestApiHandler, request)
+                response_mock.assert_called_with(412, response)
 
         cluster.members = [Member(0, 'postgresql0', 30, {'api_url': 'http'}),
                            Member(0, 'postgresql2', 30, {'api_url': 'http'})]
-        MockRestApiServer(RestApiHandler, request)
 
-        cluster.failover = None
+        # Cluster has no leader
-        MockRestApiServer(RestApiHandler, request)
+        cluster.leader.name = None
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            request = post + '53\n\n{"leader": "postgresql1"}'
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(
+                ManualFailoverPrecheckStatus.CLUSTER_NO_LEADER.value[1],
+                ManualFailoverPrecheckStatus.CLUSTER_NO_LEADER.value[0].format(leader='leader', cluster_name='dummy'))
 
-        dcs.get_cluster.side_effect = [cluster]
+        cluster.leader.name = 'postgresql1'
-        MockRestApiServer(RestApiHandler, request)
 
-        cluster2 = cluster.copy()
+        # Failover key is empty in DCS
-        cluster2.leader.name = 'postgresql0'
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
-        cluster2.is_unlocked.return_value = False
+            cluster.failover = None
-        dcs.get_cluster.side_effect = [cluster, cluster2]
+            request = post + '53\n\n{"leader": "postgresql1", "candidate": "postgresql2"}'
-        MockRestApiServer(RestApiHandler, request)
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(503, 'Switchover failed')
 
-        cluster2.leader.name = 'postgresql2'
+        # Result polling failed
-        dcs.get_cluster.side_effect = [cluster, cluster2]
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
-        MockRestApiServer(RestApiHandler, request)
+            dcs.get_cluster.side_effect = [cluster]
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(503, 'Switchover status unknown')
+
+        # Switchover to a node different from the candidate specified
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            cluster2 = cluster.copy()
+            cluster2.leader.name = 'postgresql0'
+            cluster2.is_unlocked.return_value = False
+            dcs.get_cluster.side_effect = [cluster, cluster2]
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(200, 'Switched over to "postgresql0" instead of "postgresql2"')
+
+        # Successful switchover to the candidate
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            cluster2.leader.name = 'postgresql2'
+            dcs.get_cluster.side_effect = [cluster, cluster2]
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(200, 'Successfully switched over to "postgresql2"')
+
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            dcs.manual_failover.return_value = False
+            dcs.get_cluster.side_effect = None
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(503, 'failed to write failover key into DCS')
 
-        dcs.get_cluster.side_effect = None
-        dcs.manual_failover.return_value = False
-        MockRestApiServer(RestApiHandler, request)
         dcs.manual_failover.return_value = True
 
-        with patch.object(MockHa, 'fetch_nodes_statuses', Mock(return_value=[])):
+        # Candidate is not healthy to be promoted
+        with patch.object(MockHa, 'has_members_eligible_to_promote', Mock(return_value=False)), \
+             patch.object(RestApiHandler, 'write_response') as response_mock:
             MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(
+                ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES.value[1],
+                ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES.value[0].format(action='switchover'))
+
+        # [Scheduled switchover]
 
         # Valid future date
-        request = post + '103\n\n{"leader": "postgresql1", "member": "postgresql2",' +\
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
-                         ' "scheduled_at": "6016-02-15T18:13:30.568224+01:00"}'
+            request = post + '103\n\n{"leader": "postgresql1", "member": "postgresql2",' + \
-        MockRestApiServer(RestApiHandler, request)
+                             ' "scheduled_at": "6016-02-15T18:13:30.568224+01:00"}'
-        with patch.object(GlobalConfig, 'is_paused', PropertyMock(return_value=True)), \
-                patch.object(MockPatroni, 'dcs') as d:
-            d.manual_failover.return_value = False
             MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(202, 'Switchover scheduled')
 
-        # Exception: No timezone specified
+        # Scheduled in pause mode
-        request = post + '97\n\n{"leader": "postgresql1", "member": "postgresql2",' +\
+        with patch.object(RestApiHandler, 'write_response') as response_mock, \
-                         ' "scheduled_at": "6016-02-15T18:13:30.568224"}'
+             patch.object(GlobalConfig, 'is_paused', PropertyMock(return_value=True)):
-        MockRestApiServer(RestApiHandler, request)
+            dcs.manual_failover.return_value = False
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(*ManualFailoverPrecheckStatus.SCHEDULED_SWITCHOVER_PAUSE.value[::-1])
+
+        # No timezone specified
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            request = post + '97\n\n{"leader": "postgresql1", "member": "postgresql2",' + \
+                             ' "scheduled_at": "6016-02-15T18:13:30.568224"}'
+            MockRestApiServer(RestApiHandler, request)
+            response_mock.assert_called_with(
+                ParseScheduleErrors.NO_TIMEZONE.value[1],
+                ParseScheduleErrors.NO_TIMEZONE.value[0].format(action='switchover'))
 
-        # Exception: Scheduled in the past
         request = post + '103\n\n{"leader": "postgresql1", "member": "postgresql2", "scheduled_at": "'
-        MockRestApiServer(RestApiHandler, request + '1016-02-15T18:13:30.568224+01:00"}')
+
+        # Scheduled in the past
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            MockRestApiServer(RestApiHandler, request + '1016-02-15T18:13:30.568224+01:00"}')
+            response_mock.assert_called_with(
+                ParseScheduleErrors.SCHEDULED_IN_PAST.value[1],
+                ParseScheduleErrors.SCHEDULED_IN_PAST.value[0].format(action='switchover'))
 
         # Invalid date
-        self.assertIsNotNone(MockRestApiServer(RestApiHandler, request + '2010-02-29T18:13:30.568224+01:00"}'))
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            MockRestApiServer(RestApiHandler, request + '2010-02-29T18:13:30.568224+01:00"}')
+            response_mock.assert_called_with(*ParseScheduleErrors.PARSING_ERROR.value[::-1])
 
-    def test_do_POST_failover(self):
+    @patch.object(MockPatroni, 'dcs')
+    def test_do_POST_failover(self, mock_dcs):
         post = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: '
-        MockRestApiServer(RestApiHandler, post + '14\n\n{"leader":"1"}')
+        cluster = mock_dcs.get_cluster.return_value
-        MockRestApiServer(RestApiHandler, post + '37\n\n{"candidate":"2","scheduled_at": "1"}')
+
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            MockRestApiServer(RestApiHandler, post + '19\n\n{"leader":"leader"}')
+            response_mock.assert_called_once_with(*ManualFailoverPrecheckStatus.FAILOVER_NO_CANDIDATE.value[::-1])
+
+        with patch.object(RestApiHandler, 'write_response') as response_mock:
+            MockRestApiServer(RestApiHandler, post + '37\n\n{"candidate":"2","scheduled_at": "1"}')
+            response_mock.assert_called_once_with(*ManualFailoverPrecheckStatus.SCHEDULED_FAILOVER.value[::-1])
+
+        # Candidate is not healthy to be promoted
+        cluster.members = [Member(0, 'postgresql0', 30, {'api_url': 'http'}),
+                           Member(0, 'postgresql2', 30, {'api_url': 'http'})]
+        with patch.object(MockHa, 'has_members_eligible_to_promote', Mock(return_value=False)), \
+             patch.object(RestApiHandler, 'write_response') as response_mock:
+            MockRestApiServer(RestApiHandler, post + '27\n\n{"candidate":"postgresql2"}')
+            response_mock.assert_called_with(
+                ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES.value[1],
+                ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES.value[0].format(action='failover'))
 
     @patch.object(MockHa, 'is_leader', Mock(return_value=True))
     def test_do_POST_citus(self):

tests/test_ctl.py

@@ -6,12 +6,14 @@ import unittest
 from click.testing import CliRunner
 from datetime import datetime, timedelta
 from mock import patch, Mock, PropertyMock
+from patroni.config import GlobalConfig
 from patroni.ctl import ctl, load_config, output_members, get_dcs, parse_dcs, \
     get_all_members, get_any_member, get_cursor, query_member, PatroniCtlException, apply_config_changes, \
     format_config_for_editing, show_diff, invoke_editor, format_pg_version, CONFIG_FILE_PATH, PatronictlPrettyTable
 from patroni.dcs.etcd import AbstractEtcdClientWithFailover, Cluster, Failover
+from patroni.manual_failover import ManualFailoverPrecheckStatus
 from patroni.psycopg import OperationalError
-from patroni.utils import tzutc
+from patroni.utils import ParseScheduleErrors, tzutc
 from prettytable import PrettyTable, ALL
 from urllib3 import PoolManager
 
@@ -28,6 +30,10 @@ from .test_ha import get_cluster_initialized_without_leader, get_cluster_initial
 class TestCtl(unittest.TestCase):
     TEST_ROLES = ('master', 'primary', 'leader')
 
+    SCHEDULED_TS = '2055-01-01T12:00:00+01:00'
+    SCHEDULED_TS_NO_TZ = '2055-01-01T12:00:00'
+    SCHEDULED_TS_INVALID = '2055-02-30T12:00:00'
+
     @patch('socket.getaddrinfo', socket_getaddrinfo)
     @patch.object(AbstractEtcdClientWithFailover, '_get_machines_list', Mock(return_value=['http://remotehost:2379']))
     def setUp(self):
@@ -96,91 +102,185 @@ class TestCtl(unittest.TestCase):
         mock_get_dcs.return_value = self.e
         mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
         mock_get_dcs.return_value.set_failover_value = Mock()
+
+        # Confirm
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\ny')
-        assert 'leader' in result.output
+        self.assertEqual(result.exit_code, 0)
 
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'],
+        # Abort
-                                    input='leader\nother\n2300-01-01T12:23:00\ny')
-        assert result.exit_code == 0
-
-        with patch('patroni.config.GlobalConfig.is_paused', PropertyMock(return_value=True)):
-            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
-                                              '--force', '--scheduled', '2015-01-01T12:00:00'])
-            assert result.exit_code == 1
-
-        # Aborting switchover, as we answer NO to the confirmation
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\nN')
-        assert result.exit_code == 1
+        self.assertEqual(result.exit_code, 1)
-
-        # Aborting scheduled switchover, as we answer NO to the confirmation
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
-                                    '--scheduled', '2015-01-01T12:00:00+01:00'], input='leader\nother\n\nN')
-        assert result.exit_code == 1
-
-        # Target and source are equal
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nleader\n\ny')
-        assert result.exit_code == 1
-
-        # Reality is not part of this cluster
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nReality\n\ny')
-        assert result.exit_code == 1
 
+        # Without a candidate with --force option
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0', '--force'])
-        assert 'Member' in result.output
+        self.assertEqual(result.exit_code, 0)
-
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
-                                          '--force', '--scheduled', '2015-01-01T12:00:00+01:00'])
-        assert result.exit_code == 0
-
-        # Invalid timestamp
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0', '--force', '--scheduled', 'invalid'])
-        assert result.exit_code != 0
-
-        # Invalid timestamp
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
-                                          '--force', '--scheduled', '2115-02-30T12:00:00+01:00'])
-        assert result.exit_code != 0
-
-        # Specifying wrong leader
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='dummy')
-        assert result.exit_code == 1
-
-        with patch.object(PoolManager, 'request', Mock(side_effect=Exception)):
-            # Non-responding patroni
-            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'],
-                                        input='leader\nother\n2300-01-01T12:23:00\ny')
-            assert 'falling back to DCS' in result.output
-
-        with patch.object(PoolManager, 'request') as mocked:
-            mocked.return_value.status = 500
-            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\ny')
-            assert 'Switchover failed' in result.output
-
-            mocked.return_value.status = 501
-            mocked.return_value.data = b'Server does not support this operation'
-            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\ny')
-            assert 'Switchover failed' in result.output
 
         # No members available
         mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_only_leader
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\ny')
-        assert result.exit_code == 1
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('No candidates found to switchover to', result.output)
 
         # No leader available
         mock_get_dcs.return_value.get_cluster = get_cluster_initialized_without_leader
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\ny')
-        assert result.exit_code == 1
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('This cluster has no leader', result.output)
+
+        # Citus cluster, no group number specified
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--force'], input='\n')
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('For Citus clusters the --group must me specified', result.output)
+
+        # [Scheduled]
+
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+
+        # Scheduled (confirm)
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'],
+                                    input=f'leader\nother\n{self.SCHEDULED_TS}\ny')
+        self.assertEqual(result.exit_code, 0)
+        self.assertIn(f'Are you sure you want to schedule switchover of cluster dummy '
+                      f'at {self.SCHEDULED_TS}, demoting current leader', result.output)
+
+        # Scheduled (abort)
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
+                                    '--scheduled', self.SCHEDULED_TS], input='leader\nother\n\nN')
+        self.assertEqual(result.exit_code, 1)
+
+        # Scheduled with --force option
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
+                                    '--force', '--scheduled', self.SCHEDULED_TS])
+        self.assertEqual(result.exit_code, 0)
+
+        # Scheduled in pause mode
+        with patch('patroni.config.GlobalConfig.is_paused', PropertyMock(return_value=True)):
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
+                                              '--force', '--scheduled', self.SCHEDULED_TS])
+            self.assertEqual(result.exit_code, 1)
+            self.assertIn(ManualFailoverPrecheckStatus.SCHEDULED_SWITCHOVER_PAUSE.value[0], result.output)
+
+        # Invalid timestamp with force
+        result = self.runner.invoke(ctl,['switchover', 'dummy', '--group', '0', '--force', '--scheduled',
+                                         self.SCHEDULED_TS_INVALID])
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('Unable to parse scheduled timestamp', result.output)
+
+        # Invalid timestamp
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
+                                          '--force', '--scheduled', self.SCHEDULED_TS_INVALID])
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('Unable to parse scheduled timestamp', result.output)
+
+        # Invalid timestamp - no timezone
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
+                                          '--force', '--scheduled', self.SCHEDULED_TS_NO_TZ])
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn(ParseScheduleErrors.NO_TIMEZONE.value[0].format(action='switchover'), result.output)
+
+        # [Other erroneous combinations]
+
+        # No candidate in pause mode
+        with patch('patroni.config.GlobalConfig.is_paused', PropertyMock(return_value=True)):
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\n\n\ny')
+            self.assertEqual(result.exit_code, 1)
+            self.assertIn(ManualFailoverPrecheckStatus.SWITCHOVER_PAUSE_NO_CANDIDATE.value[0], result.output)
+
+        # 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(ManualFailoverPrecheckStatus.SWITCHOVER_TO_LEADER.value[0], result.output)
+
+        # Candidate is not a member of the cluster
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nReality\n\ny')
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn(ManualFailoverPrecheckStatus.CANDIDATE_NOT_MEMEBER.value[0].format(candidate='Reality',
+                                                                                         cluster_name='dummy'),
+                      result.output)
+
+        # Specifying wrong leader
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='dummy')
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn(
+            ManualFailoverPrecheckStatus.LEADER_NOT_MEMBER.value[0].format(leader='dummy',
+                                                                           cluster_name='dummy'),
+            result.output)
+
+        mock_get_dcs.return_value.get_cluster = Mock(
+            return_value=get_cluster_initialized_with_leader(sync=('leader', 'other')))
+
+        # Candidate is not a sync standby
+        with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=True)):
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\notherMember\n\ny')
+            self.assertEqual(result.exit_code, 1)
+            self.assertIn(ManualFailoverPrecheckStatus.CANDIDATE_NOT_SYNC_STANDBY.value[0], result.output)
+
+        # No healthy nodes to promote in sync mode
+        mock_get_dcs.return_value.get_cluster = Mock(return_value=get_cluster_initialized_with_leader(sync=('leader')))
+        with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=True)):
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0', '--force'])
+            self.assertEqual(result.exit_code, 1)
+            self.assertIn(ManualFailoverPrecheckStatus.NO_SYNC_CANDIDATE.value[0].format(action='switchover'),
+                          result.output)
+
+        # No healthy nodes to promote in async mode
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_only_leader
+        with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=False)):
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0', '--force'])
+            self.assertEqual(result.exit_code, 1)
+            self.assertIn(ManualFailoverPrecheckStatus.ONLY_LEADER.value[0].format(action='switchover'),
+                          result.output)
+
+        # Cluster has no leader
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_without_leader
+        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0', '--leader', 'leader', '--force'])
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn(
+            ManualFailoverPrecheckStatus.CLUSTER_NO_LEADER.value[0].format(leader='leader', cluster_name='dummy'),
+            result.output)
+
+        # [Errors while sending Patroni REST API request]
+
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+
+        with patch.object(PoolManager, 'request', Mock(side_effect=Exception)):
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'],
+                                        input=f'leader\nother\n{self.SCHEDULED_TS}\ny')
+            self.assertIn('falling back to DCS', result.output)
+
+        with patch.object(PoolManager, 'request') as mock_api_request:
+            mock_api_request.return_value.status = 500
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\ny')
+            self.assertIn('Switchover failed', result.output)
+
+            mock_api_request.return_value.status = 501
+            mock_api_request.return_value.data = b'Server does not support this operation'
+            result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='leader\nother\n\ny')
+            self.assertIn('Switchover failed', result.output)
 
     @patch('patroni.ctl.get_dcs')
     @patch.object(PoolManager, 'request', Mock(return_value=MockResponse()))
+    @patch('patroni.ctl.request_patroni', Mock(return_value=MockResponse()))
     def test_failover(self, mock_get_dcs):
-        mock_get_dcs.return_value = self.e
-        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
         mock_get_dcs.return_value.set_failover_value = Mock()
-        result = self.runner.invoke(ctl, ['failover', 'dummy', '--force'], input='\n')
+
-        assert 'For Citus clusters the --group must me specified' in result.output
+        # No candidate specified
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
         result = self.runner.invoke(ctl, ['failover', 'dummy'], input='0\n')
-        assert 'Failover could be performed only to a specific candidate' in result.output
+        self.assertIn(ManualFailoverPrecheckStatus.FAILOVER_NO_CANDIDATE.value[0], result.output)
+
+        # Failover to an async member in sync mode (confirm)
+        cluster = get_cluster_initialized_with_leader(sync=('leader', 'other'))
+        cluster.members.append(Member(0, 'async', 28, {'api_url': 'http://127.0.0.1:8012/patroni'}))
+        cluster.config.data['synchronous_mode'] = True
+        mock_get_dcs.return_value.get_cluster = Mock(return_value=cluster)
+        result = self.runner.invoke(ctl, ['failover', 'dummy', '--group', '0', '--candidate', 'async'], input='y\ny')
+        self.assertIn('Are you sure you want to failover to the asynchronous node async', result.output)
+
+        # Failover to an async member in sync mode (abort)
+        mock_get_dcs.return_value.get_cluster = Mock(return_value=cluster)
+        result = self.runner.invoke(ctl, ['failover', 'dummy', '--group', '0', '--candidate', 'async'], input='N')
+        self.assertEqual(result.exit_code, 1)
 
     @patch('patroni.dcs.dcs_modules', Mock(return_value=['patroni.dcs.dummy', 'patroni.dcs.etcd']))
     def test_get_dcs(self):
@@ -273,12 +373,9 @@ class TestCtl(unittest.TestCase):
 
     @patch.object(PoolManager, 'request')
     @patch('patroni.ctl.get_dcs')
-    def test_restart_reinit(self, mock_get_dcs, mock_post):
+    def test_reinit(self, mock_get_dcs, mock_post):
         mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
         mock_post.return_value.status = 503
-        result = self.runner.invoke(ctl, ['restart', 'alpha'], input='now\ny\n')
-        assert 'Failed: restart for' in result.output
-        assert result.exit_code == 0
 
         result = self.runner.invoke(ctl, ['reinit', 'alpha'], input='y')
         assert result.exit_code == 1
@@ -287,67 +384,88 @@ class TestCtl(unittest.TestCase):
         result = self.runner.invoke(ctl, ['reinit', 'alpha', 'other'], input='y\ny')
         assert result.exit_code == 0
 
-        # Aborted restart
+    @patch.object(PoolManager, 'request')
+    @patch('patroni.ctl.get_dcs')
+    def test_restart(self, mock_get_dcs, mock_post):
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+        mock_post.return_value.status = 200
+
+        # Successful restart
+        result = self.runner.invoke(ctl, ['restart', 'alpha'], input='now\ny\n')
+        self.assertEqual(result.exit_code, 0)
+
+        # Aborted
         result = self.runner.invoke(ctl, ['restart', 'alpha'], input='now\nN')
-        assert result.exit_code == 1
+        self.assertEqual(result.exit_code, 1)
 
+        # With pending the flag
         result = self.runner.invoke(ctl, ['restart', 'alpha', '--pending', '--force'])
-        assert result.exit_code == 0
+        self.assertEqual(result.exit_code, 0)
-
-        # Aborted scheduled restart
-        result = self.runner.invoke(ctl, ['restart', 'alpha', '--scheduled', '2019-10-01T14:30'], input='N')
-        assert result.exit_code == 1
 
         # Not a member
         result = self.runner.invoke(ctl, ['restart', 'alpha', 'dummy', '--any'], input='now\ny')
-        assert result.exit_code == 1
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('Not a single cluster member among provided members', result.output)
+
+        # Not a member with the specified role
+        result = self.runner.invoke(ctl, ['restart', 'alpha', 'other', '--role', 'primary'], input='now\ny')
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('No primary among provided members', result.output)
 
         # Wrong pg version
         result = self.runner.invoke(ctl, ['restart', 'alpha', '--any', '--pg-version', '9.1'], input='now\ny')
-        assert 'Error: Invalid PostgreSQL version format' in result.output
+        self.assertEqual(result.exit_code, 1)
-        assert result.exit_code == 1
+        self.assertIn('Error: Invalid PostgreSQL version format', result.output)
 
+        # Restart with timeout
         result = self.runner.invoke(ctl, ['restart', 'alpha', '--pending', '--force', '--timeout', '10min'])
-        assert result.exit_code == 0
+        self.assertEqual(result.exit_code, 0)
 
-        # normal restart, the schedule is actually parsed, but not validated in patronictl
+        # Scheduled restart
-        result = self.runner.invoke(ctl, ['restart', 'alpha', 'other', '--force', '--scheduled', '2300-10-01T14:30'])
-        assert 'Failed: flush scheduled restart' in result.output
 
+        # Aborted scheduled restart
+        result = self.runner.invoke(ctl, ['restart', 'alpha', '--scheduled', self.SCHEDULED_TS], input='N')
+        self.assertEqual(result.exit_code, 1)
+
+        # Error parsing scheduled flag value (no tz)
+        result = self.runner.invoke(ctl,
+                                    ['restart', 'alpha', 'other', '--force', '--scheduled', self.SCHEDULED_TS_NO_TZ])
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn(ParseScheduleErrors.NO_TIMEZONE.value[0].format(action='restart'), result.output)
+
+        # Error parsing scheduled flag value (invalid date)
+        result = self.runner.invoke(ctl,
+                                    ['restart', 'alpha', 'other', '--force', '--scheduled', self.SCHEDULED_TS_INVALID])
+        self.assertEqual(result.exit_code, 1)
+        self.assertIn('Unable to parse scheduled timestamp', result.output)
+
+        # Successfully scheduled restart
+        result = self.runner.invoke(ctl, ['restart', 'alpha', '--scheduled', self.SCHEDULED_TS], input='Y')
+        self.assertEqual(result.exit_code, 0)
+        self.assertIn('Success: restart on member other', result.output)
+
+        # Not possible to schedule in pause mode
         with patch('patroni.config.GlobalConfig.is_paused', PropertyMock(return_value=True)):
             result = self.runner.invoke(ctl,
-                                        ['restart', 'alpha', 'other', '--force', '--scheduled', '2300-10-01T14:30'])
+                                        ['restart', 'alpha', 'other', '--force', '--scheduled', self.SCHEDULED_TS])
-            assert result.exit_code == 1
+            self.assertEqual(result.exit_code, 1)
+            self.assertIn("Can't schedule restart in the paused state", result.output)
 
-        # force restart with restart already present
+        # Force restart with restart already scheduled
-        result = self.runner.invoke(ctl, ['restart', 'alpha', 'other', '--force', '--scheduled', '2300-10-01T14:30'])
+        result = self.runner.invoke(ctl, ['restart', 'alpha', 'other', '--force', '--scheduled', self.SCHEDULED_TS])
-        assert result.exit_code == 0
+        self.assertEqual(result.exit_code, 0)
-
-        ctl_args = ['restart', 'alpha', '--pg-version', '99.0', '--scheduled', '2300-10-01T14:30']
-        # normal restart, the schedule is actually parsed, but not validated in patronictl
-        mock_post.return_value.status = 200
-        result = self.runner.invoke(ctl, ctl_args, input='y')
-        assert result.exit_code == 0
 
         # get restart with the non-200 return code
-        # normal restart, the schedule is actually parsed, but not validated in patronictl
+        ctl_args = ['restart', 'alpha', '--pg-version', '99.0', '--scheduled', self.SCHEDULED_TS]
-        mock_post.return_value.status = 204
+        for code, output in [
-        result = self.runner.invoke(ctl, ctl_args, input='y')
+            (204, 'Failed: restart for member other, status code=204'),
-        assert result.exit_code == 0
+            (202, 'Success: restart scheduled'),
-
+            (409, 'Failed: another restart is already')
-        # get restart with the non-200 return code
+        ]:
-        # normal restart, the schedule is actually parsed, but not validated in patronictl
+            mock_post.return_value.status = code
-        mock_post.return_value.status = 202
+            result = self.runner.invoke(ctl, ctl_args, input='y')
-        result = self.runner.invoke(ctl, ctl_args, input='y')
+            self.assertEqual(result.exit_code, 0)
-        assert 'Success: restart scheduled' in result.output
+            self.assertIn(output, result.output)
-        assert result.exit_code == 0
-
-        # get restart with the non-200 return code
-        # normal restart, the schedule is actually parsed, but not validated in patronictl
-        mock_post.return_value.status = 409
-        result = self.runner.invoke(ctl, ctl_args, input='y')
-        assert 'Failed: another restart is already' in result.output
-        assert result.exit_code == 0
 
     @patch('patroni.ctl.get_dcs')
     def test_remove(self, mock_get_dcs):

tests/test_ha.py

@@ -433,6 +433,7 @@ class TestHa(PostgresInit):
 
     def test_promote_without_watchdog(self):
         self.ha.has_lock = true
+        self.p.is_primary = true
         with patch.object(Watchdog, 'activate', Mock(return_value=False)):
             self.assertEqual(self.ha.run_cycle(), 'Demoting self because watchdog could not be activated')
             self.p.is_primary = false
@@ -612,6 +613,7 @@ class TestHa(PostgresInit):
         self.ha.cluster = get_cluster_not_initialized_without_leader()
         self.e.initialize = true
         self.ha.bootstrap()
+        self.p.is_primary = true
         with patch.object(Watchdog, 'activate', Mock(return_value=False)), \
                 patch('patroni.ha.logger.error') as mock_logger:
             self.assertEqual(self.ha.post_bootstrap(), 'running post_bootstrap')
@@ -685,110 +687,289 @@ class TestHa(PostgresInit):
 
     @patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
     def test_manual_failover_from_leader(self):
+        self.ha.has_lock = true  # I am the leader
+
+        # to me
+        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('%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(
+                '%s: no healthy members found, %s is not possible', 'manual failover', 'failover')
+
+        # to an existent candidate
         self.ha.fetch_node_status = get_node_status()
-        self.ha.has_lock = true
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'b', None))
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '', None))
+        self.ha.cluster.members.append(Member(0, 'b', 28, {'api_url': 'http://127.0.0.1:8011/patroni'}))
-        self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
-        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')
-        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')
-        f = Failover(0, self.p.name, '', None)
-        self.ha.cluster = get_cluster_initialized_with_leader(f)
         self.assertEqual(self.ha.run_cycle(), 'manual failover: demoting myself')
+
+        # to a candidate on an older timeline
+        with patch('patroni.ha.logger.info') as mock_info:
+            self.ha.fetch_node_status = get_node_status(timeline=1)
+            self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
+            self.assertEqual(mock_info.call_args_list[0][0],
+                             ('Timeline %s of member %s is behind the cluster timeline %s', 1, 'b', 2))
+
+        # to a lagging candidate
+        with patch('patroni.ha.logger.info') as mock_info:
+            self.ha.fetch_node_status = get_node_status(wal_position=1)
+            self.ha.cluster.config.data.update({'maximum_lag_on_failover': 5})
+            self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
+            self.assertEqual(mock_info.call_args_list[0][0],
+                             ('Member %s exceeds maximum replication lag', 'b'))
+            self.ha.cluster.members.pop()
+
+    @patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
+    def test_manual_switchover_from_leader(self):
+        self.ha.has_lock = true  # I am the leader
+
+        self.ha.fetch_node_status = get_node_status()
+
+        # different leader specified in failover key, no 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(
+                '%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(), 'switchover: demoting myself')
+
         self.ha._rewind.rewind_or_reinitialize_needed_and_possible = true
-        self.assertEqual(self.ha.run_cycle(), 'manual failover: demoting myself')
+        self.assertEqual(self.ha.run_cycle(), 'switchover: demoting myself')
-        self.ha.fetch_node_status = get_node_status(nofailover=True)
-        self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
-        self.ha.fetch_node_status = get_node_status(watchdog_failed=True)
-        self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
-        self.ha.fetch_node_status = get_node_status(timeline=1)
-        self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
-        self.ha.fetch_node_status = get_node_status(wal_position=1)
-        self.ha.cluster.config.data.update({'maximum_lag_on_failover': 5})
-        self.ha.global_config = self.ha.patroni.config.get_global_config(self.ha.cluster)
-        self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
-        # manual failover from the previous leader to us won't happen if we hold the nofailover flag
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, None))
-        self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
 
-        # Failover scheduled time must include timezone
+        # other members with failover_limitation_s
-        scheduled = datetime.datetime.now()
+        with patch('patroni.ha.logger.info') as mock_info:
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
+            self.ha.fetch_node_status = get_node_status(nofailover=True)
-        self.ha.run_cycle()
+            self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
+            self.assertEqual(mock_info.call_args_list[0][0], ('Member %s is %s', 'leader', 'not allowed to promote'))
+        with patch('patroni.ha.logger.info') as mock_info:
+            self.ha.fetch_node_status = get_node_status(watchdog_failed=True)
+            self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
+            self.assertEqual(mock_info.call_args_list[0][0], ('Member %s is %s', 'leader', 'not watchdog capable'))
+        with patch('patroni.ha.logger.info') as mock_info:
+            self.ha.fetch_node_status = get_node_status(timeline=1)
+            self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
+            self.assertEqual(mock_info.call_args_list[0][0],
+                             ('Timeline %s of member %s is behind the cluster timeline %s', 1, 'leader', 2))
+        with patch('patroni.ha.logger.info') as mock_info:
+            self.ha.fetch_node_status = get_node_status(wal_position=1)
+            self.ha.cluster.config.data.update({'maximum_lag_on_failover': 5})
+            self.ha.global_config = self.ha.patroni.config.get_global_config(self.ha.cluster)
+            self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
+            self.assertEqual(mock_info.call_args_list[0][0], ('Member %s exceeds maximum replication lag', 'leader'))
 
+    @patch('patroni.postgresql.citus.CitusHandler.is_coordinator', Mock(return_value=False))
+    def test_scheduled_switchover_from_leader(self):
+        self.ha.has_lock = true  # I am the leader
+
+        self.ha.fetch_node_status = get_node_status()
+
+        # switchover scheduled time must include timezone
+        with patch('patroni.ha.logger.warning') as mock_warning:
+            scheduled = datetime.datetime.now()
+            self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'blabla', scheduled))
+            self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
+            self.assertIn('Incorrect value of scheduled_at: %s', mock_warning.call_args_list[0][0])
+
+        # scheduled now
         scheduled = datetime.datetime.utcnow().replace(tzinfo=tzutc)
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'b', scheduled))
-        self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+        self.ha.cluster.members.append(Member(0, 'b', 28, {'api_url': 'http://127.0.0.1:8011/patroni'}))
+        self.assertEqual('switchover: demoting myself', self.ha.run_cycle())
 
-        scheduled = scheduled + datetime.timedelta(seconds=30)
+        # scheduled in the future
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
+        with patch('patroni.ha.logger.info') as mock_info:
-        self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+            scheduled = scheduled + datetime.timedelta(seconds=30)
+            self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'blabla', scheduled))
+            self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+            self.assertIn('Awaiting %s at %s (in %.0f seconds)', mock_info.call_args_list[0][0])
 
-        scheduled = scheduled + datetime.timedelta(seconds=-600)
+        # stale value
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
+        with patch('patroni.ha.logger.warning') as mock_warning:
-        self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+            scheduled = scheduled + datetime.timedelta(seconds=-600)
+            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('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+            self.assertIn('Found a stale %s value, cleaning up: %s', mock_warning.call_args_list[0][0])
 
-        scheduled = None
+    def test_manual_switchover_from_leader_in_pause(self):
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
+        self.ha.has_lock = true  # I am the leader
-        self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+        self.ha.is_paused = true
+
+        # no candidate
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, '', None))
+        with patch('patroni.ha.logger.warning') as mock_warning:
+            self.assertEqual('PAUSE: no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+            mock_warning.assert_called_with(
+                '%s is possible only to a specific candidate in a paused state', 'Switchover')
 
     def test_manual_failover_from_leader_in_pause(self):
         self.ha.has_lock = true
+        self.ha.fetch_node_status = get_node_status()
         self.ha.is_paused = true
-        scheduled = datetime.datetime.now()
+
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
+        # failover from me, candidate is healthy
-        self.assertEqual('PAUSE: no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, None, 'b', None))
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, '', None))
+        self.ha.cluster.members.append(Member(0, 'b', 28, {'api_url': 'http://127.0.0.1:8011/patroni'}))
-        self.assertEqual('PAUSE: no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+        self.assertEqual('PAUSE: manual failover: demoting myself', self.ha.run_cycle())
+        self.ha.cluster.members.pop()
 
     def test_manual_failover_from_leader_in_synchronous_mode(self):
-        self.ha.has_lock = true
         self.ha.is_synchronous_mode = true
         self.ha.process_sync_replication = Mock()
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'a', None), (self.p.name, None))
+        self.ha.fetch_node_status = get_node_status()
-        self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'a', None), (self.p.name, 'a'))
+        # I am the leader
-        self.ha.is_failover_possible = true
+        self.p.is_primary = true
+        self.ha.has_lock = true
+
+        # the candidate is not in sync members but we allow failover to an async candidate
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, None, 'b', None), sync=(self.p.name, 'a'))
+        self.ha.cluster.members.append(Member(0, 'b', 28, {'api_url': 'http://127.0.0.1:8011/patroni'}))
         self.assertEqual('manual failover: demoting myself', self.ha.run_cycle())
+        self.ha.cluster.members.pop()
+
+    def test_manual_switchover_from_leader_in_synchronous_mode(self):
+        self.ha.is_synchronous_mode = true
+        self.ha.process_sync_replication = Mock()
+
+        # I am the leader
+        self.p.is_primary = true
+        self.ha.has_lock = true
+
+        # candidate specified is not in sync members
+        with patch('patroni.ha.logger.warning') as mock_warning:
+            self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, self.p.name, 'a', None),
+                                                                  sync=(self.p.name, 'blabla'))
+            self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+            self.assertEqual(mock_warning.call_args_list[0][0],
+                             ('%s candidate=%s does not match with sync_standbys=%s', 'Switchover', 'a', 'blabla'))
+
+        # the candidate is in sync members and is healthy
+        self.ha.fetch_node_status = get_node_status(wal_position=305419896)
+        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('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:
+            self.ha.fetch_node_status = get_node_status(nofailover=true)
+            self.assertEqual('no action. I am (postgresql0), the leader with the lock', self.ha.run_cycle())
+            self.assertEqual(mock_info.call_args_list[0][0], ('Member %s is %s', 'a', 'not allowed to promote'))
 
     def test_manual_failover_process_no_leader(self):
         self.p.is_primary = false
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', self.p.name, None))
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader', None))
         self.p.set_role('replica')
-        self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
+
-        self.ha.fetch_node_status = get_node_status()  # accessible, in_recovery
+        # failover to another member, fetch_node_status for candidate fails
-        self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
+        with patch('patroni.ha.logger.warning') as mock_warning:
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, self.p.name, '', None))
+            self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader', None))
-        self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
+            self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
-        self.ha.fetch_node_status = get_node_status(reachable=False)  # inaccessible, in_recovery
+            self.assertEqual(mock_warning.call_args_list[1][0],
+                             ('%s: member %s is %s', 'manual failover', 'leader', 'not reachable'))
+
+        # failover to another member, candidate is accessible, in_recovery
         self.p.set_role('replica')
-        self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
+        self.ha.fetch_node_status = get_node_status()
-        # set failover flag to True for all members of the cluster
+        self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
+
+        # set nofailover flag to True for all members of the cluster
         # this should elect the current member, as we are not going to call the API for it.
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None))
-        self.ha.fetch_node_status = get_node_status(nofailover=True)  # accessible, in_recovery
+        self.ha.fetch_node_status = get_node_status(nofailover=True)
-        self.p.set_role('replica')
         self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
-        # same as previous, but set the current member to nofailover. In no case it should be elected as a leader
+
+        # failover to me but I am set to nofailover. In no case I should be elected as a leader
+        self.p.set_role('replica')
+        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'postgresql0', None))
         self.ha.patroni.nofailover = True
         self.assertEqual(self.ha.run_cycle(), 'following a different leader because I am not allowed to promote')
 
+        self.ha.patroni.nofailover = False
+
+        # failover to another member that is on an older timeline (only failover_limitation() is checked)
+        with patch('patroni.ha.logger.info') as mock_info:
+            self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'b', None))
+            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('%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('%s: to %s, i am %s', 'manual failover', 'b', 'postgresql0')
+
+    def test_manual_switchover_process_no_leader(self):
+        self.p.is_primary = false
+        self.p.set_role('replica')
+
+        # I was the leader, other members are healthy
+        self.ha.fetch_node_status = get_node_status()
+        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, self.p.name, '', None))
+        self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
+
+        # I was the leader, I am the only healthy member
+        with patch('patroni.ha.logger.info') as mock_info:
+            self.ha.fetch_node_status = get_node_status(reachable=False)  # inaccessible, in_recovery
+            self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
+            self.assertEqual(mock_info.call_args_list[0][0], ('Member %s is %s', 'leader', 'not reachable'))
+            self.assertEqual(mock_info.call_args_list[1][0], ('Member %s is %s', 'other', 'not reachable'))
+
     def test_manual_failover_process_no_leader_in_synchronous_mode(self):
         self.ha.is_synchronous_mode = true
         self.p.is_primary = false
+        self.ha.fetch_node_status = get_node_status(nofailover=True)  # other nodes are not healthy
 
-        # switchover to a specific node, which name doesn't match our name (postgresql0)
+        # manual failover when our name (postgresql0) isn't in the /sync key and the candidate node is not available
+        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None),
+                                                                 sync=('leader1', 'blabla'))
+        self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
+
+        # manual failover when the candidate node isn't available but our name is in the /sync key
+        # while other sync node is nofailover
+        with patch('patroni.ha.logger.warning') as mock_warning:
+            self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None),
+                                                                     sync=('leader1', 'postgresql0'))
+            self.p.sync_handler.current_state = Mock(return_value=(CaseInsensitiveSet(), CaseInsensitiveSet()))
+            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],
+                             ('%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)
+        self.p.set_role('replica')
+        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'postgresql0', None),
+                                                                 sync=('leader1', 'other'))
+        self.p.sync_handler.current_state = Mock(return_value=(CaseInsensitiveSet(['leader1']),
+                                                               CaseInsensitiveSet(['leader1'])))
+        self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
+
+    def test_manual_switchover_process_no_leader_in_synchronous_mode(self):
+        self.ha.is_synchronous_mode = true
+        self.p.is_primary = false
+
+        # to a specific node, which name doesn't match our name (postgresql0)
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'other', None))
         self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
 
-        # switchover to our node (postgresql0), which name is not in sync nodes list
+        # to our node (postgresql0), which name is not in sync nodes list
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'postgresql0', None),
                                                                  sync=('leader1', 'blabla'))
         self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
 
-        # switchover from a specific leader, but our name (postgresql0) is not in the sync nodes list
+        # without candidate, our name (postgresql0) is not in the sync nodes list
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', '', None),
                                                                  sync=('leader', 'blabla'))
         self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
@@ -798,45 +979,31 @@ class TestHa(PostgresInit):
                                                                  sync=('postgresql0'))
         self.ha.patroni.nofailover = True
         self.assertEqual(self.ha.run_cycle(), 'following a different leader because I am not allowed to promote')
-        self.ha.patroni.nofailover = False
-
-        # manual failover when our name (postgresql0) isn't in the /sync key and the `other` node is not available
-        self.ha.fetch_node_status = get_node_status(nofailover=True)  # accessible, in_recovery
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None),
-                                                                 sync=('leader1', 'blabla'))
-        self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
-
-        # manual failover when the `other` node isn't available but our name is in the /sync key
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None),
-                                                                 sync=('leader1', 'postgresql0'))
-        self.p.sync_handler.current_state = Mock(return_value=(CaseInsensitiveSet(), CaseInsensitiveSet()))
-        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')
-
-        # manual failover to our node (postgresql0),
-        # which name is not in sync nodes list (the leader and all sync nodes are not available)
-        self.p.set_role('replica')
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'postgresql0', None),
-                                                                 sync=('leader1', 'other'))
-        self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
-
-        # manual failover to our node (postgresql0),
-        # which name is not in sync nodes list (some sync nodes are available)
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'postgresql0', None),
-                                                                 sync=('leader1', 'other'))
-        self.p.set_role('replica')
-        self.p.sync_handler.current_state = Mock(return_value=(CaseInsensitiveSet(['leader1']),
-                                                               CaseInsensitiveSet(['leader1'])))
-        self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
 
     def test_manual_failover_process_no_leader_in_pause(self):
         self.ha.is_paused = true
+
+        # I am running as primary, cluster is unlocked, the candidate is allowed to promote
+        # but we are in pause
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None))
         self.assertEqual(self.ha.run_cycle(), 'PAUSE: continue to run as primary without lock')
+
+    def test_manual_switchover_process_no_leader_in_pause(self):
+        self.ha.is_paused = true
+
+        # I am running as primary, cluster is unlocked, no candidate specified
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', '', None))
         self.assertEqual(self.ha.run_cycle(), 'PAUSE: continue to run as primary without lock')
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'blabla', None))
+
-        self.assertEqual('PAUSE: acquired session lock as a leader', self.ha.run_cycle())
+        # the candidate is not running
+        with patch('patroni.ha.logger.warning') as mock_warning:
+            self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'blabla', None))
+            self.assertEqual('PAUSE: acquired session lock as a leader', self.ha.run_cycle())
+            self.assertEqual(
+                mock_warning.call_args_list[0][0],
+                ('%s: removing failover key because failover candidate is not running', 'switchover'))
+
+        # switchover to me, I am not leader
         self.p.is_primary = false
         self.p.set_role('replica')
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', self.p.name, None))
@@ -844,7 +1011,7 @@ class TestHa(PostgresInit):
 
     def test_is_healthiest_node(self):
         self.ha.is_failsafe_mode = true
-        self.p.is_primary = false
+        self.ha.state_handler.is_primary = false
         self.ha.patroni.nofailover = False
         self.ha.fetch_node_status = get_node_status()
         self.ha.dcs._last_failsafe = {'foo': ''}
@@ -1086,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 failover: 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):
@@ -1459,3 +1626,13 @@ class TestHa(PostgresInit):
             self.assertEqual(self.ha.patroni.request.call_args[1]['timeout'], 2)
             mock_logger.assert_called()
             self.assertTrue(mock_logger.call_args[0][0].startswith('Request to Citus coordinator'))
+
+    def test_has_members_eligible_to_promote(self):
+        self.ha.fetch_node_status = get_node_status()
+        members = [
+            Member(0, 'test', 1, {'api_url': 'http://127.0.0.1:8011/patroni', 'conn_url': 'postgres://127.0.0.1:5432/postgres'}),
+            Member(0, 'test2', 1, {'api_url': 'http://127.0.0.1:8011/patroni', 'conn_url': 'postgres://127.0.0.1:5432/postgres'}),
+        ]
+        with patch('patroni.ha.logger.info') as mock_logger:
+            self.assertTrue(self.ha.has_members_eligible_to_promote(members, fast_path=True))
+            mock_logger.assert_not_called()