diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 7e6a9d7631..8f08cead25 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -698,15 +698,22 @@ def list_valid_privileges_and_roles(self) -> Tuple[Set[str], Set[str]]: "superuser", }, {role[0] for role in cursor.fetchall() if role[0]} - def set_up_database(self) -> None: + def set_up_database(self, temp_location: Optional[str] = None) -> None: """Set up postgres database with the right permissions.""" connection = None + cursor = None try: - with self._connect_to_database() as connection, connection.cursor() as cursor: - cursor.execute("SELECT TRUE FROM pg_roles WHERE rolname='admin';") - if cursor.fetchone() is not None: - return + connection = self._connect_to_database() + cursor = connection.cursor() + if temp_location is not None: + cursor.execute("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';") + if cursor.fetchone() is None: + cursor.execute(f"CREATE TABLESPACE temp LOCATION '{temp_location}';") + cursor.execute("GRANT CREATE ON TABLESPACE temp TO public;") + + cursor.execute("SELECT TRUE FROM pg_roles WHERE rolname='admin';") + if cursor.fetchone() is None: # Allow access to the postgres database only to the system users. cursor.execute("REVOKE ALL PRIVILEGES ON DATABASE postgres FROM PUBLIC;") cursor.execute("REVOKE CREATE ON SCHEMA public FROM PUBLIC;") @@ -725,6 +732,8 @@ def set_up_database(self) -> None: logger.error(f"Failed to set up databases: {e}") raise PostgreSQLDatabasesSetupError() from e finally: + if cursor is not None: + cursor.close() if connection is not None: connection.close() diff --git a/metadata.yaml b/metadata.yaml index d860b58ddc..1961e2327c 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -21,8 +21,14 @@ containers: postgresql: resource: postgresql-image mounts: - - storage: pgdata +# - storage: archive +# location: /tmp/pgbackrest + - storage: data location: /var/lib/postgresql/data + - storage: logs + location: /var/lib/postgresql/logs + - storage: temp + location: /var/lib/postgresql/temp resources: postgresql-image: @@ -80,9 +86,18 @@ requires: optional: true storage: - pgdata: +# archive: +# type: filesystem +# location: /tmp/pgbackrest + data: type: filesystem location: /var/lib/postgresql/data + logs: + type: filesystem + location: /var/lib/postgresql/logs + temp: + type: filesystem + location: /var/logs/postgresql/temp assumes: - k8s-api diff --git a/src/backups.py b/src/backups.py index 9c8a68e0af..fab9bd89cc 100644 --- a/src/backups.py +++ b/src/backups.py @@ -195,7 +195,7 @@ def can_use_s3_repository(self) -> tuple[bool, str | None]: system_identifier_from_instance, error = self._execute_command([ f"/usr/lib/postgresql/{self.charm._patroni.rock_postgresql_version.split('.')[0]}/bin/pg_controldata", - "/var/lib/postgresql/data/pgdata", + "/var/lib/postgresql/data", ]) if error != "": raise Exception(error) @@ -273,7 +273,7 @@ def _create_bucket_if_not_exists(self) -> None: def _empty_data_files(self) -> None: """Empty the PostgreSQL data directory in preparation of backup restore.""" try: - self.container.exec(["rm", "-r", "/var/lib/postgresql/data/pgdata"]).wait_output() + self.container.exec(["rm", "-r", "/var/lib/postgresql/data"]).wait_output() except ExecError as e: # If previous PITR restore was unsuccessful, there is no such directory. if "No such file or directory" not in e.stderr: @@ -1040,7 +1040,7 @@ def _on_restore_action(self, event): # noqa: C901 return logger.info("Creating PostgreSQL data directory") - self.charm._create_pgdata(self.container) + self.charm._create_data(self.container) # Mark the cluster as in a restoring backup state and update the Patroni configuration. logger.info("Configuring Patroni to restore the backup") diff --git a/src/charm.py b/src/charm.py index c2f1e5699c..c75ad1271b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -132,7 +132,7 @@ EXTENSIONS_DEPENDENCY_MESSAGE = "Unsatisfied plugin dependencies. Please check the logs" EXTENSION_OBJECT_MESSAGE = "Cannot disable plugins: Existing objects depend on it. See logs" -INSUFFICIENT_SIZE_WARNING = "<10% free space on pgdata volume." +INSUFFICIENT_SIZE_WARNING = "<10% free space on data volume." ORIGINAL_PATRONI_ON_FAILURE_CONDITION = "restart" @@ -213,7 +213,7 @@ def __init__(self, *args): self.framework.observe(self.on.secret_changed, self._on_peer_relation_changed) self.framework.observe(self.on[PEER].relation_departed, self._on_peer_relation_departed) self.framework.observe(self.on.postgresql_pebble_ready, self._on_postgresql_pebble_ready) - self.framework.observe(self.on.pgdata_storage_detaching, self._on_pgdata_storage_detaching) + self.framework.observe(self.on.data_storage_detaching, self._on_data_storage_detaching) self.framework.observe(self.on.stop, self._on_stop) self.framework.observe(self.on.get_password_action, self._on_get_password) self.framework.observe(self.on.set_password_action, self._on_set_password) @@ -222,8 +222,8 @@ def __init__(self, *args): self.framework.observe(self.on.update_status, self._on_update_status) self._certs_path = "/usr/local/share/ca-certificates" - self._storage_path = self.meta.storages["pgdata"].location - self.pgdata_path = f"{self._storage_path}/pgdata" + self._storage_path = self.meta.storages["data"].location + self.data_path = self._storage_path self.upgrade = PostgreSQLUpgrade( self, @@ -496,7 +496,7 @@ def _on_peer_relation_departed(self, event: RelationDepartedEvent) -> None: # Update the sync-standby endpoint in the async replication data. self.async_replication.update_async_replication_data() - def _on_pgdata_storage_detaching(self, _) -> None: + def _on_data_storage_detaching(self, _) -> None: # Change the primary if it's the unit that is being removed. try: primary = self._patroni.get_primary(unit_name_pattern=True) @@ -504,7 +504,7 @@ def _on_pgdata_storage_detaching(self, _) -> None: # Ignore the event if the primary couldn't be retrieved. # If a switchover is needed, an automatic failover will be triggered # when the unit is removed. - logger.debug("Early exit on_pgdata_storage_detaching: primary cannot be retrieved") + logger.debug("Early exit on_data_storage_detaching: primary cannot be retrieved") return if self.unit.name != primary: @@ -917,11 +917,11 @@ def fix_leader_annotation(self) -> bool: raise e return True - def _create_pgdata(self, container: Container): + def _create_data(self, container: Container): """Create the PostgreSQL data directory.""" - if not container.exists(self.pgdata_path): + if not container.exists(self.data_path): container.make_dir( - self.pgdata_path, permissions=0o750, user=WORKLOAD_OS_USER, group=WORKLOAD_OS_GROUP + self.data_path, permissions=0o750, user=WORKLOAD_OS_USER, group=WORKLOAD_OS_GROUP ) # Also, fix the permissions from the parent directory. container.exec([ @@ -929,6 +929,16 @@ def _create_pgdata(self, container: Container): f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", self._storage_path, ]).wait() + container.exec([ + "chown", + f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", + "/var/lib/postgresql/logs", + ]).wait() + container.exec([ + "chown", + f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", + "/var/lib/postgresql/temp", + ]).wait() def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None: """Event handler for PostgreSQL container on PebbleReadyEvent.""" @@ -948,7 +958,7 @@ def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None: # Create the PostgreSQL data directory. This is needed on cloud environments # where the volume is mounted with more restrictive permissions. - self._create_pgdata(container) + self._create_data(container) self.unit.set_workload_version(self._patroni.rock_postgresql_version) @@ -1073,7 +1083,7 @@ def _initialize_cluster(self, event: WorkloadEvent) -> bool: extra_user_roles=["pg_monitor"], ) - self.postgresql.set_up_database() + self.postgresql.set_up_database(temp_location="/var/lib/postgresql/temp") access_groups = self.postgresql.list_access_groups() if access_groups != set(ACCESS_GROUPS): @@ -1427,7 +1437,7 @@ def _on_update_status_early_exit_checks(self, container) -> bool: logger.debug("on_update_status early exit: Cannot connect to container") return False - self._check_pgdata_storage_size() + self._check_data_storage_size() if ( self._has_blocked_status and self.unit.status not in S3_BLOCK_MESSAGES @@ -1441,16 +1451,16 @@ def _on_update_status_early_exit_checks(self, container) -> bool: return False return True - def _check_pgdata_storage_size(self) -> None: - """Asserts that pgdata volume has at least 10% free space and blocks charm if not.""" + def _check_data_storage_size(self) -> None: + """Asserts that data volume has at least 10% free space and blocks charm if not.""" try: - total_size, _, free_size = shutil.disk_usage(self.pgdata_path) + total_size, _, free_size = shutil.disk_usage(self.data_path) except FileNotFoundError: - logger.error("pgdata folder not found in %s", self.pgdata_path) + logger.error("data folder not found in %s", self.data_path) return logger.debug( - "pgdata free disk space: %s out of %s, ratio of %s", + "data free disk space: %s out of %s, ratio of %s", free_size, total_size, free_size / total_size, diff --git a/src/constants.py b/src/constants.py index 8fc9f92572..ff0e605466 100644 --- a/src/constants.py +++ b/src/constants.py @@ -17,7 +17,7 @@ WORKLOAD_OS_GROUP = "postgres" WORKLOAD_OS_USER = "postgres" METRICS_PORT = "9187" -POSTGRESQL_DATA_PATH = "/var/lib/postgresql/data/pgdata" +POSTGRESQL_DATA_PATH = "/var/lib/postgresql/data" POSTGRESQL_LOGS_PATH = "/var/log/postgresql" POSTGRESQL_LOGS_PATTERN = "postgresql*.log" POSTGRES_LOG_FILES = [ diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 9d34409de0..269fea5a39 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -186,7 +186,7 @@ def _configure_standby_cluster(self, event: RelationChangedEvent) -> bool: raise Exception(error) if system_identifier != relation.data[relation.app].get("system-id"): # Store current data in a tar.gz file. - logger.info("Creating backup of pgdata folder") + logger.info("Creating backup of data folder") filename = f"{POSTGRESQL_DATA_PATH}-{str(datetime.now()).replace(' ', '-').replace(':', '-')}.tar.gz" self.container.exec( f"tar -zcf {filename} {POSTGRESQL_DATA_PATH}".split() @@ -702,11 +702,11 @@ def _stop_database(self, event: RelationChangedEvent) -> bool: if not self._configure_standby_cluster(event): return False - # Remove and recreate the pgdata folder to enable replication of the data from the + # Remove and recreate the data folder to enable replication of the data from the # primary cluster. - logger.info("Removing and recreating pgdata folder") + logger.info("Removing and recreating data folder") self.container.exec(f"rm -r {POSTGRESQL_DATA_PATH}".split()).wait_output() - self.charm._create_pgdata(self.container) + self.charm._create_data(self.container) self.charm._peers.data[self.charm.unit].update({"stopped": "True"}) diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index ea8cc6d436..fbe49cf0e0 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -51,7 +51,7 @@ bootstrap: method: pgbackrest pgbackrest: command: > - pgbackrest --stanza={{ restore_stanza }} --pg1-path={{ storage_path }}/pgdata + pgbackrest --stanza={{ restore_stanza }} --pg1-path={{ storage_path }} {%- if backup_id %} --set={{ backup_id }} {%- endif %} {%- if restore_timeline %} --target-timeline="0x{{ restore_timeline }}" {% endif %} {%- if restore_to_latest %} --type=default {%- else %} @@ -71,6 +71,7 @@ bootstrap: - auth-local: trust - encoding: UTF8 - data-checksums + - waldir: /var/lib/postgresql/logs {%- endif %} pg_hba: - {{ 'hostssl' if enable_tls else 'host' }} all all 0.0.0.0/0 md5 @@ -106,6 +107,8 @@ ctl: {%- endif %} pod_ip: '{{ endpoint }}' postgresql: + basebackup: + - waldir: /var/lib/postgresql/logs connect_address: '{{ endpoint }}:5432' data_dir: {{ storage_path }}/pgdata bin_dir: /usr/lib/postgresql/{{ version }}/bin @@ -123,6 +126,7 @@ postgresql: ssl_cert_file: {{ storage_path }}/cert.pem ssl_key_file: {{ storage_path }}/key.pem {%- endif %} + temp_tablespaces: temp {%- if pg_parameters %} {%- for key, value in pg_parameters.items() %} {{key}}: {{value}} diff --git a/tests/helpers.py b/tests/helpers.py index 63a431d6d6..a4adedaf15 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -7,4 +7,4 @@ import yaml METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) -STORAGE_PATH = METADATA["storage"]["pgdata"]["location"] +STORAGE_PATH = METADATA["storage"]["data"]["location"] diff --git a/tests/integration/ha_tests/clean-data-dir.sh b/tests/integration/ha_tests/clean-data-dir.sh index c5c714405a..602ed57d1b 100755 --- a/tests/integration/ha_tests/clean-data-dir.sh +++ b/tests/integration/ha_tests/clean-data-dir.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash set -Eeuo pipefail -rm -rf /var/lib/postgresql/data/pgdata/* +rm -rf /var/lib/postgresql/data/* diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index a6ec0e49ba..cb8d230961 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -639,7 +639,7 @@ async def is_replica(ops_test: OpsTest, unit_name: str) -> bool: async def list_wal_files(ops_test: OpsTest, app: str) -> set: """Returns the list of WAL segment files in each unit.""" units = [unit.name for unit in ops_test.model.applications[app].units] - command = "ls -1 /var/lib/postgresql/data/pgdata/pg_wal/" + command = "ls -1 /var/lib/postgresql/logs/" files = {} for unit in units: complete_command = f"run --unit {unit} -- {command}" diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 8d19358d7a..24997a2bd4 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -39,7 +39,7 @@ METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) DATABASE_APP_NAME = METADATA["name"] APPLICATION_NAME = "postgresql-test-app" -STORAGE_PATH = METADATA["storage"]["pgdata"]["location"] +STORAGE_PATH = METADATA["storage"]["data"]["location"] try: check_call(["kubectl", "version", "--client=true"]) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 69a71e74ab..82552c6bce 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -143,7 +143,7 @@ async def test_settings_are_correct(ops_test: OpsTest, unit_id: int): assert settings["archive_mode"] == "on" assert settings["autovacuum"] == "on" assert settings["cluster_name"] == f"patroni-{APP_NAME}" - assert settings["data_directory"] == f"{STORAGE_PATH}/pgdata" + assert settings["data_directory"] == STORAGE_PATH assert settings["data_checksums"] == "on" assert settings["fsync"] == "on" assert settings["full_page_writes"] == "on" diff --git a/tests/integration/test_storage.py b/tests/integration/test_storage.py index 3c4ade4203..a63f05a632 100644 --- a/tests/integration/test_storage.py +++ b/tests/integration/test_storage.py @@ -18,7 +18,7 @@ logger = logging.getLogger(__name__) -INSUFFICIENT_SIZE_WARNING = "<10% free space on pgdata volume." +INSUFFICIENT_SIZE_WARNING = "<10% free space on data volume." @markers.amd64_only @@ -34,7 +34,7 @@ async def test_filling_and_emptying_pgdata_storage(ops_test: OpsTest, charm): await run_command_on_unit( ops_test, primary, - f"FREE_SPACE=$(df --output=avail {STORAGE_PATH}/pgdata | tail -1) && dd if=/dev/urandom of={STORAGE_PATH}/pgdata/tmp bs=1M count=$(( (FREE_SPACE * 91 / 100) / 1024 ))", + f"FREE_SPACE=$(df --output=avail {STORAGE_PATH} | tail -1) && dd if=/dev/urandom of={STORAGE_PATH}/pgdata/tmp bs=1M count=$(( (FREE_SPACE * 91 / 100) / 1024 ))", ) # wait for charm to get blocked diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index 49f11a0cca..da8bf14ecc 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -113,7 +113,7 @@ async def test_tls(ops_test: OpsTest) -> None: await run_command_on_unit( ops_test, replica, - 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/postgresql/data/pgdata promote"', + 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/postgresql/data promote"', ) # Check that the replica was promoted. diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index cc87a5817d..1086619156 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -208,7 +208,7 @@ def test_on_async_relation_changed(harness, wait_for_standby): patch( "charm.Patroni.member_started", new_callable=PropertyMock ) as _patroni_member_started, - patch("charm.PostgresqlOperatorCharm._create_pgdata") as _create_pgdata, + patch("charm.PostgresqlOperatorCharm._create_data") as _create_data, patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, patch("charm.PostgresqlOperatorCharm._set_active_status") as _set_active_status, patch( @@ -235,7 +235,7 @@ def test_on_async_relation_changed(harness, wait_for_standby): _stop.assert_called_once() if not wait_for_standby: _start.assert_called() - _create_pgdata.assert_called_once() + _create_data.assert_called_once() assert harness.charm.async_replication.get_primary_cluster().name == "standby" diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index b591f5ae67..11da8d2a47 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -411,7 +411,7 @@ def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): def test_empty_data_files(harness): with patch("ops.model.Container.exec") as _exec: # Test when the removal of the data files fails. - command = ["rm", "-r", "/var/lib/postgresql/data/pgdata"] + command = ["rm", "-r", "/var/lib/postgresql/data"] _exec.side_effect = ExecError(command=command, exit_code=1, stdout="", stderr="fake error") try: harness.charm.backup._empty_data_files() @@ -455,7 +455,7 @@ def test_change_connectivity_to_database(harness): def test_execute_command(harness): with patch("ops.model.Container.exec") as _exec: # Test when the command fails. - command = ["rm", "-r", "/var/lib/postgresql/data/pgdata"] + command = ["rm", "-r", "/var/lib/postgresql/data"] _exec.side_effect = ChangeError( err="fake error", change=Change( @@ -1427,7 +1427,7 @@ def test_on_restore_action(harness): with ( patch("ops.model.Container.start") as _start, patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, - patch("charm.PostgresqlOperatorCharm._create_pgdata") as _create_pgdata, + patch("charm.PostgresqlOperatorCharm._create_data") as _create_data, patch("charm.PostgreSQLBackups._empty_data_files") as _empty_data_files, patch("charm.PostgreSQLBackups._restart_database") as _restart_database, patch("lightkube.Client.delete") as _delete, @@ -1454,7 +1454,7 @@ def test_on_restore_action(harness): _delete.assert_not_called() _restart_database.assert_not_called() _empty_data_files.assert_not_called() - _create_pgdata.assert_not_called() + _create_data.assert_not_called() _update_config.assert_not_called() _start.assert_not_called() mock_event.fail.assert_not_called() @@ -1480,7 +1480,7 @@ def test_on_restore_action(harness): _delete.assert_not_called() _restart_database.assert_not_called() _empty_data_files.assert_not_called() - _create_pgdata.assert_not_called() + _create_data.assert_not_called() _update_config.assert_not_called() _start.assert_not_called() mock_event.set_results.assert_not_called() @@ -1497,7 +1497,7 @@ def test_on_restore_action(harness): _delete.assert_not_called() _restart_database.assert_not_called() _empty_data_files.assert_not_called() - _create_pgdata.assert_not_called() + _create_data.assert_not_called() _update_config.assert_not_called() _start.assert_not_called() mock_event.set_results.assert_not_called() @@ -1526,7 +1526,7 @@ def test_on_restore_action(harness): _delete.assert_not_called() _restart_database.assert_not_called() _empty_data_files.assert_not_called() - _create_pgdata.assert_not_called() + _create_data.assert_not_called() _update_config.assert_not_called() _start.assert_not_called() mock_event.set_results.assert_not_called() @@ -1541,7 +1541,7 @@ def test_on_restore_action(harness): mock_event.fail.assert_called_once() _restart_database.assert_called_once() _empty_data_files.assert_not_called() - _create_pgdata.assert_not_called() + _create_data.assert_not_called() _update_config.assert_not_called() _start.assert_not_called() mock_event.set_results.assert_not_called() @@ -1557,7 +1557,7 @@ def test_on_restore_action(harness): _empty_data_files.assert_called_once() mock_event.fail.assert_called_once() _restart_database.assert_called_once() - _create_pgdata.assert_not_called() + _create_data.assert_not_called() _update_config.assert_not_called() _start.assert_not_called() mock_event.set_results.assert_not_called() @@ -1574,7 +1574,7 @@ def test_on_restore_action(harness): "restoring-backup": "20230101-090000F", "restore-stanza": f"{harness.charm.model.name}.{harness.charm.cluster_name}", } - _create_pgdata.assert_called_once() + _create_data.assert_called_once() _update_config.assert_called_once() _start.assert_called_once_with("postgresql") mock_event.fail.assert_not_called() @@ -1583,7 +1583,7 @@ def test_on_restore_action(harness): # Test a successful PITR with the real backup id to the latest. mock_event.reset_mock() _restart_database.reset_mock() - _create_pgdata.reset_mock() + _create_data.reset_mock() _update_config.reset_mock() _start.reset_mock() mock_event.params = {"backup-id": "2023-01-01T09:00:00Z", "restore-to-time": "latest"} @@ -1595,7 +1595,7 @@ def test_on_restore_action(harness): "restore-to-time": "latest", "restore-stanza": f"{harness.charm.model.name}.{harness.charm.cluster_name}", } - _create_pgdata.assert_called_once() + _create_data.assert_called_once() _update_config.assert_called_once() _start.assert_called_once_with("postgresql") mock_event.fail.assert_not_called() @@ -1604,7 +1604,7 @@ def test_on_restore_action(harness): # Test a successful PITR with only the timestamp. mock_event.reset_mock() _restart_database.reset_mock() - _create_pgdata.reset_mock() + _create_data.reset_mock() _update_config.reset_mock() _start.reset_mock() mock_event.params = {"restore-to-time": "2025-02-24 05:00:00.001+00"} @@ -1615,7 +1615,7 @@ def test_on_restore_action(harness): "restore-to-time": "2025-02-24 05:00:00.001+00", "restore-stanza": f"{harness.charm.model.name}.{harness.charm.cluster_name}", } - _create_pgdata.assert_called_once() + _create_data.assert_called_once() _update_config.assert_called_once() _start.assert_called_once_with("postgresql") mock_event.fail.assert_not_called() @@ -1635,7 +1635,7 @@ def test_on_restore_action(harness): "restore-stanza": "", }, ) - _create_pgdata.reset_mock() + _create_data.reset_mock() _update_config.reset_mock() _start.reset_mock() mock_event.params = {"restore-to-time": "latest"} @@ -1643,7 +1643,7 @@ def test_on_restore_action(harness): _empty_data_files.assert_not_called() _restart_database.assert_not_called() assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {} - _create_pgdata.assert_not_called() + _create_data.assert_not_called() _update_config.assert_not_called() _start.assert_not_called() mock_event.set_results.assert_not_called() @@ -1664,7 +1664,7 @@ def test_on_restore_action(harness): "restore-to-time": "latest", "restore-stanza": f"{harness.charm.model.name}.{harness.charm.cluster_name}", } - _create_pgdata.assert_called_once() + _create_data.assert_called_once() _update_config.assert_called_once() _start.assert_called_once_with("postgresql") mock_event.fail.assert_not_called() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 6991e03a2c..4fda59b254 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -5,7 +5,7 @@ import logging from datetime import datetime from unittest import TestCase -from unittest.mock import MagicMock, Mock, PropertyMock, patch, sentinel +from unittest.mock import MagicMock, Mock, PropertyMock, call, patch, sentinel import psycopg2 import pytest @@ -208,7 +208,7 @@ def test_on_postgresql_pebble_ready(harness): patch("charm.PostgreSQLUpgrade.idle", new_callable=PropertyMock) as _idle, patch("charm.PostgresqlOperatorCharm._patch_pod_labels"), patch("charm.PostgresqlOperatorCharm._on_leader_elected"), - patch("charm.PostgresqlOperatorCharm._create_pgdata") as _create_pgdata, + patch("charm.PostgresqlOperatorCharm._create_data") as _create_data, ): _rock_postgresql_version.return_value = "16.6" @@ -227,7 +227,7 @@ def test_on_postgresql_pebble_ready(harness): # Check for a Waiting status when the primary k8s endpoint is not ready yet. harness.container_pebble_ready(POSTGRESQL_CONTAINER) - _create_pgdata.assert_called_once() + _create_data.assert_called_once() assert isinstance(harness.model.unit.status, WaitingStatus) _set_active_status.assert_not_called() @@ -263,7 +263,7 @@ def test_on_postgresql_pebble_ready_no_connection(harness): patch( "charm.Patroni.rock_postgresql_version", new_callable=PropertyMock ) as _rock_postgresql_version, - patch("charm.PostgresqlOperatorCharm._create_pgdata"), + patch("charm.PostgresqlOperatorCharm._create_data"), ): mock_event = MagicMock() mock_event.workload = harness.model.unit.get_container(POSTGRESQL_CONTAINER) @@ -711,7 +711,7 @@ def test_on_peer_relation_departed(harness): _remove_from_endpoints.assert_called_once_with(sentinel.units) -def test_on_pgdata_storage_detaching(harness): +def test_on_data_storage_detaching(harness): with ( patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started, patch("charm.Patroni.are_all_members_ready", new_callable=PropertyMock), @@ -721,11 +721,11 @@ def test_on_pgdata_storage_detaching(harness): ): # Early exit if not primary event = Mock() - harness.charm._on_pgdata_storage_detaching(event) + harness.charm._on_data_storage_detaching(event) assert not _member_started.called _get_primary.side_effect = [harness.charm.unit.name, "primary"] - harness.charm._on_pgdata_storage_detaching(event) + harness.charm._on_data_storage_detaching(event) _switchover.assert_called_once_with() _primary_changed.assert_called_once_with("primary") @@ -1771,28 +1771,34 @@ def test_set_active_status(harness): assert isinstance(harness.charm.unit.status, MaintenanceStatus) -def test_create_pgdata(harness): +def test_create_data_directories(harness): container = MagicMock() container.exists.return_value = False - harness.charm._create_pgdata(container) + harness.charm._create_data(container) container.make_dir.assert_called_once_with( - "/var/lib/postgresql/data/pgdata", permissions=488, user="postgres", group="postgres" + "/var/lib/postgresql/data", permissions=488, user="postgres", group="postgres" ) - container.exec.assert_called_once_with([ - "chown", - "postgres:postgres", - "/var/lib/postgresql/data", + container.exec.assert_has_calls([ + call(["chown", "postgres:postgres", "/var/lib/postgresql/data"]), + call().wait(), + call(["chown", "postgres:postgres", "/var/lib/postgresql/logs"]), + call().wait(), + call(["chown", "postgres:postgres", "/var/lib/postgresql/temp"]), + call().wait(), ]) container.make_dir.reset_mock() container.exec.reset_mock() container.exists.return_value = True - harness.charm._create_pgdata(container) + harness.charm._create_data(container) container.make_dir.assert_not_called() - container.exec.assert_called_once_with([ - "chown", - "postgres:postgres", - "/var/lib/postgresql/data", + container.exec.assert_has_calls([ + call(["chown", "postgres:postgres", "/var/lib/postgresql/data"]), + call().wait(), + call(["chown", "postgres:postgres", "/var/lib/postgresql/logs"]), + call().wait(), + call(["chown", "postgres:postgres", "/var/lib/postgresql/temp"]), + call().wait(), ])