Fix little issues with custom bootstrap (#1891)

1. Set hot_standby=off only when we do PITR
2. Restart postgres after PITR is done to avoid warnings
3. Address invalid config issue https://github.com/zalando/patroni/issues/1870#issuecomment-800088643

patroni/postgresql/bootstrap.py

@@ -90,8 +90,8 @@ class Bootstrap(object):
         self._postgresql.configure_server_parameters()
 
         # make sure there is no trigger file or postgres will be automatically promoted
-        trigger_file = 'promote_trigger_file' if self._postgresql.major_version >= 120000 else 'trigger_file'
-        trigger_file = self._postgresql.config.get('recovery_conf', {}).get(trigger_file) or 'promote'
+        trigger_file = self._postgresql.config.triggerfile_good_name
+        trigger_file = (self._postgresql.config.get('recovery_conf') or {}).get(trigger_file) or 'promote'
         trigger_file = os.path.abspath(os.path.join(self._postgresql.data_dir, trigger_file))
         if os.path.exists(trigger_file):
             os.unlink(trigger_file)

patroni/postgresql/config.py

@@ -388,12 +388,6 @@ class ConfigHandler(object):
             os.rename(self._postgresql_conf, self._postgresql_base_conf)
 
         configuration = configuration or self._server_parameters.copy()
-
-        # In case we are using custom bootstrap from spilo image with PITR it fails if it contains increasing
-        # values like Max_connections. We disable hot_standby so it will accept increasing values.
-        if self._postgresql.bootstrap.running_custom_bootstrap:
-            configuration['hot_standby'] = 'off'
-
         # Due to the permanent logical replication slots configured we have to enable hot_standby_feedback
         if self._postgresql.enforce_hot_standby_feedback:
             configuration['hot_standby_feedback'] = 'on'
@@ -559,7 +553,7 @@ class ConfigHandler(object):
         return os.path.exists(self._recovery_conf)
 
     @property
-    def _triggerfile_good_name(self):
+    def triggerfile_good_name(self):
         return 'trigger_file' if self._postgresql.major_version < 120000 else 'promote_trigger_file'
 
     @property
@@ -762,13 +756,13 @@ class ConfigHandler(object):
         return env
 
     def write_recovery_conf(self, recovery_params):
+        self._recovery_params = recovery_params
         if self._postgresql.major_version >= 120000:
             if parse_bool(recovery_params.pop('standby_mode', None)):
                 open(self._standby_signal, 'w').close()
             else:
                 self._remove_file_if_exists(self._standby_signal)
                 open(self._recovery_signal, 'w').close()
-            self._recovery_params = recovery_params
         else:
             with ConfigWriter(self._recovery_conf) as f:
                 os.chmod(self._recovery_conf, stat.S_IWRITE | stat.S_IREAD)
@@ -812,8 +806,8 @@ class ConfigHandler(object):
 
         if self.get('recovery_conf'):
             value = self._config['recovery_conf'].pop(self._triggerfile_wrong_name, None)
-            if self._triggerfile_good_name not in self._config['recovery_conf'] and value:
-                self._config['recovery_conf'][self._triggerfile_good_name] = value
+            if self.triggerfile_good_name not in self._config['recovery_conf'] and value:
+                self._config['recovery_conf'][self.triggerfile_good_name] = value
 
     def get_server_parameters(self, config):
         parameters = config['parameters'].copy()
@@ -1073,6 +1067,14 @@ class ConfigHandler(object):
             if cvalue > value:
                 effective_configuration[name] = cvalue
                 self._postgresql.set_pending_restart(True)
+
+        # If we are using custom bootstrap with PITR it could fail when values
+        # like max_connections are increased, therefore we disable hot_standby.
+        if self._postgresql.bootstrap.running_custom_bootstrap and \
+                (self._postgresql.bootstrap.keep_existing_recovery_conf or self._recovery_conf):
+            effective_configuration['hot_standby'] = 'off'
+            self._postgresql.set_pending_restart(True)
+
         return effective_configuration
 
     @property

tests/test_postgresql.py

@@ -11,6 +11,7 @@ from patroni.async_executor import CriticalTask
 from patroni.dcs import Cluster, RemoteMember, SyncState
 from patroni.exceptions import PostgresConnectionException, PatroniException
 from patroni.postgresql import Postgresql, STATE_REJECT, STATE_NO_RESPONSE
+from patroni.postgresql.bootstrap import Bootstrap
 from patroni.postgresql.postmaster import PostmasterProcess
 from patroni.utils import RetryFailedError
 from six.moves import builtins
@@ -684,6 +685,8 @@ class TestPostgresql(BaseTestPostgresql):
         self.assertEqual(self.p.get_master_timeline(), 1)
 
     @patch.object(Postgresql, 'get_postgres_role_from_data_directory', Mock(return_value='replica'))
+    @patch.object(Bootstrap, 'running_custom_bootstrap', PropertyMock(return_value=True))
+    @patch.object(Bootstrap, 'keep_existing_recovery_conf', PropertyMock(return_value=True))
     def test__build_effective_configuration(self):
         with patch.object(Postgresql, 'controldata',
                           Mock(return_value={'max_connections setting': '200',