diff --git a/features/patroni_api.feature b/features/patroni_api.feature index f97e059f..b1eded96 100644 --- a/features/patroni_api.feature +++ b/features/patroni_api.feature @@ -18,7 +18,7 @@ Scenario: check API requests on a stand-alone server And I receive a response text "failover 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 - And I receive a response text "No values given for required parameters leader and member" + And I receive a response text "No values given for required parameters leader and candidate" Scenario: check API requests for the primary-replica pair Given I start postgres1 @@ -41,7 +41,7 @@ Scenario: check the failover via the API And replication works from postgres1 to postgres0 after 15 seconds Scenario: check the scheduled failover - Given I issue a scheduled failover at http://127.0.0.1:8009 from postgres1 to postgresq0 in 10 seconds + Given I issue a scheduled failover at http://127.0.0.1:8009 from postgres1 to postgres0 in 10 seconds Then I receive a response code 200 And postgres0 is a leader after 15 seconds And replication works from postgres0 to postgres1 after 25 seconds diff --git a/patroni/api.py b/patroni/api.py index ac227619..0018e537 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -142,7 +142,7 @@ class RestApiHandler(BaseHTTPRequestHandler): self.end_headers() self.wfile.write(data) - def poll_failover_result(self, leader, member): + def poll_failover_result(self, leader, candidate): for _ in range(0, 15): time.sleep(1) try: @@ -155,18 +155,18 @@ class RestApiHandler(BaseHTTPRequestHandler): pass return 503, b'Failover status unknown' - def is_failover_possible(self, cluster, leader, member): + def is_failover_possible(self, cluster, leader, candidate): if leader and not cluster.leader or cluster.leader.name != leader: return b'leader name does not match' - if member: - members = [m for m in cluster.members if m.name == member] + if candidate: + members = [m for m in cluster.members if m.name == candidate] if not members: - return b'member does not exists' + return b'candidate does not exists' else: members = [m for m in cluster.members if m.name != cluster.leader.name and m.api_url] if not members: return b'failover is not possible: cluster does not have members except leader' - for member, reachable, _, xlog_location, tags in self.server.patroni.ha.fetch_nodes_statuses(members): + for _, reachable, _, _, tags in self.server.patroni.ha.fetch_nodes_statuses(members): if reachable and not tags.get('nofailover', False): return None return b'failover is not possible: no good candidates have been found' @@ -179,43 +179,44 @@ class RestApiHandler(BaseHTTPRequestHandler): except ValueError: request = {} leader = request.get('leader') - member = request.get('member') + candidate = request.get('candidate') or request.get('member') + scheduled_at = request.get('scheduled_at') cluster = self.server.patroni.ha.dcs.get_cluster() status_code = 500 - logger.info("received failover request with leader {0} member {1} scheduled_at {2}". - format(leader, member, request.get("scheduled_at"))) + logger.info("received failover request with leader=%s candidate=%s scheduled_at=%s", + leader, candidate, scheduled_at) data = b'' - if leader or member: - if request.get('scheduled_at'): + if leader or candidate: + if scheduled_at: try: - scheduled_at = dateutil.parser.parse(request['scheduled_at']) + scheduled_at = dateutil.parser.parse(scheduled_at) if scheduled_at.tzinfo is None: data = b'Timezone information is mandatory for scheduled_at' status_code = 400 elif scheduled_at < datetime.datetime.now(pytz.utc): data = b'Cannot schedule failover in the past' status_code = 422 - elif self.server.patroni.dcs.manual_failover(leader, member, scheduled_at=scheduled_at): + elif self.server.patroni.dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at): data = b'Failover scheduled' status_code = 200 except (ValueError, TypeError): - logger.exception('Invalid scheduled failover time: {}'.format(request['scheduled_at'])) + logger.exception('Invalid scheduled failover time: %s', request['scheduled_at']) data = b'Unable to parse scheduled timestamp. It should be in an unambiguous format, e.g. ISO 8601' status_code = 422 else: - data = self.is_failover_possible(cluster, leader, member) + data = self.is_failover_possible(cluster, leader, candidate) if not data: - if not self.server.patroni.dcs.manual_failover(leader, member): + if not self.server.patroni.dcs.manual_failover(leader, candidate): data = b'failed to write failover key into DCS' status_code = 503 else: self.server.patroni.dcs.event.set() - status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name, member) + status_code, data = self.poll_failover_result(cluster.leader and cluster.leader.name, candidate) else: status_code = 400 - data = b'No values given for required parameters leader and member' + data = b'No values given for required parameters leader and candidate' self.send_response(status_code) self.send_header('Content-Type', 'text/html') diff --git a/patroni/ctl.py b/patroni/ctl.py index 5434cebb..b14db933 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -316,8 +316,7 @@ def query( cursor = None for _ in watching(w, watch, clear=False): - output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, - connect_parameters=connect_parameters) + output, cursor = query_member(cluster, cursor, member, role, command, connect_parameters) print_output(None, output, fmt=fmt, delimiter=delimiter) if cursor is None: @@ -533,7 +532,7 @@ def failover(config_file, cluster_name, master, candidate, force, dcs, scheduled raise PatroniCtlException(message.format(scheduled)) scheduled_at = scheduled_at.isoformat() - failover_value = {'leader': master, 'member': candidate, 'scheduled_at': scheduled_at} + failover_value = {'leader': master, 'candidate': candidate, 'scheduled_at': scheduled_at} logging.debug(failover_value) # By now we have established that the leader exists and the candidate exists @@ -563,7 +562,7 @@ def failover(config_file, cluster_name, master, candidate, force, dcs, scheduled 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)) - dcs.manual_failover(leader=master, member=candidate, scheduled_at=failover_value) + dcs.manual_failover(master, candidate, scheduled_at=failover_value) output_members(cluster, name=cluster_name) diff --git a/patroni/dcs.py b/patroni/dcs.py index aa7e178a..3cb76cdb 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -89,16 +89,16 @@ class Leader(namedtuple('Leader', 'index,session,member')): return self.member.conn_url -class Failover(namedtuple('Failover', 'index,leader,member,scheduled_at')): +class Failover(namedtuple('Failover', 'index,leader,candidate,scheduled_at')): """ >>> 'Failover' in str(Failover.from_node(1, '{"leader": "cluster_leader"}')) True - >>> 'Failover' in str(Failover.from_node(1, '{"leader": "cluster_leader", "member": "cluster:member"}')) + >>> 'Failover' in str(Failover.from_node(1, '{"leader": "cluster_leader", "member": "cluster_candidate"}')) True >>> Failover.from_node(1, 'null') is None True - >>> n = '{"leader": "cluster_leader", "member": "cluster:member", "scheduled_at": "2016-01-14T10:09:57.1394Z"}' + >>> n = '{"leader": "cluster_leader", "member": "cluster_candidate", "scheduled_at": "2016-01-14T10:09:57.1394Z"}' >>> 'tzinfo=' in str(Failover.from_node(1, n)) True >>> Failover.from_node(1, None) is None @@ -258,13 +258,13 @@ class AbstractDCS(object): def set_failover_value(self, value, index=None): """Create or update `/failover` key""" - def manual_failover(self, leader, member, scheduled_at=None, index=None): - failover_value = dict() + def manual_failover(self, leader, candidate, scheduled_at=None, index=None): + failover_value = {} if leader: failover_value['leader'] = leader - if member: - failover_value['member'] = member + if candidate: + failover_value['member'] = candidate if scheduled_at: failover_value['scheduled_at'] = scheduled_at.isoformat() diff --git a/patroni/ha.py b/patroni/ha.py index 84fb1225..d9916da6 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -222,12 +222,12 @@ class Ha(object): def manual_failover_process_no_leader(self): failover = self.cluster.failover - if failover.member: # manual failover to specific member - if failover.member == self.state_handler.name: # manual failover to me + if failover.candidate: # manual failover to specific member + if failover.candidate == self.state_handler.name: # manual failover to me return True # find specific node and check that it is healthy - members = [m for m in self.cluster.members if m.name == failover.member] + members = [m for m in self.cluster.members if m.name == failover.candidate] if members: member, reachable, _, _, tags = self.fetch_node_status(members[0]) if reachable and not tags.get('nofailover', False): # node is healthy @@ -240,13 +240,13 @@ class Ha(object): logger.warning('manual failover: member %s is not allowed to promote', member.name) # at this point we should consider all members as a candidates for failover - # i.e. we assume that failover.member is None + # i.e. we assume that failover.candidate is None # try to pick some other members to failover and check that they are healthy if failover.leader: if self.state_handler.name == failover.leader: # I was the leader - # exclude me and desired member which is unhealthy (failover.member can be None) - members = [m for m in self.cluster.members if m.name not in (failover.member, failover.leader)] + # exclude me and desired member which is unhealthy (failover.candidate can be None) + members = [m for m in self.cluster.members if m.name not in (failover.candidate, failover.leader)] if self.is_failover_possible(members): # check that there are healthy members return False else: # I was the leader and it looks like currently I am the only healthy member @@ -308,8 +308,8 @@ class Ha(object): logger.warning('Incorrect value in of scheduled_at: %s', failover.scheduled_at) if not failover.leader or failover.leader == self.state_handler.name: - if not failover.member or failover.member != self.state_handler.name: - members = [m for m in self.cluster.members if not failover.member or m.name == failover.member] + if not failover.candidate or failover.candidate != self.state_handler.name: + members = [m for m in self.cluster.members if not failover.candidate or m.name == failover.candidate] if self.is_failover_possible(members): # check that there are healthy members self._async_executor.schedule('manual failover: demote') self._async_executor.run_async(self.demote) diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 7d458872..a98e4ccf 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -190,7 +190,7 @@ class ZooKeeper(AbstractDCS): def attempt_to_acquire_leader(self): ret = self._create(self.leader_path, self._name, makepath=True, ephemeral=True) - if ret: + if not ret: logger.info('Could not take out TTL lock') return ret diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 339aa4bd..d671049d 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -162,6 +162,8 @@ class TestZooKeeper(unittest.TestCase): def test_take_leader(self): self.zk.take_leader() + with patch.object(MockKazooClient, 'create', Mock(side_effect=Exception)): + self.zk.take_leader() def test_update_leader(self): self.assertTrue(self.zk.update_leader())