Test

patroni/api.py

@@ -28,11 +28,10 @@ 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 .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, split_host_port, tzutc, uri, cluster_as_json, parse_schedule, manual_failover_precheck
 
 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")
-                if _:
-                    status_code = _
+                parse_result, request[k] = parse_schedule(request[k])
+                if parse_result:
+                    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,33 +1015,18 @@ 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:
-            data = 'Failover could be performed only to a specific candidate'
-        elif action == 'switchover' and not leader:
-            data = 'Switchover could be performed only from a specific leader'
-
-        if not data and scheduled_at:
-            if action == 'failover':
-                data = "Failover can't be scheduled"
-            elif global_config.is_paused:
-                data = "Can't schedule switchover in the paused state"
-            else:
-                (status_code, data, scheduled_at) = self.parse_schedule(scheduled_at, action)
-
-        if not data and global_config.is_paused and not candidate:
-            data = 'Switchover is possible only to a specific candidate in a paused state'
-
         if action == 'failover' and leader:
             logger.warning('received failover request with leader specifed - performing switchover')
             action = 'switchover'
 
-        if not data and leader == candidate:
-            data = 'Switchover target and source are the same'
+        data, status_code = manual_failover_precheck(action, cluster, leader, candidate, bool(scheduled_at),
+                                                     global_config.is_paused, global_config.is_synchronous_mode,
+                                                     self.server.patroni).value
 
-        if not data and not scheduled_at:
-            data = self.is_failover_possible(cluster, leader, candidate, action)
-            if data:
-                status_code = 412
+        if not data and scheduled_at:
+            parse_result, scheduled_at = parse_schedule(scheduled_at)
+            if parse_result:
+                data, status_code = parse_result.value[0], parse_result.value[1]
 
         if not data:
             if self.server.patroni.dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at):
@@ -1128,12 +1040,10 @@ class RestApiHandler(BaseHTTPRequestHandler):
             else:
                 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
