From 78d3f2cac256f26139c0f9158ed344051ff89dbd Mon Sep 17 00:00:00 2001 From: Polina Bungina Date: Wed, 7 Dec 2022 09:50:54 +0100 Subject: [PATCH] Remove patronictl configure (#2475) * Remove patronictl configure command * Change name of the "secret" ENV variable (DCS->DCS_URL) and the corresponding patronictl option (to avoid mixing it up with the one from tests) --- patroni/ctl.py | 30 +++++++----------------------- tests/test_ctl.py | 40 +++++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 42 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 8d9b21f4..6c986bef 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -110,7 +110,7 @@ def parse_dcs(dcs): return yaml.safe_load(default['template'].format(host=parsed.hostname or 'localhost', port=port or default['port'])) -def load_config(path, dcs): +def load_config(path, dcs_url): from patroni.config import Config if not (os.path.exists(path) and os.access(path, os.R_OK)): @@ -123,22 +123,14 @@ def load_config(path, dcs): logging.debug('Loading configuration from file %s', path) config = Config(path, validator=None).copy() - dcs = parse_dcs(dcs) or parse_dcs(config.get('dcs_api')) or {} - if dcs: + dcs_url = parse_dcs(dcs_url) or {} + if dcs_url: for d in DCS_DEFAULTS: config.pop(d, None) - config.update(dcs) + config.update(dcs_url) return config -def store_config(config, path): - dir_path = os.path.dirname(path) - if dir_path and not os.path.isdir(dir_path): - os.makedirs(dir_path) - with open(path, 'w') as fd: - yaml.dump(config, fd) - - option_format = click.option('--format', '-f', 'fmt', help='Output format (pretty, tsv, json, yaml)', default='pretty') option_watchrefresh = click.option('-w', '--watch', type=float, help='Auto update the screen every X seconds') option_watch = click.option('-W', is_flag=True, help='Auto update the screen every 2 seconds') @@ -151,16 +143,16 @@ option_insecure = click.option('-k', '--insecure', is_flag=True, help='Allow con @click.group() @click.option('--config-file', '-c', help='Configuration file', envvar='PATRONICTL_CONFIG_FILE', default=CONFIG_FILE_PATH) -@click.option('--dcs', '-d', help='Use this DCS', envvar='DCS') +@click.option('--dcs-url', '--dcs', '-d', help='The DCS connect url', envvar='DCS_URL') @option_insecure @click.pass_context -def ctl(ctx, config_file, dcs, insecure): +def ctl(ctx, config_file, dcs_url, insecure): level = 'WARNING' for name in ('LOGLEVEL', 'PATRONI_LOGLEVEL', 'PATRONI_LOG_LEVEL'): level = os.environ.get(name, level) logging.basicConfig(format='%(asctime)s - %(levelname)s - %(message)s', level=level) logging.captureWarnings(True) # Capture eventual SSL warning - ctx.obj = load_config(config_file, dcs) + ctx.obj = load_config(config_file, dcs_url) # backward compatibility for configuration file where ctl section is not define ctx.obj.setdefault('ctl', {})['insecure'] = ctx.obj.get('ctl', {}).get('insecure') or insecure @@ -897,14 +889,6 @@ def timestamp(precision=6): return datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S.%f')[:precision - 7] -@ctl.command('configure', help='Create configuration file') -@click.option('--config-file', '-c', help='Configuration file', prompt='Configuration file', default=CONFIG_FILE_PATH) -@click.option('--dcs', '-d', help='The DCS connect url', prompt='DCS connect url', default='etcd://localhost:2379') -@click.option('--namespace', '-n', help='The namespace', prompt='Namespace', default='/service/') -def configure(config_file, dcs, namespace): - store_config({'dcs_api': str(dcs), 'namespace': str(namespace)}, config_file) - - def touch_member(config, dcs): ''' Rip-off of the ha.touch_member without inter-class dependencies ''' p = Postgresql(config['postgresql']) diff --git a/tests/test_ctl.py b/tests/test_ctl.py index cfd75b39..aadd0ef9 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -5,8 +5,8 @@ import unittest from click.testing import CliRunner from datetime import datetime, timedelta from mock import patch, Mock -from patroni.ctl import ctl, store_config, load_config, output_members, get_dcs, parse_dcs, \ - get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException, apply_config_changes, \ +from patroni.ctl import ctl, load_config, output_members, get_dcs, parse_dcs, \ + get_all_members, get_any_member, get_cursor, query_member, PatroniCtlException, apply_config_changes, \ format_config_for_editing, show_diff, invoke_editor, format_pg_version, CONFIG_FILE_PATH, PatronictlPrettyTable from patroni.dcs.etcd import AbstractEtcdClientWithFailover, Failover from patroni.psycopg import OperationalError @@ -20,17 +20,6 @@ from .test_ha import get_cluster_initialized_without_leader, get_cluster_initial get_cluster_initialized_with_only_leader, get_cluster_not_initialized_without_leader, get_cluster, Member -def test_rw_config(): - runner = CliRunner() - with runner.isolated_filesystem(): - load_config(CONFIG_FILE_PATH, None) - CONFIG_PATH = './test-ctl.yaml' - store_config({'etcd': {'host': 'localhost:2379'}}, CONFIG_PATH + '/dummy') - load_config(CONFIG_PATH + '/dummy', '0.0.0.0') - os.remove(CONFIG_PATH + '/dummy') - os.rmdir(CONFIG_PATH) - - @patch('patroni.ctl.load_config', Mock(return_value={ 'scope': 'alpha', 'restapi': {'listen': '::', 'certfile': 'a'}, 'etcd': {'host': 'localhost:2379'}, 'postgresql': {'data_dir': '.', 'pgpass': './pgpass', 'parameters': {}, 'retry_timeout': 5}})) @@ -43,11 +32,28 @@ class TestCtl(unittest.TestCase): self.runner = CliRunner() self.e = get_dcs({'etcd': {'ttl': 30, 'host': 'ok:2379', 'retry_timeout': 10}}, 'foo') - def test_load_config(self): + @patch('patroni.ctl.logging.debug') + def test_load_config(self, mock_logger_debug): runner = CliRunner() with runner.isolated_filesystem(): self.assertRaises(PatroniCtlException, load_config, './non-existing-config-file', None) - self.assertRaises(PatroniCtlException, load_config, './non-existing-config-file', None) + + with patch('os.path.exists', Mock(return_value=True)), \ + patch('patroni.config.Config.__init__', Mock(return_value=None)), \ + patch('patroni.config.Config.copy', Mock(return_value={})): + load_config(CONFIG_FILE_PATH, None) + mock_logger_debug.assert_called_once() + self.assertEqual(('Ignoring configuration file "%s". It does not exists or is not readable.', + CONFIG_FILE_PATH), + mock_logger_debug.call_args[0]) + mock_logger_debug.reset_mock() + + with patch('os.access', Mock(return_value=True)): + load_config(CONFIG_FILE_PATH, '') + mock_logger_debug.assert_called_once() + self.assertEqual(('Loading configuration from file %s', CONFIG_FILE_PATH), + mock_logger_debug.call_args[0]) + mock_logger_debug.reset_mock() @patch('patroni.psycopg.connect', psycopg_connect) def test_get_cursor(self): @@ -380,10 +386,6 @@ class TestCtl(unittest.TestCase): with patch('patroni.ctl.load_config', Mock(return_value={})): self.runner.invoke(ctl, ['list']) - def test_configure(self): - result = self.runner.invoke(configure, ['--dcs', 'abc', '-c', 'dummy', '-n', 'bla']) - assert result.exit_code == 0 - @patch('patroni.ctl.get_dcs') def test_scaffold(self, mock_get_dcs): mock_get_dcs.return_value = self.e