From 4ca94a5dabd7500b6f67a9ad5b2cd7776baf2c7b Mon Sep 17 00:00:00 2001 From: jouir Date: Tue, 4 Jul 2017 16:14:17 +0200 Subject: [PATCH] Add config_dir option for configuration files location (#466) On debian, the configuration files (postgresql.conf, pg_hba.conf, etc) are not stored in the data directory. It would be great to be able to configure the location of this separate directory. Patroni could override existing configuration files where they are used to be. The default is to store configuration files in the data directory. This setting is targeting custom installations like debian and any others moving configuration files out of the data directory. Fixes #465 --- docs/ENVIRONMENT.rst | 1 + docs/SETTINGS.rst | 1 + patroni/postgresql.py | 35 ++++++++++++++++++++++------------- postgres0.yml | 1 + postgres1.yml | 1 + postgres2.yml | 1 + tests/test_config.py | 1 + tests/test_postgresql.py | 6 ++++-- 8 files changed, 32 insertions(+), 15 deletions(-) diff --git a/docs/ENVIRONMENT.rst b/docs/ENVIRONMENT.rst index c446b725..3a8494f6 100644 --- a/docs/ENVIRONMENT.rst +++ b/docs/ENVIRONMENT.rst @@ -46,6 +46,7 @@ PostgreSQL - **PATRONI\_POSTGRESQL\_LISTEN**: IP address + port that Postgres listens to. Multiple comma-separated addresses are permitted, as long as the port component is appended after to the last one with a colon, i.e. ``listen: 127.0.0.1,127.0.0.2:5432``. Patroni will use the first address from this list to establish local connections to the PostgreSQL node. - **PATRONI\_POSTGRESQL\_CONNECT\_ADDRESS**: IP address + port through which Postgres is accessible from other nodes and applications. - **PATRONI\_POSTGRESQL\_DATA\_DIR**: The location of the Postgres data directory, either existing or to be initialized by Patroni. +- **PATRONI\_POSTGRESQL\_CONFIG\_DIR**: The location of the Postgres configuration directory, defaults to the data directory. Must be writable by Patroni. - **PATRONI\_POSTGRESQL\_BIN_DIR**: Path to PostgreSQL binaries. (pg_ctl, pg_rewind, pg_basebackup, postgres) The default value is an empty string meaning that PATH environment variable will be used to find the executables. - **PATRONI\_POSTGRESQL\_PGPASS**: path to the `.pgpass `__ password file. Patroni creates this file before executing pg\_basebackup and under some other circumstances. The location must be writable by Patroni. - **PATRONI\_REPLICATION\_USERNAME**: replication username; the user will be created during initialization. Replicas will use this user to access master via streaming replication diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst index 02c66a46..fac8a90f 100644 --- a/docs/SETTINGS.rst +++ b/docs/SETTINGS.rst @@ -82,6 +82,7 @@ PostgreSQL - **connect\_address**: IP address + port through which Postgres is accessible from other nodes and applications. - **create\_replica\_methods**: an ordered list of the create methods for turning a Patroni node into a new replica. "basebackup" is the default method; other methods are assumed to refer to scripts, each of which is configured as its own config item. - **data\_dir**: The location of the Postgres data directory, either existing or to be initialized by Patroni. +- **config\_dir**: The location of the Postgres configuration directory, defaults to the data directory. Must be writable by Patroni. - **bin\_dir**: Path to PostgreSQL binaries. (pg_ctl, pg_rewind, pg_basebackup, postgres) The default value is an empty string meaning that PATH environment variable will be used to find the executables. - **listen**: IP address + port that Postgres listens to; must be accessible from other nodes in the cluster, if you're using streaming replication. Multiple comma-separated addresses are permitted, as long as the port component is appended after to the last one with a colon, i.e. ``listen: 127.0.0.1,127.0.0.2:5432``. Patroni will use the first address from this list to establish local connections to the PostgreSQL node. - **use\_unix\_socket**: specifies that Patroni should prefer to use unix sockets to connect to the cluster. Default value is ``false``. If ``unix_socket_directories`` is definded, Patroni will use first suitable value from it to connect to the cluster and fallback to tcp if nothing is suitable. If ``unix_socket_directories`` is not specified in ``postgresql.parameters``, Patroni will assume that default value should be used and omit ``host`` from connection parameters. diff --git a/patroni/postgresql.py b/patroni/postgresql.py index aa2956f9..fb3f9b2b 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -99,6 +99,7 @@ class Postgresql(object): self._bin_dir = config.get('bin_dir') or '' self._database = config.get('database', 'postgres') self._data_dir = config['data_dir'] + self._config_dir = config.get('config_dir') or self._data_dir self._pending_restart = False self.__thread_ident = current_thread().ident @@ -120,9 +121,9 @@ class Postgresql(object): self.__cb_called = False self.__cb_pending = None config_base_name = config.get('config_base_name', 'postgresql') - self._postgresql_conf = os.path.join(self._data_dir, config_base_name + '.conf') + self._postgresql_conf = os.path.join(self._config_dir, config_base_name + '.conf') self._postgresql_base_conf_name = config_base_name + '.base.conf' - self._postgresql_base_conf = os.path.join(self._data_dir, self._postgresql_base_conf_name) + self._postgresql_base_conf = os.path.join(self._config_dir, self._postgresql_base_conf_name) self._recovery_conf = os.path.join(self._data_dir, 'recovery.conf') self._postmaster_pid = os.path.join(self._data_dir, 'postmaster.pid') self._trigger_file = config.get('recovery_conf', {}).get('trigger_file') or 'promote' @@ -162,11 +163,11 @@ class Postgresql(object): @property def _configuration_to_save(self): - configuration = [self._postgresql_conf] + configuration = [os.path.basename(self._postgresql_conf)] if 'custom_conf' not in self.config: - configuration.append(self._postgresql_base_conf) + configuration.append(os.path.basename(self._postgresql_base_conf)) if not self.config['parameters'].get('hba_file'): - configuration.append(os.path.join(self._data_dir, 'pg_hba.conf')) + configuration.append('pg_hba.conf') return configuration @property @@ -826,8 +827,9 @@ class Postgresql(object): return False start_initiated = time.time() - proc = call_self(['pg_ctl_start', self._pgcommand('postgres'), '-D', self._data_dir] + options, - close_fds=True, preexec_fn=os.setsid, stdout=subprocess.PIPE, + proc = call_self(['pg_ctl_start', self._pgcommand('postgres'), '-D', self._data_dir, + '--config-file={}'.format(self._postgresql_conf)] + options, close_fds=True, + preexec_fn=os.setsid, stdout=subprocess.PIPE, env={p: os.environ[p] for p in ('PATH', 'LC_ALL', 'LANG') if p in os.environ}) pid = int(proc.stdout.readline().strip()) proc.wait() @@ -1060,6 +1062,9 @@ class Postgresql(object): with open(self._postgresql_conf, 'w') as f: f.write('# Do not edit this file manually!\n# It will be overwritten by Patroni!\n') f.write("include '{0}'\n\n".format(self.config.get('custom_conf') or self._postgresql_base_conf_name)) + f.write("data_directory = '{}'\n".format(self._data_dir)) + f.write("hba_file = '{}'\n".format(os.path.join(self._config_dir, 'pg_hba.conf'))) + f.write("ident_file = '{}'\n".format(os.path.join(self._config_dir, 'pg_ident.conf'))) for name, value in sorted(self._server_parameters.items()): f.write("{0} = '{1}'\n".format(name, value)) @@ -1070,7 +1075,7 @@ class Postgresql(object): return True def write_pg_hba(self, config): - with open(os.path.join(self._data_dir, 'pg_hba.conf'), 'a') as f: + with open(os.path.join(self._config_dir, 'pg_hba.conf'), 'a') as f: f.write('\n{}\n'.format('\n'.join(config))) def _replace_pg_hba(self): @@ -1081,7 +1086,7 @@ class Postgresql(object): :returns: True if pg_hba.conf was rewritten. """ if not self.config['parameters'].get('hba_file') and self.config.get('pg_hba'): - with open(os.path.join(self._data_dir, 'pg_hba.conf'), 'w') as f: + with open(os.path.join(self._config_dir, 'pg_hba.conf'), 'w') as f: f.write('# Do not edit this file manually!\n# It will be overwritten by Patroni!\n') for line in self.config['pg_hba']: f.write('{0}\n'.format(line)) @@ -1366,8 +1371,10 @@ class Postgresql(object): """ try: for f in self._configuration_to_save: - if os.path.isfile(f): - shutil.copy(f, f + '.backup') + config_file = os.path.join(self._config_dir, f) + backup_file = os.path.join(self._data_dir, f + '.backup') + if os.path.isfile(config_file): + shutil.copy(config_file, backup_file) except IOError: logger.exception('unable to create backup copies of configuration files') @@ -1375,8 +1382,10 @@ class Postgresql(object): """ restore a previously saved postgresql.conf """ try: for f in self._configuration_to_save: - if not os.path.isfile(f) and os.path.isfile(f + '.backup'): - shutil.copy(f + '.backup', f) + config_file = os.path.join(self._config_dir, f) + backup_file = os.path.join(self._data_dir, f + '.backup') + if not os.path.isfile(config_file) and os.path.isfile(backup_file): + shutil.copy(backup_file, config_file) except IOError: logger.exception('unable to restore configuration files from backup') diff --git a/postgres0.yml b/postgres0.yml index f44110da..1cb634d3 100644 --- a/postgres0.yml +++ b/postgres0.yml @@ -66,6 +66,7 @@ postgresql: connect_address: 127.0.0.1:5432 data_dir: data/postgresql0 # bin_dir: +# config_dir: pgpass: /tmp/pgpass0 authentication: replication: diff --git a/postgres1.yml b/postgres1.yml index 5405ab67..9c1d141b 100644 --- a/postgres1.yml +++ b/postgres1.yml @@ -64,6 +64,7 @@ postgresql: connect_address: 127.0.0.1:5433 data_dir: data/postgresql1 # bin_dir: +# config_dir: pgpass: /tmp/pgpass1 authentication: replication: diff --git a/postgres2.yml b/postgres2.yml index ec4a7c3a..b77f5bec 100644 --- a/postgres2.yml +++ b/postgres2.yml @@ -61,6 +61,7 @@ postgresql: connect_address: 127.0.0.1:5434 data_dir: data/postgresql2 # bin_dir: +# config_dir: pgpass: /tmp/pgpass2 authentication: replication: diff --git a/tests/test_config.py b/tests/test_config.py index 21d19dae..9e7ddafa 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -39,6 +39,7 @@ class TestConfig(unittest.TestCase): 'PATRONI_POSTGRESQL_LISTEN': '0.0.0.0:5432', 'PATRONI_POSTGRESQL_CONNECT_ADDRESS': '127.0.0.1:5432', 'PATRONI_POSTGRESQL_DATA_DIR': 'data/postgres0', + 'PATRONI_POSTGRESQL_CONFIG_DIR': 'data/postgres0', 'PATRONI_POSTGRESQL_PGPASS': '/tmp/pgpass0', 'PATRONI_ETCD_HOST': '127.0.0.1:2379', 'PATRONI_ETCD_URL': 'https://127.0.0.1:2379', diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index b2376d7b..81988bf4 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -168,9 +168,11 @@ class TestPostgresql(unittest.TestCase): @patch.object(Postgresql, 'is_running', Mock(return_value=True)) def setUp(self): self.data_dir = 'data/test0' + self.config_dir = self.data_dir if not os.path.exists(self.data_dir): os.makedirs(self.data_dir) - self.p = Postgresql({'name': 'test0', 'scope': 'batman', 'data_dir': self.data_dir, 'retry_timeout': 10, + self.p = Postgresql({'name': 'test0', 'scope': 'batman', 'data_dir': self.data_dir, + 'config_dir': self.config_dir, 'retry_timeout': 10, 'listen': '127.0.0.2, 127.0.0.3:5432', 'connect_address': '127.0.0.2:5432', 'authentication': {'superuser': {'username': 'test', 'password': 'test'}, 'replication': {'username': 'replicator', 'password': 'rep-pass'}}, @@ -502,7 +504,7 @@ class TestPostgresql(unittest.TestCase): config = {'users': {'replicator': {'password': 'rep-pass', 'options': ['replication']}}} self.p.bootstrap(config) - with open(os.path.join(self.data_dir, 'pg_hba.conf')) as f: + with open(os.path.join(self.config_dir, 'pg_hba.conf')) as f: lines = f.readlines() self.assertTrue('host all all 0.0.0.0/0 md5\n' in lines)