From ab9fea7d6b5fd44336b690b2e8011fa7947d8ab4 Mon Sep 17 00:00:00 2001 From: Polina Bungina <27892524+hughcapet@users.noreply.github.com> Date: Fri, 12 May 2023 10:42:53 +0200 Subject: [PATCH 1/8] Fix openssl certificate generation in behave tests (#2672) --addext -> -addext (doesn't work on macOS) set keyfile permissions to 600 (to avoid "private key file has group or world access") --- features/environment.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/features/environment.py b/features/environment.py index b915fd99..c3fcce00 100644 --- a/features/environment.py +++ b/features/environment.py @@ -7,6 +7,7 @@ import psutil import re import shutil import signal +import stat import subprocess import sys import tempfile @@ -1060,10 +1061,11 @@ def before_all(context): try: with open(os.devnull, 'w') as null: ret = subprocess.call(['openssl', 'req', '-nodes', '-new', '-x509', '-subj', '/CN=batman.patroni', - '--addext', 'subjectAltName=IP:127.0.0.1', '-keyout', context.keyfile, + '-addext', 'subjectAltName=IP:127.0.0.1', '-keyout', context.keyfile, '-out', context.certfile], stdout=null, stderr=null) if ret != 0: raise Exception + os.chmod(context.keyfile, stat.S_IWRITE | stat.S_IREAD) except Exception: context.keyfile = context.certfile = None From fdcf8b19979e47209aac782535efbcf693175ccd Mon Sep 17 00:00:00 2001 From: Israel Date: Fri, 12 May 2023 10:38:59 -0300 Subject: [PATCH 2/8] Add docstrings and type hints to `patroni/api.py` (#2648) References: PAT-77 --- patroni/api.py | 886 +++++++++++++++++++++++++++++++++++++++------- tests/test_api.py | 29 +- 2 files changed, 786 insertions(+), 129 deletions(-) diff --git a/patroni/api.py b/patroni/api.py index 45b50a4d..0fb39157 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -1,3 +1,11 @@ +"""Implement Patroni's REST API. + +Exposes a REST API of patroni operations functions, such as status, performance and management to web clients. + +Much of what can be achieved with the command line tool patronictl can be done via the API. Patroni CLI and daemon +utilises the API to perform these functions. +""" + import base64 import hmac import json @@ -11,13 +19,15 @@ import socket import sys from http.server import BaseHTTPRequestHandler, HTTPServer -from ipaddress import ip_address, ip_network +from ipaddress import ip_address, ip_network, IPv4Network, IPv6Network from socketserver import ThreadingMixIn from threading import Thread from urllib.parse import urlparse, parse_qs -from typing import Any, Dict, Optional, Union + +from typing import Any, Callable, Dict, Iterator, List, Optional, Tuple, Union from . import psycopg +from .__main__ import Patroni from .dcs import Cluster from .exceptions import PostgresConnectionException, PostgresException from .postgresql.misc import postgres_version_to_int @@ -27,14 +37,97 @@ from .utils import deep_compare, enable_keepalive, parse_bool, patch_config, Ret logger = logging.getLogger(__name__) -class RestApiHandler(BaseHTTPRequestHandler): +def check_access(func: Callable[['RestApiHandler'], None]) -> Callable[..., None]: + """Check the source ip, authorization header, or client certificates. - def _write_status_code_only(self, status_code): + .. note:: + The actual logic to check access is implemented through :func:`RestApiServer.check_access`. + + :param func: function to be decorated. + + :returns: a decorator that executes *func* only if :func:`RestApiServer.check_access` returns ``True``. + + :Example: + + @check_access + def do_PUT_foo(): + pass + """ + + def wrapper(self: 'RestApiHandler', *args: Any, **kwargs: Any) -> None: + if self.server.check_access(self): + return func(self, *args, **kwargs) + + return wrapper + + +class RestApiHandler(BaseHTTPRequestHandler): + """Define how to handle each of the requests that are made against the REST API server.""" + + # Comment from pyi stub file. These unions can cause typing errors with IDEs, e.g. PyCharm + # + # Those are technically of types, respectively: + # * _RequestType = Union[socket.socket, Tuple[bytes, socket.socket]] + # * _AddressType = Tuple[str, int] + # But there are some concerns that having unions here would cause + # too much inconvenience to people using it (see + # https://github.com/python/typeshed/pull/384#issuecomment-234649696) + + def __init__(self, request: Any, + client_address: Any, + server: Union['RestApiServer', HTTPServer]) -> None: + """Create a :class:`RestApiHandler` instance. + + .. note:: + Currently not different from its superclass :func:`__init__`, and only used so ``pyright`` can understand + the type of ``server`` attribute. + + :param request: client request to be processed. + :param client_address: address of the client connection. + :param server: HTTP server that received the request. + """ + assert isinstance(server, RestApiServer) + super(RestApiHandler, self).__init__(request, client_address, server) + self.server: 'RestApiServer' = server + self.__start_time: float = 0.0 + self.path_query: Dict[str, List[str]] = {} + + def _write_status_code_only(self, status_code: int) -> None: + """Write a response that is composed only of the HTTP status. + + The response is written with these values separated by space: + * HTTP protocol version; + * *status_code*; + * description of *status_code*. + + .. note:: + This is usually useful for replying to requests from software like HAProxy. + + :param status_code: HTTP status code. + + :Example: + + * ``_write_status_code_only(200)`` would write a response like ``HTTP/1.0 200 OK``. + """ message = self.responses[status_code][0] self.wfile.write('{0} {1} {2}\r\n\r\n'.format(self.protocol_version, status_code, message).encode('utf-8')) self.log_request(status_code) - def _write_response(self, status_code, body, content_type='text/html', headers=None): + def _write_response(self, status_code: int, body: str, content_type: str = 'text/html', + headers: Optional[Dict[str, str]] = None) -> None: + """Write an HTTP response. + + .. note:: + Besides ``Content-Type`` header, and the HTTP headers passed through *headers*, this function will also + write the HTTP headers defined through ``restapi.http_extra_headers`` and ``restapi.https_extra_headers`` + from Patroni configuration. + + :param status_code: response HTTP status code. + :param body: response body. + :param content_type: value for ``Content-Type`` HTTP header. + :param headers: dictionary of additional HTTP headers to set for the response. Each key is the header name, and + the corresponding value is the value for the header in the response. + """ # TODO: try-catch ConnectionResetError: [Errno 104] Connection reset by peer and log it in DEBUG level self.send_response(status_code) headers = headers or {} @@ -42,31 +135,45 @@ class RestApiHandler(BaseHTTPRequestHandler): headers['Content-Type'] = content_type for name, value in headers.items(): self.send_header(name, value) - for name, value in self.server.http_extra_headers.items(): + for name, value in (self.server.http_extra_headers or {}).items(): self.send_header(name, value) self.end_headers() self.wfile.write(body.encode('utf-8')) - def _write_json_response(self, status_code, response): + def _write_json_response(self, status_code: int, response: Any) -> None: + """Write an HTTP response with a JSON content type. + + Call :func:`_write_response` with ``content_type`` as ``application/json``. + + :param status_code: response HTTP status code. + :param response: value to be dumped as a JSON string and to be used as the response body. + """ self._write_response(status_code, json.dumps(response, default=str), content_type='application/json') - def check_access(func): - """Decorator function to check the source ip, authorization header. or client certificates - - Usage example: - @check_access - def do_PUT_foo(): - pass - """ - - def wrapper(self, *args, **kwargs): - if self.server.check_access(self): - return func(self, *args, **kwargs) - - return wrapper - def _write_status_response(self, status_code: int, response: Dict[str, Any]) -> None: - """Sends HTTP response with Patroni/Postgres status in JSON format.""" + """Write an HTTP response with Patroni/Postgres status in JSON format. + + Modifies *response* before sending it to the client. Defines the ``patroni`` key, which is a + dictionary that contains the mandatory keys: + + * ``version``: Patroni version, e.g. ``3.0.2``; + * ``scope``: value of ``scope`` setting from Patroni configuration. + + May also add the following optional keys, depending on the status of this Patroni/PostgreSQL node: + + * ``tags``: tags that were set through Patroni configuration merged with dynamically applied tags; + * ``database_system_identifier``: ``Database system identifier`` from ``pg_controldata`` output; + * ``pending_restart``: ``True`` if PostgreSQL is pending to be restarted; + * ``scheduled_restart``: a dictionary with a single key ``schedule``, which is the timestamp for the scheduled + restart; + * ``watchdog_failed``: ``True`` if watchdog device is unhealthy; + * ``logger_queue_size``: log queue length if it is longer than expected; + * ``logger_records_lost``: number of log records that have been lost while the log queue was full. + + :param status_code: response HTTP status code. + :param response: represents the status of the PostgreSQL node, and is used as a basis for the HTTP response. + This dictionary is built through :func:`get_postgresql_status`. + """ patroni = self.server.patroni tags = patroni.ha.get_effective_tags() if tags: @@ -76,7 +183,7 @@ class RestApiHandler(BaseHTTPRequestHandler): if patroni.postgresql.pending_restart: response['pending_restart'] = True response['patroni'] = {'version': patroni.version, 'scope': patroni.postgresql.scope} - if patroni.scheduled_restart and isinstance(patroni.scheduled_restart, dict): + if patroni.scheduled_restart: response['scheduled_restart'] = patroni.scheduled_restart.copy() del response['scheduled_restart']['postmaster_start_time'] response['scheduled_restart']['schedule'] = (response['scheduled_restart']['schedule']).isoformat() @@ -90,15 +197,52 @@ class RestApiHandler(BaseHTTPRequestHandler): response['logger_records_lost'] = lost self._write_json_response(status_code, response) - def do_GET(self, write_status_code_only: Optional[bool] = False) -> None: - """Default method for processing all GET requests which can not be routed to other methods. + def do_GET(self, write_status_code_only: bool = False) -> None: + """Process all GET requests which can not be routed to other methods. - Is used for handling all health-checks requests. E.g. "GET /(primary|replica|sync|async|etc...)" - :param write_status_code_only: indicates that instead of normal HTTP response we should - send only HTTP Status Code and close the connection. - It is useful to when health-checks are executed by HAProxy. + Is used for handling all health-checks requests. E.g. "GET /(primary|replica|sync|async|etc...)". + + The (optional) query parameters and the HTTP response status depend on the requested path: + * ``/``, ``primary``, or ``read-write``: + * HTTP status ``200``: if a primary with the leader lock. + * ``/standby-leader``: + * HTTP status ``200``: if holds the leader lock in a standby cluster. + * ``/leader``: + * HTTP status ``200``: if holds the leader lock. + * ``/replica``: + * Query parameters: + * ``lag``: only accept replication lag up to ``lag``. Accepts either an :class:`int`, which + represents lag in bytes, or a :class:`str` representing lag in human-readable format (e.g. + ``10MB``). + * Any custom parameter: will attempt to match them against node tags. + * HTTP status ``200``: if up and running as a standby and without ``noloadbalance`` tag. + * ``/read-only``: + * HTTP status ``200``: if up and running and without ``noloadbalance`` tag. + * ``/synchronous`` or ``/sync``: + * HTTP status ``200``: if up and running as a synchronous standby. + * ``/read-only-sync``: + * HTTP status ``200``: if up and running as a synchronous standby or primary. + * ``/asynchronous``: + * Query parameters: + * ``lag``: only accept replication lag up to ``lag``. Accepts either an :class:`int`, which + represents lag in bytes, or a :class:`str` representing lag in human-readable format (e.g. + ``10MB``). + * HTTP status ``200``: if up and running as an asynchronous standby. + * ``/health``: + * HTTP status ``200``: if up and running. + + .. note:: + If not able to honor the query parameter, or not able to match the condition described for HTTP status + ``200`` in each path above, then HTTP status will be ``503``. + + .. note:: + Independently of the requested path, if *write_status_code_only* is ``False``, then it always write an HTTP + response through :func:`_write_status_response`, with the node status. + + :param write_status_code_only: indicates that instead of a normal HTTP response we should + send only the HTTP Status Code and close the connection. + Useful when health-checks are executed by HAProxy. """ - path = '/primary' if self.path == '/' else self.path response = self.get_postgresql_status() @@ -185,14 +329,33 @@ class RestApiHandler(BaseHTTPRequestHandler): else: self._write_status_response(status_code, response) - def do_OPTIONS(self): + def do_OPTIONS(self) -> None: + """Handle an ``OPTIONS`` request. + + Write a simple HTTP response that represents the current PostgreSQL status. Send only `200 OK` or + `503 Service Unavailable` as a response and nothing more, particularly no headers. + """ self.do_GET(write_status_code_only=True) - def do_HEAD(self): + def do_HEAD(self) -> None: + """Handle a ``HEAD`` request. + + Write a simple HTTP response that represents the current PostgreSQL status. Send only `200 OK` or + `503 Service Unavailable` as a response and nothing more, particularly no headers. + """ self.do_GET(write_status_code_only=True) - def do_GET_liveness(self): - patroni = self.server.patroni + def do_GET_liveness(self) -> None: + """Handle a ``GET`` request to ``/liveness`` path. + + Write a simple HTTP response with HTTP status: + * ``200``: + * If the cluster is in maintenance mode; or + * If Patroni heartbeat loop is properly running; + * ``503`` if Patroni heartbeat loop last run was more than ``ttl`` setting ago on the primary (or twice the + value of ``ttl`` on a replica). + """ + patroni: Patroni = self.server.patroni is_primary = patroni.postgresql.role in ('master', 'primary') and patroni.postgresql.is_running() # We can tolerate Patroni problems longer on the replica. # On the primary the liveness probe most likely will start failing only after the leader key expired. @@ -203,7 +366,15 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 200 if patroni.ha.is_paused() or patroni.next_run + liveness_threshold > time.time() else 503 self._write_status_code_only(status_code) - def do_GET_readiness(self): + def do_GET_readiness(self) -> None: + """Handle a ``GET`` request to ``/readiness`` path. + + Write a simple HTTP response which HTTP status can be: + * ``200``: + * If this Patroni node holds the DCS leader lock; or + * If this PostgreSQL instance is up and running; + * ``503``: if none of the previous conditions apply. + """ patroni = self.server.patroni if patroni.ha.is_leader(): status_code = 200 @@ -213,21 +384,51 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 503 self._write_status_code_only(status_code) - def do_GET_patroni(self): + def do_GET_patroni(self) -> None: + """Handle a ``GET`` request to ``/patroni`` path. + + Write an HTTP response through :func:`_write_status_response`, with HTTP status ``200`` and the status of + Postgres. + """ response = self.get_postgresql_status(True) self._write_status_response(200, response) def do_GET_cluster(self) -> None: - """Sends response with JSON representaion of Cluster topology.""" + """Handle a ``GET`` request to ``/cluster`` path. + + Write an HTTP response with JSON content based on the output of :func:`cluster_as_json`, with HTTP status + ``200`` and the JSON representation of the cluster topology. + """ cluster = self.server.patroni.dcs.get_cluster(True) global_config = self.server.patroni.config.get_global_config(cluster) self._write_json_response(200, cluster_as_json(cluster, global_config)) - def do_GET_history(self): + def do_GET_history(self) -> None: + """Handle a ``GET`` request to ``/history`` path. + + Write an HTTP response with a JSON content representing the history of events in the cluster, with HTTP status + ``200``. + + The response contains a :class:`list` of failover/switchover events. Each item is a :class:`list` with the + following items: + * Timeline when the event occurred (class:`int`); + * LSN at which the event occurred (class:`int`); + * The reason for the event (class:`str`); + * Timestamp when the new timeline was created (class:`str`); + * Name of the involved Patroni node (class:`str`). + """ cluster = self.server.patroni.dcs.cluster or self.server.patroni.dcs.get_cluster() self._write_json_response(200, cluster.history and cluster.history.lines or []) - def do_GET_config(self): + def do_GET_config(self) -> None: + """Handle a ``GET`` request to ``/config`` path. + + Write an HTTP response with a JSON content representing the Patroni configuration that is stored in the DCS, + with HTTP status ``200``. + + If the cluster information is not available in the DCS, then it will respond with no body and HTTP status + ``502`` instead. + """ cluster = self.server.patroni.dcs.cluster or self.server.patroni.dcs.get_cluster() if cluster.config: self._write_json_response(200, cluster.config.data) @@ -235,12 +436,38 @@ class RestApiHandler(BaseHTTPRequestHandler): self.send_error(502) def do_GET_metrics(self) -> None: - """Sends response in Prometheus format.""" + """Handle a ``GET`` request to ``/metrics`` path. + + Write an HTTP response with plain text content in the format used by Prometheus, with HTTP status ``200``. + + The response contains the following items: + + * ``patroni_version``: Patroni version without periods, e.g. ``030002`` for Patroni ``3.0.2``; + * ``patroni_postgres_running``: ``1`` if PostgreSQL is running, else ``0``; + * ``patroni_postmaster_start_time``: epoch timestamp since Postmaster was started; + * ``patroni_master``: ``1`` if this node holds the leader lock, else ``0``; + * ``patroni_primary``: same as ``patroni_master``; + * ``patroni_xlog_location``: ``pg_wal_lsn_diff(pg_current_wal_lsn(), '0/0')`` if leader, else ``0``; + * ``patroni_standby_leader``: ``1`` if standby leader node, else ``0``; + * ``patroni_replica``: ``1`` if a replica, else ``0``; + * ``patroni_sync_standby``: ``1`` if a sync replica, else ``0``; + * ``patroni_xlog_received_location``: ``pg_wal_lsn_diff(pg_last_wal_receive_lsn(), '0/0')``; + * ``patroni_xlog_replayed_location``: ``pg_wal_lsn_diff(pg_last_wal_replay_lsn(), '0/0)``; + * ``patroni_xlog_replayed_timestamp``: ``pg_last_xact_replay_timestamp``; + * ``patroni_xlog_paused``: ``pg_is_wal_replay_paused()``; + * ``patroni_postgres_server_version``: Postgres version without periods, e.g. ``150002`` for Postgres ``15.2``; + * ``patroni_cluster_unlocked``: ``1`` if no one holds the leader lock, else ``0``; + * ``patroni_failsafe_mode_is_active``: ``1`` if ``failmode`` is currently active, else ``0``; + * ``patroni_postgres_timeline``: PostgreSQL timeline based on current WAL file name; + * ``patroni_dcs_last_seen``: epoch timestamp when DCS was last contacted successfully; + * ``patroni_pending_restart``: ``1`` if this PostgreSQL node is pending a restart, else ``0``; + * ``patroni_is_paused``: ``1`` if Patroni is in maintenance node, else ``0``. + """ postgres = self.get_postgresql_status(True) patroni = self.server.patroni epoch = datetime.datetime(1970, 1, 1, tzinfo=tzutc) - metrics = [] + metrics: List[str] = [] scope_label = '{{scope="{0}"}}'.format(patroni.postgresql.scope) metrics.append("# HELP patroni_version Patroni semver without periods.") @@ -315,7 +542,7 @@ class RestApiHandler(BaseHTTPRequestHandler): metrics.append("# TYPE patroni_cluster_unlocked gauge") metrics.append("patroni_cluster_unlocked{0} {1}".format(scope_label, int(postgres.get('cluster_unlocked', 0)))) - metrics.append("# HELP patroni_failsafe_mode_is_active Value is 1 if the cluster is unlocked, 0 if locked.") + metrics.append("# HELP patroni_failsafe_mode_is_active Value is 1 if failsafe mode is active, 0 if inactive.") metrics.append("# TYPE patroni_failsafe_mode_is_active gauge") metrics.append("patroni_failsafe_mode_is_active{0} {1}" .format(scope_label, int(postgres.get('failsafe_mode_is_active', 0)))) @@ -340,14 +567,32 @@ class RestApiHandler(BaseHTTPRequestHandler): self._write_response(200, '\n'.join(metrics) + '\n', content_type='text/plain') - def _read_json_content(self, body_is_optional=False): + def _read_json_content(self, body_is_optional: bool = False) -> Optional[Dict[Any, Any]]: + """Read JSON from HTTP request body. + + .. note:: + Retrieves the request body based on `content-length` HTTP header. The body is expected to be a JSON + string with that length. + + If request body is expected but `content-length` HTTP header is absent, then write an HTTP response + with HTTP status ``411``. + + If request body is expected but contains nothing, or if an exception is faced, then write an HTTP + response with HTTP status ``400``. + + :param body_is_optional: if ``False`` then the request must contain a body. If ``True``, then the request may or + may not contain a body. + + :returns: deserialized JSON string from request body, if present. If body is absent, but *body_is_optional* is + ``True``, then return an empty dictionary. Returns ``None`` otherwise. + """ if 'content-length' not in self.headers: return self.send_error(411) if not body_is_optional else {} try: - content_length = int(self.headers.get('content-length')) + content_length = int(self.headers.get('content-length') or 0) if content_length == 0 and body_is_optional: return {} - request = json.loads(self.rfile.read(content_length).decode('utf-8')) + request: Union[Dict[str, Any], Any] = json.loads(self.rfile.read(content_length).decode('utf-8')) if isinstance(request, dict) and (request or body_is_optional): return request except Exception: @@ -355,7 +600,18 @@ class RestApiHandler(BaseHTTPRequestHandler): self.send_error(400) @check_access - def do_PATCH_config(self): + def do_PATCH_config(self) -> None: + """Handle a ``PATCH`` request to ``/config`` path. + + Updates the Patroni configuration based on the JSON request body, then writes a response with the new + configuration, with HTTP status ``200``. + + .. note:: + If the configuration has been previously wiped out from DCS, then write a response with + HTTP status ``503``. + + If applying a configuration value fails, then write a response with HTTP status ``409``. + """ request = self._read_json_content() if request: cluster = self.server.patroni.dcs.get_cluster(True) @@ -370,22 +626,43 @@ class RestApiHandler(BaseHTTPRequestHandler): self._write_json_response(200, data) @check_access - def do_PUT_config(self): + def do_PUT_config(self) -> None: + """Handle a ``PUT`` request to ``/config`` path. + + Overwrites the Patroni configuration based on the JSON request body, then writes a response with the new + configuration, with HTTP status ``200``. + + .. note:: + If applying the new configuration fails, then write a response with HTTP status ``502``. + """ request = self._read_json_content() if request: cluster = self.server.patroni.dcs.get_cluster() - if not deep_compare(request, cluster.config.data): + if not (cluster.config and deep_compare(request, cluster.config.data)): value = json.dumps(request, separators=(',', ':')) if not self.server.patroni.dcs.set_config_value(value): return self.send_error(502) self._write_json_response(200, request) @check_access - def do_POST_reload(self): + def do_POST_reload(self) -> None: + """Handle a ``POST`` request to ``/reload`` path. + + Schedules a reload to Patroni and writes a response with HTTP status `202`. + """ self.server.patroni.sighup_handler() self._write_response(202, 'reload scheduled') - def do_GET_failsafe(self): + def do_GET_failsafe(self) -> None: + """Handle a ``GET`` request to ``/failsafe`` path. + + Writes a response with a JSON string body containing all nodes that are known to Patroni at a given point + in time, with HTTP status ``200``. The JSON contains a dictionary, each key is the name of the Patroni node, + and the corresponding value is the URI to access `/patroni` path of its REST API. + + .. note:: + If ``failsafe_mode`` is not enabled, then write a response with HTTP status ``502``. + """ failsafe = self.server.patroni.dcs.failsafe if isinstance(failsafe, dict): self._write_json_response(200, failsafe) @@ -393,7 +670,15 @@ class RestApiHandler(BaseHTTPRequestHandler): self.send_error(502) @check_access - def do_POST_failsafe(self): + def do_POST_failsafe(self) -> None: + """Handle a ``POST`` request to ``/failsafe`` path. + + Writes a response with HTTP status ``200`` if this node is a Standby, or with HTTP status ``500`` if this is + the primary. + + .. note:: + If ``failsafe_mode`` is not enabled, then write a response with HTTP status ``502``. + """ if self.server.patroni.ha.is_failsafe_mode(): request = self._read_json_content() if request: @@ -404,16 +689,34 @@ class RestApiHandler(BaseHTTPRequestHandler): self.send_error(502) @check_access - def do_POST_sigterm(self): - """Only for behave testing on windows""" + def do_POST_sigterm(self) -> None: + """Handle a ``POST`` request to ``/sigterm`` path. + Schedule a shutdown and write a response with HTTP status ``202``. + + .. note:: + Only for behave testing on Windows. + """ if os.name == 'nt' and os.getenv('BEHAVE_DEBUG'): self.server.patroni.api_sigterm() self._write_response(202, 'shutdown scheduled') @staticmethod - def parse_schedule(schedule, action): - """ parses the given schedule and validates at """ + def parse_schedule(schedule: str, + action: str) -> Tuple[Union[int, None], Union[str, None], Union[datetime.datetime, None]]: + """Parse the given *schedule* and validate it. + + :param schedule: a string representing a timestamp, e.g. ``2023-04-14T20:27:00+00:00``. + :param action: the action to be scheduled (``restart``, ``switchover``, or ``failover``). + + :returns: a tuple composed of 3 items + * Suggested HTTP status code for a response: + * ``None``: if no issue was faced while parsing, leaving it up to the caller to decide the status; or + * ``400``: if no timezone information could be found in *schedule*; or + * ``422``: if *schedule* is invalid -- in the past or not parsable. + * An error message, if any error is faced, otherwise ``None``; + * Parsed *schedule*, if able to parse, otherwise ``None``. + """ error = None scheduled_at = None try: @@ -430,11 +733,41 @@ class RestApiHandler(BaseHTTPRequestHandler): logger.exception('Invalid scheduled %s time: %s', action, schedule) error = 'Unable to parse scheduled timestamp. It should be in an unambiguous format, e.g. ISO 8601' status_code = 422 - return (status_code, error, scheduled_at) + return status_code, error, scheduled_at @check_access def do_POST_restart(self) -> None: - """Is used to restart postgres, mainly by "patronictl restart".""" + """Handle a ``POST`` request to ``/restart`` path. + + Used to restart postgres (or schedule a restart), mainly by ``patronictl restart``. + + The request body should be a JSON dictionary, and it can contain the following keys: + * ``schedule``: timestamp at which the restart should occur; + * ``role``: restart only nodes which role is ``role``. Can be either: + * ``primary`` (or ``master``); or + * ``replica``. + * ``postgres_version``: restart only nodes which PostgreSQL version is less than ``postgres_version``, e.g. + ``15.2``; + * ``timeout``: if restart takes longer than ``timeout`` return an error and fail over to a replica; + * ``restart_pending``: if we should restart only when have ``pending restart`` flag; + + Response HTTP status codes: + * ``200``: if successfully performed an immediate restart; or + * ``202``: if successfully scheduled a restart for later; or + * ``500``: if the cluster is in maintenance mode; or + * ``400``: if + * ``role`` value is invalid; or + * ``postgres_version`` value is invalid; or + * ``timeout`` is not a number, or lesser than ``0``; or + * request contains an unknown key; or + * exception is faced while performing an immediate restart. + * ``409``: if another restart was already previously scheduled; or + * ``503``: if any issue was found while performing an immediate restart; or + * HTTP status returned by :func:`parse_schedule`, if any error was observed while parsing the schedule. + + .. note:: + If it's not able to parse the request body, then the request is silently discarded. + """ status_code = 500 data = 'restart failed' request = self._read_json_content(body_is_optional=True) @@ -492,10 +825,21 @@ class RestApiHandler(BaseHTTPRequestHandler): else: data = "Another restart is already scheduled" status_code = 409 + # pyright thinks ``data`` can be ``None`` because ``parse_schedule`` call may return ``None``. However, if + # that's the case, ``data`` will be overwritten when the ``for`` loop ends + assert isinstance(data, str) self._write_response(status_code, data) @check_access - def do_DELETE_restart(self): + def do_DELETE_restart(self) -> None: + """Handle a ``DELETE`` request to ``/restart`` path. + + Used to remove a scheduled restart of PostgreSQL. + + Response HTTP status codes: + * ``200``: if a scheduled restart was removed; or + * ``404``: if no scheduled restart could be found. + """ if self.server.patroni.ha.delete_future_restart(): data = "scheduled restart deleted" code = 200 @@ -505,7 +849,16 @@ class RestApiHandler(BaseHTTPRequestHandler): self._write_response(code, data) @check_access - def do_DELETE_switchover(self): + def do_DELETE_switchover(self) -> None: + """Handle a ``DELETE`` request to ``/switchover`` path. + + Used to remove a scheduled switchover in the cluster. + + It writes a response, and the HTTP status code can be: + * ``200``: if a scheduled switchover was removed; or + * ``404``: if no scheduled switchover could be found; or + * ``409``: if not able to update the switchover info in the DCS. + """ failover = self.server.patroni.dcs.get_cluster().failover if failover and failover.scheduled_at: if not self.server.patroni.dcs.manual_failover('', '', index=failover.index): @@ -519,7 +872,16 @@ class RestApiHandler(BaseHTTPRequestHandler): self._write_response(code, data) @check_access - def do_POST_reinitialize(self): + def do_POST_reinitialize(self) -> None: + """Handle a ``POST`` request to ``/reinitialize`` path. + + The request body may contain a JSON dictionary with the following key: + * ``force``: ``True`` if we want to cancel an already running task in order to reinit a replica. + + Response HTTP status codes: + * ``200``: if the reinit operation has started; or + * ``503``: if any error is returned by :func:`Ha.reinitialize`. + """ request = self._read_json_content(body_is_optional=True) if request: @@ -535,13 +897,25 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 503 self._write_response(status_code, data) - def poll_failover_result(self, leader, candidate, action): + def poll_failover_result(self, leader: Optional[str], candidate: Optional[str], action: str) -> Tuple[int, str]: + """Poll failover/switchover operation until it finishes or times out. + + :param leader: name of the current Patroni leader. + :param candidate: name of the Patroni node to be promoted. + :param action: the action that is ongoing (``switchover`` or ``failover``). + + :returns: a tuple composed of 2 items + * Response HTTP status codes: + * ``200``: if the operation succeeded; or + * ``503``: if the operation failed or timed out. + * A status message about the operation. + """ timeout = max(10, self.server.patroni.dcs.loop_wait) for _ in range(0, timeout * 2): time.sleep(1) try: cluster = self.server.patroni.dcs.get_cluster() - if not cluster.is_unlocked() and cluster.leader.name != leader: + if not cluster.is_unlocked() and cluster.leader and cluster.leader.name != leader: if not candidate or candidate == cluster.leader.name: return 200, 'Successfully {0}ed over to "{1}"'.format(action[:-4], cluster.leader.name) else: @@ -553,10 +927,16 @@ class RestApiHandler(BaseHTTPRequestHandler): logger.debug('Exception occurred during polling %s result: %s', action, e) return 503, action.title() + ' status unknown' - def is_failover_possible(self, cluster: Cluster, leader: str, candidate: str, action: str) -> Union[str, None]: - """Checks whether there are nodes that could take it over after demoting the primary. + def is_failover_possible(self, cluster: Cluster, leader: Optional[str], candidate: Optional[str], + action: str) -> Optional[str]: + """Checks whether there are nodes that could take over after demoting the primary. - :returns: a string with the error message or `None` if good nodes are found + :param cluster: the Patroni cluster. + :param leader: name of the current Patroni leader. + :param candidate: name of the Patroni node to be promoted. + :param action: the action to be performed (``switchover`` or ``failover``). + + :returns: a string with the error message or ``None`` if good nodes are found. """ is_synchronous_mode = self.server.patroni.config.get_global_config(cluster).is_synchronous_mode if leader and (not cluster.leader or cluster.leader.name != leader): @@ -572,7 +952,7 @@ class RestApiHandler(BaseHTTPRequestHandler): if not members: return action + ' is not possible: can not find sync_standby' else: - members = [m for m in cluster.members if m.name != cluster.leader.name and m.api_url] + members = [m for m in cluster.members if not cluster.leader or m.name != cluster.leader.name and m.api_url] if not members: return action + ' is not possible: cluster does not have members except leader' for st in self.server.patroni.ha.fetch_nodes_statuses(members): @@ -581,8 +961,29 @@ class RestApiHandler(BaseHTTPRequestHandler): return action + ' is not possible: no good candidates have been found' @check_access - def do_POST_failover(self, action: Optional[str] = 'failover') -> None: - """Handles manual failovers/switchovers, mainly from "patronictl".""" + def do_POST_failover(self, action: str = 'failover') -> None: + """Handle a ``POST`` request to ``/failover`` path. + + Handles manual failovers/switchovers, mainly from ``patronictl``. + + The request body should be a JSON dictionary, and it can contain the following keys: + * ``leader``: name of the current leader in the cluster; + * ``candidate``: name of the Patroni node to be promoted; + * ``scheduled_at``: a string representing the timestamp when to execute the switchover/failover, e.g. + ``2023-04-14T20:27:00+00:00``. + + Response HTTP status codes: + * ``202``: if operation has been scheduled; + * ``412``: if operation is not possible; + * ``503``: if unable to register the operation to the DCS; + * HTTP status returned by :func:`parse_schedule`, if any error was observed while parsing the schedule; + * HTTP status returned by :func:`poll_failover_result` if the operation has been processed immediately. + + .. note:: + If unable to parse the request body, then the request is silently discarded. + + :param action: the action to be performed (``switchover`` or ``failover``). + """ request = self._read_json_content() (status_code, data) = (400, '') if not request: @@ -630,13 +1031,29 @@ class RestApiHandler(BaseHTTPRequestHandler): else: data = 'failed to write {0} key into DCS'.format(action) status_code = 503 + # pyright thinks ``status_code`` can be ``None`` because ``parse_schedule`` call may return ``None``. However, + # if that's the case, ``status_code`` will be overwritten somewhere between ``parse_schedule`` and + # ``_write_response`` calls. + assert isinstance(status_code, int) self._write_response(status_code, data) - def do_POST_switchover(self): + def do_POST_switchover(self) -> None: + """Handle a ``POST`` request to ``/switchover`` path. + + Calls :func:`do_POST_failover` with ``switchover`` option. + """ self.do_POST_failover(action='switchover') @check_access - def do_POST_citus(self): + def do_POST_citus(self) -> None: + """Handle a ``POST`` request to ``/citus`` path. + + Call :func:`CitusHandler.handle_event` to handle the request, then write a response with HTTP status code + ``200``. + + .. note:: + If unable to parse the request body, then the request is silently discarded. + """ request = self._read_json_content() if not request: return @@ -647,16 +1064,20 @@ class RestApiHandler(BaseHTTPRequestHandler): patroni.postgresql.citus_handler.handle_event(cluster, request) self._write_response(200, 'OK') - def parse_request(self): - """Override parse_request method to enrich basic functionality of `BaseHTTPRequestHandler` class + def parse_request(self) -> bool: + """Override :func:`parse_request` method to enrich basic functionality of :class:`BaseHTTPRequestHandler`. - Original class can only invoke do_GET, do_POST, do_PUT, etc method implementations if they are defined. + Original class can only invoke :func:`do_GET`, :func:`do_POST`, :func:`do_PUT`, etc method implementations if + they are defined. But we would like to have at least some simple routing mechanism, i.e.: - GET /uri1/part2 request should invoke `do_GET_uri1()` - POST /other should invoke `do_POST_other()` + * ``GET /uri1/part2`` request should invoke :func:`do_GET_uri1()` + * ``POST /other`` should invoke :func:`do_POST_other()` - If the `do__` method does not exists we'll fallback to original behavior.""" + If the :func:`do__` method does not exist we'll fall back to original behavior. + :returns: ``True`` for success, ``False`` for failure; on failure, any relevant error response has already been + sent back. + """ ret = BaseHTTPRequestHandler.parse_request(self) if ret: urlpath = urlparse(self.path) @@ -668,18 +1089,57 @@ class RestApiHandler(BaseHTTPRequestHandler): self.command = mname return ret - def query(self, sql, *params, **kwargs): + def query(self, sql: str, *params: Any, **kwargs: Any) -> List[Tuple[Any, ...]]: + """Execute *sql* query with *params*. + + :param sql: the SQL statement to be run. + :param params: positional arguments to call :func:`RestApiServer.query` with. + :param kwargs: can contain the key ``retry``. If the key is present its value should be a :class:`bool` which + indicates whether the query should be retried upon failure or given up immediately. + + :returns: a list of rows that were fetched from the database. + """ if not kwargs.get('retry', False): return self.server.query(sql, *params) retry = Retry(delay=1, retry_exceptions=PostgresConnectionException) return retry(self.server.query, sql, *params) - def get_postgresql_status(self, retry: Optional[bool] = False) -> Dict[str, Any]: + def get_postgresql_status(self, retry: bool = False) -> Dict[str, Any]: """Builds an object representing a status of "postgres". - Some of values are collected by executing a query and other are taken from the state stored in memory. + Some of the values are collected by executing a query and other are taken from the state stored in memory. + :param retry: whether the query should be retried if failed or give up immediately - :returns: a dict with the status of Postgres/Patroni + :returns: a dict with the status of Postgres/Patroni. The keys are: + * ``state``: Postgres state among ``stopping``, ``stopped``, ``stop failed``, ``crashed``, ``running``, + ``starting``, ``start failed``, ``restarting``, ``restart failed``, ``initializing new cluster``, + ``initdb failed``, ``running custom bootstrap script``, ``custom bootstrap failed``, + ``creating replica``, or ``unknown``; + * ``postmaster_start_time``: ``pg_postmaster_start_time()``; + * ``role``: ``replica`` or ``master`` based on ``pg_is_in_recovery()`` output; + * ``server_version``: Postgres version without periods, e.g. ``150002`` for Postgres ``15.2``; + * ``xlog``: dictionary. Its structure depends on ``role``: + * If ``master``: + * ``location``: ``pg_current_wal_lsn()`` + * If ``replica``: + * ``received_location``: ``pg_wal_lsn_diff(pg_last_wal_receive_lsn(), '0/0')``; + * ``replayed_location``: ``pg_wal_lsn_diff(pg_last_wal_replay_lsn(), '0/0)``; + * ``replayed_timestamp``: ``pg_last_xact_replay_timestamp``; + * ``paused``: ``pg_is_wal_replay_paused()``; + * ``sync_standby``: ``True`` if replication mode is synchronous and this is a sync standby; + * ``timeline``: PostgreSQL primary node timeline; + * ``replication``: :class:`list` of :class:`dict` entries, one for each replication connection. Each entry + contains the following keys: + * ``application_name``: ``pg_stat_activity.application_name``; + * ``client_addr``: ``pg_stat_activity.client_addr``; + * ``state``: ``pg_stat_replication.state``; + * ``sync_priority``: ``pg_stat_replication.sync_priority``; + * ``sync_state``: ``pg_stat_replication.sync_state``; + * ``usename``: ``pg_stat_activity.usename``. + * ``pause``: ``True`` if cluster is in maintenance mode; + * ``cluster_unlocked``: ``True`` if cluster has no node holding the leader lock; + * ``failsafe_mode_is_active``: ``True`` if DCS failsafe mode is currently active; + * ``dcs_last_seen``: epoch timestamp DCS was last reached by Patroni. """ postgresql = self.server.patroni.postgresql cluster = self.server.patroni.dcs.cluster @@ -715,13 +1175,14 @@ class RestApiHandler(BaseHTTPRequestHandler): result['role'] = postgresql.role if result['role'] == 'replica' and global_config.is_synchronous_mode\ - and cluster.sync.matches(postgresql.name): + and cluster and cluster.sync.matches(postgresql.name): result['sync_standby'] = True if row[1] > 0: result['timeline'] = row[1] else: - leader_timeline = None if not cluster or cluster.is_unlocked() else cluster.leader.timeline + leader_timeline = None\ + if not cluster or cluster.is_unlocked() or not cluster.leader else cluster.leader.timeline result['timeline'] = postgresql.replica_cached_timeline(leader_timeline) if row[7]: @@ -732,7 +1193,7 @@ class RestApiHandler(BaseHTTPRequestHandler): if state == 'running': logger.exception('get_postgresql_status') state = 'unknown' - result = {'state': state, 'role': postgresql.role} + result: Dict[str, Any] = {'state': state, 'role': postgresql.role} if global_config.is_paused: result['pause'] = True @@ -743,34 +1204,72 @@ class RestApiHandler(BaseHTTPRequestHandler): result['dcs_last_seen'] = self.server.patroni.dcs.last_seen return result - def handle_one_request(self): + def handle_one_request(self) -> None: + """Parse and dispatch a request to the appropriate ``do_*`` method. + + .. note:: + This is only used to keep track of latency when logging messages through :func:`log_message`. + """ self.__start_time = time.time() BaseHTTPRequestHandler.handle_one_request(self) - def log_message(self, fmt, *args): + def log_message(self, format: str, *args: Any) -> None: + """Log a custom ``debug`` message. + + Additionally, to *format*, the log entry contains the client IP address and the current latency of the request. + + :param format: printf-style format string message to be logged. + :param args: arguments to be applied as inputs to *format*. + """ latency = 1000.0 * (time.time() - self.__start_time) - logger.debug("API thread: %s - - %s latency: %0.3f ms", self.client_address[0], fmt % args, latency) + logger.debug("API thread: %s - - %s latency: %0.3f ms", self.client_address[0], format % args, latency) class RestApiServer(ThreadingMixIn, HTTPServer, Thread): + """Patroni REST API server. + + An asynchronous thread-based HTTP server. + """ + # On 3.7+ the `ThreadingMixIn` gathers all non-daemon worker threads in order to join on them at server close. daemon_threads = True # Make worker threads "fire and forget" to prevent a memory leak. - def __init__(self, patroni, config): + def __init__(self, patroni: Patroni, config: Dict[str, Any]) -> None: + """Establish patroni configuration for the REST API daemon. + + Create a :class:`RestApiServer` instance. + + :param patroni: Patroni daemon process. + :param config: ``restapi`` section of Patroni configuration. + """ + self.connection_string = None + self.__auth_key = None + self.__allowlist_include_members: Optional[bool] = None + self.__allowlist: Tuple[Union[IPv4Network, IPv6Network], ...] = () + self.http_extra_headers: Dict[str, str] = {} self.patroni = patroni self.__listen = None self.request_queue_size = int(config.get('request_queue_size', 5)) - self.__ssl_options = None + self.__ssl_options: Dict[str, Any] = {} self.__ssl_serial_number = None self._received_new_cert = False self.reload_config(config) self.daemon = True - def query(self, sql, *params): + def query(self, sql: str, *params: Any) -> List[Tuple[Any, ...]]: + """Execute *sql* query with *params*. + + :param sql: the SQL statement to be run. + :param params: positional arguments to be used as parameters for *sql*. + + :returns: a list of rows that were fetched from the database. + :raises psycopg.Error: if had issues while executing *sql*. + :raises PostgresConnectionException: if had issues while connecting to the database. + """ cursor = None try: with self.patroni.postgresql.connection().cursor() as cursor: - cursor.execute(sql, params) + cursor.execute(sql.encode('utf-8'), params) return [r for r in cursor] except psycopg.Error as e: if cursor and cursor.connection.closed == 0: @@ -778,16 +1277,39 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): raise PostgresConnectionException('connection problems') @staticmethod - def _set_fd_cloexec(fd): + def _set_fd_cloexec(fd: socket.socket) -> None: + """Set ``FD_CLOEXEC`` for *fd*. + + It is used to avoid inheriting the REST API port when forking its process. + + .. note:: + Only takes effect on non-Windows environments. + + :param fd: socket file descriptor. + """ if os.name != 'nt': import fcntl flags = fcntl.fcntl(fd, fcntl.F_GETFD) fcntl.fcntl(fd, fcntl.F_SETFD, flags | fcntl.FD_CLOEXEC) - def check_basic_auth_key(self, key): + def check_basic_auth_key(self, key: str) -> bool: + """Check if *key* matches the password configured for the REST API. + + :param key: the password received through the Basic authorization header of an HTTP request. + + :returns: ``True`` if *key* matches the password configured for the REST API. + """ + # pyright -- ``__auth_key`` was already checked through the caller method (:func:`check_auth_header`). + assert self.__auth_key is not None return hmac.compare_digest(self.__auth_key, key.encode('utf-8')) - def check_auth_header(self, auth_header): + def check_auth_header(self, auth_header: Optional[str]) -> Optional[str]: + """Validate HTTP Basic authorization header, if present. + + :param auth_header: value of ``Authorization`` HTTP header, if present, else ``None``. + + :returns: an error message if any issue is found, ``None`` otherwise. + """ if self.__auth_key: if auth_header is None: return 'no auth header received' @@ -795,14 +1317,29 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): return 'not authenticated' @staticmethod - def __resolve_ips(host, port): + def __resolve_ips(host: str, port: int) -> Iterator[Union[IPv4Network, IPv6Network]]: + """Resolve *host* + *port* to one or more IP networks. + + :param host: hostname to be checked. + :param port: port to be checked. + + :rtype: Iterator[Union[IPv4Network, IPv6Network]] of *host* + *port* resolved to IP networks. + """ try: for _, _, _, _, sa in socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM, socket.IPPROTO_TCP): yield ip_network(sa[0], False) except Exception as e: logger.error('Failed to resolve %s: %r', host, e) - def __members_ips(self): + def __members_ips(self) -> Iterator[Union[IPv4Network, IPv6Network]]: + """Resolve each Patroni node ``restapi.connect_address`` to IP networks. + + .. note:: + Only yields object if ``restapi.allowlist_include_members`` setting is enabled. + + :rtype: Iterator[Union[IPv4Network, IPv6Network]] of each node ``restapi.connect_address`` resolved to an IP + network. + """ cluster = self.patroni.dcs.cluster if self.__allowlist_include_members and cluster: for cluster in [cluster] + list(cluster.workers.values()): @@ -810,14 +1347,27 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): if member.api_url: try: r = urlparse(member.api_url) - host = r.hostname - port = r.port or (443 if r.scheme == 'https' else 80) - for ip in self.__resolve_ips(host, port): - yield ip + if r.hostname: + port = r.port or (443 if r.scheme == 'https' else 80) + for ip in self.__resolve_ips(r.hostname, port): + yield ip except Exception as e: logger.debug('Failed to parse url %s: %r', member.api_url, e) - def check_access(self, rh): + def check_access(self, rh: RestApiHandler) -> Optional[bool]: + """Ensure client has enough privileges to perform a given request. + + Write a response back to the client if any issue is observed, and the HTTP status may be: + * ``401``: if ``Authorization`` header is missing or contain an invalid password; + * ``403``: if: + * ``restapi.allowlist`` was configured, but client IP is not in the allowed list; or + * ``restapi.allowlist_include_members`` is enabled, but client IP is not in the members list; or + * a client certificate is expected by the server, but is missing in the request. + + :param rh: the request which access should be checked. + + :returns: ``True`` if client access verification succeeded, otherwise ``None``. + """ if self.__allowlist or self.__allowlist_include_members: incoming_ip = ip_address(rh.client_address[0]) if not any(incoming_ip in net for net in self.__allowlist + tuple(self.__members_ips())): @@ -834,7 +1384,11 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): return True @staticmethod - def __has_dual_stack(): + def __has_dual_stack() -> bool: + """Check if the system has support for dual stack sockets. + + :returns: ``True`` if it has support for dual stack sockets. + """ if hasattr(socket, 'AF_INET6') and hasattr(socket, 'IPPROTO_IPV6') and hasattr(socket, 'IPV6_V6ONLY'): sock = None try: @@ -848,12 +1402,21 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): sock.close() return False - def __httpserver_init(self, host, port): - dual_stack = self.__has_dual_stack() - if host in ('', '*'): - host = None + def __httpserver_init(self, host: str, port: int) -> None: + """Start REST API HTTP server. - info = socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket.SOCK_STREAM, 0, socket.AI_PASSIVE) + .. note:: + If system has no support for dual stack sockets, then IPv4 is preferred over IPv6. + + :param host: host to bind REST API to. + :param port: port to bind REST API to. + """ + dual_stack = self.__has_dual_stack() + hostname = host + if hostname in ('', '*'): + hostname = None + + info = socket.getaddrinfo(hostname, port, socket.AF_UNSPEC, socket.SOCK_STREAM, 0, socket.AI_PASSIVE) # in case dual stack is not supported we want IPv4 to be preferred over IPv6 info.sort(key=lambda x: x[0] == socket.AF_INET, reverse=not dual_stack) @@ -862,10 +1425,32 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): HTTPServer.__init__(self, info[0][-1][:2], RestApiHandler) except socket.error: logger.error( - "Couldn't start a service on '%s:%s', please check your `restapi.listen` configuration", host, port) + "Couldn't start a service on '%s:%s', please check your `restapi.listen` configuration", hostname, port) raise - def __initialize(self, listen, ssl_options): + def __initialize(self, listen: str, ssl_options: Dict[str, Any]) -> None: + """Configure and start REST API HTTP server. + + .. note:: + This method can be called upon first initialization, and also when reloading Patroni. When reloading + Patroni, it restarts the HTTP server thread. + + :param listen: IP and port to bind REST API to. It should be a string in the format ``host:port``, where + ``host`` can be a hostname or IP address. It is the value of ``restapi.listen`` setting. + :param ssl_options: dictionary that may contain the following keys, depending on what has been configured in + ``restapi` section: + * ``certfile``: path to PEM certificate. If given, will start in HTTPS mode; + * ``keyfile``: path to key of ``certfile``; + * ``keyfile_password``: password for decrypting ``keyfile``; + * ``cafile``: path to CA file to validate client certificates; + * ``ciphers``: permitted cipher suites; + * ``verify_client``: value can be one among: + * ``none``: do not check client certificates; + * ``optional``: check client certificate only for unsafe REST API endpoints; + * ``required``: check client certificate for all REST API endpoints. + + :raises ValueError: if any issue is faced while parsing *listen*. + """ try: host, port = split_host_port(listen, None) except Exception: @@ -908,30 +1493,63 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): if reloading_config: self.start() - def process_request_thread(self, request, client_address): - enable_keepalive(request, 10, 3) + def process_request_thread(self, request: Union[socket.socket, Tuple[bytes, socket.socket]], + client_address: Tuple[str, int]) -> None: + """Process a request to the REST API. + + Wrapper for :func:`ThreadingMixIn.process_request_thread` that additionally: + * Enable TCP keepalive + * Perform SSL handshake (if an SSL socket). + + :param request: socket to handle the client request. + :param client_address: tuple containing the client IP and port. + """ + if isinstance(request, socket.socket): + enable_keepalive(request, 10, 3) if hasattr(request, 'context'): # SSLSocket - request.do_handshake() + from ssl import SSLSocket + if isinstance(request, SSLSocket): # pyright + request.do_handshake() super(RestApiServer, self).process_request_thread(request, client_address) - def shutdown_request(self, request): + def shutdown_request(self, request: Union[socket.socket, Tuple[bytes, socket.socket]]) -> None: + """Shut down a request to the REST API. + + Wrapper for :func:`HTTPServer.shutdown_request` that additionally: + * Perform SSL shutdown handshake (if a SSL socket). + + :param request: socket to handle the client request. + """ if hasattr(request, 'context'): # SSLSocket try: - request.unwrap() + from ssl import SSLSocket + if isinstance(request, SSLSocket): # pyright + request.unwrap() except Exception as e: logger.debug('Failed to shutdown SSL connection: %r', e) super(RestApiServer, self).shutdown_request(request) - def get_certificate_serial_number(self): + def get_certificate_serial_number(self) -> Optional[str]: + """Get serial number of the certificate used by the REST API. + + :returns: serial number of the certificate configured through ``restapi.certfile`` setting. + """ if self.__ssl_options.get('certfile'): import ssl try: - crt = ssl._ssl._test_decode_cert(self.__ssl_options['certfile']) - return crt.get('serialNumber') - except ssl.SSLError as e: + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + crts = ctx.load_verify_locations(self.__ssl_options['certfile']) + if crts: + return crts[0].get('serialNumber') + except Exception as e: logger.error('Failed to get serial number from certificate %s: %r', self.__ssl_options['certfile'], e) - def reload_local_certificate(self): + def reload_local_certificate(self) -> Optional[bool]: + """Reload the SSL certificate used by the REST API. + + :return: ``True`` if a different certificate has been configured through ``restapi.certfile` setting, ``None`` + otherwise. + """ if self.__protocol == 'https': on_disk_cert_serial_number = self.get_certificate_serial_number() if on_disk_cert_serial_number != self.__ssl_serial_number: @@ -939,7 +1557,14 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): self.__ssl_serial_number = on_disk_cert_serial_number return True - def _build_allowlist(self, value): + def _build_allowlist(self, value: Optional[List[str]]) -> Iterator[Union[IPv4Network, IPv6Network]]: + """Resolve each entry in *value* to an IP network object. + + :param value: list of IPs and/or networks contained in ``restapi.allowlist`` setting. Each item can be a host, + an IP, or a network in CIDR format. + + :rtype: Iterator[Union[IPv4Network, IPv6Network]] of *host* + *port* resolved to IP networks. + """ if isinstance(value, list): for v in value: if '/' in v: # netmask @@ -951,7 +1576,12 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): for ip in self.__resolve_ips(v, 8080): yield ip - def reload_config(self, config): + def reload_config(self, config: Dict[str, Any]) -> None: + """Reload REST API configuration. + + :param config: dictionary representing values under the ``restapi`` configuration section. + :raises ValueError: if ``listen`` key is not present in *config*. + """ if 'listen' not in config: # changing config in runtime raise ValueError('Can not find "restapi.listen" config') @@ -971,10 +1601,20 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): self.__initialize(config['listen'], ssl_options) self.__auth_key = base64.b64encode(config['auth'].encode('utf-8')) if 'auth' in config else None + # pyright -- ``__listen`` is initially created as ``None``, but right after that it is replaced with a string + # through :func:`__initialize`. + assert isinstance(self.__listen, str) self.connection_string = uri(self.__protocol, config.get('connect_address') or self.__listen, 'patroni') - @staticmethod - def handle_error(request, client_address): + def handle_error(self, request: Union[socket.socket, Tuple[bytes, socket.socket]], + client_address: Tuple[str, int]) -> None: + """Handle any exception that is thrown while handling a request to the REST API. + + Logs ``WARNING`` messages with the client information, and the stack trace of the faced exception. + + :param request: the request that faced an exception. + :param client_address: a tuple composed of the IP and port of the client connection. + """ logger.warning('Exception happened during processing of request from %s:%s', client_address[0], client_address[1]) logger.warning(traceback.format_exc()) diff --git a/tests/test_api.py b/tests/test_api.py index 85c01b49..166d3eb1 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -180,6 +180,7 @@ class MockRestApiServer(RestApiServer): @patch('ssl.SSLContext.load_cert_chain', Mock()) @patch('ssl.SSLContext.wrap_socket', Mock(return_value=0)) +@patch('ssl.SSLContext.load_verify_locations', Mock(return_value=[Mock()])) @patch.object(HTTPServer, '__init__', Mock()) class TestRestApiHandler(unittest.TestCase): @@ -588,6 +589,7 @@ class TestRestApiServer(unittest.TestCase): @patch('ssl.SSLContext.load_cert_chain', Mock()) @patch('ssl.SSLContext.set_ciphers', Mock()) @patch('ssl.SSLContext.wrap_socket', Mock(return_value=0)) + @patch('ssl.SSLContext.load_verify_locations', Mock(return_value=[Mock()])) @patch.object(HTTPServer, '__init__', Mock()) def setUp(self): self.srv = MockRestApiServer(Mock(), '', {'listen': '*:8008', 'certfile': 'a', 'verify_client': 'required', @@ -621,24 +623,39 @@ class TestRestApiServer(unittest.TestCase): try: raise Exception() except Exception: - self.assertIsNone(MockRestApiServer.handle_error(None, ('127.0.0.1', 55555))) + self.assertIsNone(self.srv.handle_error(None, ('127.0.0.1', 55555))) @patch.object(HTTPServer, '__init__', Mock(side_effect=socket.error)) def test_socket_error(self): self.assertRaises(socket.error, MockRestApiServer, Mock(), '', {'listen': '*:8008'}) + def __create_socket(self): + sock = socket.socket() + try: + import ssl + ctx = ssl.create_default_context(ssl.Purpose.SERVER_AUTH) + ctx.check_hostname = False + sock = ctx.wrap_socket(sock=sock) + sock.do_handshake = Mock() + sock.unwrap = Mock(side_effect=Exception) + except Exception: + pass + return sock + @patch.object(ThreadingMixIn, 'process_request_thread', Mock()) def test_process_request_thread(self): - self.srv.process_request_thread(Mock(), '2') + self.srv.process_request_thread(self.__create_socket(), ('2', 54321)) @patch.object(MockRestApiServer, 'process_request', Mock(side_effect=RuntimeError)) @patch.object(MockRestApiServer, 'get_request') def test_process_request_error(self, mock_get_request): - mock_request = Mock() - mock_request.unwrap.side_effect = Exception - mock_get_request.return_value = (mock_request, ('127.0.0.1', 55555)) + mock_get_request.return_value = (self.__create_socket(), ('127.0.0.1', 55555)) self.srv._handle_request_noblock() - @patch('ssl._ssl._test_decode_cert', Mock()) + @patch('ssl.SSLContext.load_verify_locations', Mock(return_value=[Mock()])) def test_reload_local_certificate(self): self.assertTrue(self.srv.reload_local_certificate()) + + @patch('ssl.SSLContext.load_verify_locations', Mock(side_effect=Exception)) + def test_get_certificate_serial_number(self): + self.assertIsNone(self.srv.get_certificate_serial_number()) From 66a0e4437189b59b48e49d9626f96eafefda7cf7 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 15 May 2023 11:38:40 +0200 Subject: [PATCH 3/8] Enable pyright job for every commit (#2675) And fix remaining issues that the job doesn't fail. --- .github/workflows/tests.yaml | 15 ++++++ patroni/api.py | 65 ++++++++++++----------- patroni/config.py | 15 +++--- patroni/ctl.py | 20 +++---- patroni/dcs/__init__.py | 92 +++++++++++++++++---------------- patroni/dcs/consul.py | 31 +++++------ patroni/dcs/etcd.py | 57 ++++++++++---------- patroni/dcs/etcd3.py | 90 +++++++++++++++++--------------- patroni/dcs/kubernetes.py | 90 ++++++++++++++++++-------------- patroni/dcs/raft.py | 20 +++---- patroni/dcs/zookeeper.py | 30 +++++------ patroni/ha.py | 40 +++++++------- patroni/log.py | 10 ++-- patroni/postgresql/__init__.py | 8 ++- patroni/postgresql/config.py | 4 +- patroni/postgresql/sync.py | 6 +-- patroni/scripts/wale_restore.py | 5 +- patroni/validator.py | 5 +- patroni/watchdog/base.py | 4 +- patroni/watchdog/linux.py | 1 + pyrightconfig.json | 2 +- tests/test_ha.py | 4 +- tests/test_postgresql.py | 3 +- 23 files changed, 337 insertions(+), 280 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index aea92626..f24fa76f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -157,3 +157,18 @@ jobs: steps: - run: bash <(curl -Ls https://coverage.codacy.com/get.sh) final if: ${{ env.SECRETS_AVAILABLE == 'true' }} + + pyright: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Set up Python 3.11 + uses: actions/setup-python@v4 + with: + python-version: 3.11 + + - name: Install dependencies + run: python -m pip install -r requirements.txt psycopg2-binary psycopg + + - uses: jakebailey/pyright-action@v1 diff --git a/patroni/api.py b/patroni/api.py index 0fb39157..f461811b 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -24,7 +24,7 @@ from socketserver import ThreadingMixIn from threading import Thread from urllib.parse import urlparse, parse_qs -from typing import Any, Callable, Dict, Iterator, List, Optional, Tuple, Union +from typing import Any, Callable, Dict, Iterator, List, Optional, Tuple, TYPE_CHECKING, Union from . import psycopg from .__main__ import Patroni @@ -86,7 +86,8 @@ class RestApiHandler(BaseHTTPRequestHandler): :param client_address: address of the client connection. :param server: HTTP server that received the request. """ - assert isinstance(server, RestApiServer) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(server, RestApiServer) super(RestApiHandler, self).__init__(request, client_address, server) self.server: 'RestApiServer' = server self.__start_time: float = 0.0 @@ -113,8 +114,8 @@ class RestApiHandler(BaseHTTPRequestHandler): self.wfile.write('{0} {1} {2}\r\n\r\n'.format(self.protocol_version, status_code, message).encode('utf-8')) self.log_request(status_code) - def _write_response(self, status_code: int, body: str, content_type: str = 'text/html', - headers: Optional[Dict[str, str]] = None) -> None: + def write_response(self, status_code: int, body: str, content_type: str = 'text/html', + headers: Optional[Dict[str, str]] = None) -> None: """Write an HTTP response. .. note:: @@ -143,12 +144,12 @@ class RestApiHandler(BaseHTTPRequestHandler): def _write_json_response(self, status_code: int, response: Any) -> None: """Write an HTTP response with a JSON content type. - Call :func:`_write_response` with ``content_type`` as ``application/json``. + Call :func:`write_response` with ``content_type`` as ``application/json``. :param status_code: response HTTP status code. :param response: value to be dumped as a JSON string and to be used as the response body. """ - self._write_response(status_code, json.dumps(response, default=str), content_type='application/json') + self.write_response(status_code, json.dumps(response, default=str), content_type='application/json') def _write_status_response(self, status_code: int, response: Dict[str, Any]) -> None: """Write an HTTP response with Patroni/Postgres status in JSON format. @@ -457,7 +458,7 @@ class RestApiHandler(BaseHTTPRequestHandler): * ``patroni_xlog_paused``: ``pg_is_wal_replay_paused()``; * ``patroni_postgres_server_version``: Postgres version without periods, e.g. ``150002`` for Postgres ``15.2``; * ``patroni_cluster_unlocked``: ``1`` if no one holds the leader lock, else ``0``; - * ``patroni_failsafe_mode_is_active``: ``1`` if ``failmode`` is currently active, else ``0``; + * ``patroni_failsafe_mode_is_active``: ``1`` if ``failsafe_mode`` is currently active, else ``0``; * ``patroni_postgres_timeline``: PostgreSQL timeline based on current WAL file name; * ``patroni_dcs_last_seen``: epoch timestamp when DCS was last contacted successfully; * ``patroni_pending_restart``: ``1`` if this PostgreSQL node is pending a restart, else ``0``; @@ -565,7 +566,7 @@ class RestApiHandler(BaseHTTPRequestHandler): metrics.append("# TYPE patroni_is_paused gauge") metrics.append("patroni_is_paused{0} {1}".format(scope_label, int(postgres.get('pause', 0)))) - self._write_response(200, '\n'.join(metrics) + '\n', content_type='text/plain') + self.write_response(200, '\n'.join(metrics) + '\n', content_type='text/plain') def _read_json_content(self, body_is_optional: bool = False) -> Optional[Dict[Any, Any]]: """Read JSON from HTTP request body. @@ -615,12 +616,12 @@ class RestApiHandler(BaseHTTPRequestHandler): request = self._read_json_content() if request: cluster = self.server.patroni.dcs.get_cluster(True) - if not (cluster.config and cluster.config.modify_index): + if not (cluster.config and cluster.config.modify_version): return self.send_error(503) data = cluster.config.data.copy() if patch_config(data, request): value = json.dumps(data, separators=(',', ':')) - if not self.server.patroni.dcs.set_config_value(value, cluster.config.index): + if not self.server.patroni.dcs.set_config_value(value, cluster.config.version): return self.send_error(409) self.server.patroni.ha.wakeup() self._write_json_response(200, data) @@ -651,7 +652,7 @@ class RestApiHandler(BaseHTTPRequestHandler): Schedules a reload to Patroni and writes a response with HTTP status `202`. """ self.server.patroni.sighup_handler() - self._write_response(202, 'reload scheduled') + self.write_response(202, 'reload scheduled') def do_GET_failsafe(self) -> None: """Handle a ``GET`` request to ``/failsafe`` path. @@ -684,7 +685,7 @@ class RestApiHandler(BaseHTTPRequestHandler): if request: message = self.server.patroni.ha.update_failsafe(request) or 'Accepted' code = 200 if message == 'Accepted' else 500 - self._write_response(code, message) + self.write_response(code, message) else: self.send_error(502) @@ -699,7 +700,7 @@ class RestApiHandler(BaseHTTPRequestHandler): """ if os.name == 'nt' and os.getenv('BEHAVE_DEBUG'): self.server.patroni.api_sigterm() - self._write_response(202, 'shutdown scheduled') + self.write_response(202, 'shutdown scheduled') @staticmethod def parse_schedule(schedule: str, @@ -779,7 +780,7 @@ class RestApiHandler(BaseHTTPRequestHandler): logger.debug("received restart request: {0}".format(request)) if self.server.patroni.config.get_global_config(cluster).is_paused and 'schedule' in request: - self._write_response(status_code, "Can't schedule restart in the paused state") + self.write_response(status_code, "Can't schedule restart in the paused state") return for k in request: @@ -827,8 +828,9 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 409 # pyright thinks ``data`` can be ``None`` because ``parse_schedule`` call may return ``None``. However, if # that's the case, ``data`` will be overwritten when the ``for`` loop ends - assert isinstance(data, str) - self._write_response(status_code, data) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(data, str) + self.write_response(status_code, data) @check_access def do_DELETE_restart(self) -> None: @@ -846,7 +848,7 @@ class RestApiHandler(BaseHTTPRequestHandler): else: data = "no restarts are scheduled" code = 404 - self._write_response(code, data) + self.write_response(code, data) @check_access def do_DELETE_switchover(self) -> None: @@ -861,7 +863,7 @@ class RestApiHandler(BaseHTTPRequestHandler): """ failover = self.server.patroni.dcs.get_cluster().failover if failover and failover.scheduled_at: - if not self.server.patroni.dcs.manual_failover('', '', index=failover.index): + if not self.server.patroni.dcs.manual_failover('', '', version=failover.version): return self.send_error(409) else: data = "scheduled switchover deleted" @@ -869,7 +871,7 @@ class RestApiHandler(BaseHTTPRequestHandler): else: data = "no switchover is scheduled" code = 404 - self._write_response(code, data) + self.write_response(code, data) @check_access def do_POST_reinitialize(self) -> None: @@ -895,7 +897,7 @@ class RestApiHandler(BaseHTTPRequestHandler): data = 'reinitialize started' else: status_code = 503 - self._write_response(status_code, data) + self.write_response(status_code, data) def poll_failover_result(self, leader: Optional[str], candidate: Optional[str], action: str) -> Tuple[int, str]: """Poll failover/switchover operation until it finishes or times out. @@ -1033,9 +1035,10 @@ class RestApiHandler(BaseHTTPRequestHandler): status_code = 503 # pyright thinks ``status_code`` can be ``None`` because ``parse_schedule`` call may return ``None``. However, # if that's the case, ``status_code`` will be overwritten somewhere between ``parse_schedule`` and - # ``_write_response`` calls. - assert isinstance(status_code, int) - self._write_response(status_code, data) + # ``write_response`` calls. + if TYPE_CHECKING: # pragma: no cover + assert isinstance(status_code, int) + self.write_response(status_code, data) def do_POST_switchover(self) -> None: """Handle a ``POST`` request to ``/switchover`` path. @@ -1062,7 +1065,7 @@ class RestApiHandler(BaseHTTPRequestHandler): if patroni.postgresql.citus_handler.is_coordinator() and patroni.ha.is_leader(): cluster = patroni.dcs.get_cluster(True) patroni.postgresql.citus_handler.handle_event(cluster, request) - self._write_response(200, 'OK') + self.write_response(200, 'OK') def parse_request(self) -> bool: """Override :func:`parse_request` method to enrich basic functionality of :class:`BaseHTTPRequestHandler`. @@ -1242,7 +1245,7 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): :param patroni: Patroni daemon process. :param config: ``restapi`` section of Patroni configuration. """ - self.connection_string = None + self.connection_string: str self.__auth_key = None self.__allowlist_include_members: Optional[bool] = None self.__allowlist: Tuple[Union[IPv4Network, IPv6Network], ...] = () @@ -1300,7 +1303,8 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): :returns: ``True`` if *key* matches the password configured for the REST API. """ # pyright -- ``__auth_key`` was already checked through the caller method (:func:`check_auth_header`). - assert self.__auth_key is not None + if TYPE_CHECKING: # pragma: no cover + assert self.__auth_key is not None return hmac.compare_digest(self.__auth_key, key.encode('utf-8')) def check_auth_header(self, auth_header: Optional[str]) -> Optional[str]: @@ -1371,16 +1375,16 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): if self.__allowlist or self.__allowlist_include_members: incoming_ip = ip_address(rh.client_address[0]) if not any(incoming_ip in net for net in self.__allowlist + tuple(self.__members_ips())): - return rh._write_response(403, 'Access is denied') + return rh.write_response(403, 'Access is denied') if not hasattr(rh.request, 'getpeercert') or not rh.request.getpeercert(): # valid client cert isn't present if self.__protocol == 'https' and self.__ssl_options.get('verify_client') in ('required', 'optional'): - return rh._write_response(403, 'client certificate required') + return rh.write_response(403, 'client certificate required') reason = self.check_auth_header(rh.headers.get('Authorization')) if reason: headers = {'WWW-Authenticate': 'Basic realm="' + self.patroni.__class__.__name__ + '"'} - return rh._write_response(401, reason, headers=headers) + return rh.write_response(401, reason, headers=headers) return True @staticmethod @@ -1603,7 +1607,8 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread): self.__auth_key = base64.b64encode(config['auth'].encode('utf-8')) if 'auth' in config else None # pyright -- ``__listen`` is initially created as ``None``, but right after that it is replaced with a string # through :func:`__initialize`. - assert isinstance(self.__listen, str) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(self.__listen, str) self.connection_string = uri(self.__protocol, config.get('connect_address') or self.__listen, 'patroni') def handle_error(self, request: Union[socket.socket, Tuple[bytes, socket.socket]], diff --git a/patroni/config.py b/patroni/config.py index 4d3eea8e..abd2371d 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -7,7 +7,7 @@ import yaml from collections import defaultdict from copy import deepcopy -from typing import Any, Callable, Collection, Dict, List, Optional, Union +from typing import Any, Callable, Collection, Dict, List, Optional, Union, TYPE_CHECKING from . import PATRONI_ENV_PREFIX from .collections import CaseInsensitiveDict @@ -151,7 +151,7 @@ def get_global_config(cluster: Union[Cluster, None], default: Optional[Dict[str, :returns: :class:`GlobalConfig` object """ # Try to protect from the case when DCS was wiped out - if cluster and cluster.config and cluster.config.modify_index: + if cluster and cluster.config and cluster.config.modify_version: config = cluster.config.data else: config = default or {} @@ -202,7 +202,7 @@ class Config(object): def __init__(self, configfile: str, validator: Optional[Callable[[Dict[str, Any]], List[str]]] = default_validator) -> None: - self._modify_index = -1 + self._modify_version = -1 self._dynamic_configuration = {} self.__environment_configuration = self._build_environment_configuration() @@ -257,7 +257,8 @@ class Config(object): def _load_config_file(self) -> Dict[str, Any]: """Loads config.yaml from filesystem and applies some values which were set via ENV""" - assert self._config_file is not None + if TYPE_CHECKING: # pragma: no cover + assert self._config_file is not None config = self._load_config_path(self._config_file) patch_config(config, self.__environment_configuration) return config @@ -296,9 +297,9 @@ class Config(object): # configuration could be either ClusterConfig or dict def set_dynamic_configuration(self, configuration: Union[ClusterConfig, Dict[str, Any]]) -> bool: if isinstance(configuration, ClusterConfig): - if self._modify_index == configuration.modify_index: - return False # If the index didn't changed there is nothing to do - self._modify_index = configuration.modify_index + if self._modify_version == configuration.modify_version: + return False # If the version didn't changed there is nothing to do + self._modify_version = configuration.modify_version configuration = configuration.data if not deep_compare(self._dynamic_configuration, configuration): diff --git a/patroni/ctl.py b/patroni/ctl.py index 786ea4ec..612a97db 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -32,9 +32,9 @@ if TYPE_CHECKING: # pragma: no cover from psycopg2 import cursor try: - from ydiff import markup_to_pager, PatchStream + from ydiff import markup_to_pager, PatchStream # pyright: ignore [reportMissingModuleSource] except ImportError: # pragma: no cover - from cdiff import markup_to_pager, PatchStream + from cdiff import markup_to_pager, PatchStream # pyright: ignore [reportMissingModuleSource] from .dcs import get_dcs as _get_dcs, AbstractDCS, Cluster, Member from .exceptions import PatroniException @@ -812,7 +812,8 @@ def _do_failover_or_switchover(obj: Dict[str, Any], action: str, cluster_name: s r = None try: member = cluster.leader.member if cluster.leader else candidate and cluster.get_member(candidate, False) - assert isinstance(member, Member) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(member, Member) r = request_patroni(member, 'post', action, failover_value) # probably old patroni, which doesn't support switchover yet @@ -1052,7 +1053,7 @@ def flush(obj: Dict[str, Any], cluster_name: str, group: Optional[int], logging.warning('Failing over to DCS') click.echo('{0} Could not find any accessible member of cluster {1}'.format(timestamp(), cluster_name)) - dcs.manual_failover('', '', index=failover.index) + dcs.manual_failover('', '', version=failover.version) def wait_until_pause_is_applied(dcs: AbstractDCS, paused: bool, old_cluster: Cluster) -> None: @@ -1060,7 +1061,7 @@ def wait_until_pause_is_applied(dcs: AbstractDCS, paused: bool, old_cluster: Clu config = get_global_config(old_cluster) click.echo("'{0}' request sent, waiting until it is recognized by all nodes".format(paused and 'pause' or 'resume')) - old = {m.name: m.index for m in old_cluster.members if m.api_url} + old = {m.name: m.version for m in old_cluster.members if m.api_url} loop_wait = config.get('loop_wait') or dcs.loop_wait cluster = None @@ -1072,7 +1073,7 @@ def wait_until_pause_is_applied(dcs: AbstractDCS, paused: bool, old_cluster: Clu if TYPE_CHECKING: # pragma: no cover assert cluster is not None remaining = [m.name for m in cluster.members if m.data.get('pause', False) != paused - and m.name in old and old[m.name] != m.index] + and m.name in old and old[m.name] != m.version] if remaining: return click.echo("{0} members didn't recognized pause state after {1} seconds" .format(', '.join(remaining), loop_wait)) @@ -1169,7 +1170,7 @@ def show_diff(before_editing: str, after_editing: str) -> None: ( os.path.basename(p) for p in (os.environ.get('PAGER'), "less", "more") - if p is not None and shutil.which(p) + if p is not None and bool(shutil.which(p)) ), None, ) @@ -1347,7 +1348,7 @@ def edit_config(obj: Dict[str, Any], cluster_name: str, group: Optional[int], return if force or click.confirm('Apply these changes?'): - if not dcs.set_config_value(json.dumps(changed_data), cluster.config.index): + if not dcs.set_config_value(json.dumps(changed_data), cluster.config.version): raise PatroniCtlException("Config modification aborted due to concurrent changes") click.echo("Configuration changed") @@ -1396,7 +1397,8 @@ def version(obj: Dict[str, Any], cluster_name: str, group: Optional[int], member @click.pass_obj def history(obj: Dict[str, Any], cluster_name: str, group: Optional[int], fmt: str) -> None: cluster = get_dcs(obj, cluster_name, group).get_cluster() - history: List[List[Any]] = list(map(list, cluster.history and cluster.history.lines or [])) + cluster_history = cluster.history.lines if cluster.history else [] + history: List[List[Any]] = list(map(list, cluster_history)) table_header_row = ['TL', 'LSN', 'Reason', 'Timestamp', 'New Leader'] for line in history: if len(line) < len(table_header_row): diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index f1429c6f..1ff4a60a 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -126,7 +126,7 @@ _Session = Union[int, float, str, None] class Member(NamedTuple): """Immutable object (namedtuple) which represents single member of PostgreSQL cluster. Consists of the following fields: - :param index: modification index of a given member key in a Configuration Store + :param version: modification version of a given member key in a Configuration Store :param name: name of PostgreSQL cluster member :param session: either session id or just ttl in seconds :param data: arbitrary data i.e. conn_url, api_url, xlog location, state, role, tags, etc... @@ -135,18 +135,18 @@ class Member(NamedTuple): conn_url: connection string containing host, user and password which could be used to access this member. api_url: REST API url of patroni instance """ - index: _Version + version: _Version name: str session: _Session data: Dict[str, Any] @staticmethod - def from_node(index: _Version, name: str, session: _Session, value: str) -> 'Member': + def from_node(version: _Version, name: str, session: _Session, value: str) -> 'Member': """ >>> Member.from_node(-1, '', '', '{"conn_url": "postgres://foo@bar/postgres"}') is not None True >>> Member.from_node(-1, '', '', '{') - Member(index=-1, name='', session='', data={}) + Member(version=-1, name='', session='', data={}) """ if value.startswith('postgres'): conn_url, api_url = parse_connection_string(value) @@ -157,7 +157,7 @@ class Member(NamedTuple): assert isinstance(data, dict) except (AssertionError, TypeError, ValueError): data: Dict[str, Any] = {} - return Member(index, name, session, data) + return Member(version, name, session, data) @property def conn_url(self) -> Optional[str]: @@ -229,7 +229,7 @@ class Member(NamedTuple): return self.state == 'running' @property - def version(self) -> Optional[Tuple[int, ...]]: + def patroni_version(self) -> Optional[Tuple[int, ...]]: version = self.data.get('version') if version: try: @@ -240,7 +240,9 @@ class Member(NamedTuple): class RemoteMember(Member): """Represents a remote member (typically a primary) for a standby cluster""" - def __new__(cls, name: str, data: Dict[str, Any]) -> 'RemoteMember': + + @classmethod + def from_name_and_data(cls, name: str, data: Dict[str, Any]) -> 'RemoteMember': return super(RemoteMember, cls).__new__(cls, -1, name, None, data) @staticmethod @@ -261,11 +263,11 @@ class Leader(NamedTuple): """Immutable object (namedtuple) which represents leader key. Consists of the following fields: - :param index: modification index of a leader key in a Configuration Store + :param version: modification version of a leader key in a Configuration Store :param session: either session id or just ttl in seconds :param member: reference to a `Member` object which represents current leader (see `Cluster.members`) """ - index: _Version + version: _Version session: _Session member: Member @@ -294,7 +296,7 @@ class Leader(NamedTuple): >>> Leader(1, '', Member.from_node(1, '', '', '{"version":"z"}')).checkpoint_after_promote """ - version = self.member.version + version = self.member.patroni_version # 1.5.6 is the last version which doesn't expose checkpoint_after_promote: false if version and version > (1, 5, 6): return self.data.get('role') in ('master', 'primary') and 'checkpoint_after_promote' not in self.data @@ -321,13 +323,13 @@ class Failover(NamedTuple): >>> 'abc' in Failover.from_node(1, 'abc:def') True """ - index: _Version + version: _Version leader: Optional[str] candidate: Optional[str] scheduled_at: Optional[datetime.datetime] @staticmethod - def from_node(index: _Version, value: Union[str, Dict[str, str]]) -> 'Failover': + def from_node(version: _Version, value: Union[str, Dict[str, str]]) -> 'Failover': if isinstance(value, dict): data: Dict[str, Any] = value elif value: @@ -340,26 +342,26 @@ class Failover(NamedTuple): t = [a.strip() for a in value.split(':')] leader = t[0] candidate = t[1] if len(t) > 1 else None - return Failover(index, leader, candidate, None) + return Failover(version, leader, candidate, None) else: data = {} if data.get('scheduled_at'): data['scheduled_at'] = dateutil.parser.parse(data['scheduled_at']) - return Failover(index, data.get('leader'), data.get('member'), data.get('scheduled_at')) + return Failover(version, data.get('leader'), data.get('member'), data.get('scheduled_at')) def __len__(self) -> int: return int(bool(self.leader)) + int(bool(self.candidate)) class ClusterConfig(NamedTuple): - index: _Version + version: _Version data: Dict[str, Any] - modify_index: _Version + modify_version: _Version @staticmethod - def from_node(index: _Version, value: str, modify_index: Optional[_Version] = None) -> 'ClusterConfig': + def from_node(version: _Version, value: str, modify_version: Optional[_Version] = None) -> 'ClusterConfig': """ >>> ClusterConfig.from_node(1, '{') is None False @@ -370,8 +372,8 @@ class ClusterConfig(NamedTuple): assert isinstance(data, dict) except (AssertionError, TypeError, ValueError): data: Dict[str, Any] = {} - modify_index = 0 - return ClusterConfig(index, data, index if modify_index is None else modify_index) + modify_version = 0 + return ClusterConfig(version, data, version if modify_version is None else modify_version) @property def permanent_slots(self) -> Dict[str, Any]: @@ -390,16 +392,16 @@ class ClusterConfig(NamedTuple): class SyncState(NamedTuple): """Immutable object (namedtuple) which represents last observed synhcronous replication state - :param index: modification index of a synchronization key in a Configuration Store + :param version: modification version of a synchronization key in a Configuration Store :param leader: reference to member that was leader :param sync_standby: synchronous standby list (comma delimited) which are last synchronized to leader """ - index: Optional[_Version] + version: Optional[_Version] leader: Optional[str] sync_standby: Optional[str] @staticmethod - def from_node(index: Optional[_Version], value: Union[str, Dict[str, Any], None]) -> 'SyncState': + def from_node(version: Optional[_Version], value: Union[str, Dict[str, Any], None]) -> 'SyncState': """ >>> SyncState.from_node(1, None).leader is None True @@ -418,13 +420,13 @@ class SyncState(NamedTuple): if value and isinstance(value, str): value = json.loads(value) assert isinstance(value, dict) - return SyncState(index, value.get('leader'), value.get('sync_standby')) + return SyncState(version, value.get('leader'), value.get('sync_standby')) except (AssertionError, TypeError, ValueError): - return SyncState.empty(index) + return SyncState.empty(version) @staticmethod - def empty(index: Optional[_Version] = None) -> 'SyncState': - return SyncState(index, None, None) + def empty(version: Optional[_Version] = None) -> 'SyncState': + return SyncState(version, None, None) @property def is_empty(self) -> bool: @@ -484,12 +486,12 @@ _HistoryTuple = Union[Tuple[int, int, str], Tuple[int, int, str, str], Tuple[int class TimelineHistory(NamedTuple): """Object representing timeline history file""" - index: _Version + version: _Version value: Any lines: List[_HistoryTuple] @staticmethod - def from_node(index: _Version, value: str) -> 'TimelineHistory': + def from_node(version: _Version, value: str) -> 'TimelineHistory': """ >>> h = TimelineHistory.from_node(1, 2) >>> h.lines @@ -500,7 +502,7 @@ class TimelineHistory(NamedTuple): assert isinstance(lines, list) except (AssertionError, TypeError, ValueError): lines: List[_HistoryTuple] = [] - return TimelineHistory(index, value, lines) + return TimelineHistory(version, value, lines) class Cluster(NamedTuple): @@ -537,7 +539,7 @@ class Cluster(NamedTuple): def is_empty(self): return self.initialize is None and self.config is None and self.leader is None and self.last_lsn == 0\ - and self.members == [] and self.failover is None and self.sync.index is None\ + and self.members == [] and self.failover is None and self.sync.version is None\ and self.history is None and self.slots is None and self.failsafe is None and self.workers == {} def __len__(self) -> int: @@ -701,7 +703,7 @@ class Cluster(NamedTuple): @property def min_version(self) -> Optional[Tuple[int, ...]]: - return next(iter(sorted(m.version for m in self.members if m.version)), None) + return next(iter(sorted(m.patroni_version for m in self.members if m.patroni_version)), None) class ReturnFalseException(Exception): @@ -870,7 +872,8 @@ class AbstractDCS(abc.ABC): if path is None: path = self.client_path('') cluster = self._load_cluster(path, self._cluster_loader) - assert isinstance(cluster, Cluster) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(cluster, Cluster) return cluster def is_citus_coordinator(self) -> bool: @@ -887,7 +890,6 @@ class AbstractDCS(abc.ABC): if isinstance(groups, Cluster): # Zookeeper could return a cached version cluster = groups else: - assert isinstance(groups, dict) cluster = groups.pop(CITUS_COORDINATOR_GROUP_ID, Cluster.empty()) cluster.workers.update(groups) return cluster @@ -1006,11 +1008,11 @@ class AbstractDCS(abc.ABC): process requests (hopefuly temporary), the ~DCSError exception should be raised""" @abc.abstractmethod - def set_failover_value(self, value: str, index: Optional[Any] = None) -> bool: + def set_failover_value(self, value: str, version: Optional[Any] = None) -> bool: """Create or update `/failover` key""" def manual_failover(self, leader: Optional[str], candidate: Optional[str], - scheduled_at: Optional[datetime.datetime] = None, index: Optional[Any] = None) -> bool: + scheduled_at: Optional[datetime.datetime] = None, version: Optional[Any] = None) -> bool: failover_value = {} if leader: failover_value['leader'] = leader @@ -1020,10 +1022,10 @@ class AbstractDCS(abc.ABC): if scheduled_at: failover_value['scheduled_at'] = scheduled_at.isoformat() - return self.set_failover_value(json.dumps(failover_value, separators=(',', ':')), index) + return self.set_failover_value(json.dumps(failover_value, separators=(',', ':')), version) @abc.abstractmethod - def set_config_value(self, value: str, index: Optional[Any] = None) -> bool: + def set_config_value(self, value: str, version: Optional[Any] = None) -> bool: """Create or update `/config` key""" @abc.abstractmethod @@ -1087,16 +1089,16 @@ class AbstractDCS(abc.ABC): return {'leader': leader, 'sync_standby': ','.join(sorted(sync_standby)) if sync_standby else None} def write_sync_state(self, leader: Optional[str], sync_standby: Optional[Collection[str]], - index: Optional[Any] = None) -> Optional[SyncState]: + version: Optional[Any] = None) -> Optional[SyncState]: """Write the new synchronous state to DCS. Calls :func:`sync_state` method to build a dict and than calls DCS specific :func:`set_sync_state_value` method. :param leader: name of the leader node that manages /sync key :param sync_standby: collection of currently known synchronous standby node names - :param index: for conditional update of the key/object + :param version: for conditional update of the key/object :returns: the new :class:`SyncState` object or None """ sync_value = self.sync_state(leader, sync_standby) - ret = self.set_sync_state_value(json.dumps(sync_value, separators=(',', ':')), index) + ret = self.set_sync_state_value(json.dumps(sync_value, separators=(',', ':')), version) if not isinstance(ret, bool): return SyncState.from_node(ret, sync_value) @@ -1105,23 +1107,23 @@ class AbstractDCS(abc.ABC): """""" @abc.abstractmethod - def set_sync_state_value(self, value: str, index: Optional[Any] = None) -> Union[Any, bool]: + def set_sync_state_value(self, value: str, version: Optional[Any] = None) -> Union[Any, bool]: """Set synchronous state in DCS, should be implemented in the child class. :param value: the new value of /sync key - :param index: for conditional update of the key/object + :param version: for conditional update of the key/object :returns: version of the new object or `False` in case of error """ @abc.abstractmethod - def delete_sync_state(self, index: Optional[Any] = None) -> bool: + def delete_sync_state(self, version: Optional[Any] = None) -> bool: """""" - def watch(self, leader_index: Optional[Any], timeout: float) -> bool: + def watch(self, leader_version: Optional[Any], timeout: float) -> bool: """If the current node is a leader it should just sleep. Any other node should watch for changes of leader key with a given timeout - :param leader_index: index of a leader key + :param leader_version: version of a leader key :param timeout: timeout in seconds :returns: `!True` if you would like to reschedule the next run of ha cycle""" diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py index 2d2d6708..2383a964 100644 --- a/patroni/dcs/consul.py +++ b/patroni/dcs/consul.py @@ -578,12 +578,12 @@ class Consul(AbstractDCS): return self.attempt_to_acquire_leader() @catch_consul_errors - def set_failover_value(self, value: str, index: Optional[int] = None) -> bool: - return self._client.kv.put(self.failover_path, value, cas=index) + def set_failover_value(self, value: str, version: Optional[int] = None) -> bool: + return self._client.kv.put(self.failover_path, value, cas=version) @catch_consul_errors - def set_config_value(self, value: str, index: Optional[int] = None) -> bool: - return self._client.kv.put(self.config_path, value, cas=index) + def set_config_value(self, value: str, version: Optional[int] = None) -> bool: + return self._client.kv.put(self.config_path, value, cas=version) @catch_consul_errors def _write_leader_optime(self, last_lsn: str) -> bool: @@ -622,7 +622,8 @@ class Consul(AbstractDCS): raise ConsulError('update_leader timeout') logger.warning('Recreating the leader key due to session mismatch') if cluster and cluster.leader: - self._run_and_handle_exceptions(self._client.kv.delete, self.leader_path, cas=cluster.leader.index) + self._run_and_handle_exceptions(self._client.kv.delete, self.leader_path, + cas=cluster.leader.version) retry.deadline = retry.stoptime - time.time() if retry.deadline < 0.5: @@ -653,14 +654,14 @@ class Consul(AbstractDCS): def _delete_leader(self) -> bool: cluster = self.cluster if cluster and isinstance(cluster.leader, Leader) and\ - cluster.leader.name == self._name and isinstance(cluster.leader.index, int): - return self._client.kv.delete(self.leader_path, cas=cluster.leader.index) + cluster.leader.name == self._name and isinstance(cluster.leader.version, int): + return self._client.kv.delete(self.leader_path, cas=cluster.leader.version) return True @catch_consul_errors - def set_sync_state_value(self, value: str, index: Optional[int] = None) -> Union[int, bool]: + def set_sync_state_value(self, value: str, version: Optional[int] = None) -> Union[int, bool]: retry = self._retry.copy() - ret = retry(self._client.kv.put, self.sync_path, value, cas=index) + ret = retry(self._client.kv.put, self.sync_path, value, cas=version) if ret: # We have no other choise, only read after write :( retry.deadline = retry.stoptime - time.time() if retry.deadline < 0.5: @@ -671,21 +672,21 @@ class Consul(AbstractDCS): return False @catch_consul_errors - def delete_sync_state(self, index: Optional[int] = None) -> bool: - return self.retry(self._client.kv.delete, self.sync_path, cas=index) + def delete_sync_state(self, version: Optional[int] = None) -> bool: + return self.retry(self._client.kv.delete, self.sync_path, cas=version) - def watch(self, leader_index: Optional[int], timeout: float) -> bool: + def watch(self, leader_version: Optional[int], timeout: float) -> bool: self._last_session_refresh = 0 if self.__do_not_watch: self.__do_not_watch = False return True - if leader_index: + if leader_version: end_time = time.time() + timeout while timeout >= 1: try: - idx, _ = self._client.kv.get(self.leader_path, index=leader_index, wait=str(timeout) + 's') - return str(idx) != str(leader_index) + idx, _ = self._client.kv.get(self.leader_path, index=leader_version, wait=str(timeout) + 's') + return str(idx) != str(leader_version) except (ConsulException, HTTPException, HTTPError, socket.error, socket.timeout): logger.exception('watch') diff --git a/patroni/dcs/etcd.py b/patroni/dcs/etcd.py index e53d8a67..190b79fc 100644 --- a/patroni/dcs/etcd.py +++ b/patroni/dcs/etcd.py @@ -290,7 +290,8 @@ class AbstractEtcdClientWithFailover(abc.ABC, etcd.Client): etcd_nodes = len(machines_cache) except Exception as e: logger.debug('Failed to update list of etcd nodes: %r', e) - assert isinstance(retry, Retry) # etcd.EtcdConnectionFailed is raised only if retry is not None! + if TYPE_CHECKING: # pragma: no cover + assert isinstance(retry, Retry) # etcd.EtcdConnectionFailed is raised only if retry is not None! sleeptime = retry.sleeptime remaining_time = retry.stoptime - sleeptime - time.time() nodes, timeout, retries = self._calculate_timeouts(etcd_nodes, remaining_time) @@ -502,6 +503,17 @@ class AbstractEtcd(AbstractDCS): if isinstance(raise_ex, Exception): raise raise_ex + def handle_etcd_exceptions(self, func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any: + try: + retval = func(self, *args, **kwargs) + self._has_failed = False + return retval + except (RetryFailedError, etcd.EtcdException) as e: + self._handle_exception(e) + return False + except Exception as e: + self._handle_exception(e, raise_ex=self._client.ERROR_CLS('unexpected error')) + def _run_and_handle_exceptions(self, method: Callable[..., Any], *args: Any, **kwargs: Any) -> Any: retry = kwargs.pop('retry', self.retry) try: @@ -624,16 +636,7 @@ class AbstractEtcd(AbstractDCS): def catch_etcd_errors(func: Callable[..., Any]) -> Any: def wrapper(self: AbstractEtcd, *args: Any, **kwargs: Any) -> Any: - try: - retval = func(self, *args, **kwargs) - self._has_failed = False - return retval - except (RetryFailedError, etcd.EtcdException) as e: - self._handle_exception(e) - return False - except Exception as e: - self._handle_exception(e, raise_ex=self._client.ERROR_CLS('unexpected error')) - + return self.handle_etcd_exceptions(func, *args, **kwargs) return wrapper @@ -645,7 +648,8 @@ class Etcd(AbstractEtcd): @property def _client(self) -> EtcdClient: - assert isinstance(self._abstract_client, EtcdClient) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(self._abstract_client, EtcdClient) return self._abstract_client def set_ttl(self, ttl: int) -> Optional[bool]: @@ -696,8 +700,8 @@ class Etcd(AbstractEtcd): if leader: member = Member(-1, leader.value, None, {}) member = ([m for m in members if m.name == leader.value] or [member])[0] - index = etcd_index if etcd_index > leader.modifiedIndex else leader.modifiedIndex + 1 - leader = Leader(index, leader.ttl, member) + version = etcd_index if etcd_index > leader.modifiedIndex else leader.modifiedIndex + 1 + leader = Leader(version, leader.ttl, member) # failover key failover = nodes.get(self._FAILOVER) @@ -742,7 +746,8 @@ class Etcd(AbstractEtcd): except Exception as e: self._handle_exception(e, 'get_cluster', raise_ex=EtcdError('Etcd is not responding properly')) self._has_failed = False - assert cluster is not None + if TYPE_CHECKING: # pragma: no cover + assert cluster is not None return cluster @catch_etcd_errors @@ -766,12 +771,12 @@ class Etcd(AbstractEtcd): return self._run_and_handle_exceptions(self._do_attempt_to_acquire_leader, retry=None) @catch_etcd_errors - def set_failover_value(self, value: str, index: Optional[int] = None) -> bool: - return bool(self._client.write(self.failover_path, value, prevIndex=index or 0)) + def set_failover_value(self, value: str, version: Optional[int] = None) -> bool: + return bool(self._client.write(self.failover_path, value, prevIndex=version or 0)) @catch_etcd_errors - def set_config_value(self, value: str, index: Optional[int] = None) -> bool: - return bool(self._client.write(self.config_path, value, prevIndex=index or 0)) + def set_config_value(self, value: str, version: Optional[int] = None) -> bool: + return bool(self._client.write(self.config_path, value, prevIndex=version or 0)) @catch_etcd_errors def _write_leader_optime(self, last_lsn: str) -> bool: @@ -817,24 +822,24 @@ class Etcd(AbstractEtcd): return bool(self._client.write(self.history_path, value)) @catch_etcd_errors - def set_sync_state_value(self, value: str, index: Optional[int] = None) -> Union[int, bool]: - return self.retry(self._client.write, self.sync_path, value, prevIndex=index or 0).modifiedIndex + def set_sync_state_value(self, value: str, version: Optional[int] = None) -> Union[int, bool]: + return self.retry(self._client.write, self.sync_path, value, prevIndex=version or 0).modifiedIndex @catch_etcd_errors - def delete_sync_state(self, index: Optional[int] = None) -> bool: - return bool(self.retry(self._client.delete, self.sync_path, prevIndex=index or 0)) + def delete_sync_state(self, version: Optional[int] = None) -> bool: + return bool(self.retry(self._client.delete, self.sync_path, prevIndex=version or 0)) - def watch(self, leader_index: Optional[int], timeout: float) -> bool: + def watch(self, leader_version: Optional[int], timeout: float) -> bool: if self.__do_not_watch: self.__do_not_watch = False return True - if leader_index: + if leader_version: end_time = time.time() + timeout while timeout >= 1: # when timeout is too small urllib3 doesn't have enough time to connect try: - result = self._client.watch(self.leader_path, index=leader_index, timeout=timeout + 0.5) + result = self._client.watch(self.leader_path, index=leader_version, timeout=timeout + 0.5) self._has_failed = False if result.action == 'compareAndSwap': time.sleep(0.01) diff --git a/patroni/dcs/etcd3.py b/patroni/dcs/etcd3.py index bdf5cc33..308bb2c3 100644 --- a/patroni/dcs/etcd3.py +++ b/patroni/dcs/etcd3.py @@ -13,7 +13,7 @@ from collections import defaultdict from enum import IntEnum from urllib3.exceptions import ReadTimeoutError, ProtocolError from threading import Condition, Lock, Thread -from typing import Any, Callable, Collection, Dict, Iterator, List, Optional, Tuple, Type, Union +from typing import Any, Callable, Collection, Dict, Iterator, List, Optional, Tuple, Type, TYPE_CHECKING, Union from . import ClusterConfig, Cluster, Failover, Leader, Member, SyncState,\ TimelineHistory, ReturnFalseException, catch_return_false_exception, citus_group_re @@ -145,7 +145,8 @@ errCodeToClientError = {getattr(s, 'code'): s for s in Etcd3ClientError.__subcla def _raise_for_data(data: Union[bytes, str, Dict[str, Union[Any, Dict[str, Any]]]], status_code: Optional[int] = None) -> Etcd3ClientError: try: - assert isinstance(data, dict) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(data, dict) data_error: Optional[Dict[str, Any]] = data.get('error') or data.get('Error') if isinstance(data_error, dict): # streaming response status_code = data_error.get('http_code') @@ -153,7 +154,8 @@ def _raise_for_data(data: Union[bytes, str, Dict[str, Union[Any, Dict[str, Any]] error: str = data_error['message'] else: data_code = data.get('code') or data.get('Code') - assert not isinstance(data_code, dict) + if TYPE_CHECKING: # pragma: no cover + assert not isinstance(data_code, dict) code = data_code error = str(data_error) except Exception: @@ -193,28 +195,7 @@ def build_range_request(key: str, range_end: Union[bytes, str, None] = None) -> def _handle_auth_errors(func: Callable[..., Any]) -> Any: def wrapper(self: 'Etcd3Client', *args: Any, **kwargs: Any) -> Any: - def retry(ex: Exception) -> Any: - if self.username and self.password: - self.authenticate() - return func(self, *args, **kwargs) - else: - logger.fatal('Username or password not set, authentication is not possible') - raise ex - - try: - return func(self, *args, **kwargs) - except (UserEmpty, PermissionDenied) as e: # no token provided - # PermissionDenied is raised on 3.0 and 3.1 - if self._cluster_version < (3, 3) and (not isinstance(e, PermissionDenied) - or self._cluster_version < (3, 2)): - raise UnsupportedEtcdVersion('Authentication is required by Etcd cluster but not ' - 'supported on version lower than 3.3.0. Cluster version: ' - '{0}'.format('.'.join(map(str, self._cluster_version)))) - return retry(e) - except InvalidAuthToken as e: - logger.error('Invalid auth token: %s', self._token) - return retry(e) - + return self.handle_auth_errors(func, *args, **kwargs) return wrapper @@ -322,6 +303,29 @@ class Etcd3Client(AbstractEtcdClientWithFailover): self._token = response.get('token') return old_token != self._token + def handle_auth_errors(self: 'Etcd3Client', func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any: + def retry(ex: Exception) -> Any: + if self.username and self.password: + self.authenticate() + return func(self, *args, **kwargs) + else: + logger.fatal('Username or password not set, authentication is not possible') + raise ex + + try: + return func(self, *args, **kwargs) + except (UserEmpty, PermissionDenied) as e: # no token provided + # PermissionDenied is raised on 3.0 and 3.1 + if self._cluster_version < (3, 3) and (not isinstance(e, PermissionDenied) + or self._cluster_version < (3, 2)): + raise UnsupportedEtcdVersion('Authentication is required by Etcd cluster but not ' + 'supported on version lower than 3.3.0. Cluster version: ' + '{0}'.format('.'.join(map(str, self._cluster_version)))) + return retry(e) + except InvalidAuthToken as e: + logger.error('Invalid auth token: %s', self._token) + return retry(e) + @_handle_auth_errors def range(self, key: str, range_end: Union[bytes, str, None] = None, retry: Optional[Retry] = None) -> Dict[str, Any]: @@ -401,7 +405,7 @@ class KVCache(Thread): self._leader_key = base64_encode(dcs.leader_path) self._optime_key = base64_encode(dcs.leader_optime_path) self._status_key = base64_encode(dcs.status_path) - self._name = base64_encode(dcs._name) + self._name = base64_encode(getattr(dcs, '_name')) # pyright self._is_ready = False self._response = None self._response_lock = Lock() @@ -582,9 +586,9 @@ class PatroniEtcd3Client(Etcd3Client): self._kv_cache.condition.wait(timeout) def get_cluster(self, path: str) -> List[Dict[str, Any]]: - if self._kv_cache and self._etcd3._retry.deadline is not None and path.startswith(self._etcd3.cluster_prefix): + if self._kv_cache and path.startswith(self._etcd3.cluster_prefix): with self._kv_cache.condition: - self._wait_cache(self._etcd3._retry.deadline) + self._wait_cache(self.read_timeout) ret = self._kv_cache.copy() else: ret = self._etcd3.retry(self.prefix, path).get('kvs', []) @@ -621,7 +625,6 @@ class Etcd3(AbstractEtcd): def __init__(self, config: Dict[str, Any]) -> None: super(Etcd3, self).__init__(config, PatroniEtcd3Client, (DeadlineExceeded, Unavailable, FailedPrecondition)) - assert isinstance(self._client, PatroniEtcd3Client) self.__do_not_watch = False self._lease = None self._last_lease_refresh = 0 @@ -633,12 +636,14 @@ class Etcd3(AbstractEtcd): @property def _client(self) -> PatroniEtcd3Client: - assert isinstance(self._abstract_client, PatroniEtcd3Client) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(self._abstract_client, PatroniEtcd3Client) return self._abstract_client def set_socket_options(self, sock: socket.socket, socket_options: Optional[Collection[Tuple[int, int, int]]]) -> None: - assert self._retry.deadline is not None + if TYPE_CHECKING: # pragma: no cover + assert self._retry.deadline is not None enable_keepalive(sock, self.ttl, int(self.loop_wait + self._retry.deadline)) def set_ttl(self, ttl: int) -> Optional[bool]: @@ -773,7 +778,8 @@ class Etcd3(AbstractEtcd): except Exception as e: self._handle_exception(e, 'get_cluster', raise_ex=Etcd3Error('Etcd is not responding properly')) self._has_failed = False - assert cluster is not None + if TYPE_CHECKING: # pragma: no cover + assert cluster is not None return cluster @catch_etcd_errors @@ -841,12 +847,12 @@ class Etcd3(AbstractEtcd): return ret @catch_etcd_errors - def set_failover_value(self, value: str, index: Optional[str] = None) -> bool: - return bool(self._client.put(self.failover_path, value, mod_revision=index)) + def set_failover_value(self, value: str, version: Optional[str] = None) -> bool: + return bool(self._client.put(self.failover_path, value, mod_revision=version)) @catch_etcd_errors - def set_config_value(self, value: str, index: Optional[str] = None) -> bool: - return bool(self._client.put(self.config_path, value, mod_revision=index)) + def set_config_value(self, value: str, version: Optional[str] = None) -> bool: + return bool(self._client.put(self.config_path, value, mod_revision=version)) @catch_etcd_errors def _write_leader_optime(self, last_lsn: str) -> bool: @@ -893,7 +899,7 @@ class Etcd3(AbstractEtcd): def _delete_leader(self) -> bool: cluster = self.cluster if cluster and isinstance(cluster.leader, Leader) and cluster.leader.name == self._name: - return self._client.deleterange(self.leader_path, mod_revision=cluster.leader.index) + return self._client.deleterange(self.leader_path, mod_revision=cluster.leader.version) return True @catch_etcd_errors @@ -909,15 +915,15 @@ class Etcd3(AbstractEtcd): return bool(self._client.put(self.history_path, value)) @catch_etcd_errors - def set_sync_state_value(self, value: str, index: Optional[str] = None) -> Union[str, bool]: - return self.retry(self._client.put, self.sync_path, value, mod_revision=index)\ + def set_sync_state_value(self, value: str, version: Optional[str] = None) -> Union[str, bool]: + return self.retry(self._client.put, self.sync_path, value, mod_revision=version)\ .get('header', {}).get('revision', False) @catch_etcd_errors - def delete_sync_state(self, index: Optional[str] = None) -> bool: - return self.retry(self._client.deleterange, self.sync_path, mod_revision=index) + def delete_sync_state(self, version: Optional[str] = None) -> bool: + return self.retry(self._client.deleterange, self.sync_path, mod_revision=version) - def watch(self, leader_index: Optional[str], timeout: float) -> bool: + def watch(self, leader_version: Optional[str], timeout: float) -> bool: if self.__do_not_watch: self.__do_not_watch = False return True diff --git a/patroni/dcs/kubernetes.py b/patroni/dcs/kubernetes.py index e33a8509..3e334e9d 100644 --- a/patroni/dcs/kubernetes.py +++ b/patroni/dcs/kubernetes.py @@ -135,11 +135,14 @@ class K8sConfig(object): context = context or config['current-context'] context_value = self._get_by_name(config, 'context', context) - assert isinstance(context_value, dict) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(context_value, dict) cluster = self._get_by_name(config, 'cluster', context_value['cluster']) - assert isinstance(cluster, dict) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(cluster, dict) user = self._get_by_name(config, 'user', context_value['user']) - assert isinstance(user, dict) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(user, dict) self._server = cluster['server'].rstrip('/') if self._server.startswith('https'): @@ -281,7 +284,8 @@ class K8sClient(object): try: response = self.pool_manager.request('GET', base_uri + path, **kwargs) endpoint = self._handle_server_response(response, True) - assert isinstance(endpoint, K8sObject) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(endpoint, K8sObject) for subset in endpoint.subsets: for port in subset.ports: if port.name == 'https' and port.protocol == 'TCP': @@ -412,7 +416,8 @@ class K8sClient(object): except Exception as e: logger.debug('Failed to update list of K8s master nodes: %r', e) - assert isinstance(retry, Retry) # K8sConnectionFailed is raised only if retry is not None! + if TYPE_CHECKING: # pragma: no cover + assert isinstance(retry, Retry) # K8sConnectionFailed is raised only if retry is not None! sleeptime = retry.sleeptime remaining_time = (retry.stoptime or time.time()) - sleeptime - time.time() nodes, timeout, retries = self._calculate_timeouts(api_servers, remaining_time) @@ -559,10 +564,23 @@ class CoreV1ApiProxy(object): return self._use_endpoints +def _run_and_handle_exceptions(method: Callable[..., Any], *args: Any, **kwargs: Any) -> Any: + try: + return method(*args, **kwargs) + except k8s_client.rest.ApiException as e: + if e.status == 403: + logger.exception('Permission denied') + elif e.status != 409: # Object exists or conflict in resource_version + logger.exception('Unexpected error from Kubernetes API') + return False + except (RetryFailedError, K8sException) as e: + raise KubernetesError(e) + + def catch_kubernetes_errors(func: Callable[..., Any]) -> Callable[..., Any]: def wrapper(self: 'Kubernetes', *args: Any, **kwargs: Any) -> Any: try: - return self._run_and_handle_exceptions(func, self, *args, **kwargs) + return _run_and_handle_exceptions(func, self, *args, **kwargs) except KubernetesError: return False return wrapper @@ -584,7 +602,8 @@ class ObjectCache(Thread): self._response_lock = Lock() # protect the `self._response` from concurrent access self._object_cache: Dict[str, K8sObject] = {} self._object_cache_lock = Lock() - self._annotations_map = {self._dcs.leader_path: self._dcs._LEADER, self._dcs.config_path: self._dcs._CONFIG} + self._annotations_map = {self._dcs.leader_path: getattr(self._dcs, '_LEADER'), + self._dcs.config_path: getattr(self._dcs, '_CONFIG')} # pyright self.start() def _list(self) -> K8sObject: @@ -779,19 +798,6 @@ class Kubernetes(AbstractDCS): kwargs['_retry'] = retry return retry(method, *args, **kwargs) - @staticmethod - def _run_and_handle_exceptions(method: Callable[..., Any], *args: Any, **kwargs: Any) -> Any: - try: - return method(*args, **kwargs) - except k8s_client.rest.ApiException as e: - if e.status == 403: - logger.exception('Permission denied') - elif e.status != 409: # Object exists or conflict in resource_version - logger.exception('Unexpected error from Kubernetes API') - return False - except (RetryFailedError, K8sException) as e: - raise KubernetesError(e) - def client_path(self, path: str) -> str: return super(Kubernetes, self).client_path(path)[1:].replace('/', '-') @@ -818,7 +824,8 @@ class Kubernetes(AbstractDCS): Either cause by changes in the local configuration file + SIGHUP or by changes of dynamic configuration""" super(Kubernetes, self).reload_config(config) - assert self._retry.deadline is not None + if TYPE_CHECKING: # pragma: no cover + assert self._retry.deadline is not None self._api.configure_timeouts(self.loop_wait, self._retry.deadline, self.ttl) # retriable_http_codes supposed to be either int, list of integers or comma-separated string with integers. @@ -954,7 +961,8 @@ class Kubernetes(AbstractDCS): def __load_cluster( self, group: Optional[str], loader: Callable[[Dict[str, Any]], Union[Cluster, Dict[int, Cluster]]] ) -> Union[Cluster, Dict[int, Cluster]]: - assert self._retry.deadline is not None + if TYPE_CHECKING: # pragma: no cover + assert self._retry.deadline is not None stop_time = time.time() + self._retry.deadline self._api.refresh_api_servers_cache() try: @@ -978,7 +986,8 @@ class Kubernetes(AbstractDCS): def get_citus_coordinator(self) -> Optional[Cluster]: try: ret = self.__load_cluster(str(CITUS_COORDINATOR_GROUP_ID), self._cluster_loader) - assert isinstance(ret, Cluster) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(ret, Cluster) return ret except Exception as e: logger.error('Failed to load Citus coordinator cluster from Kubernetes: %r', e) @@ -1170,8 +1179,8 @@ class Kubernetes(AbstractDCS): if kind and (kind_annotations.get(self._LEADER) != self._name or kind_resource_version == resource_version): return False - return bool(self._run_and_handle_exceptions(self._patch_or_create, self.leader_path, annotations, - kind_resource_version, ips=ips, retry=_retry)) + return bool(_run_and_handle_exceptions(self._patch_or_create, self.leader_path, annotations, + kind_resource_version, ips=ips, retry=_retry)) def update_leader(self, last_lsn: Optional[int], slots: Optional[Dict[str, int]] = None, failsafe: Optional[Dict[str, str]] = None) -> bool: @@ -1232,24 +1241,24 @@ class Kubernetes(AbstractDCS): def take_leader(self) -> bool: return self.attempt_to_acquire_leader() - def set_failover_value(self, value: str, index: Optional[str] = None) -> bool: + def set_failover_value(self, value: str, version: Optional[str] = None) -> bool: """Unused""" raise NotImplementedError # pragma: no cover def manual_failover(self, leader: Optional[str], candidate: Optional[str], - scheduled_at: Optional[datetime.datetime] = None, index: Optional[str] = None) -> bool: + scheduled_at: Optional[datetime.datetime] = None, version: Optional[str] = None) -> bool: annotations = {'leader': leader or None, 'member': candidate or None, 'scheduled_at': scheduled_at and scheduled_at.isoformat()} - patch = bool(self.cluster and isinstance(self.cluster.failover, Failover) and self.cluster.failover.index) - return bool(self.patch_or_create(self.failover_path, annotations, index, bool(index or patch), False)) + patch = bool(self.cluster and isinstance(self.cluster.failover, Failover) and self.cluster.failover.version) + return bool(self.patch_or_create(self.failover_path, annotations, version, bool(version or patch), False)) @property def _config_resource_version(self) -> Optional[str]: config = self._kinds.get(self.config_path) return config and config.metadata.resource_version - def set_config_value(self, value: str, index: Optional[str] = None) -> bool: - return self.patch_or_create_config({self._CONFIG: value}, index, bool(self._config_resource_version), False) + def set_config_value(self, value: str, version: Optional[str] = None) -> bool: + return self.patch_or_create_config({self._CONFIG: value}, version, bool(self._config_resource_version), False) @catch_kubernetes_errors def touch_member(self, data: Dict[str, Any]) -> bool: @@ -1279,7 +1288,8 @@ class Kubernetes(AbstractDCS): def initialize(self, create_new: bool = True, sysid: str = "") -> bool: cluster = self.cluster - resource_version = str(cluster.config.index) if cluster and cluster.config and cluster.config.index else None + resource_version = str(cluster.config.version)\ + if cluster and cluster.config and cluster.config.version else None return self.patch_or_create_config({self._INITIALIZE: sysid}, resource_version) def _delete_leader(self) -> bool: @@ -1308,34 +1318,34 @@ class Kubernetes(AbstractDCS): def set_history_value(self, value: str) -> bool: return self.patch_or_create_config({self._HISTORY: value}, None, bool(self._config_resource_version), False) - def set_sync_state_value(self, value: str, index: Optional[str] = None) -> bool: + def set_sync_state_value(self, value: str, version: Optional[str] = None) -> bool: """Unused""" raise NotImplementedError # pragma: no cover def write_sync_state(self, leader: Optional[str], sync_standby: Optional[Collection[str]], - index: Optional[str] = None) -> Optional[SyncState]: + version: Optional[str] = None) -> Optional[SyncState]: """Prepare and write annotations to $SCOPE-sync Endpoint or ConfigMap. :param leader: name of the leader node that manages /sync key :param sync_standby: collection of currently known synchronous standby node names - :param index: last known `resource_version` for conditional update of the object + :param version: last known `resource_version` for conditional update of the object :returns: the new :class:`SyncState` object or None """ sync_state = self.sync_state(leader, sync_standby) - ret = self.patch_or_create(self.sync_path, sync_state, index, False) + ret = self.patch_or_create(self.sync_path, sync_state, version, False) if not isinstance(ret, bool): return SyncState.from_node(ret.metadata.resource_version, sync_state) - def delete_sync_state(self, index: Optional[str] = None) -> bool: + def delete_sync_state(self, version: Optional[str] = None) -> bool: """Patch annotations of $SCOPE-sync Endpoint or ConfigMap with empty values. Effectively it removes "leader" and "sync_standby" annotations from the object. - :param index: last known `resource_version` for conditional update of the object + :param version: last known `resource_version` for conditional update of the object :returns: `True` if "delete" was successful """ - return self.write_sync_state(None, None, index=index) is not None + return self.write_sync_state(None, None, version=version) is not None - def watch(self, leader_index: Optional[str], timeout: float) -> bool: + def watch(self, leader_version: Optional[str], timeout: float) -> bool: if self.__do_not_watch: self.__do_not_watch = False return True diff --git a/patroni/dcs/raft.py b/patroni/dcs/raft.py index 10113931..9d9b5f6d 100644 --- a/patroni/dcs/raft.py +++ b/patroni/dcs/raft.py @@ -430,11 +430,11 @@ class Raft(AbstractDCS): return self._sync_obj.set(self.leader_path, self._name, ttl=self._ttl, handle_raft_error=False, prevExist=False) is not False - def set_failover_value(self, value: str, index: Optional[int] = None) -> bool: - return self._sync_obj.set(self.failover_path, value, prevIndex=index) is not False + def set_failover_value(self, value: str, version: Optional[int] = None) -> bool: + return self._sync_obj.set(self.failover_path, value, prevIndex=version) is not False - def set_config_value(self, value: str, index: Optional[int] = None) -> bool: - return self._sync_obj.set(self.config_path, value, prevIndex=index) is not False + def set_config_value(self, value: str, version: Optional[int] = None) -> bool: + return self._sync_obj.set(self.config_path, value, prevIndex=version) is not False def touch_member(self, data: Dict[str, Any]) -> bool: value = json.dumps(data, separators=(',', ':')) @@ -458,17 +458,17 @@ class Raft(AbstractDCS): def set_history_value(self, value: str) -> bool: return self._sync_obj.set(self.history_path, value) is not False - def set_sync_state_value(self, value: str, index: Optional[int] = None) -> Union[int, bool]: - ret = self._sync_obj.set(self.sync_path, value, prevIndex=index) + def set_sync_state_value(self, value: str, version: Optional[int] = None) -> Union[int, bool]: + ret = self._sync_obj.set(self.sync_path, value, prevIndex=version) if isinstance(ret, dict): return ret['index'] return ret - def delete_sync_state(self, index: Optional[int] = None) -> bool: - return self._sync_obj.delete(self.sync_path, prevIndex=index) + def delete_sync_state(self, version: Optional[int] = None) -> bool: + return self._sync_obj.delete(self.sync_path, prevIndex=version) - def watch(self, leader_index: Optional[int], timeout: float) -> bool: + def watch(self, leader_version: Optional[int], timeout: float) -> bool: try: - return super(Raft, self).watch(leader_index, timeout) + return super(Raft, self).watch(leader_version, timeout) finally: self.event.clear() diff --git a/patroni/dcs/zookeeper.py b/patroni/dcs/zookeeper.py index 3d5f7b4b..5ec9cd04 100644 --- a/patroni/dcs/zookeeper.py +++ b/patroni/dcs/zookeeper.py @@ -273,7 +273,7 @@ class ZooKeeper(AbstractDCS): member = Member(-1, leader[0], None, {}) member = ([m for m in members if m.name == leader[0]] or [member])[0] leader = Leader(leader[1].version, leader[1].ephemeralOwner, member) - self._fetch_cluster = member.index == -1 + self._fetch_cluster = member.version == -1 # get last known leader lsn and slots last_lsn, slots = self.get_status(path, leader) @@ -357,19 +357,19 @@ class ZooKeeper(AbstractDCS): logger.info('Could not take out TTL lock') return False - def _set_or_create(self, key: str, value: str, index: Optional[int] = None, + def _set_or_create(self, key: str, value: str, version: Optional[int] = None, retry: bool = False, do_not_create_empty: bool = False) -> Union[int, bool]: value_bytes = value.encode('utf-8') try: if retry: - ret = self._client.retry(self._client.set, key, value_bytes, version=index or -1) + ret = self._client.retry(self._client.set, key, value_bytes, version=version or -1) else: - ret = self._client.set_async(key, value_bytes, version=index or -1).get(timeout=1) + ret = self._client.set_async(key, value_bytes, version=version or -1).get(timeout=1) return ret.version except NoNodeError: if do_not_create_empty and not value_bytes: return True - elif index is None: + elif version is None: if self._create(key, value_bytes, retry): return 0 else: @@ -378,11 +378,11 @@ class ZooKeeper(AbstractDCS): logger.exception('Failed to update %s', key) return False - def set_failover_value(self, value: str, index: Optional[int] = None) -> bool: - return self._set_or_create(self.failover_path, value, index) is not False + def set_failover_value(self, value: str, version: Optional[int] = None) -> bool: + return self._set_or_create(self.failover_path, value, version) is not False - def set_config_value(self, value: str, index: Optional[int] = None) -> bool: - return self._set_or_create(self.config_path, value, index, retry=True) is not False + def set_config_value(self, value: str, version: Optional[int] = None) -> bool: + return self._set_or_create(self.config_path, value, version, retry=True) is not False def initialize(self, create_new: bool = True, sysid: str = "") -> bool: sysid_bytes = sysid.encode('utf-8') @@ -494,14 +494,14 @@ class ZooKeeper(AbstractDCS): def set_history_value(self, value: str) -> bool: return self._set_or_create(self.history_path, value) is not False - def set_sync_state_value(self, value: str, index: Optional[int] = None) -> Union[int, bool]: - return self._set_or_create(self.sync_path, value, index, retry=True, do_not_create_empty=True) + def set_sync_state_value(self, value: str, version: Optional[int] = None) -> Union[int, bool]: + return self._set_or_create(self.sync_path, value, version, retry=True, do_not_create_empty=True) - def delete_sync_state(self, index: Optional[int] = None) -> bool: - return self.set_sync_state_value("{}", index) is not False + def delete_sync_state(self, version: Optional[int] = None) -> bool: + return self.set_sync_state_value("{}", version) is not False - def watch(self, leader_index: Optional[int], timeout: float) -> bool: - ret = super(ZooKeeper, self).watch(leader_index, timeout + 0.5) + def watch(self, leader_version: Optional[int], timeout: float) -> bool: + ret = super(ZooKeeper, self).watch(leader_version, timeout + 0.5) if ret and not self._fetch_status: self._fetch_cluster = True return ret or self._fetch_cluster diff --git a/patroni/ha.py b/patroni/ha.py index e1559a27..1190ad1c 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -100,10 +100,10 @@ class Failsafe(object): @property def leader(self) -> Optional[Leader]: with self._lock: - if self._last_update + self._dcs.ttl > time.time(): - return Leader('', '', RemoteMember(self._name, {'api_url': self._api_url, - 'conn_url': self._conn_url, - 'slots': self._slots})) + if self._last_update + self._dcs.ttl > time.time() and self._name: + return Leader('', '', RemoteMember.from_name_and_data(self._name, {'api_url': self._api_url, + 'conn_url': self._conn_url, + 'slots': self._slots})) def update_cluster(self, cluster: Cluster) -> Cluster: # Enreach cluster with the real leader if there was a ping from it @@ -596,7 +596,7 @@ class Ha(object): if sync_common != current: logger.info("Updating synchronous privilege temporarily from %s to %s", list(current), list(sync_common)) - sync = self.dcs.write_sync_state(self.state_handler.name, sync_common, index=sync.index) + sync = self.dcs.write_sync_state(self.state_handler.name, sync_common, version=sync.version) if not sync: return logger.info('Synchronous replication key updated by someone else.') @@ -614,11 +614,11 @@ class Ha(object): time.sleep(2) _, allow_promote = self.state_handler.sync_handler.current_state(self.cluster) if allow_promote and allow_promote != sync_common: - if not self.dcs.write_sync_state(self.state_handler.name, allow_promote, index=sync.index): + if not self.dcs.write_sync_state(self.state_handler.name, allow_promote, version=sync.version): return logger.info("Synchronous replication key updated by someone else") logger.info("Synchronous standby status assigned to %s", list(allow_promote)) else: - if not self.cluster.sync.is_empty and self.dcs.delete_sync_state(index=self.cluster.sync.index): + if not self.cluster.sync.is_empty and self.dcs.delete_sync_state(version=self.cluster.sync.version): logger.info("Disabled synchronous replication") self.state_handler.sync_handler.set_synchronous_standby_names(CaseInsensitiveSet()) @@ -727,7 +727,7 @@ class Ha(object): if self.is_synchronous_mode(): # Just set ourselves as the authoritative source of truth for now. We don't want to wait for standbys # to connect. We will try finding a synchronous standby in the next cycle. - if not self.dcs.write_sync_state(self.state_handler.name, None, index=self.cluster.sync.index): + if not self.dcs.write_sync_state(self.state_handler.name, None, version=self.cluster.sync.version): # Somebody else updated sync state, it may be due to us losing the lock. To be safe, postpone # promotion until next cycle. TODO: trigger immediate retry of run_cycle return 'Postponing promotion because synchronous replication state was updated by somebody else' @@ -800,9 +800,8 @@ class Ha(object): data['slots'] = self.state_handler.slots() except Exception: logger.exception('Exception when called state_handler.slots()') - members = [RemoteMember(name, {'api_url': url}) - for name, url in failsafe.items() - if name != self.state_handler.name] + members = [RemoteMember.from_name_and_data(name, {'api_url': url}) + for name, url in failsafe.items() if name != self.state_handler.name] if not members: # A sinlge node cluster return True pool = ThreadPool(len(members)) @@ -907,7 +906,7 @@ class Ha(object): if not self.cluster.get_member(failover.candidate, fallback_to_leader=False)\ and self.state_handler.is_leader(): logger.warning("manual failover: removing failover key because failover candidate is not running") - self.dcs.manual_failover('', '', index=failover.index) + self.dcs.manual_failover('', '', version=failover.version) return None return False @@ -998,7 +997,8 @@ class Ha(object): if failsafe_members and self.state_handler.name not in failsafe_members: return False # Race among not only existing cluster members, but also all known members from the failsafe config - all_known_members += [RemoteMember(name, {'api_url': url}) for name, url in failsafe_members.items()] + all_known_members += [RemoteMember.from_name_and_data(name, {'api_url': url}) + for name, url in failsafe_members.items()] all_known_members += self.cluster.members # When in sync mode, only last known primary and sync standby are allowed to promote automatically. @@ -1148,7 +1148,7 @@ class Ha(object): if (failover.scheduled_at and not self.should_run_scheduled_action("failover", failover.scheduled_at, lambda: - self.dcs.manual_failover('', '', index=failover.index))): + self.dcs.manual_failover('', '', version=failover.version))): return if not failover.leader or failover.leader == self.state_handler.name: @@ -1178,7 +1178,7 @@ class Ha(object): failover.leader, self.state_handler.name) logger.info('Cleaning up failover key') - self.dcs.manual_failover('', '', index=failover.index) + self.dcs.manual_failover('', '', version=failover.version) def process_unhealthy_cluster(self) -> str: """Cluster has no leader key""" @@ -1189,7 +1189,7 @@ class Ha(object): if failover: if self.is_paused() and failover.leader and failover.candidate: logger.info('Updating failover key after acquiring leader lock...') - self.dcs.manual_failover('', failover.candidate, failover.scheduled_at, failover.index) + self.dcs.manual_failover('', failover.candidate, failover.scheduled_at, failover.version) else: logger.info('Cleaning up failover key after acquiring leader lock...') self.dcs.manual_failover('', '') @@ -1839,11 +1839,11 @@ class Ha(object): def watch(self, timeout: float) -> bool: # watch on leader key changes if the postgres is running and leader is known and current node is not lock owner if self._async_executor.busy or not self.cluster or self.cluster.is_unlocked() or self.has_lock(False): - leader_index = None + leader_version = None else: - leader_index = self.cluster.leader.index if self.cluster.leader else None + leader_version = self.cluster.leader.version if self.cluster.leader else None - return self.dcs.watch(leader_index, timeout) + return self.dcs.watch(leader_version, timeout) def wakeup(self) -> None: """Call of this method will trigger the next run of HA loop if there is @@ -1868,4 +1868,4 @@ class Ha(object): data['conn_kwargs'] = conn_kwargs name = member.name if member else 'remote_member:{}'.format(uuid.uuid1()) - return RemoteMember(name, data) + return RemoteMember.from_name_and_data(name, data) diff --git a/patroni/log.py b/patroni/log.py index 955594ff..55b63386 100644 --- a/patroni/log.py +++ b/patroni/log.py @@ -13,7 +13,7 @@ from patroni.utils import deep_compare from queue import Queue, Full from threading import Lock, Thread -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional, Union, TYPE_CHECKING _LOGGER = logging.getLogger(__name__) @@ -249,8 +249,9 @@ class PatroniLogger(Thread): if not isinstance(self.log_handler, RotatingFileHandler): new_handler = RotatingFileHandler(os.path.join(config['dir'], __name__)) handler = new_handler or self.log_handler - assert isinstance(handler, RotatingFileHandler) - handler.maxBytes = int(config.get('file_size', 25000000)) + if TYPE_CHECKING: # pragma: no cover + assert isinstance(handler, RotatingFileHandler) + handler.maxBytes = int(config.get('file_size', 25000000)) # pyright: ignore [reportGeneralTypeIssues] handler.backupCount = int(config.get('file_num', 4)) else: if self.log_handler is None or isinstance(self.log_handler, RotatingFileHandler): @@ -306,7 +307,8 @@ class PatroniLogger(Thread): while True: self._close_old_handlers() - assert self.log_handler is not None + if TYPE_CHECKING: # pragma: no cover + assert self.log_handler is not None record = self._queue_handler.queue.get(True) # special message that indicates Patroni is shutting down diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py index 7f792826..305aa4ce 100644 --- a/patroni/postgresql/__init__.py +++ b/patroni/postgresql/__init__.py @@ -192,7 +192,7 @@ class Postgresql(object): "FROM pg_catalog.pg_stat_get_wal_senders() w," " pg_catalog.pg_stat_get_activity(w.pid)" " WHERE w.state = 'streaming') r)").format(self.wal_name, self.lsn_name) - if (not self._global_config or self._global_config.is_synchronous_mode) + if (not self.global_config or self.global_config.is_synchronous_mode) and self.role in ('master', 'primary', 'promoted') else "'on', '', NULL") if self._major_version >= 90600: @@ -375,6 +375,10 @@ class Postgresql(object): self.config.write_postgresql_conf() self.reload() + @property + def global_config(self) -> Optional['GlobalConfig']: + return self._global_config + def reset_cluster_info_state(self, cluster: Union[Cluster, None], nofailover: bool = False, global_config: Optional['GlobalConfig'] = None) -> None: """Reset monitoring query cache. @@ -388,7 +392,7 @@ class Postgresql(object): :param global_config: last known :class:`GlobalConfig` object """ self._cluster_info_state = {} - if cluster and cluster.config and cluster.config.modify_index: + if cluster and cluster.config and cluster.config.modify_version: self._has_permanent_logical_slots =\ cluster.has_permanent_logical_slots(self.name, nofailover, self.major_version) diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index 4d6bacfb..76a3fb7b 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -860,9 +860,9 @@ class ConfigHandler(object): parameters = config['parameters'].copy() listen_addresses, port = split_host_port(config['listen'], 5432) parameters.update(cluster_name=self._postgresql.scope, listen_addresses=listen_addresses, port=str(port)) - if not self._postgresql._global_config or self._postgresql._global_config.is_synchronous_mode: + if not self._postgresql.global_config or self._postgresql.global_config.is_synchronous_mode: if self._synchronous_standby_names is None: - if self._postgresql._global_config and self._postgresql._global_config.is_synchronous_mode_strict\ + if self._postgresql.global_config and self._postgresql.global_config.is_synchronous_mode_strict\ and self._postgresql.role in ('master', 'primary', 'promoted'): parameters['synchronous_standby_names'] = '*' else: diff --git a/patroni/postgresql/sync.py b/patroni/postgresql/sync.py index 78e7b427..c56bdbcd 100644 --- a/patroni/postgresql/sync.py +++ b/patroni/postgresql/sync.py @@ -240,10 +240,10 @@ class SyncHandler(object): if len(replica_list) > 1 else self._postgresql.last_operation() if TYPE_CHECKING: # pragma: no cover - assert self._postgresql._global_config is not None - sync_node_count = self._postgresql._global_config.synchronous_node_count\ + assert self._postgresql.global_config is not None + sync_node_count = self._postgresql.global_config.synchronous_node_count\ if self._postgresql.supports_multiple_sync else 1 - sync_node_maxlag = self._postgresql._global_config.maximum_lag_on_syncnode + sync_node_maxlag = self._postgresql.global_config.maximum_lag_on_syncnode candidates = CaseInsensitiveSet() sync_nodes = CaseInsensitiveSet() diff --git a/patroni/scripts/wale_restore.py b/patroni/scripts/wale_restore.py index c2d3545c..7ef5d2c9 100755 --- a/patroni/scripts/wale_restore.py +++ b/patroni/scripts/wale_restore.py @@ -32,7 +32,7 @@ import sys import time from enum import IntEnum -from typing import Any, List, NamedTuple, Optional, Tuple +from typing import Any, List, NamedTuple, Optional, Tuple, TYPE_CHECKING from .. import psycopg @@ -365,7 +365,8 @@ def main() -> int: break time.sleep(RETRY_SLEEP_INTERVAL) - assert exit_code is not None + if TYPE_CHECKING: # pragma: no cover + assert exit_code is not None return exit_code diff --git a/patroni/validator.py b/patroni/validator.py index 569894d1..e867ce58 100644 --- a/patroni/validator.py +++ b/patroni/validator.py @@ -11,7 +11,7 @@ import shutil import socket import subprocess -from typing import Any, Dict, Union, Iterator, List, Optional as OptionalType +from typing import Any, Dict, Union, Iterator, List, Optional as OptionalType, TYPE_CHECKING from .utils import parse_int, split_host_port, data_directory_is_empty from .dcs import dcs_modules @@ -196,7 +196,8 @@ def get_major_version(bin_dir: OptionalType[str] = None) -> str: binary = os.path.join(bin_dir, 'postgres') version = subprocess.check_output([binary, '--version']).decode() version = re.match(r'^[^\s]+ [^\s]+ (\d+)(\.(\d+))?', version) - assert version is not None + if TYPE_CHECKING: # pragma: no cover + assert version is not None return '.'.join([version.group(1), version.group(3)]) if int(version.group(1)) < 10 else version.group(1) diff --git a/patroni/watchdog/base.py b/patroni/watchdog/base.py index bebdabe2..7d1cdca6 100644 --- a/patroni/watchdog/base.py +++ b/patroni/watchdog/base.py @@ -34,7 +34,7 @@ def parse_mode(mode: Union[bool, str]) -> str: def synchronized(func: Callable[..., Any]) -> Callable[..., Any]: def wrapped(self: 'Watchdog', *args: Any, **kwargs: Any) -> Any: - with self._lock: + with self.lock: return func(self, *args, **kwargs) return wrapped @@ -90,7 +90,7 @@ class Watchdog(object): def __init__(self, config: Config) -> None: self.config = WatchdogConfig(config) self.active_config: WatchdogConfig = self.config - self._lock = RLock() + self.lock = RLock() self.active = False if self.config.mode == MODE_OFF: diff --git a/patroni/watchdog/linux.py b/patroni/watchdog/linux.py index 3c96a268..6404f303 100644 --- a/patroni/watchdog/linux.py +++ b/patroni/watchdog/linux.py @@ -1,3 +1,4 @@ +# pyright: reportConstantRedefinition=false import ctypes import os import platform diff --git a/pyrightconfig.json b/pyrightconfig.json index 20980afb..2394c6de 100644 --- a/pyrightconfig.json +++ b/pyrightconfig.json @@ -19,7 +19,7 @@ "reportMissingImports": true, "reportMissingTypeStubs": false, - "pythonVersion": "3.6", + "pythonVersion": "3.11", "pythonPlatform": "All", "typeCheckingMode": "strict" diff --git a/tests/test_ha.py b/tests/test_ha.py index c9734517..8495a1f0 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -1201,7 +1201,7 @@ class TestHa(PostgresInit): # When we just became primary nobody is sync self.assertEqual(self.ha.enforce_primary_role('msg', 'promote msg'), 'promote msg') mock_set_sync.assert_called_once_with(CaseInsensitiveSet()) - mock_write_sync.assert_called_once_with('leader', None, index=0) + mock_write_sync.assert_called_once_with('leader', None, version=0) mock_set_sync.reset_mock() @@ -1239,7 +1239,7 @@ class TestHa(PostgresInit): mock_acquire.assert_called_once() mock_follow.assert_not_called() mock_promote.assert_called_once() - mock_write_sync.assert_called_once_with('other', None, index=0) + mock_write_sync.assert_called_once_with('other', None, version=0) def test_disable_sync_when_restarting(self): self.ha.is_synchronous_mode = true diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 1a44b671..f7c0c155 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -339,7 +339,8 @@ class TestPostgresql(BaseTestPostgresql): @patch.object(Postgresql, 'start', Mock()) def test_follow(self): self.p.call_nowait(CallbackAction.ON_START) - m = RemoteMember('1', {'restore_command': '2', 'primary_slot_name': 'foo', 'conn_kwargs': {'host': 'bar'}}) + m = RemoteMember.from_name_and_data('1', {'restore_command': '2', 'primary_slot_name': 'foo', + 'conn_kwargs': {'host': 'bar'}}) self.p.follow(m) with patch.object(Postgresql, 'ensure_major_version_is_known', Mock(return_value=False)): self.assertIsNone(self.p.follow(m)) From 44e58a1ba11bb47e3ea186797bbe5c477f5a4d9e Mon Sep 17 00:00:00 2001 From: Polina Bungina <27892524+hughcapet@users.noreply.github.com> Date: Mon, 15 May 2023 11:40:35 +0200 Subject: [PATCH 4/8] Dev docker images improvements (#2677) - configurable image - ETCD_UNSUPPORTED_ARCH env in docker-compose - Build confd and citus for arm64 images --- Dockerfile | 18 +++++++++++++++--- Dockerfile.citus | 37 +++++++++++++++++++++++++++++++------ docker-compose-citus.yml | 19 ++++++++++--------- docker-compose.yml | 11 ++++++----- 4 files changed, 62 insertions(+), 23 deletions(-) diff --git a/Dockerfile b/Dockerfile index e424f588..5892e1ef 100644 --- a/Dockerfile +++ b/Dockerfile @@ -53,14 +53,26 @@ RUN set -ex \ && curl -sL "https://github.com/coreos/etcd/releases/download/v$ETCDVERSION/etcd-v$ETCDVERSION-linux-$(dpkg --print-architecture).tar.gz" \ | tar xz -C /usr/local/bin --strip=1 --wildcards --no-anchored etcd etcdctl \ \ - # Download confd - && curl -sL "https://github.com/kelseyhightower/confd/releases/download/v$CONFDVERSION/confd-$CONFDVERSION-linux-$(dpkg --print-architecture)" \ - > /usr/local/bin/confd && chmod +x /usr/local/bin/confd \ + && if [ $(dpkg --print-architecture) = 'arm64' ]; then \ + # Build confd + apt-get install -y git make \ + && curl -sL https://go.dev/dl/go1.20.4.linux-arm64.tar.gz | tar xz -C /usr/local go \ + && export GOROOT=/usr/local/go && export PATH=$PATH:$GOROOT/bin \ + && git clone --recurse-submodules https://github.com/kelseyhightower/confd.git \ + && make -C confd \ + && cp confd/bin/confd /usr/local/bin/confd \ + && rm -rf /confd /usr/local/go; \ + else \ + # Download confd + curl -sL "https://github.com/kelseyhightower/confd/releases/download/v$CONFDVERSION/confd-$CONFDVERSION-linux-$(dpkg --print-architecture)" \ + > /usr/local/bin/confd && chmod +x /usr/local/bin/confd; \ + fi \ \ # Clean up all useless packages and some files && apt-get purge -y --allow-remove-essential python3-pip gzip bzip2 util-linux e2fsprogs \ libmagic1 bsdmainutils login ncurses-bin libmagic-mgc e2fslibs bsdutils \ exim4-config gnupg-agent dirmngr libpython2.7-stdlib libpython2.7-minimal \ + git make \ && apt-get autoremove -y \ && apt-get clean -y \ && rm -rf /var/lib/apt/lists/* \ diff --git a/Dockerfile.citus b/Dockerfile.citus index d5432b15..06a84683 100644 --- a/Dockerfile.citus +++ b/Dockerfile.citus @@ -20,14 +20,26 @@ RUN set -ex \ && export DEBIAN_FRONTEND=noninteractive \ && echo 'APT::Install-Recommends "0";\nAPT::Install-Suggests "0";' > /etc/apt/apt.conf.d/01norecommend \ && apt-get update -y \ - # postgres:10 is based on debian, which has the patroni package. We will install all required dependencies + # postgres:PG_MAJOR is based on debian, which has the patroni package. We will install all required dependencies && apt-cache depends patroni | sed -n -e 's/.*Depends: \(python3-.\+\)$/\1/p' \ | grep -Ev '^python3-(sphinx|etcd|consul|kazoo|kubernetes)' \ | xargs apt-get install -y vim curl less jq locales haproxy sudo \ python3-etcd python3-kazoo python3-pip busybox \ net-tools iputils-ping --fix-missing \ - && curl https://install.citusdata.com/community/deb.sh | bash \ - && apt-get -y install postgresql-$PG_MAJOR-citus-11.2 \ + && if [ $(dpkg --print-architecture) = 'arm64' ]; then \ + apt-get install -y postgresql-server-dev-$PG_MAJOR \ + git gcc make autoconf \ + libc6-dev flex libcurl4-gnutls-dev \ + libicu-dev libkrb5-dev liblz4-dev \ + libpam0g-dev libreadline-dev libselinux1-dev\ + libssl-dev libxslt1-dev libzstd-dev uuid-dev \ + && git clone -b "main" https://github.com/citusdata/citus.git \ + && MAKEFLAGS="-j $(grep -c ^processor /proc/cpuinfo)" \ + && cd citus && ./configure && make install && cd ../ && rm -rf /citus; \ + else \ + curl https://install.citusdata.com/community/deb.sh | bash \ + && apt-get -y install postgresql-$PG_MAJOR-citus-11.2; \ + fi \ && pip3 install dumb-init \ \ # Cleanup all locales but en_US.UTF-8 @@ -55,9 +67,19 @@ RUN set -ex \ && curl -sL https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-$(dpkg --print-architecture).tar.gz \ | tar xz -C /usr/local/bin --strip=1 --wildcards --no-anchored etcd etcdctl \ \ - # Download confd - && curl -sL https://github.com/kelseyhightower/confd/releases/download/v${CONFDVERSION}/confd-${CONFDVERSION}-linux-$(dpkg --print-architecture) \ - > /usr/local/bin/confd && chmod +x /usr/local/bin/confd \ + && if [ $(dpkg --print-architecture) = 'arm64' ]; then \ + # Build confd + curl -sL https://go.dev/dl/go1.20.4.linux-arm64.tar.gz | tar xz -C /usr/local go \ + && export GOROOT=/usr/local/go && export PATH=$PATH:$GOROOT/bin \ + && git clone --recurse-submodules https://github.com/kelseyhightower/confd.git \ + && make -C confd \ + && cp confd/bin/confd /usr/local/bin/confd \ + && rm -rf /confd /usr/local/go; \ + else \ + # Download confd + curl -sL "https://github.com/kelseyhightower/confd/releases/download/v$CONFDVERSION/confd-$CONFDVERSION-linux-$(dpkg --print-architecture)" \ + > /usr/local/bin/confd && chmod +x /usr/local/bin/confd; \ + fi \ # Prepare client cert for HAProxy && cat /etc/ssl/private/ssl-cert-snakeoil.key /etc/ssl/certs/ssl-cert-snakeoil.pem > /etc/ssl/private/ssl-cert-snakeoil.crt \ \ @@ -65,6 +87,9 @@ RUN set -ex \ && apt-get purge -y --allow-remove-essential python3-pip gzip bzip2 util-linux e2fsprogs \ libmagic1 bsdmainutils login ncurses-bin libmagic-mgc e2fslibs bsdutils \ exim4-config gnupg-agent dirmngr libpython2.7-stdlib libpython2.7-minimal \ + postgresql-server-dev-$PG_MAJOR git gcc make autoconf \ + libc6-dev flex libcurl4-gnutls-dev libicu-dev libkrb5-dev liblz4-dev \ + libpam0g-dev libreadline-dev libselinux1-dev libssl-dev libxslt1-dev libzstd-dev uuid-dev \ && apt-get autoremove -y \ && apt-get clean -y \ && rm -rf /var/lib/apt/lists/* \ diff --git a/docker-compose-citus.yml b/docker-compose-citus.yml index f141bc3b..7ff2a2c5 100644 --- a/docker-compose-citus.yml +++ b/docker-compose-citus.yml @@ -16,7 +16,7 @@ networks: services: etcd1: &etcd - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] environment: ETCDCTL_API: 3 @@ -25,6 +25,7 @@ services: ETCD_INITIAL_CLUSTER: etcd1=http://etcd1:2380,etcd2=http://etcd2:2380,etcd3=http://etcd3:2380 ETCD_INITIAL_CLUSTER_STATE: new ETCD_INITIAL_CLUSTER_TOKEN: tutorial + ETCD_UNSUPPORTED_ARCH: arm64 container_name: demo-etcd1 hostname: etcd1 command: etcd -name etcd1 -initial-advertise-peer-urls http://etcd1:2380 @@ -42,7 +43,7 @@ services: command: etcd -name etcd3 -initial-advertise-peer-urls http://etcd3:2380 haproxy: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: haproxy @@ -64,7 +65,7 @@ services: PGSSLROOTCERT: /etc/ssl/certs/ssl-cert-snakeoil.pem coord1: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: coord1 @@ -75,7 +76,7 @@ services: PATRONI_CITUS_GROUP: 0 coord2: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: coord2 @@ -85,7 +86,7 @@ services: PATRONI_NAME: coord2 coord3: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: coord3 @@ -96,7 +97,7 @@ services: work1-1: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: work1-1 @@ -107,7 +108,7 @@ services: PATRONI_CITUS_GROUP: 1 work1-2: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: work1-2 @@ -118,7 +119,7 @@ services: work2-1: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: work2-1 @@ -129,7 +130,7 @@ services: PATRONI_CITUS_GROUP: 2 work2-2: - image: patroni-citus + image: ${PATRONI_TEST_IMAGE:-patroni-citus} networks: [ demo ] env_file: docker/patroni.env hostname: work2-2 diff --git a/docker-compose.yml b/docker-compose.yml index 626391de..996c2c82 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,7 +14,7 @@ networks: services: etcd1: &etcd - image: patroni + image: ${PATRONI_TEST_IMAGE:-patroni} networks: [ demo ] environment: ETCD_LISTEN_PEER_URLS: http://0.0.0.0:2380 @@ -22,6 +22,7 @@ services: ETCD_INITIAL_CLUSTER: etcd1=http://etcd1:2380,etcd2=http://etcd2:2380,etcd3=http://etcd3:2380 ETCD_INITIAL_CLUSTER_STATE: new ETCD_INITIAL_CLUSTER_TOKEN: tutorial + ETCD_UNSUPPORTED_ARCH: arm64 container_name: demo-etcd1 hostname: etcd1 command: etcd -name etcd1 -initial-advertise-peer-urls http://etcd1:2380 @@ -39,7 +40,7 @@ services: command: etcd -name etcd3 -initial-advertise-peer-urls http://etcd3:2380 haproxy: - image: patroni + image: ${PATRONI_TEST_IMAGE:-patroni} networks: [ demo ] env_file: docker/patroni.env hostname: haproxy @@ -54,7 +55,7 @@ services: PATRONI_SCOPE: demo patroni1: - image: patroni + image: ${PATRONI_TEST_IMAGE:-patroni} networks: [ demo ] env_file: docker/patroni.env hostname: patroni1 @@ -64,7 +65,7 @@ services: PATRONI_NAME: patroni1 patroni2: - image: patroni + image: ${PATRONI_TEST_IMAGE:-patroni} networks: [ demo ] env_file: docker/patroni.env hostname: patroni2 @@ -74,7 +75,7 @@ services: PATRONI_NAME: patroni2 patroni3: - image: patroni + image: ${PATRONI_TEST_IMAGE:-patroni} networks: [ demo ] env_file: docker/patroni.env hostname: patroni3 From 506b5bec482923d6a09a177d3ecfd4ef0d73001c Mon Sep 17 00:00:00 2001 From: Polina Bungina <27892524+hughcapet@users.noreply.github.com> Date: Mon, 15 May 2023 13:40:22 +0200 Subject: [PATCH 5/8] Validate-config fixes (#2678) - fix --validate-config not to error out if bin_dir is an empty string in the yaml config - mention bin_dir optionality in the docs - validate bin_dir even if it is not in the yaml config (add optional default value for Optional config params in validator) - make rewind user optional --- docs/yaml_configuration.rst | 2 +- patroni/validator.py | 38 ++++++++++++++++++++++---------- tests/test_validator.py | 44 +++++++++++++++++++++++++++---------- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/docs/yaml_configuration.rst b/docs/yaml_configuration.rst index a5b32a20..51f769ca 100644 --- a/docs/yaml_configuration.rst +++ b/docs/yaml_configuration.rst @@ -262,7 +262,7 @@ PostgreSQL own config item. See :ref:`custom replica creation methods documentation ` for further explanation. - **data\_dir**: The location of the Postgres data directory, either :ref:`existing ` or to be initialized by Patroni. - **config\_dir**: The location of the Postgres configuration directory, defaults to the data directory. Must be writable by Patroni. - - **bin\_dir**: Path to PostgreSQL binaries (pg_ctl, pg_rewind, pg_basebackup, postgres). The default value is an empty string meaning that PATH environment variable will be used to find the executables. + - **bin\_dir**: (optional) Path to PostgreSQL binaries (pg_ctl, pg_rewind, pg_basebackup, postgres). If not provided or is an empty string, PATH environment variable will be used to find the executables. - **listen**: IP address + port that Postgres listens to; must be accessible from other nodes in the cluster, if you're using streaming replication. Multiple comma-separated addresses are permitted, as long as the port component is appended after to the last one with a colon, i.e. ``listen: 127.0.0.1,127.0.0.2:5432``. Patroni will use the first address from this list to establish local connections to the PostgreSQL node. - **use\_unix\_socket**: specifies that Patroni should prefer to use unix sockets to connect to the cluster. Default value is ``false``. If ``unix_socket_directories`` is defined, Patroni will use the first suitable value from it to connect to the cluster and fallback to tcp if nothing is suitable. If ``unix_socket_directories`` is not specified in ``postgresql.parameters``, Patroni will assume that the default value should be used and omit ``host`` from the connection parameters. - **use\_unix\_socket\_repl**: specifies that Patroni should prefer to use unix sockets for replication user cluster connection. Default value is ``false``. If ``unix_socket_directories`` is defined, Patroni will use the first suitable value from it to connect to the cluster and fallback to tcp if nothing is suitable. If ``unix_socket_directories`` is not specified in ``postgresql.parameters``, Patroni will assume that the default value should be used and omit ``host`` from the connection parameters. diff --git a/patroni/validator.py b/patroni/validator.py index e867ce58..3d30ca2c 100644 --- a/patroni/validator.py +++ b/patroni/validator.py @@ -341,14 +341,17 @@ class Optional(object): """Mark a configuration option as optional. :ivar name: name of the configuration option. + :ivar default: value to set if the configuration option is not explicitly provided """ - def __init__(self, name: str) -> None: + def __init__(self, name: str, default: OptionalType[Any] = None) -> None: """Create an :class:`Optional` object. :param name: name of the configuration option. + :param default: value to set if the configuration option is not explicitly provided """ self.name = name + self.default = default class Directory(object): @@ -370,14 +373,27 @@ class Directory(object): self.contains = contains self.contains_executable = contains_executable + def _check_executables(self, path: OptionalType[str] = None) -> Iterator[Result]: + """Check that all executables from contains_executable list exist within the given directory or within PATH. + + :param path: optional path to the base directory against which executables will be validated. + If not provided, check within PATH. + :rtype: Iterator[:class:`Result`] objects with the error message containing the name of the executable, + if any check fails. + """ + for program in self.contains_executable or []: + if not shutil.which(program, path=path): + yield Result(False, f"does not contain '{program}' in '{(path or '$PATH')}'") + def validate(self, name: str) -> Iterator[Result]: """Check if the expected paths and executables can be found under *name* directory. :param name: path to the base directory against which paths and executables will be validated. + Check against PATH if name is not provided. :rtype: Iterator[:class:`Result`] objects with the error message related to the failure, if any check fails. """ if not name: - yield Result(False, "is an empty string") + yield from self._check_executables() elif not os.path.exists(name): yield Result(False, "Directory '{}' does not exist.".format(name)) elif not os.path.isdir(name): @@ -387,10 +403,7 @@ class Directory(object): for path in self.contains: if not os.path.exists(os.path.join(name, path)): yield Result(False, "'{}' does not contain '{}'".format(name, path)) - if self.contains_executable: - for program in self.contains_executable: - if not shutil.which(program, path=name): - yield Result(False, "'{}' does not contain '{}'".format(name, program)) + yield from self._check_executables(path=name) class Schema(object): @@ -472,8 +485,7 @@ class Schema(object): * It must contain a ``bind.host`` entry which value should be valid as per function ``validate_host``; * It must contain a ``bind.port`` entry which value should be an :class:`int` instance; * It must contain a ``aliases`` entry which value should be a :class:`list` of :class:`str` instances; - * It may optionally contain a ``data_directory`` entry. If not given it will assume the value - ``/var/lib/myapp``; + * It may optionally contain a ``data_directory`` entry, with a value which should be a string; * It must contain at least one of ``log_to_file`` or ``log_to_db``, with a value which should be a :class:`bool` instance; * It must contain a ``version`` entry which value should be either an :class:`int` or a :class:`float` @@ -583,9 +595,11 @@ class Schema(object): for d in self._data_key(key): if d not in self.data and not isinstance(key, Optional): yield Result(False, "is not defined.", path=d) - elif d not in self.data and isinstance(key, Optional): + elif d not in self.data and isinstance(key, Optional) and key.default is None: continue else: + if d not in self.data and isinstance(key, Optional): + self.data[d] = key.default validator = self.validator[key] if isinstance(key, Or) and isinstance(self.validator[key], Case): validator = self.validator[key]._schema[d] @@ -807,11 +821,11 @@ schema = Schema({ "authentication": { "replication": userattributes, "superuser": userattributes, - "rewind": userattributes + Optional("rewind"): userattributes }, "data_dir": validate_data_dir, - Optional("bin_dir"): Directory(contains_executable=["pg_ctl", "initdb", "pg_controldata", "pg_basebackup", - "postgres", "pg_isready"]), + Optional("bin_dir", ""): Directory(contains_executable=["pg_ctl", "initdb", "pg_controldata", "pg_basebackup", + "postgres", "pg_isready"]), Optional("parameters"): { Optional("unix_socket_directories"): str }, diff --git a/tests/test_validator.py b/tests/test_validator.py index df017500..87ea78b7 100644 --- a/tests/test_validator.py +++ b/tests/test_validator.py @@ -7,7 +7,7 @@ import unittest from io import StringIO from mock import Mock, patch, mock_open from patroni.dcs import dcs_modules -from patroni.validator import schema +from patroni.validator import schema, Directory, Schema available_dcs = [m.split(".")[-1] for m in dcs_modules()] config = { @@ -92,6 +92,16 @@ config = { } } +config_2 = { + "some_dir": "very_interesting_dir" +} + +schema2 = Schema({ + "some_dir": Directory(contains=["very_interesting_subdir", "another_interesting_subdir"]) +}) + +required_binaries = ["pg_ctl", "initdb", "pg_controldata", "pg_basebackup", "postgres", "pg_isready"] + directories = [] files = [] binaries = [] @@ -190,6 +200,14 @@ class TestValidator(unittest.TestCase): self.assertEqual(['consul.host', 'etcd.host', 'postgresql.bin_dir', 'postgresql.data_dir', 'postgresql.listen', 'raft.bind_addr', 'raft.self_addr', 'restapi.connect_address'], parse_output(output)) + def test_bin_dir_is_empty_string_excutables_in_path(self, mock_out, mock_err): + binaries.extend(required_binaries) + c = copy.deepcopy(config) + c["postgresql"]["bin_dir"] = "" + errors = schema(c) + output = "\n".join(errors) + self.assertEqual(['raft.bind_addr', 'raft.self_addr'], parse_output(output)) + @patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 12.1")) def test_data_dir_contains_pg_version(self, mock_out, mock_err): directories.append(config["postgresql"]["data_dir"]) @@ -197,14 +215,11 @@ class TestValidator(unittest.TestCase): directories.append(os.path.join(config["postgresql"]["data_dir"], "pg_wal")) files.append(os.path.join(config["postgresql"]["data_dir"], "global", "pg_control")) files.append(os.path.join(config["postgresql"]["data_dir"], "PG_VERSION")) - binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_ctl")) - binaries.append(os.path.join(config["postgresql"]["bin_dir"], "initdb")) - binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_controldata")) - binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_basebackup")) - binaries.append(os.path.join(config["postgresql"]["bin_dir"], "postgres")) - binaries.append(os.path.join(config["postgresql"]["bin_dir"], "pg_isready")) + binaries.extend(required_binaries) + c = copy.deepcopy(config) + c["postgresql"]["bin_dir"] = "" # to cover postgres --version call from PATH with patch('patroni.validator.open', mock_open(read_data='12')): - errors = schema(config) + errors = schema(c) output = "\n".join(errors) self.assertEqual(['raft.bind_addr', 'raft.self_addr'], parse_output(output)) @@ -215,10 +230,10 @@ class TestValidator(unittest.TestCase): directories.append(os.path.join(config["postgresql"]["data_dir"], "pg_wal")) files.append(os.path.join(config["postgresql"]["data_dir"], "global", "pg_control")) files.append(os.path.join(config["postgresql"]["data_dir"], "PG_VERSION")) + binaries.extend([os.path.join(config["postgresql"]["bin_dir"], i) for i in required_binaries]) c = copy.deepcopy(config) c["etcd"]["hosts"] = [] c["postgresql"]["listen"] = '127.0.0.2,*:543' - del c["postgresql"]["bin_dir"] with patch('patroni.validator.open', mock_open(read_data='11')): errors = schema(c) output = "\n".join(errors) @@ -227,18 +242,19 @@ class TestValidator(unittest.TestCase): @patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 12.1")) def test_pg_wal_doesnt_exist(self, mock_out, mock_err): + binaries.extend([os.path.join(config["postgresql"]["bin_dir"], i) for i in required_binaries]) directories.append(config["postgresql"]["data_dir"]) directories.append(config["postgresql"]["bin_dir"]) files.append(os.path.join(config["postgresql"]["data_dir"], "global", "pg_control")) files.append(os.path.join(config["postgresql"]["data_dir"], "PG_VERSION")) c = copy.deepcopy(config) - del c["postgresql"]["bin_dir"] with patch('patroni.validator.open', mock_open(read_data='11')): errors = schema(c) output = "\n".join(errors) self.assertEqual(['postgresql.data_dir', 'raft.bind_addr', 'raft.self_addr'], parse_output(output)) def test_data_dir_is_empty_string(self, mock_out, mock_err): + binaries.extend(required_binaries) directories.append(config["postgresql"]["data_dir"]) directories.append(config["postgresql"]["bin_dir"]) c = copy.deepcopy(config) @@ -248,5 +264,11 @@ class TestValidator(unittest.TestCase): c["postgresql"]["bin_dir"] = "" errors = schema(c) output = "\n".join(errors) - self.assertEqual(['kubernetes', 'postgresql.bin_dir', 'postgresql.data_dir', + self.assertEqual(['kubernetes', 'postgresql.data_dir', 'postgresql.pg_hba', 'raft.bind_addr', 'raft.self_addr'], parse_output(output)) + + def test_directory_contains(self, mock_out, mock_err): + directories.extend([config_2["some_dir"], os.path.join(config_2["some_dir"], "very_interesting_subdir")]) + errors = schema2(config_2) + output = "\n".join(errors) + self.assertEqual(['some_dir'], parse_output(output)) From e0a4a0c6a674523f659539ffe44d75370054e9da Mon Sep 17 00:00:00 2001 From: Polina Bungina <27892524+hughcapet@users.noreply.github.com> Date: Mon, 22 May 2023 15:30:24 +0200 Subject: [PATCH 6/8] Fix pyright complaints about partner_addrs, change Citus repo URL in CI (#2682) * Fix pyright complaints about partner_addrs * Pin pyright version in workflow * Change Citus repo URL in CI --- .github/workflows/tests.yaml | 6 ++++-- patroni/dcs/raft.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f24fa76f..e35a2a81 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -116,8 +116,8 @@ jobs: sudo apt-get install -y wget ca-certificates gnupg debian-archive-keyring apt-transport-https sudo sh -c 'echo "deb http://apt.postgresql.org/pub/repos/apt $(lsb_release -cs)-pgdg main" > /etc/apt/sources.list.d/pgdg.list' sudo sh -c 'wget -qO - https://www.postgresql.org/media/keys/ACCC4CF8.asc | gpg --dearmor > /etc/apt/trusted.gpg.d/apt.postgresql.org.gpg' - sudo sh -c 'echo "deb [signed-by=/etc/apt/trusted.gpg.d/citusdata_community.gpg] https://repos.citusdata.com/community/ubuntu/ $(lsb_release -cs) main" > /etc/apt/sources.list.d/citusdata_community.list' - sudo sh -c 'wget -qO - https://repos.citusdata.com/community/gpgkey | gpg --dearmor > /etc/apt/trusted.gpg.d/citusdata_community.gpg' + sudo sh -c 'echo "deb [signed-by=/etc/apt/trusted.gpg.d/citusdata_community.gpg] https://packagecloud.io/citusdata/community/ubuntu/ $(lsb_release -cs) main" > /etc/apt/sources.list.d/citusdata_community.list' + sudo sh -c 'wget -qO - https://packagecloud.io/citusdata/community/gpgkey | gpg --dearmor > /etc/apt/trusted.gpg.d/citusdata_community.gpg' if: matrix.os == 'ubuntu' - name: Install dependencies run: python .github/workflows/install_deps.py @@ -172,3 +172,5 @@ jobs: run: python -m pip install -r requirements.txt psycopg2-binary psycopg - uses: jakebailey/pyright-action@v1 + with: + version: 1.1.309 diff --git a/patroni/dcs/raft.py b/patroni/dcs/raft.py index 9d9b5f6d..c068f7f0 100644 --- a/patroni/dcs/raft.py +++ b/patroni/dcs/raft.py @@ -10,7 +10,7 @@ from pysyncobj.dns_resolver import globalDnsResolver from pysyncobj.node import TCPNode from pysyncobj.transport import TCPTransport, CONNECTION_STATE from pysyncobj.utility import TcpUtility -from typing import Any, Callable, Collection, Dict, List, Optional, Union, TYPE_CHECKING +from typing import Any, Callable, Collection, Dict, List, Optional, Set, Union, TYPE_CHECKING from . import AbstractDCS, ClusterConfig, Cluster, Failover, Leader, Member, SyncState, TimelineHistory, citus_group_re from ..exceptions import DCSError @@ -114,7 +114,7 @@ class KVStoreTTL(DynMemberSyncObj): self.set_retry_timeout(int(config.get('retry_timeout') or 10)) self_addr = config.get('self_addr') - partner_addrs = set(config.get('partner_addrs', [])) + partner_addrs: Set[str] = set(config.get('partner_addrs', [])) if config.get('patronictl'): if self_addr: partner_addrs.add(self_addr) From db71ba3955b6658db7f68370f0ba08d25bbf2796 Mon Sep 17 00:00:00 2001 From: Polina Bungina <27892524+hughcapet@users.noreply.github.com> Date: Mon, 22 May 2023 16:15:40 +0200 Subject: [PATCH 7/8] Fix dev Dockerfile.citus for arm (#2683) - Fix dev Dockerfile.citus for arm Don't purge lib required for citus run * Change citus repo url, update citus version --- Dockerfile.citus | 8 +++++--- kubernetes/Dockerfile.citus | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Dockerfile.citus b/Dockerfile.citus index 06a84683..b0920557 100644 --- a/Dockerfile.citus +++ b/Dockerfile.citus @@ -37,8 +37,10 @@ RUN set -ex \ && MAKEFLAGS="-j $(grep -c ^processor /proc/cpuinfo)" \ && cd citus && ./configure && make install && cd ../ && rm -rf /citus; \ else \ - curl https://install.citusdata.com/community/deb.sh | bash \ - && apt-get -y install postgresql-$PG_MAJOR-citus-11.2; \ + echo "deb [signed-by=/etc/apt/trusted.gpg.d/citusdata_community.gpg] https://packagecloud.io/citusdata/community/debian/ $(lsb_release -cs) main" > /etc/apt/sources.list.d/citusdata_community.list \ + && curl -sL https://packagecloud.io/citusdata/community/gpgkey | gpg --dearmor > /etc/apt/trusted.gpg.d/citusdata_community.gpg \ + && apt-get update -y \ + && apt-get -y install postgresql-$PG_MAJOR-citus-11.3 \ fi \ && pip3 install dumb-init \ \ @@ -88,7 +90,7 @@ RUN set -ex \ libmagic1 bsdmainutils login ncurses-bin libmagic-mgc e2fslibs bsdutils \ exim4-config gnupg-agent dirmngr libpython2.7-stdlib libpython2.7-minimal \ postgresql-server-dev-$PG_MAJOR git gcc make autoconf \ - libc6-dev flex libcurl4-gnutls-dev libicu-dev libkrb5-dev liblz4-dev \ + libc6-dev flex libicu-dev libkrb5-dev liblz4-dev \ libpam0g-dev libreadline-dev libselinux1-dev libssl-dev libxslt1-dev libzstd-dev uuid-dev \ && apt-get autoremove -y \ && apt-get clean -y \ diff --git a/kubernetes/Dockerfile.citus b/kubernetes/Dockerfile.citus index de3ecb0b..195bb8e9 100644 --- a/kubernetes/Dockerfile.citus +++ b/kubernetes/Dockerfile.citus @@ -10,8 +10,10 @@ RUN export DEBIAN_FRONTEND=noninteractive \ | xargs apt-get install -y busybox vim-tiny curl jq less locales git python3-pip python3-wheel \ ## Make sure we have a en_US.UTF-8 locale available && localedef -i en_US -c -f UTF-8 -A /usr/share/locale/locale.alias en_US.UTF-8 \ - && curl https://install.citusdata.com/community/deb.sh | bash \ - && apt-get -y install postgresql-15-citus-11.2 \ + && echo "deb [signed-by=/etc/apt/trusted.gpg.d/citusdata_community.gpg] https://packagecloud.io/citusdata/community/debian/ $(lsb_release -cs) main" > /etc/apt/sources.list.d/citusdata_community.list \ + && curl -sL https://packagecloud.io/citusdata/community/gpgkey | gpg --dearmor > /etc/apt/trusted.gpg.d/citusdata_community.gpg \ + && apt-get update -y \ + && apt-get -y install postgresql-$PG_MAJOR-citus-11.3 \ && pip3 install setuptools \ && pip3 install 'git+https://github.com/zalando/patroni.git#egg=patroni[kubernetes]' \ && PGHOME=/home/postgres \ From 2f5bcbd8777181cb6cd7a81de62353eadb8f71d5 Mon Sep 17 00:00:00 2001 From: Polina Bungina <27892524+hughcapet@users.noreply.github.com> Date: Tue, 23 May 2023 08:17:51 +0200 Subject: [PATCH 8/8] Change PostgreSQL Slack invite link (#2680) --- .github/ISSUE_TEMPLATE/bug_report.yml | 2 +- .github/ISSUE_TEMPLATE/config.yml | 2 +- README.rst | 2 +- docs/CONTRIBUTING.rst | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 7c97a4e3..fca0377a 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -6,7 +6,7 @@ body: - type: markdown attributes: value: | - If you have a question please post it on channel [#patroni](https://postgresteam.slack.com/archives/C9XPYG92A) in the [PostgreSQL Slack](https://join.slack.com/t/postgresteam/shared_invite/zt-1qj14i9sj-E9WqIFlvcOiHsEk2yFEMjA). + If you have a question please post it on channel [#patroni](https://postgresteam.slack.com/archives/C9XPYG92A) in the [PostgreSQL Slack](https://pgtreats.info/slack-invite). Before reporting a bug please make sure to **reproduce it with the latest Patroni version**! Please fill the form below and provide as much information as possible. Not doing so may result in your bug not being addressed in a timely manner. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index d63cedc1..5c40952c 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,5 +1,5 @@ blank_issues_enabled: false contact_links: - name: Question - url: https://join.slack.com/t/postgresteam/shared_invite/zt-1qj14i9sj-E9WqIFlvcOiHsEk2yFEMjA + url: https://pgtreats.info/slack-invite about: "Please ask questions on channel #patroni in the PostgreSQL Slack" diff --git a/README.rst b/README.rst index c8cf44b0..c26daf9a 100644 --- a/README.rst +++ b/README.rst @@ -49,7 +49,7 @@ We report new releases information `here `__, via Issues and PRs, and on channel `#patroni `__ in the `PostgreSQL Slack `__. If you're using Patroni, or just interested, please join us. +There are two places to connect with the Patroni community: `on github `__, via Issues and PRs, and on channel `#patroni `__ in the `PostgreSQL Slack `__. If you're using Patroni, or just interested, please join us. =================================== Technical Requirements/Installation diff --git a/docs/CONTRIBUTING.rst b/docs/CONTRIBUTING.rst index a4ce4d38..58e5f7bc 100644 --- a/docs/CONTRIBUTING.rst +++ b/docs/CONTRIBUTING.rst @@ -8,7 +8,7 @@ Wanna contribute to Patroni? Yay - here is how! Chatting -------- -Just want to chat with other Patroni users? Looking for interactive troubleshooting help? Join us on channel `#patroni `__ in the `PostgreSQL Slack `__. +Just want to chat with other Patroni users? Looking for interactive troubleshooting help? Join us on channel `#patroni `__ in the `PostgreSQL Slack `__. Running tests -------------