diff --git a/features/steps/patroni_api.py b/features/steps/patroni_api.py index 4d2f1709..28aaa18a 100644 --- a/features/steps/patroni_api.py +++ b/features/steps/patroni_api.py @@ -106,13 +106,8 @@ def scheduled_failover(context, at_url, from_host, to_host, in_seconds): @step('I issue a scheduled restart at {url:url} in {in_seconds:d} seconds with {data}') def scheduled_restart(context, url, in_seconds, data): data = data and json.loads(data) or {} - restart_options = ['"schedule": "{0}"'.format((datetime.now(pytz.utc) + timedelta(seconds=int(in_seconds))).isoformat())] - for key in data: - if key == 'schedule': - continue - restart_options.append('"{0}": "{1}"'.format(key, data[key])) - context.execute_steps(u"""Given I issue a POST request to {0}/restart with {{{1}}}""". - format(url, ','.join(restart_options))) + data.update(schedule='{0}'.format((datetime.now(pytz.utc) + timedelta(seconds=int(in_seconds))).isoformat())) + context.execute_steps(u"""Given I issue a POST request to {0}/restart with {1}""".format(url, json.dumps(data))) @step('I add tag {tag:w} {value:w} to {pg_name:w} config') diff --git a/patroni/api.py b/patroni/api.py index c6b5e225..c9d64408 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -119,7 +119,7 @@ class RestApiHandler(BaseHTTPRequestHandler): def _read_json_content(self, body_is_optional=False): if 'content-length' not in self.headers: - return self.send_error(411) if body_is_optional else None + return self.send_error(411) if not body_is_optional else {} try: content_length = int(self.headers.get('content-length')) if content_length == 0 and body_is_optional: @@ -195,7 +195,9 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 500 data = 'restart failed' request = self._read_json_content(body_is_optional=True) - request = request or {} + if request is None: + # failed to parse the json + return if request: logger.debug("received restart request: {0}".format(request)) for k in request: @@ -225,7 +227,6 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 200 if status else 503 except Exception: logger.exception('Exception during restart') - data = "Schedule required for the scheduled restart" status_code = 400 else: request['postmaster_start_time'] = self.server.patroni.ha.state_handler.postmaster_start_time() @@ -239,9 +240,12 @@ class RestApiHandler(BaseHTTPRequestHandler): @check_auth def do_DELETE_restart(self): - self.server.patroni.ha.delete_future_restart() - data = "scheduled restart deleted" - code = 200 + if self.server.patroni.ha.delete_future_restart(): + data = "scheduled restart deleted" + code = 200 + else: + data = "no restarts are scheduled" + code = 404 self._write_response(code, data) @check_auth diff --git a/patroni/ha.py b/patroni/ha.py index b24bef94..d989ae06 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -174,7 +174,7 @@ class Ha(object): tags = json.get('tags', dict()) return (member, True, not is_master, xlog_location, tags) except: - logging.exception('request failed: GET %s', member.api_url) + logger.exception('request failed: GET %s', member.api_url) return (member, False, None, 0, {}) def fetch_nodes_statuses(self, members): @@ -293,33 +293,35 @@ class Ha(object): def should_run_scheduled_action(self, action_name, scheduled_at, cleanup_fn): if scheduled_at: - # If the faildover is in the far future, we shouldn't do anything and just return. - # If the failover is in the past, we consider the value to be stale and we remove + # If the scheduled action is in the far future, we shouldn't do anything and just return. + # If the scheduled action is in the past, we consider the value to be stale and we remove # the value. - # If the value is close to now, we initiate the failover + # If the value is close to now, we initiate the scheduled action + # Additionally, if the scheduled action cannot be executed altogether, i.e. there is an error + # or the action is in the past - we take care of cleaning it up. now = datetime.datetime.now(pytz.utc) try: delta = (scheduled_at - now).total_seconds() if delta > self.patroni.nap_time: - logging.info('Awaiting {0} at %s (in %.0f seconds)'.format(action_name), - scheduled_at.isoformat(), delta) + logger.info('Awaiting %s at %s (in %.0f seconds)', + action_name, scheduled_at.isoformat(), delta) return False elif delta < - int(self.patroni.nap_time * 1.5): - logger.warning('Found a stale {0} value, cleaning up: %s'.format(action_name), - scheduled_at.isoformat()) + logger.warning('Found a stale %s value, cleaning up: %s', + action_name, scheduled_at.isoformat()) cleanup_fn() self.dcs.manual_failover('', '', index=self.cluster.failover.index) - return None + return False # The value is very close to now sleep(max(delta, 0)) logger.info('Manual scheduled {0} at %s'.format(action_name), scheduled_at.isoformat()) return True except TypeError: - logger.warning('Incorrect value in of scheduled_at: %s', scheduled_at) + logger.warning('Incorrect value of scheduled_at: %s', scheduled_at) cleanup_fn() - return None + return False def process_manual_failover_from_leader(self): failover = self.cluster.failover @@ -401,11 +403,11 @@ class Ha(object): if (restart_data and self.should_run_scheduled_action('restart', restart_data['schedule'], self.delete_future_restart)): try: - ret, message = self.restart(restart_data) - if ret: - logger.info("Scheduled restart successfull") - else: - logger.warning("Scheduled restart: {0}".format(message)) + ret, message = self.restart(restart_data) + if ret: + logger.info("Scheduled restart successful") + else: + logger.warning("Scheduled restart: {0}".format(message)) finally: self.delete_future_restart() @@ -442,8 +444,12 @@ class Ha(object): return False def delete_future_restart(self): + ret = False with self._async_executor: - self.patroni.scheduled_restart = {} + if self.patroni.scheduled_restart: + self.patroni.scheduled_restart = {} + ret = True + return ret def immediate_restart_scheduled(self): return self._async_executor.scheduled_action == 'restart' diff --git a/tests/test_api.py b/tests/test_api.py index e50b58bb..9a722b42 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -72,7 +72,7 @@ class MockPatroni(object): version = '0.00' noloadbalance = Mock(return_value=False) scheduled_restart = {'schedule': dateutil.parser.parse('2016-08-29 12:45TZ+1'), - 'postmaster_start_time': '2016-08-20 12:00TZ+1'} + 'postmaster_start_time': postgresql.postmaster_start_time()} @staticmethod def sighup_handler(): @@ -184,24 +184,27 @@ class TestRestApiHandler(unittest.TestCase): MockRestApiServer(RestApiHandler, request) post = request + '\nContent-Length: ' - # wrong version - request = post + '84\n\n{"schedule": "2016-08-20 12:45TZ+1", "role": "unknown", "postgres_version": "9.5.3"}' - MockRestApiServer(RestApiHandler, request) + + def make_request(request): + return '{0}{1}\n\n{2}'.format(post, len(request), request) # wrong role - request = post + '85\n\n{"schedule": "2016-08-20 12:45TZ+1", "role": "master", "postgres_version": "9.5.3.1"}' + request = make_request('{"schedule": "2016-08-20 12:45TZ+1", "role": "unknown", "postgres_version": "9.5.3"}') + MockRestApiServer(RestApiHandler, request) + # wrong version + request = make_request('{"schedule": "2016-08-20 12:45TZ+1", "role": "master", "postgres_version": "9.5.3.1"}') MockRestApiServer(RestApiHandler, request) # unknown filter - request = post + '55\n\n{"schedule": "2016-08-29 12:45TZ+1", "batman": "lives"}' + request = make_request('{"schedule": "2016-08-29 12:45TZ+1", "batman": "lives"}') MockRestApiServer(RestApiHandler, request) # incorrect schedule - request = post + '55\n\n{"schedule": "2016-08-42 12:45TZ+1", "role": "master"}' + request = make_request('{"schedule": "2016-08-42 12:45TZ+1", "role": "master"}') MockRestApiServer(RestApiHandler, request) # everything fine, but the schedule is missing - request = post + '47\n\n{"role": "master", "postgres_version": "9.5.2"}' + request = make_request('{"role": "master", "postgres_version": "9.5.2"}') MockRestApiServer(RestApiHandler, request) for retval in (True, False): with patch.object(MockHa, 'schedule_future_restart', Mock(return_value=retval)): - request = post + '36\n\n{"schedule": "2016-08-29 12:45TZ+1"}' + request = make_request('{"schedule": "2016-08-29 12:45TZ+1"}') MockRestApiServer(RestApiHandler, request) def test_do_DELETE_restart(self):