Implemented patroni --version (#1291)

That required a refactoring of `Config` and `Patroni` classes. Now one has to explicitely create the instance of `Config` before creating `Patroni`.

The Config file can optionally call the validate function.

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:

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

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:

patroni/exceptions.py

@@ -27,3 +27,7 @@ class PostgresConnectionException(PostgresException):
 
 class WatchdogError(PatroniException):
     pass
+
+
+class ConfigParseError(PatroniException):
+    pass

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

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

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

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

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