diff --git a/patroni/api.py b/patroni/api.py index 0ca7f2e3..cd2023c2 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -253,7 +253,7 @@ class RestApiHandler(BaseHTTPRequestHandler): return {'tags': self.server.patroni.tags} def log_message(self, fmt, *args): - logger.debug("API thread: " + fmt % args) + logger.debug("API thread: %s - - [%s] %s", self.client_address[0], self.log_date_time_string() + fmt % args) class RestApiServer(ThreadingMixIn, HTTPServer, Thread): diff --git a/patroni/ctl.py b/patroni/ctl.py index 12cc443e..fb4c241b 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -61,7 +61,7 @@ def load_config(path, dcs): try: with open(path, 'rb') as fd: config = yaml.safe_load(fd) - except: + except (IOError, yaml.YAMLError): logging.exception('Could not load configuration file') if dcs: @@ -81,7 +81,7 @@ def store_config(config, path): option_config_file = click.option('--config-file', '-c', help='Configuration file', default=CONFIG_FILE_PATH) -option_format = click.option('--format', '-f', help='Output format (pretty, json)', default='pretty') +option_format = click.option('--format', '-f', 'fmt', help='Output format (pretty, json)', default='pretty') option_dcs = click.option('--dcs', '-d', help='Use this DCS', envvar='DCS') 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') @@ -114,9 +114,9 @@ def post_patroni(member, endpoint, content, headers=None): data=json.dumps(content), timeout=60) -def print_output(columns, rows=None, alignment=None, format='pretty', header=True, delimiter='\t'): +def print_output(columns, rows=None, alignment=None, fmt='pretty', header=True, delimiter='\t'): rows = rows or [] - if format == 'pretty': + if fmt == 'pretty': t = PrettyTable(columns) for k, v in (alignment or {}).items(): t.align[k] = v @@ -125,14 +125,14 @@ def print_output(columns, rows=None, alignment=None, format='pretty', header=Tru click.echo(t) return - if format == 'json': + if fmt == 'json': elements = list() for r in rows: elements.append(dict(zip(columns, r))) click.echo(json.dumps(elements)) - if format == 'tsv': + if fmt == 'tsv': if columns is not None and header: click.echo(delimiter.join(columns) + '\n') @@ -251,8 +251,8 @@ def dsn(cluster_name, config_file, dcs, role, member): @click.argument('cluster_name') @option_config_file @option_format -@click.option('--format', help='Output format (pretty, json)', default='tsv') -@click.option('--file', '-f', help='Execute the SQL commands from this file', type=click.File('rb')) +@click.option('--format', 'fmt', help='Output format (pretty, json)', default='tsv') +@click.option('--file', '-f', 'p_file', help='Execute the SQL commands from this file', type=click.File('rb')) @click.option('--password', help='force password prompt', is_flag=True) @click.option('-U', '--username', help='database user name', type=str) @option_dcs @@ -274,21 +274,21 @@ def query( watch, delimiter, command, - file, + p_file, password, username, dbname, - format='tsv', + fmt='tsv', ): if role is not None and member is not None: raise PatroniCtlException('--role and --member are mutually exclusive options') if member is None and role is None: role = 'master' - if file is not None and command is not None: + if p_file is not None and command is not None: raise PatroniCtlException('--file and --command are mutually exclusive options') - if file is None and command is None: + if p_file is None and command is None: raise PatroniCtlException('You need to specify either --command or --file') connect_parameters = dict() @@ -299,8 +299,8 @@ def query( if dbname: connect_parameters['database'] = dbname - if file is not None: - command = file.read() + if p_file is not None: + command = p_file.read() config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs) @@ -309,7 +309,7 @@ def query( output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, connect_parameters=connect_parameters) - print_output(None, output, format=format, delimiter=delimiter) + print_output(None, output, fmt=fmt, delimiter=delimiter) if cursor is None: cluster = dcs.get_cluster() @@ -351,13 +351,13 @@ def query_member(cluster, cursor, member, role, command, connect_parameters=None @option_config_file @option_format @option_dcs -def remove(config_file, cluster_name, format, dcs): +def remove(config_file, cluster_name, fmt, dcs): config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs) if not isinstance(dcs, Etcd): raise PatroniCtlException('We have not implemented this for DCS of type {0}'.format(type(dcs))) - output_members(cluster, format=format) + output_members(cluster, fmt=fmt) confirm = click.prompt('Please confirm the cluster name to remove', type=str) if confirm != cluster_name: @@ -431,11 +431,11 @@ def ctl_load_config(cluster_name, config_file, dcs): @click.argument('member_names', nargs=-1) @click.option('--role', '-r', help='Restart only members with this role', default='any', type=click.Choice(['master', 'replica', 'any'])) -@click.option('--any', help='Restart a single member only', is_flag=True) +@click.option('--any', 'p_any', help='Restart a single member only', is_flag=True) @option_config_file @option_force @option_dcs -def restart(cluster_name, member_names, config_file, dcs, force, role, any): +def restart(cluster_name, member_names, config_file, dcs, force, role, p_any): config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs) role_names = [m.name for m in get_all_members(cluster=cluster, role=role)] @@ -445,7 +445,7 @@ def restart(cluster_name, member_names, config_file, dcs, force, role, any): else: member_names = role_names - if any: + if p_any: random.shuffle(member_names) member_names = member_names[:1] @@ -552,7 +552,7 @@ def failover(config_file, cluster_name, master, candidate, force, dcs): output_members(cluster, name=cluster_name) -def output_members(cluster, name=None, format='pretty'): +def output_members(cluster, name=None, fmt='pretty'): rows = [] logging.debug(cluster) leader_name = None @@ -597,7 +597,7 @@ def output_members(cluster, name=None, format='pretty'): ] alignment = {'Cluster': 'l', 'Member': 'l', 'Host': 'l', 'Lag in MB': 'r'} - print_output(columns, rows, alignment, format) + print_output(columns, rows, alignment, fmt) @ctl.command('list', help='List the Patroni members for a given Patroni') @@ -607,7 +607,7 @@ def output_members(cluster, name=None, format='pretty'): @option_watch @option_watchrefresh @option_dcs -def members(config_file, cluster_names, format, watch, w, dcs): +def members(config_file, cluster_names, fmt, watch, w, dcs): if not cluster_names: logging.warning('Listing members: No cluster names were provided') return @@ -617,7 +617,7 @@ def members(config_file, cluster_names, format, watch, w, dcs): dcs = get_dcs(config, cn) for _ in watching(w, watch): - output_members(dcs.get_cluster(), name=cn, format=format) + output_members(dcs.get_cluster(), name=cn, fmt=fmt) def timestamp(precision=6): diff --git a/patroni/scripts/wale_restore.py b/patroni/scripts/wale_restore.py index 190d00ad..6513ef2d 100755 --- a/patroni/scripts/wale_restore.py +++ b/patroni/scripts/wale_restore.py @@ -107,23 +107,18 @@ class WALERestore(object): # construct the LSN from the segment and offset backup_start_lsn = '{0}/{1}'.format(lsn_segment, lsn_offset) - conn = None - cursor = None diff_in_bytes = long(backup_size) if not self.no_master: try: # get the difference in bytes between the current WAL location and the backup start offset - conn = psycopg2.connect(self.master_connection) - conn.autocommit = True - cursor = conn.cursor() - cursor.execute("SELECT pg_xlog_location_diff(pg_current_xlog_location(), %s)", (backup_start_lsn,)) - diff_in_bytes = long(cursor.fetchone()[0]) + with psycopg2.connect(self.master_connection) as con: + con.autocommit = True + with con.cursor() as cur: + cur.execute("SELECT pg_xlog_location_diff(pg_current_xlog_location(), %s)", (backup_start_lsn,)) + diff_in_bytes = long(cur.fetchone()[0]) except psycopg2.Error as e: - logger.error('could not determine difference with the master location: {}'.format(e)) + logger.error('could not determine difference with the master location: %s', e) return False - finally: - cursor and cursor.close() - conn and conn.close() else: # always try to use WAL-E if base backup is available diff_in_bytes = 0 diff --git a/patronictl.py b/patronictl.py index 5b06c153..50e65c87 100755 --- a/patronictl.py +++ b/patronictl.py @@ -2,4 +2,4 @@ from patroni.ctl import ctl if __name__ == '__main__': - ctl() + ctl(None) diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 868a1442..fb63aebd 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -53,6 +53,7 @@ class TestCtl(unittest.TestCase): @patch('socket.getaddrinfo', socket_getaddrinfo) def setUp(self): + self.runner = CliRunner() with patch.object(Client, 'machines') as mock_machines: mock_machines.__get__ = Mock(return_value=['http://remotehost:2379']) self.p = MockPostgresql() @@ -83,9 +84,9 @@ class TestCtl(unittest.TestCase): def test_output_members(self): cluster = get_cluster_initialized_with_leader() - output_members(cluster, name='abc', format='pretty') - output_members(cluster, name='abc', format='json') - output_members(cluster, name='abc', format='tsv') + output_members(cluster, name='abc', fmt='pretty') + output_members(cluster, name='abc', fmt='json') + output_members(cluster, name='abc', fmt='tsv') @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) @patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None)) @@ -95,49 +96,47 @@ class TestCtl(unittest.TestCase): @patch('requests.post', requests_get) @patch('patroni.ctl.post_patroni', Mock(return_value=MockResponse())) def test_failover(self): - runner = CliRunner() - with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())): - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') assert 'Failing over to new leader' in result.output - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other N''') assert 'Aborting failover' in str(result.output) - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader leader y''') assert 'target and source are the same' in str(result.output) - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader Reality y''') assert 'Reality does not exist' in str(result.output) - result = runner.invoke(ctl, ['failover', 'dummy', '--force']) + result = self.runner.invoke(ctl, ['failover', 'dummy', '--force']) assert 'Failing over to new leader' in result.output - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='dummy') + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='dummy') assert 'is not the leader of cluster' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_only_leader())): - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') assert 'No candidates found to failover to' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_without_leader())): - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') assert 'This cluster has no master' in str(result.output) with patch('patroni.ctl.post_patroni', Mock(side_effect=Exception())): - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') assert 'falling back to DCS' in result.output @@ -146,27 +145,27 @@ y''') mocked = Mock() mocked.return_value.status_code = 500 with patch('patroni.ctl.post_patroni', Mock(return_value=mocked)): - result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader + result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') assert 'Failover failed, details' in result.output # with patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())): -# result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='nonsense') +# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='nonsense') # assert 'is not the leader of cluster' in str(result.output) - - # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8', '--master', 'nonsense']) - # assert 'is not the leader of cluster' in str(result.output) - - # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nn') - # assert 'Aborting failover' in str(result.output) - - # with patch('patroni.ctl.wait_for_leader', Mock(return_value = get_cluster_initialized_with_leader())): - # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY') - # assert 'master did not change after' in result.output - - # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY') - # assert 'Failover failed' in result.output +# +# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8', '--master', 'nonsense']) +# assert 'is not the leader of cluster' in str(result.output) +# +# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nn') +# assert 'Aborting failover' in str(result.output) +# +# with patch('patroni.ctl.wait_for_leader', Mock(return_value = get_cluster_initialized_with_leader())): +# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY') +# assert 'master did not change after' in result.output +# +# result = self.runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY') +# assert 'Failover failed' in result.output def test_(self): self.assertRaises(patroni.exceptions.PatroniCtlException, get_dcs, {'scheme': 'dummy'}, 'dummy') @@ -174,10 +173,8 @@ y''') @patch('psycopg2.connect', psycopg2_connect) @patch('patroni.ctl.query_member', Mock(return_value=([['mock column']], None))) def test_query(self): - runner = CliRunner() - with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)): - result = runner.invoke(ctl, [ + result = self.runner.invoke(ctl, [ 'query', 'alpha', '--member', @@ -187,23 +184,23 @@ y''') ]) assert 'mutually exclusive' in str(result.output) - with runner.isolated_filesystem(): + with self.runner.isolated_filesystem(): with open('dummy', 'w') as dummy_file: dummy_file.write('SELECT 1') - result = runner.invoke(ctl, [ + result = self.runner.invoke(ctl, [ 'query', 'alpha' ]) assert 'You need to specify' in str(result.output) - result = runner.invoke(ctl, [ + result = self.runner.invoke(ctl, [ 'query', 'alpha' ]) assert 'You need to specify' in str(result.output) - result = runner.invoke(ctl, [ + result = self.runner.invoke(ctl, [ 'query', 'alpha', '--file', @@ -213,15 +210,15 @@ y''') ]) assert 'mutually exclusive' in str(result.output) - result = runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy']) + result = self.runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy']) os.remove('dummy') - result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1']) + result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1']) assert 'mock column' in result.output - result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--dbname', 'dummy', - '--password', '--username', 'dummy'], input='password\n') + result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--dbname', 'dummy', + '--password', '--username', 'dummy'], input='password\n') assert 'mock column' in result.output @patch('patroni.ctl.get_cursor', Mock(return_value=MockConnect().cursor())) @@ -247,13 +244,11 @@ y''') @patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) def test_dsn(self): - runner = CliRunner() - with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)): - result = runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8']) + result = self.runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8']) assert 'host=127.0.0.1 port=5435' in result.output - result = runner.invoke(ctl, [ + result = self.runner.invoke(ctl, [ 'dsn', 'alpha', '--role', @@ -263,10 +258,10 @@ y''') ]) assert 'mutually exclusive' in str(result.output) - result = runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy']) + result = self.runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy']) assert 'Can not find' in str(result.output) - # result = runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8', '--role', 'replica']) + # result = self.runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8', '--role', 'replica']) # assert 'host=127.0.0.1 port=5436' in result.output @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) @@ -274,13 +269,11 @@ y''') @patch('requests.get', requests_get) @patch('requests.post', requests_get) def test_restart_reinit(self): - runner = CliRunner() + result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y') + result = self.runner.invoke(ctl, ['reinit', 'alpha', '--dcs', '8.8.8.8'], input='y') - result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y') - result = runner.invoke(ctl, ['reinit', 'alpha', '--dcs', '8.8.8.8'], input='y') - - result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='N') - result = runner.invoke(ctl, [ + result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='N') + result = self.runner.invoke(ctl, [ 'restart', 'alpha', '--dcs', @@ -291,36 +284,34 @@ y''') assert 'not a member' in str(result.output) with patch('requests.post', Mock(return_value=MockResponse())): - result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y') + result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y') @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) @patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None)) def test_remove(self): - runner = CliRunner() - - result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='alpha\nslave') + result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='alpha\nslave') assert 'Please confirm' in result.output assert 'You are about to remove all' in result.output assert 'You did not exactly type' in str(result.output) - result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha + result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha Yes I am aware slave''') assert 'You did not specify the current master of the cluster' in str(result.output) - result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='beta\nleader') + result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='beta\nleader') assert 'Cluster names specified do not match' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', get_cluster_initialized_with_leader): - result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], - input='''alpha + result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], + input='''alpha Yes I am aware leader''') assert 'object has no attribute' in str(result.exception) with patch('patroni.ctl.get_dcs', Mock(return_value=Mock())): - result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], - input='''alpha + result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], + input='''alpha Yes I am aware leader''') assert 'We have not implemented this for DCS of type' in str(result.output) @@ -340,11 +331,9 @@ leader''') self.assertRaises(requests.exceptions.ConnectionError, post_patroni, member, 'dummy', {}) def test_ctl(self): - runner = CliRunner() + self.runner.invoke(ctl, ['list']) - runner.invoke(ctl, ['list']) - - result = runner.invoke(ctl, ['--help']) + result = self.runner.invoke(ctl, ['--help']) assert 'Usage:' in result.output def test_get_any_member(self): @@ -374,15 +363,11 @@ leader''') @patch('requests.get', requests_get) @patch('requests.post', requests_get) def test_members(self): - runner = CliRunner() - - result = runner.invoke(members, ['alpha']) + result = self.runner.invoke(members, ['alpha']) assert result.exit_code == 0 def test_configure(self): - runner = CliRunner() - - result = runner.invoke(configure, [ + result = self.runner.invoke(configure, [ '--dcs', 'abc', '-c',