Implement reload of config.yml with REST API call

and acceptance tests for that

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

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):

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)

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())
 

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

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':

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))

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))

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)