From f747ab7a52c1e3413c15027d1e6ef05f3423dbdd Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 24 Oct 2023 08:43:27 -0300 Subject: [PATCH] [DPE-1781] Configuration support (#239) * Add missing parameters and add configuration support * Use fixed snap * Fix parameters validation * Add validations * Update library * Revert changes on charm.py * Add configuration logic and tests * PR feedback (including the feedback from https://github.com/canonical/postgresql-k8s-operator/pull/281) * Fix TLS tests * Increase timeout --- config.yaml | 178 ++++++++++++++++++ lib/charms/postgresql_k8s/v0/postgresql.py | 91 ++++++++-- src/charm.py | 45 ++++- src/cluster.py | 14 +- src/config.py | 199 +++++++++++++++++++++ src/constants.py | 2 +- templates/patroni.yml.j2 | 5 +- tests/integration/helpers.py | 17 +- tests/integration/test_charm.py | 62 ++++++- tests/integration/test_tls.py | 8 +- tests/unit/test_charm.py | 13 +- tests/unit/test_db.py | 2 + tests/unit/test_postgresql_provider.py | 3 +- 13 files changed, 595 insertions(+), 44 deletions(-) diff --git a/config.yaml b/config.yaml index 8885803da9..781c213e62 100644 --- a/config.yaml +++ b/config.yaml @@ -2,6 +2,103 @@ # See LICENSE file for licensing details. options: + durability_synchronous_commit: + description: | + Sets the current transactions synchronization level. This charm allows only the + “on”, “remote_apply” and “remote_write” values to avoid data loss if the primary + crashes and there are replicas. + type: string + default: "on" + instance_default_text_search_config: + description: | + Selects the text search configuration that is used by those variants of the text + search functions that do not have an explicit argument specifying it. + Allowed values start with “pg_catalog.” followed by a language name, like + “pg_catalog.english”. + type: string + default: "pg_catalog.simple" + instance_password_encryption: + description: | + Determines the algorithm to use to encrypt the password. + Allowed values are: “md5” and “scram-sha-256”. + type: string + default: "scram-sha-256" + logging_log_connections: + description: | + Logs each successful connection. + type: boolean + default: false + logging_log_disconnections: + description: | + Logs end of a session, including duration. + type: boolean + default: false + logging_log_lock_waits: + description: | + Logs long lock waits. + type: boolean + default: false + logging_log_min_duration_statement: + description: | + Sets the minimum running time (milliseconds) above which statements will be logged. + Allowed values are: from -1 to 2147483647 (-1 disables logging + statement durations). + type: int + default: -1 + memory_maintenance_work_mem: + description: | + Sets the maximum memory (KB) to be used for maintenance operations. + Allowed values are: from 1024 to 2147483647. + type: int + default: 65536 + memory_max_prepared_transactions: + description: | + Sets the maximum number of simultaneously prepared transactions. + Allowed values are: from 0 to 262143. + type: int + default: 0 + memory_shared_buffers: + description: | + Sets the number of shared memory buffers (8 kB) used by the server. This charm allows + to set this value up to 40% of the available memory from the unit, as it is unlikely + that an allocation of more than that will work better than a smaller amount. + Allowed values are: from 16 to 1073741823. + type: int + memory_temp_buffers: + description: | + Sets the maximum number of temporary buffers (8 kB) used by each session. + Allowed values are: from 100 to 1073741823. + type: int + default: 1024 + memory_work_mem: + description: | + Sets the maximum memory (KB) to be used for query workspaces. + Allowed values are: from 64 to 2147483647. + type: int + default: 4096 + optimizer_constraint_exclusion: + description: | + Enables the planner to use constraints to optimize queries. + Allowed values are: “on”, “off” and “partition”. + type: string + default: "partition" + optimizer_default_statistics_target: + description: | + Sets the default statistics target. Allowed values are: from 1 to 10000. + type: int + default: 100 + optimizer_from_collapse_limit: + description: | + Sets the FROM-list size beyond which subqueries are not collapsed. + Allowed values are: from 1 to 2147483647. + type: int + default: 8 + optimizer_join_collapse_limit: + description: | + Sets the FROM-list size beyond which JOIN constructs are not flattened. + Allowed values are: from 1 to 2147483647. + type: int + default: 8 plugin_citext_enable: default: false type: boolean @@ -40,3 +137,84 @@ options: Amount of memory in Megabytes to limit PostgreSQL and associated process to. If unset, this will be decided according to the default memory limit in the selected profile. Only comes into effect when the `production` profile is selected. + request_date_style: + description: | + Sets the display format for date and time values. Allowed formats are explained + in https://www.postgresql.org/docs/14/runtime-config-client.html#GUC-DATESTYLE. + type: string + default: "ISO, MDY" + request_standard_conforming_strings: + description: | + Causes ... strings to treat backslashes literally. + type: boolean + default: true + request_time_zone: + description: | + Sets the time zone for displaying and interpreting time stamps. + Allowed values are the ones from IANA time zone data, a time zone abbreviation + like PST and POSIX-style time zone specifications. + type: string + default: "UTC" + response_bytea_output: + description: | + Sets the output format for bytes. + Allowed values are: “escape” and “hex”. + type: string + default: "hex" + response_lc_monetary: + description: | + Sets the locale for formatting monetary amounts. + Allowed values are the locales available in the unit. + type: string + default: "C" + response_lc_numeric: + description: | + Sets the locale for formatting numbers. + Allowed values are the locales available in the unit. + type: string + default: "C" + response_lc_time: + description: | + Sets the locale for formatting date and time values. + Allowed values are the locales available in the unit. + type: string + default: "C" + vacuum_autovacuum_analyze_scale_factor: + description: | + Specifies a fraction of the table size to add to autovacuum_vacuum_threshold when + deciding whether to trigger a VACUUM. The default, 0.1, means 10% of table size. + Allowed values are: from 0 to 100. + type: float + default: 0.1 + vacuum_autovacuum_analyze_threshold: + description: | + Sets the minimum number of inserted, updated or deleted tuples needed to trigger + an ANALYZE in any one table. Allowed values are: from 0 to 2147483647. + type: int + default: 50 + vacuum_autovacuum_freeze_max_age: + description: | + Maximum age (in transactions) before triggering autovacuum on a table to prevent + transaction ID wraparound. Allowed values are: from 100000 to 2000000000. + type: int + default: 200000000 + vacuum_autovacuum_vacuum_cost_delay: + description: | + Sets cost delay value (milliseconds) that will be used in automatic VACUUM operations. + Allowed values are: from -1 to 100 (-1 tells PostgreSQL to use the regular + vacuum_cost_delay value). + type: float + default: 2.0 + vacuum_autovacuum_vacuum_scale_factor: + description: | + Specifies a fraction of the table size to add to autovacuum_vacuum_threshold when + deciding whether to trigger a VACUUM. The default, 0.2, means 20% of table size. + Allowed values are: from 0 to 100. + type: float + default: 0.2 + vacuum_vacuum_freeze_table_age: + description: | + Age (in transactions) at which VACUUM should scan whole table to freeze tuples. + Allowed values are: from 0 to 2000000000. + type: int + default: 150000000 diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 3efcda0c41..debac9ae11 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -32,7 +32,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 17 +LIBPATCH = 18 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles" @@ -310,6 +310,32 @@ def enable_disable_extension(self, extension: str, enable: bool, database: str = if connection is not None: connection.close() + def get_postgresql_text_search_configs(self) -> Set[str]: + """Returns the PostgreSQL available text search configs. + + Returns: + Set of PostgreSQL text search configs. + """ + with self._connect_to_database( + connect_to_current_host=True + ) as connection, connection.cursor() as cursor: + cursor.execute("SELECT CONCAT('pg_catalog.', cfgname) FROM pg_ts_config;") + text_search_configs = cursor.fetchall() + return {text_search_config[0] for text_search_config in text_search_configs} + + def get_postgresql_timezones(self) -> Set[str]: + """Returns the PostgreSQL available timezones. + + Returns: + Set of PostgreSQL timezones. + """ + with self._connect_to_database( + connect_to_current_host=True + ) as connection, connection.cursor() as cursor: + cursor.execute("SELECT name FROM pg_timezone_names;") + timezones = cursor.fetchall() + return {timezone[0] for timezone in timezones} + def get_postgresql_version(self) -> str: """Returns the PostgreSQL version. @@ -445,12 +471,12 @@ def is_restart_pending(self) -> bool: @staticmethod def build_postgresql_parameters( - profile: str, available_memory: int, limit_memory: Optional[int] = None - ) -> Optional[Dict[str, str]]: + config_options: Dict, available_memory: int, limit_memory: Optional[int] = None + ) -> Optional[Dict]: """Builds the PostgreSQL parameters. Args: - profile: the profile to use. + config_options: charm config options containing profile and PostgreSQL parameters. available_memory: available memory to use in calculation in bytes. limit_memory: (optional) limit memory to use in calculation in bytes. @@ -459,19 +485,60 @@ def build_postgresql_parameters( """ if limit_memory: available_memory = min(available_memory, limit_memory) + profile = config_options["profile"] logger.debug(f"Building PostgreSQL parameters for {profile=} and {available_memory=}") + parameters = {} + for config, value in config_options.items(): + # Filter config option not related to PostgreSQL parameters. + if not config.startswith( + ( + "durability", + "instance", + "logging", + "memory", + "optimizer", + "request", + "response", + "vacuum", + ) + ): + continue + parameter = "_".join(config.split("_")[1:]) + if parameter in ["date_style", "time_zone"]: + parameter = "".join(x.capitalize() for x in parameter.split("_")) + parameters[parameter] = value + shared_buffers_max_value = int(int(available_memory * 0.4) / 10**6) + if parameters.get("shared_buffers", 0) > shared_buffers_max_value: + raise Exception( + f"Shared buffers config option should be at most 40% of the available memory, which is {shared_buffers_max_value}MB" + ) if profile == "production": # Use 25% of the available memory for shared_buffers. # and the remaind as cache memory. shared_buffers = int(available_memory * 0.25) effective_cache_size = int(available_memory - shared_buffers) - - parameters = { - "shared_buffers": f"{int(shared_buffers/10**6)}MB", - "effective_cache_size": f"{int(effective_cache_size/10**6)}MB", - } - - return parameters + parameters.setdefault("shared_buffers", f"{int(shared_buffers/10**6)}MB") + parameters.update({"effective_cache_size": f"{int(effective_cache_size/10**6)}MB"}) else: # Return default - return {"shared_buffers": "128MB"} + parameters.setdefault("shared_buffers", "128MB") + return parameters + + def validate_date_style(self, date_style: str) -> bool: + """Validate a date style against PostgreSQL. + + Returns: + Whether the date style is valid. + """ + try: + with self._connect_to_database( + connect_to_current_host=True + ) as connection, connection.cursor() as cursor: + cursor.execute( + sql.SQL( + "SET DateStyle to {};", + ).format(sql.Identifier(date_style)) + ) + return True + except psycopg2.Error: + return False diff --git a/src/charm.py b/src/charm.py index 60824a4599..87a4ea40e7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -5,7 +5,9 @@ """Charmed Machine Operator for the PostgreSQL database.""" import json import logging +import os import subprocess +import time from typing import Dict, List, Optional, Set from charms.data_platform_libs.v0.data_models import TypedCharmBase @@ -1411,14 +1413,14 @@ def _is_workload_running(self) -> bool: def update_config(self, is_creating_backup: bool = False) -> bool: """Updates Patroni config file based on the existence of the TLS files.""" - enable_tls = all(self.tls.get_tls_files()) + enable_tls = self.is_tls_enabled limit_memory = None if self.config.profile_limit_memory: limit_memory = self.config.profile_limit_memory * 10**6 # Build PostgreSQL parameters. pg_parameters = self.postgresql.build_postgresql_parameters( - self.config.profile, self.get_available_memory(), limit_memory + self.model.config, self.get_available_memory(), limit_memory ) # Update and reload configuration based on TLS files availability. @@ -1444,10 +1446,21 @@ def update_config(self, is_creating_backup: bool = False) -> bool: logger.debug("Early exit update_config: Patroni not started yet") return False - restart_postgresql = ( - enable_tls != self.postgresql.is_tls_enabled() - ) or self.postgresql.is_restart_pending() + self._validate_config_options() + + self._patroni.update_parameter_controller_by_patroni( + "max_connections", max(4 * os.cpu_count(), 100) + ) + self._patroni.update_parameter_controller_by_patroni( + "max_prepared_transactions", self.config.memory_max_prepared_transactions + ) + + restart_postgresql = self.is_tls_enabled != self.postgresql.is_tls_enabled() self._patroni.reload_patroni_configuration() + # Sleep the same time as Patroni's loop_wait default value, which tells how much time + # Patroni will wait before checking the configuration file again to reload it. + time.sleep(10) + restart_postgresql = restart_postgresql or self.postgresql.is_restart_pending() self.unit_peer_data.update({"tls": "enabled" if enable_tls else ""}) # Restart PostgreSQL if TLS configuration has changed @@ -1473,6 +1486,28 @@ def update_config(self, is_creating_backup: bool = False) -> bool: return True + def _validate_config_options(self) -> None: + """Validates specific config options that need access to the database or to the TLS status.""" + if ( + self.config.instance_default_text_search_config is not None + and self.config.instance_default_text_search_config + not in self.postgresql.get_postgresql_text_search_configs() + ): + raise Exception( + "instance_default_text_search_config config option has an invalid value" + ) + + if self.config.request_date_style is not None and not self.postgresql.validate_date_style( + self.config.request_date_style + ): + raise Exception("request_date_style config option has an invalid value") + + if ( + self.config.request_time_zone is not None + and self.config.request_time_zone not in self.postgresql.get_postgresql_timezones() + ): + raise Exception("request_time_zone config option has an invalid value") + def _update_relation_endpoints(self) -> None: """Updates endpoints and read-only endpoint in all relations.""" self.postgresql_client_relation.update_endpoints() diff --git a/src/cluster.py b/src/cluster.py index ddce946939..8b896da42f 100644 --- a/src/cluster.py +++ b/src/cluster.py @@ -7,7 +7,7 @@ import os import pwd import subprocess -from typing import Dict, List, Optional, Set +from typing import Any, Dict, List, Optional, Set import requests from charms.operator_libs_linux.v2 import snap @@ -620,6 +620,18 @@ def reinitialize_postgresql(self) -> None: """Reinitialize PostgreSQL.""" requests.post(f"{self._patroni_url}/reinitialize", verify=self.verify) + @retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=2, max=10)) + def update_parameter_controller_by_patroni(self, parameter: str, value: Any) -> None: + """Update the value of a parameter controller by Patroni. + + For more information, check https://patroni.readthedocs.io/en/latest/patroni_configuration.html#postgresql-parameters-controlled-by-patroni. + """ + requests.patch( + f"{self._patroni_url}/config", + verify=self.verify, + json={"postgresql": {"parameters": {parameter: value}}}, + ) + def update_synchronous_node_count(self, units: int = None) -> None: """Update synchronous_node_count to the minority of the planned cluster.""" if units is None: diff --git a/src/config.py b/src/config.py index c612d90702..795357815d 100644 --- a/src/config.py +++ b/src/config.py @@ -4,6 +4,7 @@ """Structured configuration for the PostgreSQL charm.""" import logging +import subprocess from typing import Optional from charms.data_platform_libs.v0.data_models import BaseConfigModel @@ -15,6 +16,22 @@ class CharmConfig(BaseConfigModel): """Manager for the structured configuration.""" + durability_synchronous_commit: Optional[str] + instance_default_text_search_config: Optional[str] + instance_password_encryption: Optional[str] + logging_log_connections: Optional[bool] + logging_log_disconnections: Optional[bool] + logging_log_lock_waits: Optional[bool] + logging_log_min_duration_statement: Optional[int] + memory_maintenance_work_mem: Optional[int] + memory_max_prepared_transactions: Optional[int] + memory_shared_buffers: Optional[int] + memory_temp_buffers: Optional[int] + memory_work_mem: Optional[int] + optimizer_constraint_exclusion: Optional[str] + optimizer_default_statistics_target: Optional[int] + optimizer_from_collapse_limit: Optional[int] + optimizer_join_collapse_limit: Optional[int] profile: str profile_limit_memory: Optional[int] plugin_citext_enable: bool @@ -23,6 +40,19 @@ class CharmConfig(BaseConfigModel): plugin_pg_trgm_enable: bool plugin_plpython3u_enable: bool plugin_unaccent_enable: bool + request_date_style: Optional[str] + request_standard_conforming_strings: Optional[bool] + request_time_zone: Optional[str] + response_bytea_output: Optional[str] + response_lc_monetary: Optional[str] + response_lc_numeric: Optional[str] + response_lc_time: Optional[str] + vacuum_autovacuum_analyze_scale_factor: Optional[float] + vacuum_autovacuum_analyze_threshold: Optional[int] + vacuum_autovacuum_freeze_max_age: Optional[int] + vacuum_autovacuum_vacuum_cost_delay: Optional[float] + vacuum_autovacuum_vacuum_scale_factor: Optional[float] + vacuum_vacuum_freeze_table_age: Optional[int] @classmethod def keys(cls) -> list[str]: @@ -34,6 +64,106 @@ def plugin_keys(cls) -> filter: """Return plugin config names in a iterable.""" return filter(lambda x: x.startswith("plugin_"), cls.keys()) + @validator("durability_synchronous_commit") + @classmethod + def durability_synchronous_commit_values(cls, value: str) -> Optional[str]: + """Check durability_synchronous_commit config option is one of `on`, `remote_apply` or `remote_write`.""" + if value not in ["on", "remote_apply", "remote_write"]: + raise ValueError("Value not one of 'on', 'remote_apply' or 'remote_write'") + + return value + + @validator("instance_password_encryption") + @classmethod + def instance_password_encryption_values(cls, value: str) -> Optional[str]: + """Check instance_password_encryption config option is one of `md5` or `scram-sha-256`.""" + if value not in ["md5", "scram-sha-256"]: + raise ValueError("Value not one of 'md5' or 'scram-sha-256'") + + return value + + @validator("logging_log_min_duration_statement") + @classmethod + def logging_log_min_duration_statement_values(cls, value: int) -> Optional[int]: + """Check logging_log_min_duration_statement config option is between -1 and 2147483647.""" + if value < -1 or value > 2147483647: + raise ValueError("Value is not between -1 and 2147483647") + + return value + + @validator("memory_maintenance_work_mem") + @classmethod + def memory_maintenance_work_mem_values(cls, value: int) -> Optional[int]: + """Check memory_maintenance_work_mem config option is between 1024 and 2147483647.""" + if value < 1024 or value > 2147483647: + raise ValueError("Value is not between 1024 and 2147483647") + + return value + + @validator("memory_max_prepared_transactions") + @classmethod + def memory_max_prepared_transactions_values(cls, value: int) -> Optional[int]: + """Check memory_max_prepared_transactions config option is between 0 and 262143.""" + if value < 0 or value > 262143: + raise ValueError("Value is not between 0 and 262143") + + return value + + @validator("memory_shared_buffers") + @classmethod + def memory_shared_buffers_values(cls, value: int) -> Optional[int]: + """Check memory_shared_buffers config option is greater or equal than 16.""" + if value < 16 or value > 1073741823: + raise ValueError("Shared buffers config option should be at least 16") + + return value + + @validator("memory_temp_buffers") + @classmethod + def memory_temp_buffers_values(cls, value: int) -> Optional[int]: + """Check memory_temp_buffers config option is between 100 and 1073741823.""" + if value < 100 or value > 1073741823: + raise ValueError("Value is not between 100 and 1073741823") + + return value + + @validator("memory_work_mem") + @classmethod + def memory_work_mem_values(cls, value: int) -> Optional[int]: + """Check memory_work_mem config option is between 64 and 2147483647.""" + if value < 64 or value > 2147483647: + raise ValueError("Value is not between 64 and 2147483647") + + return value + + @validator("optimizer_constraint_exclusion") + @classmethod + def optimizer_constraint_exclusion_values(cls, value: str) -> Optional[str]: + """Check optimizer_constraint_exclusion config option is one of `on`, `off` or `partition`.""" + if value not in ["on", "off", "partition"]: + raise ValueError("Value not one of 'on', 'off' or 'partition'") + + return value + + @validator("optimizer_default_statistics_target") + @classmethod + def optimizer_default_statistics_target_values(cls, value: int) -> Optional[int]: + """Check optimizer_default_statistics_target config option is between 1 and 10000.""" + if value < 1 or value > 10000: + raise ValueError("Value is not between 1 and 10000") + + return value + + @validator("optimizer_from_collapse_limit", allow_reuse=True) + @validator("optimizer_join_collapse_limit", allow_reuse=True) + @classmethod + def optimizer_collapse_limit_values(cls, value: int) -> Optional[int]: + """Check optimizer collapse_limit config option is between 1 and 2147483647.""" + if value < 1 or value > 2147483647: + raise ValueError("Value is not between 1 and 2147483647") + + return value + @validator("profile") @classmethod def profile_values(cls, value: str) -> Optional[str]: @@ -53,3 +183,72 @@ def profile_limit_memory_validator(cls, value: int) -> Optional[int]: raise ValueError("`profile-limit-memory` limited to 7 digits (9999999MB)") return value + + @validator("response_bytea_output") + @classmethod + def response_bytea_output_values(cls, value: str) -> Optional[str]: + """Check response_bytea_output config option is one of `escape` or `hex`.""" + if value not in ["escape", "hex"]: + raise ValueError("Value not one of 'escape' or 'hex'") + + return value + + @validator("response_lc_monetary", allow_reuse=True) + @validator("response_lc_numeric", allow_reuse=True) + @validator("response_lc_time", allow_reuse=True) + @classmethod + def response_lc_values(cls, value: str) -> Optional[str]: + """Check if the requested locale is available in the system.""" + output = subprocess.check_output(["ls", "/snap/charmed-postgresql/current/usr/lib/locale"]) + locales = [locale.decode() for locale in output.splitlines()] + locales.append("C") + if value not in locales: + raise ValueError("Value not one of the locales available in the system") + + return value + + @validator("vacuum_autovacuum_analyze_scale_factor", allow_reuse=True) + @validator("vacuum_autovacuum_vacuum_scale_factor", allow_reuse=True) + @classmethod + def vacuum_autovacuum_vacuum_scale_factor_values(cls, value: float) -> Optional[float]: + """Check autovacuum scale_factor config option is between 0 and 100.""" + if value < 0 or value > 100: + raise ValueError("Value is not between 0 and 100") + + return value + + @validator("vacuum_autovacuum_analyze_threshold") + @classmethod + def vacuum_autovacuum_analyze_threshold_values(cls, value: int) -> Optional[int]: + """Check vacuum_autovacuum_analyze_threshold config option is between 0 and 2147483647.""" + if value < 0 or value > 2147483647: + raise ValueError("Value is not between 0 and 2147483647") + + return value + + @validator("vacuum_autovacuum_freeze_max_age") + @classmethod + def vacuum_autovacuum_freeze_max_age_values(cls, value: int) -> Optional[int]: + """Check vacuum_autovacuum_freeze_max_age config option is between 100000 and 2000000000.""" + if value < 100000 or value > 2000000000: + raise ValueError("Value is not between 100000 and 2000000000") + + return value + + @validator("vacuum_autovacuum_vacuum_cost_delay") + @classmethod + def vacuum_autovacuum_vacuum_cost_delay_values(cls, value: float) -> Optional[float]: + """Check vacuum_autovacuum_vacuum_cost_delay config option is between -1 and 100.""" + if value < -1 or value > 100: + raise ValueError("Value is not between -1 and 100") + + return value + + @validator("vacuum_vacuum_freeze_table_age") + @classmethod + def vacuum_vacuum_freeze_table_age_values(cls, value: int) -> Optional[int]: + """Check vacuum_vacuum_freeze_table_age config option is between 0 and 2000000000.""" + if value < 0 or value > 2000000000: + raise ValueError("Value is not between 0 and 2000000000") + + return value diff --git a/src/constants.py b/src/constants.py index 9b0503cf90..02a0b40a9c 100644 --- a/src/constants.py +++ b/src/constants.py @@ -32,7 +32,7 @@ # Snap constants. PGBACKREST_EXECUTABLE = "charmed-postgresql.pgbackrest" POSTGRESQL_SNAP_NAME = "charmed-postgresql" -SNAP_PACKAGES = [(POSTGRESQL_SNAP_NAME, {"revision": "85"})] +SNAP_PACKAGES = [(POSTGRESQL_SNAP_NAME, {"revision": "86"})] SNAP_COMMON_PATH = "/var/snap/charmed-postgresql/common" SNAP_CURRENT_PATH = "/var/snap/charmed-postgresql/current" diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 3b31336634..0732ec7156 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -59,7 +59,6 @@ bootstrap: remove_data_directory_on_rewind_failure: true remove_data_directory_on_diverged_timelines: true parameters: - synchronous_commit: on synchronous_standby_names: "*" {%- if enable_pgbackrest %} archive_command: 'pgbackrest {{ pgbackrest_configuration_file }} --stanza={{ stanza }} archive-push %p' @@ -67,6 +66,10 @@ bootstrap: archive_command: /bin/true {%- endif %} archive_mode: on + autovacuum: true + fsync: true + full_page_writes: true + lc_messages: 'en_US.UTF8' log_autovacuum_min_duration: 60000 log_checkpoints: 'on' log_destination: 'stderr' diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index b704da8a3c..24e7a6c0a9 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -423,21 +423,6 @@ async def deploy_and_relate_bundle_with_postgresql( return relation.id -def enable_connections_logging(ops_test: OpsTest, unit_name: str) -> None: - """Turn on the log of all connections made to a PostgreSQL instance. - - Args: - ops_test: The ops test framework instance - unit_name: The name of the unit to turn on the connection logs - """ - unit_address = get_unit_address(ops_test, unit_name) - requests.patch( - f"https://{unit_address}:8008/config", - json={"postgresql": {"parameters": {"log_connections": True}}}, - verify=False, - ) - - async def ensure_correct_relation_data( ops_test: OpsTest, database_units: int, app_name: str, relation_name: str ) -> None: @@ -879,7 +864,7 @@ async def scale_application(ops_test: OpsTest, application_name: str, count: int ] await ops_test.model.applications[application_name].destroy_units(*units) await ops_test.model.wait_for_idle( - apps=[application_name], status="active", timeout=1000, wait_for_exact_units=count + apps=[application_name], status="active", timeout=1500, wait_for_exact_units=count ) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index b0503c7ad5..935051c306 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -96,10 +96,20 @@ async def test_settings_are_correct(ops_test: OpsTest, unit_id: int): settings_names = [ "archive_command", "archive_mode", + "autovacuum", "data_directory", "cluster_name", "data_checksums", + "fsync", + "full_page_writes", + "lc_messages", "listen_addresses", + "log_autovacuum_min_duration", + "log_checkpoints", + "log_destination", + "log_temp_files", + "log_timezone", + "max_connections", "wal_level", ] with connection.cursor() as cursor: @@ -116,10 +126,20 @@ async def test_settings_are_correct(ops_test: OpsTest, unit_id: int): # Validate each configuration set by Patroni on PostgreSQL. assert settings["archive_command"] == "/bin/true" assert settings["archive_mode"] == "on" + assert settings["autovacuum"] == "on" assert settings["cluster_name"] == DATABASE_APP_NAME assert settings["data_directory"] == f"{STORAGE_PATH}/var/lib/postgresql" assert settings["data_checksums"] == "on" + assert settings["fsync"] == "on" + assert settings["full_page_writes"] == "on" + assert settings["lc_messages"] == "en_US.UTF8" assert settings["listen_addresses"] == host + assert settings["log_autovacuum_min_duration"] == "60000" + assert settings["log_checkpoints"] == "on" + assert settings["log_destination"] == "stderr" + assert settings["log_temp_files"] == "1" + assert settings["log_timezone"] == "UTC" + assert settings["max_connections"] == "100" assert settings["wal_level"] == "logical" # Retrieve settings from Patroni REST API. @@ -127,9 +147,9 @@ async def test_settings_are_correct(ops_test: OpsTest, unit_id: int): settings = result.json() # Validate each configuration related to Patroni - assert settings["postgresql"]["use_pg_rewind"] - assert settings["postgresql"]["remove_data_directory_on_rewind_failure"] - assert settings["postgresql"]["remove_data_directory_on_diverged_timelines"] + assert settings["postgresql"]["use_pg_rewind"] is True + assert settings["postgresql"]["remove_data_directory_on_rewind_failure"] is True + assert settings["postgresql"]["remove_data_directory_on_diverged_timelines"] is True assert settings["loop_wait"] == 10 assert settings["retry_timeout"] == 10 assert settings["maximum_lag_on_failover"] == 1048576 @@ -141,6 +161,42 @@ async def test_settings_are_correct(ops_test: OpsTest, unit_id: int): assert unit.data["port-ranges"][0]["protocol"] == "tcp" +async def test_postgresql_parameters_change(ops_test: OpsTest) -> None: + """Test that's possible to change PostgreSQL parameters.""" + await ops_test.model.applications[DATABASE_APP_NAME].set_config( + { + "memory_max_prepared_transactions": "100", + "memory_shared_buffers": "128", + "response_lc_monetary": "en_GB.utf8", + } + ) + await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active", idle_period=30) + any_unit_name = ops_test.model.applications[DATABASE_APP_NAME].units[0].name + password = await get_password(ops_test, any_unit_name) + + # Connect to PostgreSQL. + for unit_id in UNIT_IDS: + host = get_unit_address(ops_test, f"{DATABASE_APP_NAME}/{unit_id}") + logger.info("connecting to the database host: %s", host) + with db_connect(host, password) as connection: + settings_names = ["max_prepared_transactions", "shared_buffers", "lc_monetary"] + with connection.cursor() as cursor: + cursor.execute( + sql.SQL("SELECT name,setting FROM pg_settings WHERE name IN ({});").format( + sql.SQL(", ").join(sql.Placeholder() * len(settings_names)) + ), + settings_names, + ) + records = cursor.fetchall() + settings = convert_records_to_dict(records) + connection.close() + + # Validate each configuration set by Patroni on PostgreSQL. + assert settings["max_prepared_transactions"] == "100" + assert settings["shared_buffers"] == "128" + assert settings["lc_monetary"] == "en_GB.utf8" + + async def test_scale_down_and_up(ops_test: OpsTest): """Test data is replicated to new units after a scale up.""" # Ensure the initial number of units in the application. diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index 7146d8a43a..963afe13f1 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -16,7 +16,6 @@ check_tls, check_tls_patroni_api, db_connect, - enable_connections_logging, get_password, get_primary, get_unit_address, @@ -79,7 +78,12 @@ async def test_tls_enabled(ops_test: OpsTest) -> None: # Enable additional logs on the PostgreSQL instance to check TLS # being used in a later step and make the fail-over to happens faster. - enable_connections_logging(ops_test, primary) + await ops_test.model.applications[DATABASE_APP_NAME].set_config( + {"logging_log_connections": "True"} + ) + await ops_test.model.wait_for_idle( + apps=[DATABASE_APP_NAME], status="active", idle_period=30 + ) change_primary_start_timeout(ops_test, primary, 0) for attempt in Retrying( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 00b19f1c80..5344dd9ce4 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -250,7 +250,8 @@ def test_on_config_changed( _enable_disable_extensions.assert_called_once() _set_up_relation.assert_called_once() - def test_enable_disable_extensions(self): + @patch("subprocess.check_output", return_value=b"C") + def test_enable_disable_extensions(self, _): with patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock: # Test when all extensions install/uninstall succeed. postgresql_mock.enable_disable_extension.side_effect = None @@ -457,6 +458,7 @@ def test_on_start_replica( self.assertTrue(isinstance(self.harness.model.unit.status, WaitingStatus)) @patch_network_get(private_address="1.1.1.1") + @patch("subprocess.check_output", return_value=b"C") @patch("charm.snap.SnapCache") @patch("charm.PostgresqlOperatorCharm.postgresql") @patch("charm.Patroni") @@ -471,6 +473,7 @@ def test_on_start_no_patroni_member( patroni, _postgresql, _snap_cache, + _, ): # Mock the passwords. patroni.return_value.member_started = False @@ -1055,9 +1058,12 @@ def test_restart(self, _are_all_members_ready, _restart_postgresql): mock_event.defer.assert_not_called() @patch_network_get(private_address="1.1.1.1") + @patch("subprocess.check_output", return_value=b"C") @patch("charm.snap.SnapCache") @patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_acquire_lock") @patch("charm.Patroni.reload_patroni_configuration") + @patch("charm.Patroni.update_parameter_controller_by_patroni") + @patch("charm.PostgresqlOperatorCharm._validate_config_options") @patch("charm.Patroni.member_started", new_callable=PropertyMock) @patch("charm.PostgresqlOperatorCharm._is_workload_running", new_callable=PropertyMock) @patch("charm.Patroni.render_patroni_yml_file") @@ -1068,9 +1074,12 @@ def test_update_config( _render_patroni_yml_file, _is_workload_running, _member_started, + _, + __, _reload_patroni_configuration, _restart, - _, + ___, + ____, ): with patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock: # Mock some properties. diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index 5bcef37e95..6fe44a9b88 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -182,6 +182,7 @@ def test_get_extensions(self): ([extensions[1], extensions[2]], {extensions[2]}), ) + @patch("subprocess.check_output", return_value=b"C") @patch("relations.db.DbProvides._update_unit_status") @patch("relations.db.DbProvides.update_endpoints") @patch("charm.PostgresqlOperatorCharm.enable_disable_extensions") @@ -194,6 +195,7 @@ def test_set_up_relation( _enable_disable_extensions, _update_endpoints, _update_unit_status, + _, ): with patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock: # Define some mocks' side effects. diff --git a/tests/unit/test_postgresql_provider.py b/tests/unit/test_postgresql_provider.py index 28d20093c3..ea9dd0d796 100644 --- a/tests/unit/test_postgresql_provider.py +++ b/tests/unit/test_postgresql_provider.py @@ -73,6 +73,7 @@ def request_database(self): {"database": DATABASE, "extra-user-roles": EXTRA_USER_ROLES}, ) + @patch("subprocess.check_output", return_value=b"C") @patch("charm.PostgreSQLProvider.update_endpoints") @patch("relations.postgresql_provider.new_password", return_value="test-password") @patch.object(EventBase, "defer") @@ -82,7 +83,7 @@ def request_database(self): ) @patch("charm.Patroni.member_started", new_callable=PropertyMock) def test_on_database_requested( - self, _member_started, _primary_endpoint, _defer, _new_password, _update_endpoints + self, _member_started, _primary_endpoint, _defer, _new_password, _update_endpoints, _ ): with patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock: # Set some side effects to test multiple situations.