-        # ``write_response`` calls.
-        if TYPE_CHECKING:  # pragma: no cover
-            assert isinstance(status_code, int)
-        self.write_response(status_code, data)
+
+        status_code = status_code or 400
+        self.write_response(status_code, data.format(action=action, leader=leader, candidate=candidate,
+                                                     cluster_name=self.server.patroni.postgresql.scope))
 
     def do_POST_switchover(self) -> None:
         """Handle a ``POST`` request to ``/switchover`` path.

patroni/ctl.py

@@ -50,7 +50,7 @@ from .config import Config, get_global_config
 from .dcs import get_dcs as _get_dcs, AbstractDCS, Cluster, Member
 from .exceptions import PatroniException
 from .postgresql.misc import postgres_version_to_int
-from .utils import cluster_as_json, patch_config, polling_loop
+from .utils import cluster_as_json, manual_failover_precheck, parse_schedule, patch_config, polling_loop
 from .request import PatroniRequest
 from .version import __version__
 
@@ -943,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)
@@ -1067,7 +1030,9 @@ def restart(obj: Dict[str, Any], cluster_name: str, group: Optional[int], member
         scheduled = click.prompt('When should the restart take place (e.g. ' + next_hour + ') ',
                                  type=str, default='now')
 
-    scheduled_at = parse_scheduled(scheduled)
+    parse_result, scheduled_at = parse_schedule(scheduled if scheduled != 'now' else None)
+    if parse_result:
+        raise PatroniCtlException(parse_result.value[0].format(action='restart'))
     confirm_members_action(members, force, 'restart', scheduled_at)
 
     if p_any:
@@ -1210,6 +1175,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')
@@ -1220,7 +1188,7 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
 
     global_config = get_global_config(cluster)
 
-    # leader has to be be defined for switchover only
+    # Leader is required for switchover only
     if action == 'switchover':
         if cluster.leader is None or not cluster.leader.name:
             raise PatroniCtlException('This cluster has no leader')
@@ -1232,60 +1200,46 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
                 prompt = 'Standby Leader' if global_config.is_standby_cluster else 'Primary'
                 leader = click.prompt(prompt, type=str, default=(cluster.leader and cluster.leader.name))
 
-        if cluster.leader.name != leader:
-            raise PatroniCtlException(f'Member {leader} is not the leader of cluster {cluster_name}')
-
-    # excluding members with nofailover tag
+    # 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]
-    # 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))
 
+    candidate_names.sort()   # we sort the names for consistent output to the client
     if candidate is None and not force:
         candidate = click.prompt('Candidate ' + str(candidate_names), type=str, default='')
 
-    if action == 'failover' and not candidate:
-        raise PatroniCtlException('Failover could be performed only to a specific candidate')
-
-    if candidate == leader:
-        raise PatroniCtlException(action.title() + ' target and source are the same.')
-
-    if candidate and candidate not in candidate_names:
-        raise PatroniCtlException(
-            f'Member {candidate} does not exist in cluster {cluster_name} or is tagged as nofailover')
-
+    # We allow manual failover to an aync node in the sync mode, so we better ask for the confirmation
     if not force and action == 'failover':
-        if global_config.is_synchronous_mode and not cluster.sync.is_empty\
-           and not cluster.sync.matches(candidate, True)\
+        if global_config.is_synchronous_mode and not cluster.sync.is_empty \
+           and not cluster.sync.matches(candidate, True) \
            and not click.confirm(f'Are you sure you want to failover to the asynchronous node {candidate}'):
             raise PatroniCtlException('Aborting ' + action)
 
+    if action == 'switchover' and scheduled is None and not force:
+        next_hour = (datetime.datetime.now() + datetime.timedelta(hours=1)).strftime('%Y-%m-%dT%H:%M')
+        scheduled = click.prompt('When should the switchover take place (e.g. ' + next_hour + ' ) ',
+                                 type=str, default='now')
+
+    # Now, when we collected all the possible info, run checks
+
+    result_text, _ = manual_failover_precheck(action, cluster, leader, candidate, bool(scheduled),
+                                              global_config.is_paused, global_config.is_synchronous_mode).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:
-            next_hour = (datetime.datetime.now() + datetime.timedelta(hours=1)).strftime('%Y-%m-%dT%H:%M')
-            scheduled = click.prompt('When should the switchover take place (e.g. ' + next_hour + ' ) ',
-                                     type=str, default='now')
-
-        scheduled_at = parse_scheduled(scheduled)
+        parse_result, scheduled_at = parse_schedule(scheduled if scheduled != 'now' else None)
+        if parse_result:
+            raise PatroniCtlException(parse_result.value[0].format(action=action))
         if scheduled_at:
-            if global_config.is_paused:
-                raise PatroniCtlException("Can't schedule switchover in the paused state")
             scheduled_at_str = scheduled_at.isoformat()
 
-    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)
-
-    # By now we have established that the leader exists and the candidate exists
+    # By now we have established that the leader exists and the candidate exists,
+    # so confirm the action that is about to be run
     if not force:
         demote_msg = f', demoting current leader {cluster.leader.name}' if cluster.leader else ''
         if scheduled_at_str:
@@ -1297,6 +1251,15 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s
             if not click.confirm(f'Are you sure you want to {action} cluster {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)

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
@@ -32,8 +35,9 @@ from .exceptions import PatroniException
 from .version import __version__
 
 if TYPE_CHECKING:  # pragma: no cover
-    from .dcs import Cluster
+    from .dcs import Cluster, Member
     from .config import GlobalConfig
+    from .ha import Patroni
 
 tzutc = tz.tzutc()
 
@@ -1061,3 +1065,95 @@ 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)
+
+
+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)
+
+
+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
+
+
+def manual_failover_precheck(action: str, cluster: 'Cluster',
+                             leader: Optional[str], candidate: Optional[str], scheduled: bool = False,
+                             paused: bool = False, sync_mode: bool = False,
+                             patroni_obj: Optional['Patroni'] = None) -> ManualFailoverPrecheckStatus:
+    if action == 'failover' and not candidate:
+        return ManualFailoverPrecheckStatus.FAILOVER_NO_CANDIDATE
+    elif action == 'switchover' and not leader:
+        return ManualFailoverPrecheckStatus.SWITCHOVER_NO_LEADER
+
+    if scheduled:
+        if action == 'failover':
+            return ManualFailoverPrecheckStatus.SCHEDULED_FAILOVER
+        elif paused:
+            return ManualFailoverPrecheckStatus.SCHEDULED_SWITCHOVER_PAUSE
+
+    if paused and not candidate:
+        return ManualFailoverPrecheckStatus.SWITCHOVER_PAUSE_NO_CANDIDATE
+
+    if leader == candidate:
+        return ManualFailoverPrecheckStatus.SWITCHOVER_TO_LEADER
+
+    if action == 'switchover':
+        if cluster.leader is None or not cluster.leader.name:
+            return ManualFailoverPrecheckStatus.CLUSTER_NO_LEADER
+        if cluster.leader.name != leader:
+            return ManualFailoverPrecheckStatus.LEADER_NOT_MEMBER
+
+    if candidate:
+        if action == 'switchover' and sync_mode and not cluster.sync.matches(candidate):
+            return ManualFailoverPrecheckStatus.CANDIDATE_NOT_SYNC_STANDBY
+        members = [m for m in cluster.members if m.name == candidate]
+        if not members:
+            return ManualFailoverPrecheckStatus.CANDIDATE_NOT_MEMEBER
+    elif sync_mode:
+        members = [m for m in cluster.members if cluster.sync.matches(m.name)]
+        if not members:
+            return ManualFailoverPrecheckStatus.NO_SYNC_CANDIDATE
+    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 ManualFailoverPrecheckStatus.ONLY_LEADER
+
+    if patroni_obj:
+        for st in patroni_obj.ha.fetch_nodes_statuses(members):
+            if st.failover_limitation() is None:
+                break
+        else:
+            return ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES
+
+    return ManualFailoverPrecheckStatus.CHECK_PASSED

tests/test_api.py

@@ -546,13 +546,14 @@ class TestRestApiHandler(unittest.TestCase):
             cluster.leader.name = 'postgresql2'
             request = post + '53\n\n{"leader": "postgresql1", "candidate": "postgresql2"}'
             MockRestApiServer(RestApiHandler, request)
-            response_mock.assert_called_with(412, 'leader name does not match')
+            response_mock.assert_called_with(412, 'Member postgresql1 is not the leader of cluster dummy')
 
         # Candidate to promote is not a member of the cluster
         cluster.leader.name = 'postgresql1'
         cluster.sync.matches.return_value = False
         for is_synchronous_mode, response in (
-                (True, 'candidate name does not match with sync_standby'), (False, 'candidate does not exists')):
+                (True, 'candidate name does not match with sync_standby'),
+                (False, 'Member postgresql2 does not exist in cluster dummy or is tagged as nofailover')):
             with patch.object(GlobalConfig, 'is_synchronous_mode', PropertyMock(return_value=is_synchronous_mode)), \
                  patch.object(RestApiHandler, 'write_response') as response_mock:
                 MockRestApiServer(RestApiHandler, request)
@@ -642,9 +643,9 @@ class TestRestApiHandler(unittest.TestCase):
     def test_do_POST_failover(self):
         post = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: '
 
-        with patch.object(RestApiHandler, 'write_response') as response_mock:
-            MockRestApiServer(RestApiHandler, post + '14\n\n{"leader":"1"}')
-            response_mock.assert_called_once_with(400, 'Failover could be performed only to a specific candidate')
+        # with patch.object(RestApiHandler, 'write_response') as response_mock:
+            # MockRestApiServer(RestApiHandler, post + '14\n\n{"leader":"1"}')
+            # response_mock.assert_called_once_with(400, 'Failover could be performed only to a specific candidate')
 
         with patch.object(RestApiHandler, 'write_response') as response_mock:
             MockRestApiServer(RestApiHandler, post + '37\n\n{"candidate":"2","scheduled_at": "1"}')
@@ -652,7 +653,7 @@ class TestRestApiHandler(unittest.TestCase):
 
         with patch.object(RestApiHandler, 'write_response') as response_mock:
             MockRestApiServer(RestApiHandler, post + '30\n\n{"leader":"1","candidate":"2"}')
-            response_mock.assert_called_once_with(412, 'leader name does not match')
+            response_mock.assert_called_once_with(412, 'Member 1 is not the leader of cluster dummy')
 
     @patch.object(MockHa, 'is_leader', Mock(return_value=True))
     def test_do_POST_citus(self):

tests/test_ctl.py

@@ -28,6 +28,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):
@@ -111,23 +115,23 @@ class TestCtl(unittest.TestCase):
 
         # Scheduled (confirm)
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'],
-                                    input='leader\nother\n2300-01-01T12:23:00\ny')
+                                    input=f'leader\nother\n{self.SCHEDULED_TS}\ny')
         self.assertEqual(result.exit_code, 0)
 
         # Scheduled (abort)
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0',
-                                    '--scheduled', '2015-01-01T12:00:00+01:00'], input='leader\nother\n\nN')
+                                    '--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', '2015-01-01T12:00:00+01:00'])
+                                    '--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', '2015-01-01T12:00:00'])
+                                              '--force', '--scheduled', self.SCHEDULED_TS])
             self.assertEqual(result.exit_code, 1)
             self.assertIn("Can't schedule switchover in the paused state", result.output)
 
@@ -141,17 +145,24 @@ class TestCtl(unittest.TestCase):
         self.assertEqual(result.exit_code, 1)
         self.assertIn('Member Reality does not exist in cluster dummy or is tagged as nofailover', result.output)
 
-        # Invalid timestamp
-        result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0', '--force', '--scheduled', 'invalid'])
+        # 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', '2115-02-30T12:00:00+01:00'])
+                                          '--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('Timezone information is mandatory for the scheduled switchover', result.output)
+
         # Specifying wrong leader
         result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'], input='dummy')
         self.assertEqual(result.exit_code, 1)
@@ -160,7 +171,7 @@ class TestCtl(unittest.TestCase):
         # Errors while sending Patroni REST API request
         with patch.object(PoolManager, 'request', Mock(side_effect=Exception)):
             result = self.runner.invoke(ctl, ['switchover', 'dummy', '--group', '0'],
-                                        input='leader\nother\n2300-01-01T12:23:00\ny')
+                                        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:
@@ -327,7 +338,7 @@ class TestCtl(unittest.TestCase):
         assert result.exit_code == 0
 
         # Aborted scheduled restart
-        result = self.runner.invoke(ctl, ['restart', 'alpha', '--scheduled', '2019-10-01T14:30'], input='N')
+        result = self.runner.invoke(ctl, ['restart', 'alpha', '--scheduled', self.SCHEDULED_TS], input='N')
         assert result.exit_code == 1
 
         # Not a member
@@ -343,19 +354,19 @@ class TestCtl(unittest.TestCase):
         assert result.exit_code == 0
 
         # normal restart, the schedule is actually parsed, but not validated in patronictl
-        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 'Failed: flush scheduled restart' in result.output
 
         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
 
         # force restart with restart already present
-        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
 
-        ctl_args = ['restart', 'alpha', '--pg-version', '99.0', '--scheduled', '2300-10-01T14:30']
+        ctl_args = ['restart', 'alpha', '--pg-version', '99.0', '--scheduled', self.SCHEDULED_TS]
         # 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')