From 6cf63d1366d4807fe5ff88ae64b21a5876e192aa Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 16 Jun 2016 08:45:52 +0200 Subject: [PATCH 1/4] Implement `copy` method It returns copy of `effective_configuration`. Don't check that PATRONI_*_USERNAME and PATRONI_*_PASSWORD are set together. User may want to set only PASSWORD. --- patroni/config.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/patroni/config.py b/patroni/config.py index e749612e..ce81de17 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -66,8 +66,7 @@ class Config(object): format(self.PATRONI_CONFIG_VARIABLE)) exit(1) - self.__effective_configuration = self._build_effective_configuration(self._dynamic_configuration, - self._local_configuration) + self.__effective_configuration = self._build_effective_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() @@ -202,7 +201,7 @@ class Config(object): value = _popenv(name + '_' + param) if value: ret[param] = value - return len(ret) == 2 and ret or None + return ret restapi_auth = _get_auth('restapi') if restapi_auth: @@ -306,3 +305,6 @@ class Config(object): def __getitem__(self, key): return self.__effective_configuration[key] + + def copy(self): + return deepcopy(self.__effective_configuration) From c1b6f1ef24eea6859b41ee892b3db4e3eb2f9573 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 16 Jun 2016 08:48:49 +0200 Subject: [PATCH 2/4] Make list of available dcs implementations unique. And exclude AbstractDCS from it. --- patroni/dcs/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index c068ef8e..9662a708 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -31,7 +31,7 @@ def parse_connection_string(value): def get_dcs(config): - available_implementations = [] + available_implementations = set() for name in os.listdir(os.path.dirname(__file__)): if name.endswith('.py') and not name.startswith('__'): # find module module = importlib.import_module(__package__ + '.' + name[:-3]) @@ -40,8 +40,8 @@ def get_dcs(config): value = getattr(module, name) name = name.lower() # try to find implementation of AbstractDCS interface - if inspect.isclass(value) and issubclass(value, AbstractDCS): - available_implementations.append(name) + if inspect.isclass(value) and issubclass(value, AbstractDCS) and value != AbstractDCS: + available_implementations.add(name) if name in config: # which has configuration section in the config file # propagate some parameters config[name].update({p: config[p] for p in ('namespace', 'name', From bd6070e2b02ecb4b6a42649d4f63476106cdeeef Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 16 Jun 2016 08:50:44 +0200 Subject: [PATCH 3/4] Make patronictl use config.py for loading config_file config.py is not only loading config_file but also can build configuration from environment variables. --- patroni/ctl.py | 23 +++++++++++++++++------ tests/test_ctl.py | 27 ++++++--------------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 499f6563..2352aace 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -11,11 +11,13 @@ import os import psycopg2 import random import requests +import sys import time import tzlocal import yaml from click import ClickException +from patroni.config import Config from patroni.dcs import get_dcs as _get_dcs from patroni.exceptions import PatroniException from patroni.postgresql import parseurl @@ -56,14 +58,23 @@ def parse_dcs(dcs): def load_config(path, dcs): logging.debug('Loading configuration from file %s', path) - config = dict() + config = {} + old_argv = list(sys.argv) try: - with open(path, 'rb') as fd: - config = yaml.safe_load(fd) - except (IOError, yaml.YAMLError): - logging.exception('Could not load configuration file') + sys.argv[1] = path + if Config.PATRONI_CONFIG_VARIABLE not in os.environ: + for p in ('PATRONI_RESTAPI_LISTEN', 'PATRONI_POSTGRESQL_DATA_DIR'): + if p not in os.environ: + os.environ[p] = '.' + config = Config().copy() + finally: + sys.argv = old_argv - config.update(parse_dcs(dcs) or parse_dcs(config.get('dcs_api')) or {}) + dcs = parse_dcs(dcs) or parse_dcs(config.get('dcs_api')) or {} + if dcs: + for d in DCS_DEFAULTS: + config.pop(d, None) + config.update(dcs) return config diff --git a/tests/test_ctl.py b/tests/test_ctl.py index eb63e153..8742561a 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -1,7 +1,7 @@ import etcd import os -import pytest -import requests.exceptions +import requests +import sys import unittest from click.testing import CliRunner @@ -19,29 +19,14 @@ CONFIG_FILE_PATH = './test-ctl.yaml' def test_rw_config(): runner = CliRunner() - config = {'a': 'b'} with runner.isolated_filesystem(): - store_config(config, CONFIG_FILE_PATH + '/dummy') + store_config({'etcd': {'host': 'localhost:2379'}}, CONFIG_FILE_PATH + '/dummy') + sys.argv = ['patronictl.py', ''] + load_config(CONFIG_FILE_PATH + '/dummy', None) + load_config(CONFIG_FILE_PATH + '/dummy', '0.0.0.0') os.remove(CONFIG_FILE_PATH + '/dummy') os.rmdir(CONFIG_FILE_PATH) - with pytest.raises(Exception): - result = load_config(CONFIG_FILE_PATH, None) - assert 'Could not load configuration file' in result.output - - os.mkdir(CONFIG_FILE_PATH) - with pytest.raises(Exception): - store_config(config, CONFIG_FILE_PATH) - - os.rmdir(CONFIG_FILE_PATH) - - store_config(config, CONFIG_FILE_PATH) - load_config(CONFIG_FILE_PATH, None) - load_config(CONFIG_FILE_PATH, '0.0.0.0') - - store_config({'dcs_api': None}, CONFIG_FILE_PATH) - load_config(CONFIG_FILE_PATH, None) - @patch('patroni.ctl.load_config', Mock(return_value={'etcd': {'host': 'localhost:4001'}})) class TestCtl(unittest.TestCase): From fe3a999cb27f99e6a47c310db6808b9771827ab5 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 16 Jun 2016 10:26:06 +0200 Subject: [PATCH 4/4] Enforce name requirements for dcs implementations Class implementing AbstractDCS must have name similar to the module name. I.e. Patroni will load ZooKeeper from zookeeper.py, but not from exhibitor.py, although it (ZooKeeper) is also available there. --- patroni/dcs/__init__.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index 9662a708..c1c31174 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -32,21 +32,21 @@ def parse_connection_string(value): def get_dcs(config): available_implementations = set() - for name in os.listdir(os.path.dirname(__file__)): - if name.endswith('.py') and not name.startswith('__'): # find module - module = importlib.import_module(__package__ + '.' + name[:-3]) - for name in dir(module): # iterate through module content - if not name.startswith('__'): # skip internal stuff - value = getattr(module, name) - name = name.lower() - # try to find implementation of AbstractDCS interface - if inspect.isclass(value) and issubclass(value, AbstractDCS) and value != AbstractDCS: - available_implementations.add(name) - if name in config: # which has configuration section in the config file - # propagate some parameters - config[name].update({p: config[p] for p in ('namespace', 'name', - 'scope', 'ttl', 'retry_timeout') if p in config}) - return value(config[name]) + for module in os.listdir(os.path.dirname(__file__)): + if module.endswith('.py') and not module.startswith('__'): # find module + module_name = module[:-3].lower() + module = importlib.import_module(__package__ + '.' + module[:-3]) + for name in filter(lambda name: not name.startswith('__'), dir(module)): # iterate through module content + value = getattr(module, name) + name = name.lower() + # try to find implementation of AbstractDCS interface, class name must match with module_name + if inspect.isclass(value) and issubclass(value, AbstractDCS) and name == module_name: + available_implementations.add(name) + if name in config: # which has configuration section in the config file + # propagate some parameters + config[name].update({p: config[p] for p in ('namespace', 'name', + 'scope', 'ttl', 'retry_timeout') if p in config}) + return value(config[name]) raise PatroniException("""Can not find suitable configuration of distributed configuration store Available implementations: """ + ', '.join(available_implementations))