From 073ef3784f1068afa1e3094fed8045585a5e09d4 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 27 May 2016 16:29:33 +0200 Subject: [PATCH] Implement PATCH /config --- features/basic_replication.feature | 6 ++-- features/patroni_api.feature | 6 ++-- features/steps/basic_replication.py | 17 --------- features/steps/patroni_api.py | 29 ++++++--------- patroni/__init__.py | 3 +- patroni/api.py | 56 +++++++++++++++++++++-------- patroni/config.py | 18 +++++----- patroni/ctl.py | 2 +- patroni/dcs/__init__.py | 6 ++-- tests/test_api.py | 40 ++++++++++++++------- 10 files changed, 100 insertions(+), 83 deletions(-) diff --git a/features/basic_replication.feature b/features/basic_replication.feature index 438b1dd7..ea5176be 100644 --- a/features/basic_replication.feature +++ b/features/basic_replication.feature @@ -9,14 +9,14 @@ Feature: basic replication Then table foo is present on postgres1 after 20 seconds Scenario: check local configuration reload - When I issue an empty POST request to http://127.0.0.1:8008/reload + Given I issue an empty POST request to http://127.0.0.1:8008/reload Then I receive a response code 304 When I add tag new_tag new_value to postgres0 config And I issue an empty POST request to http://127.0.0.1:8008/reload - Then I receive a response code 200 + Then I receive a response code 202 Scenario: check dynamic configuration change via DCS - When I patch global configuration with {"ttl": 20, "loop_wait": 5, "postgresql": {"parameters": {"max_connections": 101}}} + Given I issue a PATCH request to http://127.0.0.1:8008/config with {"ttl": 20, "loop_wait": 5, "postgresql": {"parameters": {"max_connections": 101}}} Then Response on GET http://127.0.0.1:8008/patroni contains restart_pending after 11 seconds And Response on GET http://127.0.0.1:8009/patroni contains restart_pending after 11 seconds And Response on GET http://127.0.0.1:8008/patroni contains new_value after 1 seconds diff --git a/features/patroni_api.feature b/features/patroni_api.feature index 31bbbdb2..22dce74f 100644 --- a/features/patroni_api.feature +++ b/features/patroni_api.feature @@ -13,7 +13,7 @@ Scenario: check API requests on a stand-alone server When I issue an empty POST request to http://127.0.0.1:8008/reinitialize Then I receive a response code 503 And I receive a response text "I am the leader, can not reinitialize" - When I issue a POST request to http://127.0.0.1:8008/failover with leader=postgres0 + 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" When I issue an empty POST request to http://127.0.0.1:8008/failover @@ -36,7 +36,7 @@ Scenario: check API requests for the primary-replica pair Then postgres1 role is the secondary after 15 seconds Scenario: check the failover via the API - Given I issue a POST request to http://127.0.0.1:8008/failover with leader=postgres0,candidate=postgres1 + Given I issue a POST request to http://127.0.0.1:8008/failover 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 5 seconds @@ -45,7 +45,7 @@ Scenario: check the failover via the API Scenario: check the scheduled failover Given I issue a scheduled failover at http://127.0.0.1:8009 from postgres1 to postgres0 in 1 seconds - Then I receive a response code 200 + Then I receive a response code 202 And postgres0 is a leader after 20 seconds And postgres0 role is the primary after 5 seconds And postgres1 role is the secondary after 10 seconds diff --git a/features/steps/basic_replication.py b/features/steps/basic_replication.py index c3033627..05acf288 100644 --- a/features/steps/basic_replication.py +++ b/features/steps/basic_replication.py @@ -1,4 +1,3 @@ -import json import psycopg2 as pg import requests @@ -56,22 +55,6 @@ def replication_works(context, master, replica, time_limit): """.format(int(time()), master, replica, time_limit)) -def patch_config_with_data(config, data): - for name, value in data.items(): - if isinstance(value, dict): - patch_config_with_data(config[name], value) - else: - config[name] = value - - -@step('I patch global configuration with {data}') -def patch_config(context, data): - data = json.loads(data) - config = json.loads(context.dcs_ctl.query('config')) - patch_config_with_data(config, data) - context.dcs_ctl.set('config', json.dumps(config)) - - @then('Response on GET {url} contains {value} after {timeout:d} seconds') def check_http_response(context, url, value, timeout): for _ in range(int(timeout)): diff --git a/features/steps/patroni_api.py b/features/steps/patroni_api.py index d94086b5..d41eb373 100644 --- a/features/steps/patroni_api.py +++ b/features/steps/patroni_api.py @@ -1,3 +1,4 @@ +import json import parse import pytz import requests @@ -12,12 +13,7 @@ def parse_url(text): return text -@parse.with_pattern(r'(?:\w+=(?:\w|\.|:|-|\+|\s)+,?)+') -def parse_data(text): - return text - - -register_type(url=parse_url, data=parse_data) +register_type(url=parse_url) # there is no way we can find out if the node has already @@ -56,20 +52,17 @@ def do_get(context, url): @step('I issue an empty POST request to {url:url}') def do_post_empty(context, url): - do_post(context, url, None) + do_request(context, 'POST', url, None) -@step('I issue a POST request to {url:url} with {data:data}') -def do_post(context, url, data): - post_data = {} - if data: - post_components = data.split(',') - for pc in post_components: - if '=' in pc: - k, v = pc.split('=', 2) - post_data[k.strip()] = v.strip() +@step('I issue a {request_method:w} request to {url:url} with {data}') +def do_request(context, request_method, url, data): + data = data and json.loads(data) or {} try: - r = requests.post(url, json=post_data) + if request_method == 'PATCH': + r = requests.patch(url, json=data) + else: + r = requests.post(url, json=data) except requests.exceptions.RequestException: context.status_code = None context.response = None @@ -96,5 +89,5 @@ def check_response(context, component, data): @step('I issue a scheduled failover at {at_url:url} from {from_host:w} to {to_host:w} in {in_seconds:d} seconds') def scheduled_failover(context, at_url, from_host, to_host, in_seconds): context.execute_steps(u""" - Given I issue a POST request to {0}/failover with leader={1},candidate={2},scheduled_at={3} + Given I issue a POST request to {0}/failover with {{"leader": "{1}", "candidate": "{2}", "scheduled_at": "{3}"}} """.format(at_url, from_host, to_host, datetime.now(pytz.utc) + timedelta(seconds=int(in_seconds)))) diff --git a/patroni/__init__.py b/patroni/__init__.py index b8df602e..a8107362 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -100,8 +100,7 @@ class Patroni(object): logger.info(self.ha.run_cycle()) cluster = self.dcs.cluster - if cluster and cluster.config and cluster.config.data and \ - self.config.set_dynamic_configuration(cluster.config.data): + if cluster and cluster.config and self.config.set_dynamic_configuration(cluster.config.data): self.reload_config() if not self.postgresql.data_directory_empty(): diff --git a/patroni/api.py b/patroni/api.py index d84ce7ff..8858a4f5 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -45,6 +45,9 @@ class RestApiHandler(BaseHTTPRequestHandler): self.end_headers() self.wfile.write(body.encode('utf-8')) + def _write_json_response(self, status_code, response): + self._write_response(status_code, json.dumps(response), {'Content-Type': 'application/json'}) + def send_auth_request(self, body): headers = {'WWW-Authenticate': 'Basic realm="' + self.server.patroni.__class__.__name__ + '"'} self._write_response(401, body, headers) @@ -65,17 +68,15 @@ class RestApiHandler(BaseHTTPRequestHandler): def _write_status_response(self, status_code, response, options=False): if options: - body = None - else: - patroni = self.server.patroni - response.update({'tags': patroni.tags} if patroni.tags else {}) - if patroni.postgresql.sysid: - response['database_system_identifier'] = patroni.postgresql.sysid - if patroni.postgresql.restart_pending: - response['restart_pending'] = True - response['patroni'] = {'version': patroni.version, 'scope': patroni.postgresql.scope} - body = json.dumps(response) - self._write_response(status_code, body, {'Content-Type': 'application/json'}) + return self._write_response(status_code, None) + patroni = self.server.patroni + response.update({'tags': patroni.tags} if patroni.tags else {}) + if patroni.postgresql.sysid: + response['database_system_identifier'] = patroni.postgresql.sysid + if patroni.postgresql.restart_pending: + response['restart_pending'] = True + response['patroni'] = {'version': patroni.version, 'scope': patroni.postgresql.scope} + self._write_json_response(status_code, response) def do_GET(self, options=False): """Default method for processing all GET requests which can not be routed to other methods""" @@ -112,12 +113,39 @@ class RestApiHandler(BaseHTTPRequestHandler): response = self.get_postgresql_status(True) self._write_status_response(200, response) + def do_GET_config(self): + self._write_json_response(200, self.server.patroni.config.dynamic_configuration) + + @staticmethod + def _patch_config(config, data): + for name, value in data.items(): + if isinstance(value, dict) and name in config: + RestApiHandler._patch_config(config[name], value) + elif value is None: + config.pop(name, None) + else: + config[name] = value + + @check_auth + def do_PATCH_config(self): + content_length = int(self.headers.get('content-length', 0)) + request = json.loads(self.rfile.read(content_length).decode('utf-8')) + cluster = self.server.patroni.ha.dcs.get_cluster() + data = cluster.config.data.copy() + RestApiHandler._patch_config(data, request) + response_code = data == cluster.config.data and 304 or 200 + if response_code == 200: + self.server.patroni.ha.dcs.set_config_value(json.dumps(data, separators=(',', ':')), cluster.config.index) + self._write_json_response(200, data) + else: + self._write_response(304, None) + @check_auth def do_POST_reload(self): try: configuration_is_changed = self.server.patroni.config.reload_local_configuration(True) - status_code = configuration_is_changed and 200 or 304 - response = configuration_is_changed and 'reload scheduled' or '' + status_code = configuration_is_changed and 202 or 304 + response = configuration_is_changed and 'reload scheduled' or None if configuration_is_changed: self.server.patroni.sighup_handler() except Exception as e: @@ -218,7 +246,7 @@ class RestApiHandler(BaseHTTPRequestHandler): elif self.server.patroni.dcs.manual_failover(leader, candidate, scheduled_at=scheduled_at): self.server.patroni.dcs.event.set() data = 'Failover scheduled' - status_code = 200 + status_code = 202 else: data = 'failed to write failover key into DCS' status_code = 503 diff --git a/patroni/config.py b/patroni/config.py index 8cb43458..2fb24135 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -43,7 +43,8 @@ class Config(object): self._config_file = None if config_env else config_file self._dynamic_configuration = {} self._local_configuration = yaml.safe_load(config_env) if config_env else self._load_config_file() - self._build_effective_configuration(self._dynamic_configuration, self._local_configuration) + self.__effective_configuration = self._build_effective_configuration(self._dynamic_configuration, + self._local_configuration) self._data_dir = self.__effective_configuration['postgresql']['data_dir'] self._cache_file = os.path.join(self._data_dir, self.__CACHE_FILENAME) self._load_cache() @@ -93,9 +94,10 @@ class Config(object): logger.error('Can not remove temporary file %s', tmpfile) def set_dynamic_configuration(self, configuration): - if configuration and self._dynamic_configuration != configuration: + if self._dynamic_configuration != configuration: try: - self._build_effective_configuration(configuration, self._local_configuration) + self.__effective_configuration = self._build_effective_configuration(configuration, + self._local_configuration) self._dynamic_configuration = configuration self._cache_needs_saving = True return True @@ -107,13 +109,11 @@ class Config(object): try: configuration = self._load_config_file() if self._local_configuration != configuration: - old_effective_configuration = self.__effective_configuration - self._build_effective_configuration(self._dynamic_configuration, configuration) + new_configuration = self._build_effective_configuration(self._dynamic_configuration, configuration) if dry_run: - ret = old_effective_configuration != self.__effective_configuration - self.__effective_configuration = old_effective_configuration - return ret + return new_configuration != self.__effective_configuration self._local_configuration = configuration + self.__effective_configuration = new_configuration return True except Exception: logger.exception('Exception when reloading local configuration from %s', self.config_file) @@ -173,7 +173,7 @@ class Config(object): pg_config.update({p: config[p] for p in ('name', 'scope', 'retry_timeout', 'maximum_lag_on_failover') if p in config}) - self.__effective_configuration = config + return config def get(self, key, default=None): return self.__effective_configuration.get(key, default) diff --git a/patroni/ctl.py b/patroni/ctl.py index 08622086..3d93d2fd 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -534,7 +534,7 @@ def failover(config_file, cluster_name, master, candidate, force, dcs, scheduled r = None try: r = post_patroni(cluster.leader.member, 'failover', failover_value) - if r.status_code == 200: + if r.status_code in (200, 202): logging.debug(r) cluster = dcs.get_cluster() logging.debug(cluster) diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index fcf258b7..209214c0 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -169,14 +169,14 @@ class ClusterConfig(namedtuple('ClusterConfig', 'index,data')): @staticmethod def from_node(index, data): """ - >>> ClusterConfig.from_node(1, '{').data - {} + >>> ClusterConfig.from_node(1, '{') is None + True """ try: data = json.loads(data) except (TypeError, ValueError): - data = {} + return None return ClusterConfig(index, data) diff --git a/tests/test_api.py b/tests/test_api.py index 9cfdd4ec..edc0ebc7 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -4,7 +4,7 @@ import unittest from mock import Mock, patch from patroni.api import RestApiHandler, RestApiServer -from patroni.dcs import Member +from patroni.dcs import ClusterConfig, Member from six import BytesIO as IO from six.moves import BaseHTTPServer from six.moves.BaseHTTPServer import BaseHTTPRequestHandler @@ -90,6 +90,8 @@ class MockRestApiServer(RestApiServer): @patch('ssl.wrap_socket', Mock(return_value=0)) class TestRestApiHandler(unittest.TestCase): + _authorization = '\nAuthorization: Basic dGVzdDp0ZXN0' + def test_do_GET(self): MockRestApiServer(RestApiHandler, 'GET /replica') with patch.object(RestApiHandler, 'get_postgresql_status', Mock(return_value={})): @@ -127,12 +129,25 @@ class TestRestApiHandler(unittest.TestCase): self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'POST /restart HTTP/1.0')) MockRestApiServer(RestApiHandler, 'POST /restart HTTP/1.0\nAuthorization:') + @patch.object(MockPatroni, 'config') + def test_do_GET_config(self, mock_config): + mock_config.dynamic_configuration = {} + self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /config')) + + @patch.object(MockHa, 'dcs') + def test_do_PATCH_config(self, mock_dcs): + mock_dcs.get_cluster.return_value.config = \ + ClusterConfig.from_node(1, '{"postgresql": {"use_slots": false, "parameters": {"wal_level": "logical"}}}') + request = 'PATCH /config HTTP/1.0' + self._authorization + '\nContent-Length: ' + self.assertIsNotNone(MockRestApiServer(RestApiHandler, request + '2\n\n{}')) + MockRestApiServer(RestApiHandler, request + '59\n\n{"ttl":5,"use_slots":true,"postgresql":{"parameters":null}}') + @patch.object(MockPatroni, 'sighup_handler', Mock(side_effect=Exception)) def test_do_POST_reload(self): - MockRestApiServer(RestApiHandler, 'POST /reload HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0') + self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'POST /reload HTTP/1.0' + self._authorization)) def test_do_POST_restart(self): - request = 'POST /restart HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0' + request = 'POST /restart HTTP/1.0' + self._authorization self.assertIsNotNone(MockRestApiServer(RestApiHandler, request)) with patch.object(MockHa, 'restart', Mock(side_effect=Exception)): MockRestApiServer(RestApiHandler, request) @@ -140,7 +155,7 @@ class TestRestApiHandler(unittest.TestCase): @patch.object(MockHa, 'dcs') def test_do_POST_reinitialize(self, dcs): cluster = dcs.get_cluster.return_value - request = 'POST /reinitialize HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0' + request = 'POST /reinitialize HTTP/1.0' + self._authorization MockRestApiServer(RestApiHandler, request) cluster.is_unlocked.return_value = False MockRestApiServer(RestApiHandler, request) @@ -161,19 +176,18 @@ class TestRestApiHandler(unittest.TestCase): def test_do_POST_failover(self, dcs): cluster = dcs.get_cluster.return_value - request = 'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\nContent-Length: 0\n\n' + request = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: 0\n\n' MockRestApiServer(RestApiHandler, request) cluster.leader.name = 'postgresql1' MockRestApiServer(RestApiHandler, request) - request = 'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\n' +\ - 'Content-Length: 25\n\n{"leader": "postgresql1"}' + request = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: 25\n\n{"leader": "postgresql1"}' MockRestApiServer(RestApiHandler, request) cluster.leader.name = 'postgresql2' - request = 'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\n' +\ - 'Content-Length: 53\n\n{"leader": "postgresql1", "candidate": "postgresql2"}' + request = 'POST /failover HTTP/1.0' + self._authorization +\ + '\nContent-Length: 53\n\n{"leader": "postgresql1", "candidate": "postgresql2"}' MockRestApiServer(RestApiHandler, request) cluster.leader.name = 'postgresql1' @@ -199,7 +213,7 @@ class TestRestApiHandler(unittest.TestCase): MockRestApiServer(RestApiHandler, request) # Valid future date - request = 'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\nContent-Length: 103\n\n{"leader": ' +\ + request = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: 103\n\n{"leader": ' +\ '"postgresql1", "member": "postgresql2", "scheduled_at": "6016-02-15T18:13:30.568224+01:00"}' MockRestApiServer(RestApiHandler, request) with patch.object(MockPatroni, 'dcs') as d: @@ -207,16 +221,16 @@ class TestRestApiHandler(unittest.TestCase): MockRestApiServer(RestApiHandler, request) # Exception: No timezone specified - request = 'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\nContent-Length: 97\n\n{"leader": ' +\ + request = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: 97\n\n{"leader": ' +\ '"postgresql1", "member": "postgresql2", "scheduled_at": "6016-02-15T18:13:30.568224"}' MockRestApiServer(RestApiHandler, request) # Exception: Scheduled in the past - request = 'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\nContent-Length: 103\n\n{"leader": ' +\ + request = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: 103\n\n{"leader": ' +\ '"postgresql1", "member": "postgresql2", "scheduled_at": "1016-02-15T18:13:30.568224+01:00"}' MockRestApiServer(RestApiHandler, request) # Invalid date - request = 'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\nContent-Length: 103\n\n{"leader": ' +\ + request = 'POST /failover HTTP/1.0' + self._authorization + '\nContent-Length: 103\n\n{"leader": ' +\ '"postgresql1", "member": "postgresql2", "scheduled_at": "2010-02-29T18:13:30.568224+01:00"}' self.assertIsNotNone(MockRestApiServer(RestApiHandler, request))