From 78f8392dd8612858ce1e4ae1cd113193bfd71f4c Mon Sep 17 00:00:00 2001 From: onkolahmet Date: Thu, 6 Nov 2025 15:34:10 +0100 Subject: [PATCH 1/5] config expose for postgresql-14 VM --- config.yaml | 50 +++ lib/charms/postgresql_k8s/v0/postgresql.py | 79 ++++- src/charm.py | 18 +- src/config.py | 44 +++ tests/integration/test_config.py | 22 ++ tests/unit/test_charm.py | 354 +++++++++++++++++++++ 6 files changed, 565 insertions(+), 2 deletions(-) diff --git a/config.yaml b/config.yaml index b20e0bf2ed..7fd084a3c3 100644 --- a/config.yaml +++ b/config.yaml @@ -42,6 +42,13 @@ options: type: int description: | [EXPERIMENTAL] Force set max_connections. + wal_compression: + description: | + Enables compression of full-page writes written to WAL. + Compression can reduce the volume of WAL written and improve I/O performance. + Allowed values are: "on" or "off". + type: string + default: "on" instance_default_text_search_config: description: | Selects the text search configuration that is used by those variants of the text @@ -148,6 +155,49 @@ options: Allowed values are: from 64 to 2147483647. type: int default: 4096 + max_worker_processes: + description: | + Sets the maximum number of background processes that the system can support. + Allowed values are: "auto" or a number. + When set to "auto", defaults to minimum(8, 2*vCores). + If a number is provided, it will be capped to 10 * vCores with a warning logged if exceeded. + type: string + default: "auto" + max_parallel_workers: + description: | + Sets the maximum number of workers that can be started for parallel operations. + Allowed values are: "auto" or a number. + When set to "auto", defaults to max_worker_processes. + type: string + default: "auto" + max_parallel_maintenance_workers: + description: | + Sets the maximum number of parallel workers for maintenance operations. + Allowed values are: "auto" or a number. + When set to "auto", defaults to max_worker_processes. + type: string + default: "auto" + max_logical_replication_workers: + description: | + Sets the maximum number of logical replication workers. + Allowed values are: "auto" or a number. + When set to "auto", defaults to max_worker_processes. + type: string + default: "auto" + max_sync_workers_per_subscription: + description: | + Sets the maximum number of synchronization workers per subscription. + Allowed values are: "auto" or a number. + When set to "auto", defaults to max_worker_processes. + type: string + default: "auto" + max_parallel_apply_workers_per_subscription: + description: | + Sets the maximum number of parallel apply workers per subscription. + Allowed values are: "auto" or a number. + When set to "auto", defaults to max_worker_processes. + type: string + default: "auto" optimizer_constraint_exclusion: description: | Enables the planner to use constraints to optimize queries. diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 480dac02ba..b3bdaa792a 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -897,7 +897,10 @@ def build_postgresql_group_map(group_map: Optional[str]) -> List[Tuple]: @staticmethod def build_postgresql_parameters( - config_options: dict, available_memory: int, limit_memory: Optional[int] = None + config_options: dict, + available_memory: int, + limit_memory: Optional[int] = None, + available_cores: Optional[int] = None, ) -> Optional[dict]: """Builds the PostgreSQL parameters. @@ -905,6 +908,7 @@ def build_postgresql_parameters( 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. + available_cores: (optional) number of available CPU cores for worker process calculations. Returns: Dictionary with the PostgreSQL parameters. @@ -915,6 +919,19 @@ def build_postgresql_parameters( logger.debug(f"Building PostgreSQL parameters for {profile=} and {available_memory=}") parameters = {} for config, value in config_options.items(): + # Handle direct PostgreSQL parameter names (no prefix) + if config in [ + "max_worker_processes", + "max_parallel_workers", + "max_parallel_maintenance_workers", + "max_logical_replication_workers", + "max_sync_workers_per_subscription", + "max_parallel_apply_workers_per_subscription", + "wal_compression", + ]: + parameters[config] = value + continue + # Filter config option not related to PostgreSQL parameters. if not config.startswith(( "connection", @@ -935,6 +952,66 @@ def build_postgresql_parameters( if parameter in ["date_style", "time_zone"]: parameter = "".join(x.capitalize() for x in parameter.split("_")) parameters[parameter] = value + + # Handle worker process parameters with "auto" support + if available_cores is not None: + # Calculate max_worker_processes first (needed as base for other workers) + max_worker_processes_value = None + if "max_worker_processes" in parameters: + if parameters["max_worker_processes"] == "auto": + # auto = minimum(8, 2*vCores) + max_worker_processes_value = min(8, 2 * available_cores) + parameters["max_worker_processes"] = max_worker_processes_value + else: + # It's a number - validate minimum and maximum + max_worker_processes_value = int(parameters["max_worker_processes"]) + if max_worker_processes_value < 0: + raise ValueError( + f"max_worker_processes value {max_worker_processes_value} is below " + f"minimum allowed value of 0." + ) + max_allowed = 10 * available_cores + if max_worker_processes_value > max_allowed: + raise ValueError( + f"max_worker_processes value {max_worker_processes_value} exceeds " + f"maximum allowed limit of {max_allowed} (10 * vCores)." + ) + parameters["max_worker_processes"] = max_worker_processes_value + + # Handle other worker parameters that default to max_worker_processes + worker_params = [ + "max_parallel_workers", + "max_parallel_maintenance_workers", + "max_logical_replication_workers", + "max_sync_workers_per_subscription", + "max_parallel_apply_workers_per_subscription", + ] + + for worker_param in worker_params: + if worker_param in parameters: + if parameters[worker_param] == "auto": + # auto = max_worker_processes + if max_worker_processes_value is not None: + parameters[worker_param] = max_worker_processes_value + else: + # Fallback if max_worker_processes not configured + parameters[worker_param] = min(8, 2 * available_cores) + else: + # It's a number - validate minimum and maximum + worker_value = int(parameters[worker_param]) + if worker_value < 0: + raise ValueError( + f"{worker_param} value {worker_value} is below " + f"minimum allowed value of 0." + ) + max_allowed = 10 * available_cores + if worker_value > max_allowed: + raise ValueError( + f"{worker_param} value {worker_value} exceeds " + f"maximum allowed limit of {max_allowed} (10 * vCores)." + ) + parameters[worker_param] = worker_value + shared_buffers_max_value_in_mb = int(available_memory * 0.4 / 10**6) shared_buffers_max_value = int(shared_buffers_max_value_in_mb * 10**3 / 8) if parameters.get("shared_buffers", 0) > shared_buffers_max_value: diff --git a/src/charm.py b/src/charm.py index 4005b48e0f..8f73cdea89 100755 --- a/src/charm.py +++ b/src/charm.py @@ -2000,7 +2000,10 @@ def update_config(self, is_creating_backup: bool = False, no_peers: bool = False # Build PostgreSQL parameters. pg_parameters = self.postgresql.build_postgresql_parameters( - self.model.config, self.get_available_memory(), limit_memory + self.model.config, + self.get_available_memory(), + limit_memory, + self.get_available_cores(), ) # Update and reload configuration based on TLS files availability. @@ -2152,6 +2155,19 @@ def get_available_memory(self) -> int: return 0 + def get_available_cores(self) -> int: + """Returns the number of available CPU cores. + + This method uses os.cpu_count() which returns the number of logical CPUs + available to the system. This is appropriate for calculating resource + limits for PostgreSQL worker processes. + + Returns: + Number of available CPU cores, or 1 as fallback. + """ + cores = os.cpu_count() + return cores if cores is not None else 1 + @property def client_relations(self) -> list[Relation]: """Return the list of established client relations.""" diff --git a/src/config.py b/src/config.py index 16dda4d06f..0273caa55d 100644 --- a/src/config.py +++ b/src/config.py @@ -25,6 +25,7 @@ class CharmConfig(BaseConfigModel): durability_synchronous_commit: str | None durability_wal_keep_size: int | None experimental_max_connections: int | None + wal_compression: str | None instance_default_text_search_config: str | None instance_max_locks_per_transaction: int | None instance_password_encryption: str | None @@ -42,6 +43,12 @@ class CharmConfig(BaseConfigModel): memory_shared_buffers: int | None memory_temp_buffers: int | None memory_work_mem: int | None + max_worker_processes: str | None + max_parallel_workers: str | None + max_parallel_maintenance_workers: str | None + max_logical_replication_workers: str | None + max_sync_workers_per_subscription: str | None + max_parallel_apply_workers_per_subscription: str | None optimizer_constraint_exclusion: str | None optimizer_cpu_index_tuple_cost: float | None optimizer_cpu_operator_cost: float | None @@ -289,6 +296,43 @@ def memory_work_mem_values(cls, value: int) -> int | None: return value + @validator("wal_compression") + @classmethod + def wal_compression_values(cls, value: str) -> str | None: + """Check wal_compression config option is one of `on` or `off`.""" + if value not in ["on", "off"]: + raise ValueError("Value not one of 'on' or 'off'") + + return value + + @validator( + "max_worker_processes", + "max_parallel_workers", + "max_parallel_maintenance_workers", + "max_logical_replication_workers", + "max_sync_workers_per_subscription", + "max_parallel_apply_workers_per_subscription", + ) + @classmethod + def worker_values(cls, value: str) -> str | None: + """Check worker config options are 'auto' or a valid number. + + Note: The actual cap validation (10 * vCores) is performed at runtime + in the charm layer where CPU information is available. + """ + if value == "auto": + return value + + # Validate that if not "auto", it's a valid positive integer + try: + num_value = int(value) + if num_value < 0: + raise ValueError("Value must be 'auto' or a positive number") + except ValueError: + raise ValueError("Value must be 'auto' or a valid number") + + return value + @validator("optimizer_constraint_exclusion") @classmethod def optimizer_constraint_exclusion_values(cls, value: str) -> str | None: diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py index af36bff263..30ac5d95a0 100644 --- a/tests/integration/test_config.py +++ b/tests/integration/test_config.py @@ -20,11 +20,18 @@ async def test_config_parameters(ops_test: OpsTest, charm) -> None: """Build and deploy one unit of PostgreSQL and then test config with wrong parameters.""" # Build and deploy the PostgreSQL charm. async with ops_test.fast_forward(): + # Get the current system architecture for deployment constraints + import subprocess + arch = subprocess.run( + ["dpkg", "--print-architecture"], capture_output=True, check=True, encoding="utf-8" + ).stdout.strip() + await ops_test.model.deploy( charm, num_units=1, base=CHARM_BASE, config={"profile": "testing"}, + constraints=f"arch={arch}", ) await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active", timeout=1500) @@ -211,6 +218,21 @@ async def test_config_parameters(ops_test: OpsTest, charm) -> None: { "vacuum_vacuum_multixact_freeze_table_age": ["-1", "150000000"] }, # config option is between 0 and 2000000000 + {"wal_compression": [test_string, "on"]}, # config option is one of `on` or `off` + {"max_worker_processes": [test_string, "auto"]}, # config option is `auto` or a valid number + {"max_parallel_workers": [test_string, "auto"]}, # config option is `auto` or a valid number + { + "max_parallel_maintenance_workers": [test_string, "auto"] + }, # config option is `auto` or a valid number + { + "max_logical_replication_workers": [test_string, "auto"] + }, # config option is `auto` or a valid number + { + "max_sync_workers_per_subscription": [test_string, "auto"] + }, # config option is `auto` or a valid number + { + "max_parallel_apply_workers_per_subscription": [test_string, "auto"] + }, # config option is `auto` or a valid number ] charm_config = {} diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1747cf08a2..b65a19adbf 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -2957,3 +2957,357 @@ def test_on_secret_remove(harness, only_with_juju_secrets): event.secret.label = None harness.charm._on_secret_remove(event) assert not event.remove_revision.called + + +def test_build_postgresql_parameters_wal_compression(): + """Test wal_compression parameter is correctly passed through.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # Test with wal_compression on + config = {"profile": "testing", "wal_compression": "on"} + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + assert params["wal_compression"] == "on" + + # Test with wal_compression off + config = {"profile": "testing", "wal_compression": "off"} + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + assert params["wal_compression"] == "off" + + +def test_build_postgresql_parameters_max_worker_processes_auto(): + """Test max_worker_processes auto calculation.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # With 4 cores: min(8, 2*4) = 8 + config = {"profile": "testing", "max_worker_processes": "auto"} + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + assert params["max_worker_processes"] == 8 + + # With 16 cores: min(8, 2*16) = 8 (capped at 8) + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=16) + assert params["max_worker_processes"] == 8 + + # With 2 cores: min(8, 2*2) = 4 + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=2) + assert params["max_worker_processes"] == 4 + + +def test_build_postgresql_parameters_max_worker_processes_numeric(): + """Test max_worker_processes with valid numeric values.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # Valid value within limit (4 cores * 10 = 40 max) + config = {"profile": "testing", "max_worker_processes": "16"} + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + assert params["max_worker_processes"] == 16 + + # At the maximum limit + config = {"profile": "testing", "max_worker_processes": "40"} + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + assert params["max_worker_processes"] == 40 + + +def test_build_postgresql_parameters_max_worker_processes_exceeds_limit(): + """Test max_worker_processes blocks when exceeding 10*vCores.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # 4 cores * 10 = 40 max, trying 50 should raise ValueError + config = {"profile": "testing", "max_worker_processes": "50"} + with pytest.raises(ValueError, match="exceeds maximum allowed limit of 40"): + PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + +def test_build_postgresql_parameters_max_worker_processes_negative(): + """Test max_worker_processes blocks negative values.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = {"profile": "testing", "max_worker_processes": "-5"} + with pytest.raises(ValueError, match="is below minimum allowed value of 0"): + PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + +def test_build_postgresql_parameters_max_parallel_workers_auto(): + """Test max_parallel_workers auto uses max_worker_processes value.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = { + "profile": "testing", + "max_worker_processes": "12", + "max_parallel_workers": "auto", + } + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + assert params["max_parallel_workers"] == 12 + + +def test_build_postgresql_parameters_max_parallel_workers_numeric(): + """Test max_parallel_workers with valid numeric value.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = {"profile": "testing", "max_parallel_workers": "20"} + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + assert params["max_parallel_workers"] == 20 + + +def test_build_postgresql_parameters_max_parallel_workers_exceeds_limit(): + """Test max_parallel_workers blocks when exceeding limit.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # 4 cores * 10 = 40 max, trying 45 should raise ValueError + config = {"profile": "testing", "max_parallel_workers": "45"} + with pytest.raises(ValueError, match="exceeds maximum allowed limit of 40"): + PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + +def test_build_postgresql_parameters_max_parallel_workers_negative(): + """Test max_parallel_workers blocks negative values.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = {"profile": "testing", "max_parallel_workers": "-3"} + with pytest.raises(ValueError, match="is below minimum allowed value of 0"): + PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + +def test_build_postgresql_parameters_all_worker_params_auto(): + """Test all worker parameters with auto setting.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = { + "profile": "testing", + "max_worker_processes": "auto", + "max_parallel_workers": "auto", + "max_parallel_maintenance_workers": "auto", + "max_logical_replication_workers": "auto", + "max_sync_workers_per_subscription": "auto", + "max_parallel_apply_workers_per_subscription": "auto", + } + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + # All should be 8 (min(8, 2*4)) + assert params["max_worker_processes"] == 8 + assert params["max_parallel_workers"] == 8 + assert params["max_parallel_maintenance_workers"] == 8 + assert params["max_logical_replication_workers"] == 8 + assert params["max_sync_workers_per_subscription"] == 8 + assert params["max_parallel_apply_workers_per_subscription"] == 8 + + +def test_build_postgresql_parameters_mixed_worker_params(): + """Test mix of auto and numeric values for worker parameters.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = { + "profile": "testing", + "max_worker_processes": "10", + "max_parallel_workers": "auto", + "max_parallel_maintenance_workers": "6", + "max_logical_replication_workers": "auto", + } + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + assert params["max_worker_processes"] == 10 + assert params["max_parallel_workers"] == 10 # auto = max_worker_processes + assert params["max_parallel_maintenance_workers"] == 6 + assert params["max_logical_replication_workers"] == 10 # auto = max_worker_processes + + +def test_get_available_cores(harness): + """Test get_available_cores returns CPU count from OS.""" + with patch("os.cpu_count", return_value=8): + assert harness.charm.get_available_cores() == 8 + + with patch("os.cpu_count", return_value=4): + assert harness.charm.get_available_cores() == 4 + + with patch("os.cpu_count", return_value=1): + assert harness.charm.get_available_cores() == 1 + + +def test_get_available_cores_fallback_when_none(harness): + """Test get_available_cores returns 1 when os.cpu_count() returns None.""" + with patch("os.cpu_count", return_value=None): + assert harness.charm.get_available_cores() == 1 + + +def test_build_postgresql_parameters_no_cores_provided(): + """Test that worker parameters are not validated when available_cores is None.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # When available_cores is None, even out-of-range values should pass through + # (they remain as strings and validation is skipped) + config = { + "profile": "testing", + "max_worker_processes": "1000", # Would exceed any reasonable limit + "max_parallel_workers": "auto", # Should remain as "auto" + } + + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=None) + + # Parameters should be passed through without conversion or validation + assert params["max_worker_processes"] == "1000" + assert params["max_parallel_workers"] == "auto" + + +def test_build_postgresql_parameters_zero_value(): + """Test that zero is a valid value for worker parameters.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = { + "profile": "testing", + "max_worker_processes": "0", + "max_parallel_workers": "0", + } + + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + # Zero should be accepted and converted to int + assert params["max_worker_processes"] == 0 + assert params["max_parallel_workers"] == 0 + + +def test_build_postgresql_parameters_boundary_max_value(): + """Test maximum boundary value (exactly 10*vCores).""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # With 4 cores, max is 40 + config = { + "profile": "testing", + "max_worker_processes": "40", + "max_parallel_workers": "40", + } + + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + # Exactly at the limit should be accepted + assert params["max_worker_processes"] == 40 + assert params["max_parallel_workers"] == 40 + + +def test_build_postgresql_parameters_boundary_just_over_max(): + """Test value just over maximum boundary (10*vCores + 1).""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # With 4 cores, max is 40, so 41 should fail + config = {"profile": "testing", "max_worker_processes": "41"} + + with pytest.raises(ValueError, match="exceeds maximum allowed limit of 40"): + PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + +def test_build_postgresql_parameters_auto_with_many_cores(): + """Test auto calculation with many cores (should cap at 8).""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # With 16 cores, 2*16=32, but auto should cap at 8 + config = {"profile": "testing", "max_worker_processes": "auto"} + + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=16) + + # Should be min(8, 2*16) = 8 + assert params["max_worker_processes"] == 8 + + +def test_build_postgresql_parameters_auto_with_single_core(): + """Test auto calculation with single core.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # With 1 core, 2*1=2 + config = {"profile": "testing", "max_worker_processes": "auto"} + + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=1) + + # Should be min(8, 2*1) = 2 + assert params["max_worker_processes"] == 2 + + +def test_build_postgresql_parameters_dependent_workers_when_max_not_set(): + """Test dependent worker params use fallback when max_worker_processes not configured.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + # Don't set max_worker_processes, but set dependent params + config = { + "profile": "testing", + "max_parallel_workers": "auto", + "max_logical_replication_workers": "auto", + } + + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + # Should use fallback: min(8, 2*4) = 8 + assert params["max_parallel_workers"] == 8 + assert params["max_logical_replication_workers"] == 8 + + +def test_build_postgresql_parameters_all_worker_params_different_values(): + """Test all worker parameters can have different valid numeric values.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = { + "profile": "testing", + "max_worker_processes": "20", + "max_parallel_workers": "15", + "max_parallel_maintenance_workers": "10", + "max_logical_replication_workers": "8", + "max_sync_workers_per_subscription": "5", + "max_parallel_apply_workers_per_subscription": "3", + } + + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + # All should be converted to int and within limit (0-40) + assert params["max_worker_processes"] == 20 + assert params["max_parallel_workers"] == 15 + assert params["max_parallel_maintenance_workers"] == 10 + assert params["max_logical_replication_workers"] == 8 + assert params["max_sync_workers_per_subscription"] == 5 + assert params["max_parallel_apply_workers_per_subscription"] == 3 + + +def test_build_postgresql_parameters_string_not_number_or_auto(): + """Test that invalid string values (not 'auto' or numeric) raise error.""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = {"profile": "testing", "max_worker_processes": "invalid"} + + # Should raise ValueError when trying to convert to int + with pytest.raises(ValueError, match="invalid literal for int"): + PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + +def test_build_postgresql_parameters_float_value(): + """Test that float values are handled (truncated to int).""" + from charms.postgresql_k8s.v0.postgresql import PostgreSQL + + config = {"profile": "testing", "max_worker_processes": "8.7"} + + # Should raise ValueError as int() doesn't accept float strings + with pytest.raises(ValueError, match="invalid literal for int"): + PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) + + +def test_update_config_calls_get_available_cores(harness): + """Test that update_config properly calls get_available_cores and passes it through.""" + with ( + patch("charm.snap.SnapCache"), + patch( + "charm.PostgresqlOperatorCharm._is_workload_running", new_callable=PropertyMock + ) as _is_workload_running, + patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started, + patch.object(harness.charm, "get_available_cores", return_value=8) as mock_cores, + patch.object(harness.charm, "get_available_memory", return_value=1073741824), + patch.object(harness.charm.postgresql, "build_postgresql_parameters", return_value={}) as mock_build, + patch.object(harness.charm._patroni, "render_patroni_yml_file"), + patch.object(harness.charm._patroni, "reload_patroni_configuration"), + patch("charm.Patroni.bulk_update_parameters_controller_by_patroni"), + ): + _is_workload_running.return_value = True + _member_started.return_value = True + + harness.charm.update_config() + + # Verify get_available_cores was called + mock_cores.assert_called_once() + + # Verify build_postgresql_parameters was called with the cores value + mock_build.assert_called_once() + call_args = mock_build.call_args + assert call_args[0][3] == 8 # available_cores is 4th positional arg From 959ccfd22957671d6bc02099b83aa111569038e7 Mon Sep 17 00:00:00 2001 From: onkolahmet Date: Thu, 6 Nov 2025 15:48:43 +0100 Subject: [PATCH 2/5] fix unit testlint --- src/charm.py | 4 +- src/config.py | 10 ++--- tests/integration/test_config.py | 11 ++++-- tests/unit/test_charm.py | 68 ++++++++++++++++---------------- 4 files changed, 50 insertions(+), 43 deletions(-) diff --git a/src/charm.py b/src/charm.py index 8f73cdea89..55058b8b88 100755 --- a/src/charm.py +++ b/src/charm.py @@ -2157,11 +2157,11 @@ def get_available_memory(self) -> int: def get_available_cores(self) -> int: """Returns the number of available CPU cores. - + This method uses os.cpu_count() which returns the number of logical CPUs available to the system. This is appropriate for calculating resource limits for PostgreSQL worker processes. - + Returns: Number of available CPU cores, or 1 as fallback. """ diff --git a/src/config.py b/src/config.py index 0273caa55d..6024f03cea 100644 --- a/src/config.py +++ b/src/config.py @@ -316,21 +316,21 @@ def wal_compression_values(cls, value: str) -> str | None: @classmethod def worker_values(cls, value: str) -> str | None: """Check worker config options are 'auto' or a valid number. - + Note: The actual cap validation (10 * vCores) is performed at runtime in the charm layer where CPU information is available. """ if value == "auto": return value - + # Validate that if not "auto", it's a valid positive integer try: num_value = int(value) if num_value < 0: raise ValueError("Value must be 'auto' or a positive number") - except ValueError: - raise ValueError("Value must be 'auto' or a valid number") - + except ValueError as e: + raise ValueError("Value must be 'auto' or a valid number") from e + return value @validator("optimizer_constraint_exclusion") diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py index 30ac5d95a0..0d0424e178 100644 --- a/tests/integration/test_config.py +++ b/tests/integration/test_config.py @@ -22,10 +22,11 @@ async def test_config_parameters(ops_test: OpsTest, charm) -> None: async with ops_test.fast_forward(): # Get the current system architecture for deployment constraints import subprocess + arch = subprocess.run( ["dpkg", "--print-architecture"], capture_output=True, check=True, encoding="utf-8" ).stdout.strip() - + await ops_test.model.deploy( charm, num_units=1, @@ -219,8 +220,12 @@ async def test_config_parameters(ops_test: OpsTest, charm) -> None: "vacuum_vacuum_multixact_freeze_table_age": ["-1", "150000000"] }, # config option is between 0 and 2000000000 {"wal_compression": [test_string, "on"]}, # config option is one of `on` or `off` - {"max_worker_processes": [test_string, "auto"]}, # config option is `auto` or a valid number - {"max_parallel_workers": [test_string, "auto"]}, # config option is `auto` or a valid number + { + "max_worker_processes": [test_string, "auto"] + }, # config option is `auto` or a valid number + { + "max_parallel_workers": [test_string, "auto"] + }, # config option is `auto` or a valid number { "max_parallel_maintenance_workers": [test_string, "auto"] }, # config option is `auto` or a valid number diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index b65a19adbf..88e92330a3 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3114,10 +3114,10 @@ def test_get_available_cores(harness): """Test get_available_cores returns CPU count from OS.""" with patch("os.cpu_count", return_value=8): assert harness.charm.get_available_cores() == 8 - + with patch("os.cpu_count", return_value=4): assert harness.charm.get_available_cores() == 4 - + with patch("os.cpu_count", return_value=1): assert harness.charm.get_available_cores() == 1 @@ -3131,7 +3131,7 @@ def test_get_available_cores_fallback_when_none(harness): def test_build_postgresql_parameters_no_cores_provided(): """Test that worker parameters are not validated when available_cores is None.""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + # When available_cores is None, even out-of-range values should pass through # (they remain as strings and validation is skipped) config = { @@ -3139,9 +3139,9 @@ def test_build_postgresql_parameters_no_cores_provided(): "max_worker_processes": "1000", # Would exceed any reasonable limit "max_parallel_workers": "auto", # Should remain as "auto" } - + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=None) - + # Parameters should be passed through without conversion or validation assert params["max_worker_processes"] == "1000" assert params["max_parallel_workers"] == "auto" @@ -3150,15 +3150,15 @@ def test_build_postgresql_parameters_no_cores_provided(): def test_build_postgresql_parameters_zero_value(): """Test that zero is a valid value for worker parameters.""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + config = { "profile": "testing", "max_worker_processes": "0", "max_parallel_workers": "0", } - + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) - + # Zero should be accepted and converted to int assert params["max_worker_processes"] == 0 assert params["max_parallel_workers"] == 0 @@ -3167,16 +3167,16 @@ def test_build_postgresql_parameters_zero_value(): def test_build_postgresql_parameters_boundary_max_value(): """Test maximum boundary value (exactly 10*vCores).""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + # With 4 cores, max is 40 config = { "profile": "testing", "max_worker_processes": "40", "max_parallel_workers": "40", } - + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) - + # Exactly at the limit should be accepted assert params["max_worker_processes"] == 40 assert params["max_parallel_workers"] == 40 @@ -3185,10 +3185,10 @@ def test_build_postgresql_parameters_boundary_max_value(): def test_build_postgresql_parameters_boundary_just_over_max(): """Test value just over maximum boundary (10*vCores + 1).""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + # With 4 cores, max is 40, so 41 should fail config = {"profile": "testing", "max_worker_processes": "41"} - + with pytest.raises(ValueError, match="exceeds maximum allowed limit of 40"): PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) @@ -3196,12 +3196,12 @@ def test_build_postgresql_parameters_boundary_just_over_max(): def test_build_postgresql_parameters_auto_with_many_cores(): """Test auto calculation with many cores (should cap at 8).""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + # With 16 cores, 2*16=32, but auto should cap at 8 config = {"profile": "testing", "max_worker_processes": "auto"} - + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=16) - + # Should be min(8, 2*16) = 8 assert params["max_worker_processes"] == 8 @@ -3209,12 +3209,12 @@ def test_build_postgresql_parameters_auto_with_many_cores(): def test_build_postgresql_parameters_auto_with_single_core(): """Test auto calculation with single core.""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + # With 1 core, 2*1=2 config = {"profile": "testing", "max_worker_processes": "auto"} - + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=1) - + # Should be min(8, 2*1) = 2 assert params["max_worker_processes"] == 2 @@ -3222,16 +3222,16 @@ def test_build_postgresql_parameters_auto_with_single_core(): def test_build_postgresql_parameters_dependent_workers_when_max_not_set(): """Test dependent worker params use fallback when max_worker_processes not configured.""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + # Don't set max_worker_processes, but set dependent params config = { "profile": "testing", "max_parallel_workers": "auto", "max_logical_replication_workers": "auto", } - + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) - + # Should use fallback: min(8, 2*4) = 8 assert params["max_parallel_workers"] == 8 assert params["max_logical_replication_workers"] == 8 @@ -3240,7 +3240,7 @@ def test_build_postgresql_parameters_dependent_workers_when_max_not_set(): def test_build_postgresql_parameters_all_worker_params_different_values(): """Test all worker parameters can have different valid numeric values.""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + config = { "profile": "testing", "max_worker_processes": "20", @@ -3250,9 +3250,9 @@ def test_build_postgresql_parameters_all_worker_params_different_values(): "max_sync_workers_per_subscription": "5", "max_parallel_apply_workers_per_subscription": "3", } - + params = PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) - + # All should be converted to int and within limit (0-40) assert params["max_worker_processes"] == 20 assert params["max_parallel_workers"] == 15 @@ -3265,9 +3265,9 @@ def test_build_postgresql_parameters_all_worker_params_different_values(): def test_build_postgresql_parameters_string_not_number_or_auto(): """Test that invalid string values (not 'auto' or numeric) raise error.""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + config = {"profile": "testing", "max_worker_processes": "invalid"} - + # Should raise ValueError when trying to convert to int with pytest.raises(ValueError, match="invalid literal for int"): PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) @@ -3276,9 +3276,9 @@ def test_build_postgresql_parameters_string_not_number_or_auto(): def test_build_postgresql_parameters_float_value(): """Test that float values are handled (truncated to int).""" from charms.postgresql_k8s.v0.postgresql import PostgreSQL - + config = {"profile": "testing", "max_worker_processes": "8.7"} - + # Should raise ValueError as int() doesn't accept float strings with pytest.raises(ValueError, match="invalid literal for int"): PostgreSQL.build_postgresql_parameters(config, 1073741824, available_cores=4) @@ -3294,19 +3294,21 @@ def test_update_config_calls_get_available_cores(harness): patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started, patch.object(harness.charm, "get_available_cores", return_value=8) as mock_cores, patch.object(harness.charm, "get_available_memory", return_value=1073741824), - patch.object(harness.charm.postgresql, "build_postgresql_parameters", return_value={}) as mock_build, + patch.object( + harness.charm.postgresql, "build_postgresql_parameters", return_value={} + ) as mock_build, patch.object(harness.charm._patroni, "render_patroni_yml_file"), patch.object(harness.charm._patroni, "reload_patroni_configuration"), patch("charm.Patroni.bulk_update_parameters_controller_by_patroni"), ): _is_workload_running.return_value = True _member_started.return_value = True - + harness.charm.update_config() - + # Verify get_available_cores was called mock_cores.assert_called_once() - + # Verify build_postgresql_parameters was called with the cores value mock_build.assert_called_once() call_args = mock_build.call_args From 65d1e40d1dbd89afacb244610baf8220049e6d44 Mon Sep 17 00:00:00 2001 From: onkolahmet Date: Mon, 10 Nov 2025 11:52:57 +0100 Subject: [PATCH 3/5] improve test coverage --- tests/unit/test_charm.py | 88 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 88e92330a3..94e3f86c4b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3313,3 +3313,91 @@ def test_update_config_calls_get_available_cores(harness): mock_build.assert_called_once() call_args = mock_build.call_args assert call_args[0][3] == 8 # available_cores is 4th positional arg + + +# Tests for config.py validators to increase coverage +def test_config_wal_compression_invalid_value(): + """Test that wal_compression validator rejects invalid values.""" + from pydantic import ValidationError + + from src.config import CharmConfig + + # Create a minimal valid config + base_config = { + "wal_compression": "invalid_value", # Invalid + } + + with pytest.raises(ValidationError) as exc_info: + CharmConfig(**base_config) + + # Check that validation error mentions wal_compression + errors = exc_info.value.errors() + assert any("wal_compression" in str(error) for error in errors) + + +def test_config_worker_processes_negative_value(): + """Test that worker process validators reject negative values.""" + from pydantic import ValidationError + + from src.config import CharmConfig + + base_config = { + "max_worker_processes": "-5", # Negative + } + + with pytest.raises(ValidationError) as exc_info: + CharmConfig(**base_config) + + errors = exc_info.value.errors() + assert any("max_worker_processes" in str(error) for error in errors) + + +def test_config_worker_processes_invalid_string(): + """Test that worker process validators reject invalid string values.""" + from pydantic import ValidationError + + from src.config import CharmConfig + + base_config = { + "max_worker_processes": "not_a_number", # Invalid string + } + + with pytest.raises(ValidationError) as exc_info: + CharmConfig(**base_config) + + errors = exc_info.value.errors() + assert any("max_worker_processes" in str(error) for error in errors) + + +def test_config_max_parallel_workers_negative(): + """Test that max_parallel_workers validator rejects negative values.""" + from pydantic import ValidationError + + from src.config import CharmConfig + + base_config = { + "max_parallel_workers": "-10", + } + + with pytest.raises(ValidationError) as exc_info: + CharmConfig(**base_config) + + errors = exc_info.value.errors() + assert any("max_parallel_workers" in str(error) for error in errors) + + +def test_config_max_parallel_maintenance_workers_invalid(): + """Test that max_parallel_maintenance_workers validator rejects invalid values.""" + from pydantic import ValidationError + + from src.config import CharmConfig + + base_config = { + "max_parallel_maintenance_workers": "INVALID", + } + + with pytest.raises(ValidationError) as exc_info: + CharmConfig(**base_config) + + errors = exc_info.value.errors() + assert any("max_parallel_maintenance_workers" in str(error) for error in errors) From 547ceb1ce3db473947b07d624b00d523c480300f Mon Sep 17 00:00:00 2001 From: onkolahmet Date: Mon, 10 Nov 2025 12:09:59 +0100 Subject: [PATCH 4/5] improve test coverage --- tests/unit/test_charm.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 94e3f86c4b..54d078ef75 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3401,3 +3401,28 @@ def test_config_max_parallel_maintenance_workers_invalid(): errors = exc_info.value.errors() assert any("max_parallel_maintenance_workers" in str(error) for error in errors) + + +def test_config_worker_values_validator_auto(): + """Test that worker_values validator accepts 'auto' value.""" + from src.config import CharmConfig + + # Test the validator directly by calling it + result = CharmConfig.worker_values("auto") + assert result == "auto" + + +def test_config_worker_values_validator_positive_number(): + """Test that worker_values validator accepts valid positive numbers.""" + from src.config import CharmConfig + + # Test the validator directly by calling it with a valid positive number string + result = CharmConfig.worker_values("8") + assert result == "8" + + result = CharmConfig.worker_values("0") + assert result == "0" + + result = CharmConfig.worker_values("100") + assert result == "100" + From 8668a20cd21bf2b8c1855e41c2f827d7de736fce Mon Sep 17 00:00:00 2001 From: onkolahmet Date: Mon, 10 Nov 2025 12:16:30 +0100 Subject: [PATCH 5/5] fix lint for tests --- tests/unit/test_charm.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 54d078ef75..bac6174d88 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3419,10 +3419,9 @@ def test_config_worker_values_validator_positive_number(): # Test the validator directly by calling it with a valid positive number string result = CharmConfig.worker_values("8") assert result == "8" - + result = CharmConfig.worker_values("0") assert result == "0" - + result = CharmConfig.worker_values("100") assert result == "100" -