From c4032f4ce86e459f2f239eef21621eb11e3dbded Mon Sep 17 00:00:00 2001 From: Polina Bungina Date: Tue, 22 Aug 2023 10:29:47 +0200 Subject: [PATCH] Test --- patroni/api.py | 120 ++++++---------------------------------------- patroni/ctl.py | 117 ++++++++++++++++---------------------------- patroni/utils.py | 98 ++++++++++++++++++++++++++++++++++++- tests/test_api.py | 13 ++--- tests/test_ctl.py | 37 +++++++++----- 5 files changed, 183 insertions(+), 202 deletions(-) diff --git a/patroni/api.py b/patroni/api.py index d526b460..360247a1 100644 --- a/patroni/api.py +++ b/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. diff --git a/patroni/ctl.py b/patroni/ctl.py index aa567bcd..0962b17e 100644 --- a/patroni/ctl.py +++ b/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) diff --git a/patroni/utils.py b/patroni/utils.py index be468d2e..de7a02c6 100644 --- a/patroni/utils.py +++ b/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 diff --git a/tests/test_api.py b/tests/test_api.py index 264286db..5025dd61 100644 --- a/tests/test_api.py +++ b/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): diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 2a74a2f7..c2c452e0 100644 --- a/tests/test_ctl.py +++ b/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')