From 18786464a14d9bbab7f4006b524ecea18deeeb78 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 5 Jan 2018 15:17:56 +0100 Subject: [PATCH] Rename failover to switchover and make new failover work without leader (#588) In addition to that implement /switchover endpoint as an alias to /failover endpoint and implement more checks like: * candidate must be provided for a failover * switchover can't be scheduled in a pause state * and so on Fixes https://github.com/zalando/patroni/issues/585 Fixes https://github.com/zalando/patroni/issues/520 --- features/patroni_api.feature | 24 ++++----- features/steps/patroni_api.py | 6 +-- patroni/api.py | 77 ++++++++++++++------------- patroni/ctl.py | 98 +++++++++++++++++++++-------------- tests/test_api.py | 19 +++++-- tests/test_ctl.py | 45 +++++++++------- 6 files changed, 158 insertions(+), 111 deletions(-) diff --git a/features/patroni_api.feature b/features/patroni_api.feature index 8d70bb22..05eedaf0 100644 --- a/features/patroni_api.feature +++ b/features/patroni_api.feature @@ -13,17 +13,17 @@ Scenario: check API requests on a stand-alone server When I run patronictl.py reinit batman postgres0 --force Then I receive a response returncode 0 And I receive a response output "Failed: reinitialize for member postgres0, status code=503, (I am the leader, can not reinitialize)" - When I run patronictl.py failover batman --master postgres0 --force + When I run patronictl.py switchover batman --master postgres0 --force Then I receive a response returncode 1 - And I receive a response output "Error: No candidates found to failover to" - When I issue a POST request to http://127.0.0.1:8008/failover with {"leader": "postgres0"} - Then I receive a response code 500 - And I receive a response text failover is not possible: cluster does not have members except leader + And I receive a response output "Error: No candidates found to switchover to" + When I issue a POST request to http://127.0.0.1:8008/switchover with {"leader": "postgres0"} + Then I receive a response code 412 + And I receive a response text switchover is not possible: cluster does not have members except leader When I issue an empty POST request to http://127.0.0.1:8008/failover Then I receive a response code 400 When I issue a POST request to http://127.0.0.1:8008/failover with {"foo": "bar"} Then I receive a response code 400 - And I receive a response text "No values given for required parameters leader and candidate" + And I receive a response text "Failover could be performed only to a specific candidate" Scenario: check local configuration reload Given I issue an empty POST request to http://127.0.0.1:8008/reload @@ -64,21 +64,21 @@ Scenario: check API requests for the primary-replica pair in the pause mode When I sleep for 10 seconds Then postgres1 role is the secondary after 15 seconds -Scenario: check the failover via the API in the pause mode - Given I issue a POST request to http://127.0.0.1:8008/failover with {"leader": "postgres0", "candidate": "postgres1"} +Scenario: check the switchover via the API in the pause mode + Given I issue a POST request to http://127.0.0.1:8008/switchover with {"leader": "postgres0", "candidate": "postgres1"} Then I receive a response code 200 And postgres1 is a leader after 5 seconds And postgres1 role is the primary after 10 seconds And postgres0 role is the secondary after 10 seconds And replication works from postgres1 to postgres0 after 20 seconds -Scenario: check the scheduled failover - Given I issue a scheduled failover from postgres1 to postgres0 in 3 seconds +Scenario: check the scheduled switchover + Given I issue a scheduled switchover from postgres1 to postgres0 in 3 seconds Then I receive a response returncode 1 - And I receive a response output "Can't schedule failover in the paused state" + And I receive a response output "Can't schedule switchover in the paused state" When I run patronictl.py resume batman Then I receive a response returncode 0 - Given I issue a scheduled failover from postgres1 to postgres0 in 3 seconds + Given I issue a scheduled switchover from postgres1 to postgres0 in 3 seconds Then I receive a response returncode 0 And postgres0 is a leader after 20 seconds And postgres0 role is the primary after 10 seconds diff --git a/features/steps/patroni_api.py b/features/steps/patroni_api.py index ba0ee086..4c732cbf 100644 --- a/features/steps/patroni_api.py +++ b/features/steps/patroni_api.py @@ -125,10 +125,10 @@ def check_response(context, component, data): assert str(context.response[component]) == str(data), "{0} does not contain {1}".format(component, data) -@step('I issue a scheduled failover from {from_host:w} to {to_host:w} in {in_seconds:d} seconds') -def scheduled_failover(context, from_host, to_host, in_seconds): +@step('I issue a scheduled switchover from {from_host:w} to {to_host:w} in {in_seconds:d} seconds') +def scheduled_switchover(context, from_host, to_host, in_seconds): context.execute_steps(u""" - Given I run patronictl.py failover batman --master {0} --candidate {1} --scheduled "{2}" --force + Given I run patronictl.py switchover batman --master {0} --candidate {1} --scheduled "{2}" --force """.format(from_host, to_host, datetime.now(tzutc) + timedelta(seconds=int(in_seconds)))) diff --git a/patroni/api.py b/patroni/api.py index 82406833..e70726dd 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -24,9 +24,9 @@ def check_auth(func): def do_PUT_foo(): pass """ - def wrapper(handler): + def wrapper(handler, *args, **kwargs): if handler.check_auth_header(): - return func(handler) + return func(handler, *args, **kwargs) return wrapper @@ -300,11 +300,11 @@ class RestApiHandler(BaseHTTPRequestHandler): logger.debug('Exception occured during polling failover result: %s', e) return 503, 'Failover status unknown' - def is_failover_possible(self, cluster, leader, candidate): + def is_failover_possible(self, cluster, leader, candidate, action): if leader and (not cluster.leader or cluster.leader.name != leader): return 'leader name does not match' if candidate: - if cluster.is_synchronous_mode() and cluster.sync.sync_standby != candidate: + if action == 'switchover' and cluster.is_synchronous_mode() and cluster.sync.sync_standby != candidate: return 'candidate name does not match with sync_standby' members = [m for m in cluster.members if m.name == candidate] if not members: @@ -312,20 +312,20 @@ class RestApiHandler(BaseHTTPRequestHandler): elif cluster.is_synchronous_mode(): members = [m for m in cluster.members if m.name == cluster.sync.sync_standby] if not members: - return 'failover is not possible: can not find sync_standby' + return action + ' is not possible: can not find sync_standby' else: members = [m for m in cluster.members if m.name != cluster.leader.name and m.api_url] if not members: - return 'failover is not possible: cluster does not have members except leader' + 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 'failover is not possible: no good candidates have been found' + return action + ' is not possible: no good candidates have been found' @check_auth - def do_POST_failover(self): + def do_POST_failover(self, action='failover'): request = self._read_json_content() - status_code = 500 + (status_code, data) = (400, '') if not request: return @@ -334,39 +334,46 @@ class RestApiHandler(BaseHTTPRequestHandler): scheduled_at = request.get('scheduled_at') cluster = self.server.patroni.dcs.get_cluster() - if scheduled_at and cluster.is_paused(): - self._write_response(status_code, "Can't schedule failover in the paused state") + logger.info("received %s request with leader=%s candidate=%s scheduled_at=%s", + action, leader, candidate, scheduled_at) - logger.info("received failover request with leader=%s candidate=%s scheduled_at=%s", - 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' - data = '' - if leader or candidate: - if scheduled_at: - (_, data, scheduled_at) = self.parse_schedule(scheduled_at, "failover") - if _: - status_code = _ - elif self.server.patroni.dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at): - self.server.patroni.ha.wakeup() - data = 'Failover scheduled' + if not data and scheduled_at: + if not leader: + data = 'Scheduled {0} is possible only from a specific leader'.format(action) + if not data and cluster.is_paused(): + 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 cluster.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): + self.server.patroni.ha.wakeup() + if scheduled_at: + data = action.title() + ' scheduled' status_code = 202 else: - data = 'failed to write failover key into DCS' - status_code = 503 + status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name, candidate) else: - data = self.is_failover_possible(cluster, leader, candidate) - if not data: - if self.server.patroni.dcs.manual_failover(leader, candidate): - self.server.patroni.ha.wakeup() - status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name, candidate) - else: - data = 'failed to write failover key into DCS' - status_code = 503 - else: - status_code = 400 - data = 'No values given for required parameters leader and candidate' + data = 'failed to write {0} key into DCS'.format(action) + status_code = 503 self._write_response(status_code, data) + def do_POST_switchover(self): + self.do_POST_failover(action='switchover') + def parse_request(self): """Override parse_request method to enrich basic functionality of `BaseHTTPRequestHandler` class diff --git a/patroni/ctl.py b/patroni/ctl.py index 496c9fe6..c9454023 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -535,33 +535,26 @@ def reinit(obj, cluster_name, member_names, force): break -@ctl.command('failover', help='Failover to a replica') -@arg_cluster_name -@click.option('--master', help='The name of the current master', default=None) -@click.option('--candidate', help='The name of the candidate', default=None) -@click.option('--scheduled', help='Timestamp of a scheduled failover in unambiguous format (e.g. ISO 8601)', - default=None) -@option_force -@click.pass_obj -def failover(obj, cluster_name, master, candidate, force, scheduled): +def _do_failover_or_switchover(obj, action, cluster_name, master, candidate, force, scheduled=None): """ - We want to trigger a failover for the specified cluster name. + We want to trigger a failover or switchover for the specified cluster name. We verify that the cluster name, master name and candidate name are correct. - If so, we trigger a failover and keep the client up to date. + If so, we trigger an action and keep the client up to date. """ dcs = get_dcs(obj, cluster_name) cluster = dcs.get_cluster() - if cluster.leader is None and not cluster.is_paused(): - raise PatroniCtlException('This cluster has no master') + if action == 'switchover': + if cluster.leader is None: + raise PatroniCtlException('This cluster has no master') - if master is None and (not cluster.is_paused() or cluster.leader): - if force: - master = cluster.leader.member.name - else: - master = click.prompt('Master', type=str, default=cluster.leader.member.name) + if master is None: + if force: + master = cluster.leader.member.name + else: + master = click.prompt('Master', type=str, default=cluster.leader.member.name) if master is not None and cluster.leader and cluster.leader.member.name != master: raise PatroniCtlException('Member {0} is not the leader of cluster {1}'.format(master, cluster_name)) @@ -571,28 +564,32 @@ def failover(obj, cluster_name, master, candidate, force, scheduled): candidate_names.sort() if not candidate_names: - raise PatroniCtlException('No candidates found to failover to') + raise PatroniCtlException('No candidates found to {0} to'.format(action)) 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 == master: - raise PatroniCtlException('Failover target and source are the same.') + raise PatroniCtlException(action.title() + ' target and source are the same.') if candidate and candidate not in candidate_names: raise PatroniCtlException('Member {0} does not exist in cluster {1}'.format(candidate, cluster_name)) - if scheduled is None and not force: - scheduled = click.prompt('When should the failover take place (e.g. 2015-10-01T14:30) ', type=str, - default='now') - - scheduled_at = parse_scheduled(scheduled) - scheduled_at_str = None - if scheduled_at: - if cluster.is_paused(): - raise PatroniCtlException("Can't schedule failover in the paused state") - scheduled_at_str = scheduled_at.isoformat() + + if action == 'switchover': + if scheduled is None and not force: + scheduled = click.prompt('When should the switchover take place (e.g. 2015-10-01T14:30) ', + type=str, default='now') + + scheduled_at = parse_scheduled(scheduled) + if scheduled_at: + if cluster.is_paused(): + raise PatroniCtlException("Can't schedule switchover in the paused state") + scheduled_at_str = scheduled_at.isoformat() failover_value = {'leader': master, 'candidate': candidate, 'scheduled_at': scheduled_at_str} @@ -603,35 +600,56 @@ def failover(obj, cluster_name, master, candidate, force, scheduled): output_members(dcs.get_cluster(), cluster_name) if not force: - a = \ - click.confirm('Are you sure you want to failover cluster {0}, demoting current master {1}?'.format( - cluster_name, master)) - if not a: - raise PatroniCtlException('Aborting failover') + demote_msg = ', demoting current master ' + master if master else '' + + if not click.confirm('Are you sure you want to {0} cluster {1}{2}?'.format(action, cluster_name, demote_msg)): + raise PatroniCtlException('Aborting ' + action) r = None try: - member = cluster.leader.member if cluster.leader else [m for m in cluster.members if m.name == candidate][0] + member = cluster.leader.member if cluster.leader else cluster.get_member(candidate, False) - r = request_patroni(member, 'post', 'failover', failover_value, auth_header(obj)) + r = request_patroni(member, 'post', action, failover_value, auth_header(obj)) if r.status_code in (200, 202): logging.debug(r) cluster = dcs.get_cluster() logging.debug(cluster) click.echo('{0} {1}'.format(timestamp(), r.text)) else: - click.echo('Failover failed, details: {0}, {1}'.format(r.status_code, r.text)) + click.echo('{0} failed, details: {1}, {2}'.format(action.title(), r.status_code, r.text)) return except Exception: logging.exception(r) logging.warning('Failing over to DCS') - click.echo(timestamp() + ' Could not failover using Patroni api, falling back to DCS') - click.echo(timestamp() + ' Initializing failover from master {0}'.format(master)) + click.echo('{0} Could not {1} using Patroni api, falling back to DCS'.format(timestamp(), action)) dcs.manual_failover(master, candidate, scheduled_at=scheduled_at) output_members(cluster, cluster_name) +@ctl.command('failover', help='Failover to a replica') +@arg_cluster_name +@click.option('--master', help='The name of the current master', default=None) +@click.option('--candidate', help='The name of the candidate', default=None) +@option_force +@click.pass_obj +def failover(obj, cluster_name, master, candidate, force): + action = 'switchover' if master else 'failover' + _do_failover_or_switchover(obj, action, cluster_name, master, candidate, force) + + +@ctl.command('switchover', help='Switchover to a replica') +@arg_cluster_name +@click.option('--master', help='The name of the current master', default=None) +@click.option('--candidate', help='The name of the candidate', default=None) +@click.option('--scheduled', help='Timestamp of a scheduled switchover in unambiguous format (e.g. ISO 8601)', + default=None) +@option_force +@click.pass_obj +def switchover(obj, cluster_name, master, candidate, force, scheduled): + _do_failover_or_switchover(obj, 'switchover', cluster_name, master, candidate, force, scheduled) + + def output_members(cluster, name, extended=False, fmt='pretty'): rows = [] logging.debug(cluster) diff --git a/tests/test_api.py b/tests/test_api.py index 1bc40cdf..9ed644ef 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -282,12 +282,13 @@ class TestRestApiHandler(unittest.TestCase): @patch('time.sleep', Mock()) @patch.object(MockPatroni, 'dcs') - def test_do_POST_failover(self, dcs): + def test_do_POST_switchover(self, dcs): dcs.loop_wait = 10 cluster = dcs.get_cluster.return_value cluster.is_synchronous_mode.return_value = False + cluster.is_paused.return_value = False - post = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: ' + post = 'POST /switchover HTTP/1.0' + self._authorization + '\nContent-Length: ' MockRestApiServer(RestApiHandler, post + '7\n\n{"1":2}') @@ -297,8 +298,14 @@ class TestRestApiHandler(unittest.TestCase): cluster.leader.name = 'postgresql1' MockRestApiServer(RestApiHandler, request) + request = post + '25\n\n{"leader": "postgresql1"}' + + cluster.is_paused.return_value = True + MockRestApiServer(RestApiHandler, request) + + cluster.is_paused.return_value = False for cluster.is_synchronous_mode.return_value in (True, False): - MockRestApiServer(RestApiHandler, post + '25\n\n{"leader": "postgresql1"}') + MockRestApiServer(RestApiHandler, request) cluster.leader.name = 'postgresql2' request = post + '53\n\n{"leader": "postgresql1", "candidate": "postgresql2"}' @@ -354,3 +361,9 @@ class TestRestApiHandler(unittest.TestCase): # Invalid date self.assertIsNotNone(MockRestApiServer(RestApiHandler, request + '2010-02-29T18:13:30.568224+01:00"}')) + + @patch.object(MockPatroni, 'dcs', Mock()) + def test_do_POST_failover(self): + post = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: ' + MockRestApiServer(RestApiHandler, post + '14\n\n{"leader":"1"}') + MockRestApiServer(RestApiHandler, post + '37\n\n{"candidate":"2","scheduled_at": "1"}') diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 0cda4baf..db79607e 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -75,70 +75,79 @@ class TestCtl(unittest.TestCase): @patch('patroni.ctl.get_dcs') @patch('patroni.ctl.request_patroni', Mock(return_value=MockResponse())) - def test_failover(self, mock_get_dcs): + def test_switchover(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'], input='leader\nother\n\ny') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n\ny') assert 'leader' in result.output - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n2300-01-01T12:23:00\ny') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n2300-01-01T12:23:00\ny') assert result.exit_code == 0 with patch('patroni.dcs.Cluster.is_paused', Mock(return_value=True)): - result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', '2015-01-01T12:00:00']) + result = self.runner.invoke(ctl, ['switchover', 'dummy', '--force', '--scheduled', '2015-01-01T12:00:00']) assert result.exit_code == 1 - # Aborting failover,as we anser NO to the confirmation - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n\nN') + # Aborting switchover, as we anser NO to the confirmation + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n\nN') assert result.exit_code == 1 # Target and source are equal - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nleader\n\ny') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nleader\n\ny') assert result.exit_code == 1 # Reality is not part of this cluster - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nReality\n\ny') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nReality\n\ny') assert result.exit_code == 1 - result = self.runner.invoke(ctl, ['failover', 'dummy', '--force']) + result = self.runner.invoke(ctl, ['switchover', 'dummy', '--force']) assert 'Member' in result.output - result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', '2015-01-01T12:00:00+01:00']) + result = self.runner.invoke(ctl, ['switchover', 'dummy', '--force', '--scheduled', '2015-01-01T12:00:00+01:00']) assert result.exit_code == 0 # Invalid timestamp - result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', 'invalid']) + result = self.runner.invoke(ctl, ['switchover', 'dummy', '--force', '--scheduled', 'invalid']) assert result.exit_code != 0 # Invalid timestamp - result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', '2115-02-30T12:00:00+01:00']) + result = self.runner.invoke(ctl, ['switchover', 'dummy', '--force', '--scheduled', '2115-02-30T12:00:00+01:00']) assert result.exit_code != 0 # Specifying wrong leader - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='dummy') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='dummy') assert result.exit_code == 1 with patch('patroni.ctl.request_patroni', Mock(side_effect=Exception)): # Non-responding patroni - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n2300-01-01T12:23:00\ny') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n2300-01-01T12:23:00\ny') assert 'falling back to DCS' in result.output with patch('patroni.ctl.request_patroni') as mocked: mocked.return_value.status_code = 500 - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n\ny') - assert 'Failover failed' in result.output + result = self.runner.invoke(ctl, ['switchover', 'dummy'], 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, ['failover', 'dummy'], input='leader\nother\n\ny') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n\ny') assert result.exit_code == 1 # No master available mock_get_dcs.return_value.get_cluster = get_cluster_initialized_without_leader - result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n\ny') + result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n\ny') assert result.exit_code == 1 + @patch('patroni.ctl.get_dcs') + @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'], input='\n') + assert 'Failover could be performed only to a specific candidate' in result.output + def test_get_dcs(self): self.assertRaises(PatroniCtlException, get_dcs, {'dummy': {}}, 'dummy')