From 6700cd0aa67fa703413b61f54d33ba5643dc16dd Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 26 May 2016 17:09:40 +0200 Subject: [PATCH] Implement reload of config.yml with REST API call and acceptance tests for that --- features/basic_replication.feature | 8 ++++++ features/environment.py | 12 +++++++- features/steps/basic_replication.py | 5 ++++ patroni/__init__.py | 4 +-- patroni/api.py | 13 +++++++++ patroni/config.py | 43 ++++++++++++++++++++--------- tests/test_api.py | 9 ++++++ tests/test_config.py | 11 +++++++- tests/test_patroni.py | 1 + 9 files changed, 89 insertions(+), 17 deletions(-) diff --git a/features/basic_replication.feature b/features/basic_replication.feature index ba347f45..438b1dd7 100644 --- a/features/basic_replication.feature +++ b/features/basic_replication.feature @@ -8,10 +8,18 @@ Feature: basic replication When I add the table foo to postgres0 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 + 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 + Scenario: check dynamic configuration change via DCS When I patch global configuration 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 Scenario: check the basic failover And I kill postgres0 diff --git a/features/environment.py b/features/environment.py index 01e94ce0..40b2f377 100644 --- a/features/environment.py +++ b/features/environment.py @@ -98,6 +98,13 @@ class PatroniController(AbstractController): except IOError: return None + def add_tag_to_config(self, tag, value): + with open(self._config) as r: + config = yaml.safe_load(r) + config['tags']['tag'] = value + with open(self._config, 'w') as w: + yaml.safe_dump(config, w, default_flow_style=False) + def _start(self): return subprocess.Popen(['coverage', 'run', '--source=patroni', '-p', 'patroni.py', self._config], stdout=self._log, stderr=subprocess.STDOUT, cwd=self._work_directory) @@ -126,6 +133,9 @@ class PatroniController(AbstractController): 'logging_collector': 'on', 'log_destination': 'csvlog', 'log_directory': self._output_dir, 'log_filename': name + '.log', 'log_statement': 'all', 'log_min_messages': 'debug1'}) + if 'bootstrap' in config and 'initdb' in config['bootstrap']: + config['bootstrap']['initdb'].extend([{'auth': 'md5'}, {'auth-host': 'md5'}]) + if tags: config['tags'] = tags @@ -347,7 +357,7 @@ class PatroniPoolController(object): self._processes[pg_name].start(max_wait_limit) def __getattr__(self, func): - if func not in ['stop', 'query', 'write_label', 'read_label', 'check_role_has_changed_to']: + if func not in ['stop', 'query', 'write_label', 'read_label', 'check_role_has_changed_to', 'add_tag_to_config']: raise AttributeError("PatroniPoolController instance has no attribute '{0}'".format(func)) def wrapper(pg_name, *args, **kwargs): diff --git a/features/steps/basic_replication.py b/features/steps/basic_replication.py index 4f441dac..c3033627 100644 --- a/features/steps/basic_replication.py +++ b/features/steps/basic_replication.py @@ -82,3 +82,8 @@ def check_http_response(context, url, value, timeout): else: assert False,\ "Value {0} is not present in response after {1} seconds".format(value, timeout) + + +@step('I add tag {tag:w} {value:w} to {pg_name:w} config') +def add_tag_to_config(context, tag, value, pg_name): + context.pctl.add_tag_to_config(pg_name, tag, value) diff --git a/patroni/__init__.py b/patroni/__init__.py index 34c7d84e..b8df602e 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -94,8 +94,8 @@ class Patroni(object): while True: if self._received_sighup: self._received_sighup = False - self.config.reload_local_configuration() - self.reload_config() + if self.config.reload_local_configuration(): + self.reload_config() logger.info(self.ha.run_cycle()) diff --git a/patroni/api.py b/patroni/api.py index 33404f28..d84ce7ff 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -112,6 +112,19 @@ class RestApiHandler(BaseHTTPRequestHandler): response = self.get_postgresql_status(True) self._write_status_response(200, response) + @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 '' + if configuration_is_changed: + self.server.patroni.sighup_handler() + except Exception as e: + status_code = 500 + response = str(e) + self._write_response(status_code, response) + @check_auth def do_POST_restart(self): status_code = 500 diff --git a/patroni/config.py b/patroni/config.py index cf5ba85d..8cb43458 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -43,7 +43,7 @@ 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._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() @@ -94,14 +94,31 @@ class Config(object): def set_dynamic_configuration(self, configuration): if configuration and self._dynamic_configuration != configuration: - self._dynamic_configuration = configuration - self._build_effective_configuration() - self._cache_needs_saving = True - return True + try: + self._build_effective_configuration(configuration, self._local_configuration) + self._dynamic_configuration = configuration + self._cache_needs_saving = True + return True + except Exception: + logger.exception('Exception when setting dynamic_configuration') - def reload_local_configuration(self): - self._local_configuration = self._load_config_file() - self._build_effective_configuration() + def reload_local_configuration(self, dry_run=False): + if self.config_file: + 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) + if dry_run: + ret = old_effective_configuration != self.__effective_configuration + self.__effective_configuration = old_effective_configuration + return ret + self._local_configuration = configuration + return True + except Exception: + logger.exception('Exception when reloading local configuration from %s', self.config_file) + if dry_run: + raise def _process_postgresql_parameters(self, parameters, is_local=False): ret = {} @@ -114,10 +131,10 @@ class Config(object): ret[name] = value return ret - def _safe_copy_dynamic_configuration(self): + def _safe_copy_dynamic_configuration(self, dynamic_configuration): config = deepcopy(self.__DEFAULT_CONFIG) - for name, value in self._dynamic_configuration.items(): + for name, value in dynamic_configuration.items(): if name == 'postgresql': for name, value in (value or {}).items(): if name == 'parameters': @@ -128,9 +145,9 @@ class Config(object): config[name] = value return config - def _build_effective_configuration(self): - config = self._safe_copy_dynamic_configuration() - for name, value in self._local_configuration.items(): + def _build_effective_configuration(self, dynamic_configuration, local_configuration): + config = self._safe_copy_dynamic_configuration(dynamic_configuration) + for name, value in local_configuration.items(): if name == 'postgresql': for name, value in (value or {}).items(): if name == 'parameters': diff --git a/tests/test_api.py b/tests/test_api.py index 23541cec..9cfdd4ec 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -50,6 +50,7 @@ class MockHa(object): class MockPatroni(object): + config = Mock() postgresql = MockPostgresql() ha = MockHa() dcs = Mock() @@ -57,6 +58,10 @@ class MockPatroni(object): version = '0.00' noloadbalance = Mock(return_value=False) + @staticmethod + def sighup_handler(): + pass + class MockRequest(object): @@ -122,6 +127,10 @@ class TestRestApiHandler(unittest.TestCase): self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'POST /restart HTTP/1.0')) MockRestApiServer(RestApiHandler, 'POST /restart HTTP/1.0\nAuthorization:') + @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') + def test_do_POST_restart(self): request = 'POST /restart HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0' self.assertIsNotNone(MockRestApiServer(RestApiHandler, request)) diff --git a/tests/test_config.py b/tests/test_config.py index ca2f2511..6987fa67 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -13,8 +13,17 @@ class TestConfig(unittest.TestCase): def setUp(self): self.config = Config(config_env='postgresql: {data_dir: foo}') + @patch.object(Config, '_build_effective_configuration', Mock(side_effect=Exception)) + def test_set_dynamic_configuration(self): + self.assertIsNone(self.config.set_dynamic_configuration({'foo': 'bar'})) + def test_reload_local_configuration(self): - self.assertIsNone(Config(config_file='postgres0.yml').reload_local_configuration()) + config = Config(config_file='postgres0.yml') + with patch.object(Config, '_load_config_file', Mock(return_value={})): + with patch.object(Config, '_build_effective_configuration', Mock(side_effect=Exception)): + self.assertRaises(Exception, config.reload_local_configuration, True) + self.assertTrue(config.reload_local_configuration(True)) + self.assertTrue(config.reload_local_configuration()) @patch('tempfile.mkstemp', Mock(return_value=[3000, 'blabla'])) @patch('os.path.exists', Mock(return_value=True)) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 1cff2611..71575e82 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -64,6 +64,7 @@ class TestPatroni(unittest.TestCase): del os.environ[Patroni.PATRONI_CONFIG_VARIABLE] @patch('patroni.config.Config.save_cache', Mock()) + @patch('patroni.config.Config.reload_local_configuration', Mock(return_value=True)) def test_run(self): self.p.sighup_handler() self.p.ha.dcs.watch = Mock(side_effect=SleepException)