From bd847fd2ccf4a6132d676439559373354bb3764a Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 28 Nov 2017 12:10:05 +0100 Subject: [PATCH] Patronictl extended info (#567) * Show information about scheduled failover and maintenance mode when showing list of cluster members. Fixes https://github.com/zalando/patroni/issues/557 * Fix postgres version check functions (postgres 10 and above compatibility) and apply pep8 formatting to the tests. * Bump some configuration parameters to match with postgres 10 defaults. * Fix name of contributor in release notes. --- docs/releases.rst | 2 +- patroni/api.py | 10 ++++--- patroni/ctl.py | 60 ++++++++++++++++++++++------------------ patroni/postgresql.py | 46 ++++++++++++++++-------------- patroni/utils.py | 5 ---- tests/test_ctl.py | 11 +++++--- tests/test_postgresql.py | 3 +- tests/test_postmaster.py | 9 +++--- 8 files changed, 79 insertions(+), 67 deletions(-) diff --git a/docs/releases.rst b/docs/releases.rst index 8a1a3ff7..cee3e70c 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -24,7 +24,7 @@ Version 1.3.6 **Consul improvements** -- Make it possible to provide datacenter configuration for Consul (DeathBorn, Alexander) +- Make it possible to provide datacenter configuration for Consul (Vilius Okockis, Alexander) Before that Patroni was always communicating with datacenter of the host it runs on. diff --git a/patroni/api.py b/patroni/api.py index ee8ff2d5..8684a82a 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -7,8 +7,8 @@ import time import dateutil.parser import datetime -from patroni.exceptions import PostgresConnectionException -from patroni.utils import deep_compare, patch_config, Retry, RetryFailedError, is_valid_pg_version, parse_int, tzutc +from patroni.postgresql import PostgresConnectionException, PostgresException, Postgresql +from patroni.utils import deep_compare, patch_config, Retry, RetryFailedError, parse_int, tzutc from six.moves.BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer from six.moves.socketserver import ThreadingMixIn from threading import Thread @@ -221,9 +221,11 @@ class RestApiHandler(BaseHTTPRequestHandler): data = "PostgreSQL role should be either master or replica" break elif k == 'postgres_version': - if not is_valid_pg_version(request[k]): + try: + Postgresql.postgres_version_to_int(request[k]) + except PostgresException as e: status_code = 400 - data = "PostgreSQL version should be in the first.major.minor format" + data = e.value break elif k == 'timeout': request[k] = parse_int(request[k], 's') diff --git a/patroni/ctl.py b/patroni/ctl.py index 7400f807..2afec1b4 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -30,7 +30,7 @@ from patroni.config import Config from patroni.dcs import get_dcs as _get_dcs from patroni.exceptions import PatroniException from patroni.postgresql import Postgresql -from patroni.utils import is_valid_pg_version, patch_config +from patroni.utils import patch_config from prettytable import PrettyTable from six.moves.urllib_parse import urlparse from six import text_type @@ -469,11 +469,12 @@ def restart(obj, cluster_name, member_names, force, role, p_any, scheduled, vers content['restart_pending'] = True if version: - if not is_valid_pg_version(version): - message = 'PostgreSQL version should be in the first.major.minor format' - raise PatroniCtlException(message) - else: - content['postgres_version'] = version + try: + Postgresql.postgres_version_to_int(version) + except PatroniException as e: + raise PatroniCtlException(e.value) + + content['postgres_version'] = version if scheduled is None and not force: scheduled = click.prompt('When should the restart take place (e.g. 2015-10-01T14:30) ', type=str, default='now') @@ -622,12 +623,15 @@ def output_members(cluster, name, extended=False, fmt='pretty'): logging.debug(cluster) leader_name = None if cluster.leader: - leader_name = cluster.leader.member.name + leader_name = cluster.leader.name xlog_location_cluster = cluster.last_leader_operation or 0 # Mainly for consistent pretty printing and watching we sort the output cluster.members.sort(key=lambda x: x.name) + + extended = extended or any(m.data.get('scheduled_restart') for m in cluster.members) + for m in cluster.members: logging.debug(m) @@ -637,21 +641,15 @@ def output_members(cluster, name, extended=False, fmt='pretty'): elif m.name == cluster.sync.sync_standby: role = 'Sync standby' - host = m.conn_kwargs()['host'] - - xlog_location = m.data.get('xlog_location') or 0 + xlog_location = m.data.get('xlog_location') lag = '' - if xlog_location_cluster >= xlog_location: + if xlog_location is None: + lag = 'unknown' + elif xlog_location_cluster >= xlog_location: lag = round((xlog_location_cluster - xlog_location)/1024/1024) - row = [ - name, - m.name, - host, - role, - m.data.get('state', ''), - lag, - ] + row = [name, m.name, m.conn_kwargs()['host'], role, m.data.get('state', ''), lag] + if extended: value = '' scheduled_restart = m.data.get('scheduled_restart') @@ -664,14 +662,7 @@ def output_members(cluster, name, extended=False, fmt='pretty'): rows.append(row) - columns = [ - 'Cluster', - 'Member', - 'Host', - 'Role', - 'State', - 'Lag in MB', - ] + columns = ['Cluster', 'Member', 'Host', 'Role', 'State', 'Lag in MB'] alignment = {'Cluster': 'l', 'Member': 'l', 'Host': 'l', 'Lag in MB': 'r'} if extended: @@ -680,6 +671,21 @@ def output_members(cluster, name, extended=False, fmt='pretty'): print_output(columns, rows, alignment, fmt) + service_info = [] + if cluster.is_paused(): + service_info.append('Maintenance mode: on') + + if cluster.failover and cluster.failover.scheduled_at: + info = 'Failover scheduled at: ' + cluster.failover.scheduled_at.isoformat() + if cluster.failover.leader: + info += '\n from: ' + cluster.failover.leader + if cluster.failover.candidate: + info += '\n to: ' + cluster.failover.candidate + service_info.append(info) + + if service_info: + click.echo(' ' + '\n '.join(service_info)) + @ctl.command('list', help='List the Patroni members for a given Patroni') @click.argument('cluster_names', nargs=-1) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 25fd9484..0480c4d9 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -12,7 +12,7 @@ import time from collections import defaultdict from contextlib import contextmanager from patroni.callback_executor import CallbackExecutor -from patroni.exceptions import PostgresConnectionException +from patroni.exceptions import PostgresConnectionException, PostgresException from patroni.utils import compare_values, parse_bool, parse_int, Retry, RetryFailedError, polling_loop from patroni.postmaster import PostmasterProcess from six import string_types @@ -86,12 +86,12 @@ class Postgresql(object): 'wal_level': ('hot_standby', lambda v: v.lower() in ('hot_standby', 'replica', 'logical'), 90100), 'hot_standby': ('on', lambda _: False, 90100), 'max_connections': (100, lambda v: int(v) >= 100, 90100), - 'max_wal_senders': (5, lambda v: int(v) >= 5, 90100), + 'max_wal_senders': (10, lambda v: int(v) >= 10, 90100), 'wal_keep_segments': (8, lambda v: int(v) >= 8, 90100), 'max_prepared_transactions': (0, lambda v: int(v) >= 0, 90100), 'max_locks_per_transaction': (64, lambda v: int(v) >= 64, 90100), 'track_commit_timestamp': ('off', lambda v: parse_bool(v) is not None, 90500), - 'max_replication_slots': (5, lambda v: int(v) >= 5, 90400), + 'max_replication_slots': (10, lambda v: int(v) >= 10, 90400), 'max_worker_processes': (8, lambda v: int(v) >= 8, 90400), 'wal_log_hints': ('on', lambda _: False, 90400) } @@ -918,7 +918,8 @@ class Postgresql(object): return True, True - def terminate_starting_postmaster(self, postmaster): + @staticmethod + def terminate_starting_postmaster(postmaster): """Terminates a postmaster that has not yet opened ports or possibly even written a pid file. Blocks until the process goes away.""" postmaster.signal_stop('immediate') @@ -1615,7 +1616,7 @@ $$""".format(name, ' '.join(options)), name, password, password) @staticmethod def postgres_version_to_int(pg_version): - """ Convert the server_version to integer + """Convert the server_version to integer >>> Postgresql.postgres_version_to_int('9.5.3') 90503 @@ -1623,29 +1624,34 @@ $$""".format(name, ' '.join(options)), name, password, password) 90313 >>> Postgresql.postgres_version_to_int('10.1') 100001 - >>> Postgresql.postgres_version_to_int('10') + >>> Postgresql.postgres_version_to_int('10') # doctest: +IGNORE_EXCEPTION_DETAIL Traceback (most recent call last): ... - Exception: Invalid PostgreSQL format: X.Y or X.Y.Z is accepted: 10 - >>> Postgresql.postgres_version_to_int('a.b.c') + PostgresException: 'Invalid PostgreSQL version format: X.Y or X.Y.Z is accepted: 10' + >>> Postgresql.postgres_version_to_int('9.6') # doctest: +IGNORE_EXCEPTION_DETAIL Traceback (most recent call last): ... - Exception: Invalid PostgreSQL version: a.b.c + PostgresException: 'Invalid PostgreSQL version format: X.Y or X.Y.Z is accepted: 9.6' + >>> Postgresql.postgres_version_to_int('a.b.c') # doctest: +IGNORE_EXCEPTION_DETAIL + Traceback (most recent call last): + ... + PostgresException: 'Invalid PostgreSQL version: a.b.c' """ - components = pg_version.split('.') - result = [] - if len(components) < 2 or len(components) > 3: - raise Exception("Invalid PostgreSQL format: X.Y or X.Y.Z is accepted: {0}".format(pg_version)) + try: + components = list(map(int, pg_version.split('.'))) + except ValueError: + raise PostgresException('Invalid PostgreSQL version: {0}'.format(pg_version)) + + if len(components) < 2 or len(components) == 2 and components[0] < 10 or len(components) > 3: + raise PostgresException('Invalid PostgreSQL version format: X.Y or X.Y.Z is accepted: {0}' + .format(pg_version)) + if len(components) == 2: # new style verion numbers, i.e. 10.1 becomes 100001 - components.insert(1, '0') - try: - result = [c if int(c) > 10 else '0{0}'.format(c) for c in components] - result = int(''.join(result)) - except ValueError: - raise Exception("Invalid PostgreSQL version: {0}".format(pg_version)) - return result + components.insert(1, 0) + + return int(''.join('{0:02d}'.format(c) for c in components)) @staticmethod def postgres_major_version_to_int(pg_version): diff --git a/patroni/utils.py b/patroni/utils.py index bdf663ab..89d69797 100644 --- a/patroni/utils.py +++ b/patroni/utils.py @@ -1,6 +1,5 @@ import random import time -import re from dateutil import tz from patroni.exceptions import PatroniException @@ -194,10 +193,6 @@ def _sleep(interval): time.sleep(interval) -def is_valid_pg_version(version): - return re.match(r'[1-9][0-9]?(\.(0|([1-9][0-9]?))){2}$', version) - - class RetryFailedError(PatroniException): """Raised when retrying an operation ultimately failed, after retrying the maximum number of attempts.""" diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 8c02cd2c..ca030129 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -5,11 +5,13 @@ import sys import unittest from click.testing import CliRunner +from datetime import datetime, timedelta from mock import patch, Mock from patroni.ctl import ctl, members, store_config, load_config, output_members, request_patroni, get_dcs, parse_dcs, \ get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException, apply_config_changes, \ format_config_for_editing, show_diff, invoke_editor -from patroni.dcs.etcd import Client +from patroni.dcs.etcd import Client, Failover +from patroni.utils import tzutc from psycopg2 import OperationalError from test_etcd import etcd_read, requests_get, socket_getaddrinfo, MockResponse from test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader, \ @@ -64,7 +66,8 @@ class TestCtl(unittest.TestCase): self.assertRaises(PatroniCtlException, parse_dcs, 'invalid://test') def test_output_members(self): - cluster = get_cluster_initialized_with_leader() + scheduled_at = datetime.now(tzutc) + timedelta(seconds=600) + cluster = get_cluster_initialized_with_leader(Failover(1, 'foo', 'bar', scheduled_at)) self.assertIsNone(output_members(cluster, name='abc', fmt='pretty')) self.assertIsNone(output_members(cluster, name='abc', fmt='json')) self.assertIsNone(output_members(cluster, name='abc', fmt='tsv')) @@ -235,7 +238,7 @@ class TestCtl(unittest.TestCase): # Wrong pg version result = self.runner.invoke(ctl, ['restart', 'alpha', '--any', '--pg-version', '9.1'], input='y') - assert 'Error: PostgreSQL version' in result.output + assert 'Error: Invalid PostgreSQL version format' in result.output assert result.exit_code == 1 result = self.runner.invoke(ctl, ['restart', 'alpha', '--pending', '--force', '--timeout', '10min']) @@ -261,7 +264,7 @@ class TestCtl(unittest.TestCase): with patch('requests.post', Mock(return_value=MockResponse(204))): # get restart with the non-200 return code # normal restart, the schedule is actually parsed, but not validated in patronictl - result = self.runner.invoke(ctl, ['restart', 'alpha', '--pg-version', '42.0.0', + result = self.runner.invoke(ctl, ['restart', 'alpha', '--pg-version', '42.0', '--scheduled', '2300-10-01T14:30'], input='y') assert result.exit_code == 0 diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 39ef2467..a3928434 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -1,8 +1,6 @@ -import errno import mock # for the mock.call method, importing it without a namespace breaks python3 import os import psycopg2 -import psutil import shutil import subprocess import unittest @@ -105,6 +103,7 @@ class MockPostmaster(object): self.signal_stop = Mock(return_value=None) self.wait = Mock() + def pg_controldata_string(*args, **kwargs): return b""" pg_control version number: 942 diff --git a/tests/test_postmaster.py b/tests/test_postmaster.py index 95ff0cbb..7c8b556c 100644 --- a/tests/test_postmaster.py +++ b/tests/test_postmaster.py @@ -4,6 +4,7 @@ from mock import Mock, patch from patroni.postmaster import PostmasterProcess import psutil + class TestPostmasterProcess(unittest.TestCase): @patch('psutil.Process.__init__', Mock()) def test_init(self): @@ -20,9 +21,9 @@ class TestPostmasterProcess(unittest.TestCase): mock_init.side_effect = None with patch.object(psutil.Process, 'pid', 123), \ - patch.object(psutil.Process, 'parent', return_value=124), \ - patch('os.getpid', return_value=125) as mock_ospid, \ - patch('os.getppid', return_value=126): + patch.object(psutil.Process, 'parent', return_value=124), \ + patch('os.getpid', return_value=125) as mock_ospid, \ + patch('os.getppid', return_value=126): self.assertNotEquals(PostmasterProcess.from_pidfile({"pid": "123"}), None) @@ -62,7 +63,7 @@ class TestPostmasterProcess(unittest.TestCase): c2.cmdline = Mock(return_value=["postgres: postgres postgres [local] idle"]) with patch('psutil.Process.children', Mock(return_value=[c1, c2])): proc = PostmasterProcess(123) - proc.wait_for_user_backends_to_close() + self.assertIsNone(proc.wait_for_user_backends_to_close()) mock_wait.assert_called_with([c2]) @patch('subprocess.Popen')