diff --git a/patroni/__init__.py b/patroni/__init__.py index 4534ad9d..f0e06df4 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -4,6 +4,8 @@ import signal import sys import time +from patroni.version import __version__ + logger = logging.getLogger(__name__) PATRONI_ENV_PREFIX = 'PATRONI_' @@ -11,9 +13,8 @@ PATRONI_ENV_PREFIX = 'PATRONI_' class Patroni(object): - def __init__(self): + def __init__(self, conf): from patroni.api import RestApiServer - from patroni.config import Config from patroni.dcs import get_dcs from patroni.ha import Ha from patroni.log import PatroniLogger @@ -26,7 +27,7 @@ class Patroni(object): self.version = __version__ self.logger = PatroniLogger() - self.config = Config() + self.config = conf self.logger.reload_config(self.config.get('log', {})) self.dcs = get_dcs(self.config) self.watchdog = Watchdog(self.config) @@ -164,7 +165,23 @@ class Patroni(object): def patroni_main(): - patroni = Patroni() + import argparse + from patroni.config import Config, ConfigParseError + + parser = argparse.ArgumentParser() + parser.add_argument('--version', action='version', version='%(prog)s {0}'.format(__version__)) + parser.add_argument('configfile', nargs='?', default='', + help='Patroni may also read the configuration from the {0} environment variable' + .format(Config.PATRONI_CONFIG_VARIABLE)) + args = parser.parse_args() + try: + conf = Config(args.configfile) + except ConfigParseError as e: + if e.value: + print(e.value) + parser.print_help() + sys.exit(1) + patroni = Patroni(conf) try: patroni.run() except KeyboardInterrupt: diff --git a/patroni/config.py b/patroni/config.py index 2bf6cdd7..6d8e740e 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -2,13 +2,13 @@ import json import logging import os import shutil -import sys import tempfile import yaml from collections import defaultdict from copy import deepcopy from patroni import PATRONI_ENV_PREFIX +from patroni.exceptions import ConfigParseError from patroni.dcs import ClusterConfig from patroni.postgresql.config import CaseInsensitiveDict, ConfigHandler from patroni.utils import deep_compare, parse_bool, parse_int, patch_config @@ -26,6 +26,11 @@ _AUTH_ALLOWED_PARAMETERS = ( ) +def default_validator(conf): + if not conf: + return "Config is empty." + + class Config(object): """ This class is responsible for: @@ -75,27 +80,26 @@ class Config(object): } } - def __init__(self): + def __init__(self, configfile, validator=default_validator): self._modify_index = -1 self._dynamic_configuration = {} self.__environment_configuration = self._build_environment_configuration() # Patroni reads the configuration from the command-line argument if it exists, otherwise from the environment - self._config_file = len(sys.argv) >= 2 and os.path.isfile(sys.argv[1]) and sys.argv[1] + self._config_file = configfile and os.path.isfile(configfile) and configfile if self._config_file: self._local_configuration = self._load_config_file() else: config_env = os.environ.pop(self.PATRONI_CONFIG_VARIABLE, None) self._local_configuration = config_env and yaml.safe_load(config_env) or self.__environment_configuration - if not self._local_configuration: - print('Usage: {0} config.yml'.format(sys.argv[0])) - print('\tPatroni may also read the configuration from the {0} environment variable'. - format(self.PATRONI_CONFIG_VARIABLE)) - sys.exit(1) + if validator: + error = validator(self._local_configuration) + if error: + raise ConfigParseError(error) self.__effective_configuration = self._build_effective_configuration({}, self._local_configuration) - self._data_dir = self.__effective_configuration['postgresql']['data_dir'] + self._data_dir = self.__effective_configuration.get('postgresql', {}).get('data_dir', "") self._cache_file = os.path.join(self._data_dir, self.__CACHE_FILENAME) self._load_cache() self._cache_needs_saving = False @@ -340,7 +344,7 @@ class Config(object): config[name] = deepcopy(value) if value else {} # restapi server expects to get restapi.auth = 'username:password' - if 'authentication' in config['restapi']: + if 'restapi' in config and 'authentication' in config['restapi']: config['restapi']['auth'] = '{username}:{password}'.format(**config['restapi']['authentication']) # special treatment for old config diff --git a/patroni/ctl.py b/patroni/ctl.py index f5fa0f95..ead8173d 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -72,17 +72,7 @@ def load_config(path, dcs): logging.debug('Ignoring configuration file "%s". It does not exists or is not readable.', path) else: logging.debug('Loading configuration from file %s', path) - config = {} - old_argv = list(sys.argv) - try: - 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 = Config(path, validator=None).copy() dcs = parse_dcs(dcs) or parse_dcs(config.get('dcs_api')) or {} if dcs: diff --git a/patroni/exceptions.py b/patroni/exceptions.py index 4601f093..43980721 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -27,3 +27,7 @@ class PostgresConnectionException(PostgresException): class WatchdogError(PatroniException): pass + + +class ConfigParseError(PatroniException): + pass diff --git a/tests/test_config.py b/tests/test_config.py index a60046f2..b7bf9aba 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -15,10 +15,7 @@ class TestConfig(unittest.TestCase): def setUp(self): sys.argv = ['patroni.py'] os.environ[Config.PATRONI_CONFIG_VARIABLE] = 'restapi: {}\npostgresql: {data_dir: foo}' - self.config = Config() - - def test_no_config(self): - self.assertRaises(SystemExit, Config) + self.config = Config(None) def test_set_dynamic_configuration(self): with patch.object(Config, '_build_effective_configuration', Mock(side_effect=Exception)): @@ -66,8 +63,7 @@ class TestConfig(unittest.TestCase): 'PATRONI_admin_PASSWORD': 'admin', 'PATRONI_admin_OPTIONS': 'createrole,createdb' }) - sys.argv = ['patroni.py', 'postgres0.yml'] - config = Config() + config = Config('postgres0.yml') with patch.object(Config, '_load_config_file', Mock(return_value={'restapi': {}})): with patch.object(Config, '_build_effective_configuration', Mock(side_effect=Exception)): config.reload_local_configuration() diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 10b40433..be858262 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -25,7 +25,6 @@ CONFIG_FILE_PATH = './test-ctl.yaml' def test_rw_config(): runner = CliRunner() with runner.isolated_filesystem(): - sys.argv = ['patronictl.py', ''] load_config(CONFIG_FILE_PATH + '/dummy', None) store_config({'etcd': {'host': 'localhost:2379'}}, CONFIG_FILE_PATH + '/dummy') load_config(CONFIG_FILE_PATH + '/dummy', '0.0.0.0') diff --git a/tests/test_ha.py b/tests/test_ha.py index db0e0b33..01cc52ce 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -122,7 +122,7 @@ zookeeper: # all the extra values that are coming from py.test sys.argv = sys.argv[:1] - self.config = Config() + self.config = Config(None) self.config.set_dynamic_configuration({'maximum_lag_on_failover': 5}) self.version = '1.5.7' self.postgresql = p diff --git a/tests/test_log.py b/tests/test_log.py index b86ae4cf..5733ed6f 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -35,7 +35,7 @@ class TestPatroniLogger(unittest.TestCase): sys.argv = ['patroni.py'] os.environ[Config.PATRONI_CONFIG_VARIABLE] = yaml.dump(config, default_flow_style=False) logger = PatroniLogger() - patroni_config = Config() + patroni_config = Config(None) logger.reload_config(patroni_config['log']) logger.start() diff --git a/tests/test_patroni.py b/tests/test_patroni.py index a53fb354..9e36de14 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -5,6 +5,7 @@ import signal import time import unittest +import patroni.config as config from mock import Mock, PropertyMock, patch from patroni.api import RestApiServer from patroni.async_executor import AsyncExecutor @@ -40,7 +41,9 @@ class MockFrozenImporter(object): @patch.object(etcd.Client, 'read', etcd_read) class TestPatroni(unittest.TestCase): - @patch('sys.argv', ['patroni.py', 'postgres0.yml']) + def test_no_config(self): + self.assertRaises(SystemExit, patroni_main) + @patch('pkgutil.get_importer', Mock(return_value=MockFrozenImporter())) @patch('sys.frozen', Mock(return_value=True), create=True) @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock()) @@ -53,7 +56,8 @@ class TestPatroni(unittest.TestCase): RestApiServer._BaseServer__shutdown_request = True RestApiServer.socket = 0 os.environ['PATRONI_POSTGRESQL_DATA_DIR'] = 'data/test0' - self.p = Patroni() + conf = config.Config('postgres0.yml') + self.p = Patroni(conf) def tearDown(self): logging.getLogger().handlers[:] = self._handlers