diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 7e6a9d7631..d2438d5b91 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -35,7 +35,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 51 +LIBPATCH = 52 # Groups to distinguish HBA access ACCESS_GROUP_IDENTITY = "identity_access" @@ -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 c71efb15f6..299a8afcea 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -62,9 +62,22 @@ requires: optional: true storage: - pgdata: + archive: type: filesystem - location: /var/snap/charmed-postgresql/common + description: Storage mount used for holding local backups (before typically sending them to remote object storage) when relevant/needed. + location: /var/snap/charmed-postgresql/common/data/archive + data: + type: filesystem + description: Storage mount used for storing all tables, indexes, and so on (except those from temporary tablespaces). + location: /var/snap/charmed-postgresql/common/var/lib/postgresql + logs: + type: filesystem + description: Storage mount used for storing all the logs that are part of the transactional commit path (WAL files). + location: /var/snap/charmed-postgresql/common/data/logs + temp: + type: filesystem + description: Storage mount used for storing temporary tablespaces (where typically sort operations happen). + location: /var/snap/charmed-postgresql/common/data/temp assumes: - juju diff --git a/src/backups.py b/src/backups.py index 4c414d75e8..dba0b61459 100644 --- a/src/backups.py +++ b/src/backups.py @@ -97,7 +97,7 @@ def _tls_ca_chain_filename(self) -> str: """Returns the path to the TLS CA chain file.""" s3_parameters, _ = self._retrieve_s3_parameters() if s3_parameters.get("tls-ca-chain") is not None: - return f"{self.charm._storage_path}/pgbackrest-tls-ca-chain.crt" + return f"{PGBACKREST_CONF_PATH}/pgbackrest-tls-ca-chain.crt" return "" def _get_s3_session_resource(self, s3_parameters: dict): @@ -327,12 +327,25 @@ def _create_bucket_if_not_exists(self) -> None: def _empty_data_files(self) -> bool: """Empty the PostgreSQL data directory in preparation of backup restore.""" + paths = [ + "/var/snap/charmed-postgresql/common/data/archive", + POSTGRESQL_DATA_PATH, + "/var/snap/charmed-postgresql/common/data/logs", + "/var/snap/charmed-postgresql/common/data/temp", + ] + path = None try: - path = Path(POSTGRESQL_DATA_PATH) - if path.exists() and path.is_dir(): - shutil.rmtree(path) + for path in paths: + path_object = Path(path) + if path_object.exists() and path_object.is_dir(): + for item in os.listdir(path): + item_path = os.path.join(path, item) + if os.path.isfile(item_path) or os.path.islink(item_path): + os.remove(item_path) + elif os.path.isdir(item_path): + shutil.rmtree(item_path) except OSError as e: - logger.warning(f"Failed to remove contents of the data directory with error: {e!s}") + logger.warning(f"Failed to remove contents from {path} with error: {e!s}") return False return True diff --git a/src/charm.py b/src/charm.py index 3cd071e251..6ed06d7732 100755 --- a/src/charm.py +++ b/src/charm.py @@ -86,6 +86,7 @@ PATRONI_PASSWORD_KEY, PEER, PLUGIN_OVERRIDES, + POSTGRESQL_DATA_PATH, POSTGRESQL_SNAP_NAME, RAFT_PASSWORD_KEY, REPLICATION_PASSWORD_KEY, @@ -197,9 +198,8 @@ def __init__(self, *args): self.framework.observe(self.on.update_status, self._on_update_status) self.cluster_name = self.app.name self._member_name = self.unit.name.replace("/", "-") - self._certs_path = "/usr/local/share/ca-certificates" - self._storage_path = self.meta.storages["pgdata"].location + self._storage_path = self.meta.storages["data"].location self.upgrade = PostgreSQLUpgrade( self, @@ -1469,7 +1469,9 @@ def _start_primary(self, event: StartEvent) -> None: event.defer() return - self.postgresql.set_up_database() + self.postgresql.set_up_database( + temp_location="/var/snap/charmed-postgresql/common/data/temp" + ) access_groups = self.postgresql.list_access_groups() if access_groups != set(ACCESS_GROUPS): @@ -1722,6 +1724,11 @@ def _handle_processes_failures(self) -> bool: # Restart the PostgreSQL process if it was frozen (in that case, the Patroni # process is running by the PostgreSQL process not). if self._unit_ip in self.members_ips and self._patroni.member_inactive: + data_directory_contents = os.listdir(POSTGRESQL_DATA_PATH) + if len(data_directory_contents) == 1 and data_directory_contents[0] == "pg_wal": + os.remove(os.path.join(POSTGRESQL_DATA_PATH, "pg_wal")) + logger.info("PostgreSQL data directory was not empty. Removed pg_wal") + return True try: self._patroni.restart_patroni() logger.info("restarted PostgreSQL because it was not running") diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 2939f06ca5..3d452e68da 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" # Input is hardcoded subprocess.check_call(f"tar -zcf {filename} {POSTGRESQL_DATA_PATH}".split()) # noqa: S603 @@ -649,19 +649,26 @@ def _re_emit_async_relation_changed_event(self) -> None: ) def _reinitialise_pgdata(self) -> None: - """Reinitialise the pgdata folder.""" + """Reinitialise the data folder.""" + paths = [ + "/var/snap/charmed-postgresql/common/data/archive", + POSTGRESQL_DATA_PATH, + "/var/snap/charmed-postgresql/common/data/logs", + "/var/snap/charmed-postgresql/common/data/temp", + ] + path = None try: - path = Path(POSTGRESQL_DATA_PATH) - if path.exists() and path.is_dir(): - shutil.rmtree(path) + for path in paths: + path_object = Path(path) + if path_object.exists() and path_object.is_dir(): + for item in os.listdir(path): + item_path = os.path.join(path, item) + if os.path.isfile(item_path) or os.path.islink(item_path): + os.remove(item_path) + elif os.path.isdir(item_path): + shutil.rmtree(item_path) except OSError as e: - raise Exception( - f"Failed to remove contents of the data directory with error: {e!s}" - ) from e - os.mkdir(POSTGRESQL_DATA_PATH) - # Expected permissions - os.chmod(POSTGRESQL_DATA_PATH, 0o750) # noqa: S103 - self.charm._patroni._change_owner(POSTGRESQL_DATA_PATH) + raise Exception(f"Failed to remove contents from {path} with error: {e!s}") from e @property def _relation(self) -> Relation: @@ -712,9 +719,9 @@ 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._reinitialise_pgdata() # Remove previous cluster information to make it possible to initialise a new cluster. diff --git a/src/upgrade.py b/src/upgrade.py index a8ba18a8d1..38f579c995 100644 --- a/src/upgrade.py +++ b/src/upgrade.py @@ -247,7 +247,9 @@ def _prepare_upgrade_from_legacy(self) -> None: self.charm.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY), extra_user_roles="pg_monitor", ) - self.charm.postgresql.set_up_database() + self.charm.postgresql.set_up_database( + temp_location="/var/snap/charmed-postgresql/common/data/temp" + ) self._set_up_new_access_roles_for_legacy() def _set_up_new_access_roles_for_legacy(self) -> None: diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 11dd0dbd48..63632084aa 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -126,9 +126,12 @@ bootstrap: initdb: - encoding: UTF8 - data-checksums + - waldir: /var/snap/charmed-postgresql/common/data/logs {%- endif %} postgresql: + basebackup: + - waldir: /var/snap/charmed-postgresql/common/data/logs listen: '{{ self_ip }}:5432' connect_address: '{{ self_ip }}:5432' # Path to PostgreSQL binaries used in the database bootstrap process. @@ -147,6 +150,7 @@ postgresql: ssl_cert_file: {{ conf_path }}/cert.pem ssl_key_file: {{ conf_path }}/key.pem {%- endif %} + temp_tablespaces: temp unix_socket_directories: /tmp {%- if pg_parameters %} {%- for key, value in pg_parameters.items() %} diff --git a/tests/helpers.py b/tests/helpers.py index 029c42a446..cbc7d6be41 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/helpers.py b/tests/integration/ha_tests/helpers.py index 0c56054775..eb42e37eb8 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -914,12 +914,13 @@ def storage_type(ops_test, app): return line.split()[3] -def storage_id(ops_test, unit_name): - """Retrieves storage id associated with provided unit. +def get_storage_ids(ops_test, unit_name): + """Retrieves storages ids associated with provided unit. Note: this function exists as a temporary solution until this issue is ported to libjuju 2: https://github.com/juju/python-libjuju/issues/694 """ + storage_ids = [] model_name = ops_test.model.info.name proc = subprocess.check_output(f"juju storage --model={model_name}".split()) proc = proc.decode("utf-8") @@ -934,18 +935,22 @@ def storage_id(ops_test, unit_name): continue if line.split()[0] == unit_name: - return line.split()[1] + storage_ids.append(line.split()[1]) + return storage_ids -async def add_unit_with_storage(ops_test, app, storage): - """Adds unit with storage. +async def add_unit_with_storage(ops_test, app, storages): + """Adds unit with storages. Note: this function exists as a temporary solution until this issue is resolved: https://github.com/juju/python-libjuju/issues/695 """ original_units = {unit.name for unit in ops_test.model.applications[app].units} model_name = ops_test.model.info.name - add_unit_cmd = f"add-unit {app} --model={model_name} --attach-storage={storage}".split() + add_unit_cmd = f"add-unit {app} --model={model_name}" + for storage in storages: + add_unit_cmd = add_unit_cmd + f" --attach-storage={storage}" + add_unit_cmd = add_unit_cmd.split() return_code, _, _ = await ops_test.juju(*add_unit_cmd) assert return_code == 0, "Failed to add unit with storage" async with ops_test.fast_forward(): @@ -958,7 +963,9 @@ async def add_unit_with_storage(ops_test, app, storage): # verify storage attached new_unit = (current_units - original_units).pop() - assert storage_id(ops_test, new_unit) == storage, "unit added with incorrect storage" + assert sorted(get_storage_ids(ops_test, new_unit)) == sorted(storages), ( + "unit added with incorrect storage" + ) # return a reference to newly added unit for unit in ops_test.model.applications[app].units: @@ -1050,8 +1057,8 @@ async def check_db(ops_test: OpsTest, app: str, db: str) -> bool: return db in query -async def get_any_deatached_storage(ops_test: OpsTest) -> str: - """Returns any of the current available deatached storage.""" +async def get_detached_storages(ops_test: OpsTest) -> list[str]: + """Returns the current available detached storage.""" return_code, storages_list, stderr = await ops_test.juju( "storage", "-m", f"{ops_test.controller_name}:{ops_test.model.info.name}", "--format=json" ) @@ -1059,9 +1066,13 @@ async def get_any_deatached_storage(ops_test: OpsTest) -> str: raise Exception(f"failed to get storages info with error: {stderr}") parsed_storages_list = json.loads(storages_list) + detached_storages = [] for storage_name, storage in parsed_storages_list["storage"].items(): if (str(storage["status"]["current"]) == "detached") and (str(storage["life"] == "alive")): - return storage_name + detached_storages.append(storage_name) + + if len(detached_storages) > 0: + return detached_storages raise Exception("failed to get deatached storage") diff --git a/tests/integration/ha_tests/test_restore_cluster.py b/tests/integration/ha_tests/test_restore_cluster.py index 48c3ef2b44..81bcfecf37 100644 --- a/tests/integration/ha_tests/test_restore_cluster.py +++ b/tests/integration/ha_tests/test_restore_cluster.py @@ -17,8 +17,8 @@ ) from .helpers import ( add_unit_with_storage, + get_storage_ids, reused_full_cluster_recovery_storage, - storage_id, ) FIRST_APPLICATION = "first-cluster" @@ -40,7 +40,12 @@ async def test_build_and_deploy(ops_test: OpsTest, charm) -> None: application_name=FIRST_APPLICATION, num_units=3, base=CHARM_BASE, - storage={"pgdata": {"pool": "lxd-btrfs", "size": 2048}}, + storage={ + "archive": {"pool": "lxd-btrfs", "size": 8046}, + "data": {"pool": "lxd-btrfs", "size": 8046}, + "logs": {"pool": "lxd-btrfs", "size": 8046}, + "temp": {"pool": "lxd-btrfs", "size": 8046}, + }, config={"profile": "testing"}, ) @@ -91,7 +96,7 @@ async def test_cluster_restore(ops_test): logger.info("Downscaling the existing cluster") storages = [] for unit in ops_test.model.applications[FIRST_APPLICATION].units: - storages.append(storage_id(ops_test, unit.name)) + storages.append(get_storage_ids(ops_test, unit.name)) await ops_test.model.destroy_unit(unit.name) await ops_test.model.remove_application(FIRST_APPLICATION, block_until_done=True) diff --git a/tests/integration/ha_tests/test_self_healing.py b/tests/integration/ha_tests/test_self_healing.py index 8524452865..13ca586376 100644 --- a/tests/integration/ha_tests/test_self_healing.py +++ b/tests/integration/ha_tests/test_self_healing.py @@ -35,6 +35,7 @@ get_controller_machine, get_patroni_setting, get_primary, + get_storage_ids, get_unit_ip, is_cluster_updated, is_connection_possible, @@ -48,7 +49,6 @@ reused_replica_storage, send_signal_to_process, start_continuous_writes, - storage_id, storage_type, update_restart_condition, wait_network_restore, @@ -76,7 +76,12 @@ async def test_build_and_deploy(ops_test: OpsTest, charm) -> None: charm, num_units=3, base=CHARM_BASE, - storage={"pgdata": {"pool": "lxd-btrfs", "size": 2048}}, + storage={ + "archive": {"pool": "lxd-btrfs", "size": 2048}, + "data": {"pool": "lxd-btrfs", "size": 2048}, + "logs": {"pool": "lxd-btrfs", "size": 2048}, + "temp": {"pool": "lxd-btrfs", "size": 2048}, + }, config={"profile": "testing"}, ) # Deploy the continuous writes application charm if it wasn't already deployed. @@ -121,7 +126,7 @@ async def test_storage_re_use(ops_test, continuous_writes): for unit in ops_test.model.applications[app].units: if await is_replica(ops_test, unit.name): break - unit_storage_id = storage_id(ops_test, unit.name) + unit_storage_id = get_storage_ids(ops_test, unit.name) expected_units = len(ops_test.model.applications[app].units) - 1 await ops_test.model.destroy_unit(unit.name) await ops_test.model.wait_for_idle( diff --git a/tests/integration/ha_tests/test_smoke.py b/tests/integration/ha_tests/test_smoke.py index b160d38f2b..7f9c78c754 100644 --- a/tests/integration/ha_tests/test_smoke.py +++ b/tests/integration/ha_tests/test_smoke.py @@ -19,10 +19,10 @@ check_db, check_password_auth, create_db, - get_any_deatached_storage, + get_detached_storages, + get_storage_ids, is_postgresql_ready, is_storage_exists, - storage_id, ) TEST_DATABASE_NAME = "test_database" @@ -42,7 +42,12 @@ async def test_app_force_removal(ops_test: OpsTest, charm: str): application_name=APPLICATION_NAME, num_units=1, base=CHARM_BASE, - storage={"pgdata": {"pool": "lxd-btrfs", "size": 8046}}, + storage={ + "archive": {"pool": "lxd-btrfs", "size": 8046}, + "data": {"pool": "lxd-btrfs", "size": 8046}, + "logs": {"pool": "lxd-btrfs", "size": 8046}, + "temp": {"pool": "lxd-btrfs", "size": 8046}, + }, config={"profile": "testing"}, ) @@ -58,13 +63,16 @@ async def test_app_force_removal(ops_test: OpsTest, charm: str): assert await is_postgresql_ready(ops_test, primary_name) logger.info("getting storage id") - storage_id_str = storage_id(ops_test, primary_name) + storage_ids = get_storage_ids(ops_test, primary_name) # Check if storage exists after application deployed logger.info("werifing is storage exists") - for attempt in Retrying(stop=stop_after_delay(15 * 3), wait=wait_fixed(3), reraise=True): - with attempt: - assert await is_storage_exists(ops_test, storage_id_str) + for storage_id in storage_ids: + for attempt in Retrying( + stop=stop_after_delay(15 * 3), wait=wait_fixed(3), reraise=True + ): + with attempt: + assert await is_storage_exists(ops_test, storage_id) # Create test database to check there is no resources conflicts logger.info("creating db") @@ -82,9 +90,12 @@ async def test_app_force_removal(ops_test: OpsTest, charm: str): # Storage should remain logger.info("werifing is storage exists") - for attempt in Retrying(stop=stop_after_delay(15 * 3), wait=wait_fixed(3), reraise=True): - with attempt: - assert await is_storage_exists(ops_test, storage_id_str) + for storage_id in storage_ids: + for attempt in Retrying( + stop=stop_after_delay(15 * 3), wait=wait_fixed(3), reraise=True + ): + with attempt: + assert await is_storage_exists(ops_test, storage_id) @pytest.mark.abort_on_fail @@ -92,13 +103,13 @@ async def test_charm_garbage_ignorance(ops_test: OpsTest, charm: str): """Test charm deploy in dirty environment with garbage storage.""" async with ops_test.fast_forward(): logger.info("checking garbage storage") - garbage_storage = None + garbage_storages = None for attempt in Retrying(stop=stop_after_delay(30 * 3), wait=wait_fixed(3), reraise=True): with attempt: - garbage_storage = await get_any_deatached_storage(ops_test) + garbage_storages = await get_detached_storages(ops_test) logger.info("add unit with attached storage") - await add_unit_with_storage(ops_test, APPLICATION_NAME, garbage_storage) + await add_unit_with_storage(ops_test, APPLICATION_NAME, garbage_storages) primary_name = ops_test.model.applications[APPLICATION_NAME].units[0].name @@ -108,15 +119,19 @@ async def test_charm_garbage_ignorance(ops_test: OpsTest, charm: str): assert await is_postgresql_ready(ops_test, primary_name) logger.info("getting storage id") - storage_id_str = storage_id(ops_test, primary_name) + storage_ids = get_storage_ids(ops_test, primary_name) - assert storage_id_str == garbage_storage + for storage_id in storage_ids: + assert storage_id in garbage_storages # Check if storage exists after application deployed logger.info("werifing is storage exists") - for attempt in Retrying(stop=stop_after_delay(15 * 3), wait=wait_fixed(3), reraise=True): - with attempt: - assert await is_storage_exists(ops_test, storage_id_str) + for storage_id in storage_ids: + for attempt in Retrying( + stop=stop_after_delay(15 * 3), wait=wait_fixed(3), reraise=True + ): + with attempt: + assert await is_storage_exists(ops_test, storage_id) # Check that test database exists for new unit logger.info("checking db") @@ -134,7 +149,7 @@ async def test_app_resources_conflicts_v3(ops_test: OpsTest, charm: str): garbage_storage = None for attempt in Retrying(stop=stop_after_delay(30 * 3), wait=wait_fixed(3), reraise=True): with attempt: - garbage_storage = await get_any_deatached_storage(ops_test) + garbage_storage = await get_detached_storages(ops_test) logger.info("deploying duplicate application with attached storage") await ops_test.model.deploy( @@ -142,7 +157,7 @@ async def test_app_resources_conflicts_v3(ops_test: OpsTest, charm: str): application_name=DUP_APPLICATION_NAME, num_units=1, base=CHARM_BASE, - attach_storage=[tag.storage(garbage_storage)], + attach_storage=[tag.storage(storage) for storage in garbage_storage], config={"profile": "testing"}, ) diff --git a/tests/integration/ha_tests/test_upgrade.py b/tests/integration/ha_tests/test_upgrade.py index e639b8ff60..df1b30a7e4 100644 --- a/tests/integration/ha_tests/test_upgrade.py +++ b/tests/integration/ha_tests/test_upgrade.py @@ -32,11 +32,17 @@ @pytest.mark.abort_on_fail async def test_deploy_latest(ops_test: OpsTest) -> None: """Simple test to ensure that the PostgreSQL and application charms get deployed.""" - await ops_test.model.deploy( + await ops_test.juju( + "deploy", DATABASE_APP_NAME, - num_units=3, - channel="16/edge", - config={"profile": "testing"}, + "-n", + 3, + "--channel", + "16/edge/multiple-storages", + "--config", + "profile=testing", + "--base", + "ubuntu@24.04", ) await ops_test.model.deploy( APPLICATION_NAME, diff --git a/tests/integration/ha_tests/test_upgrade_from_stable.py b/tests/integration/ha_tests/test_upgrade_from_stable.py index 679c9a639f..32cab6c03c 100644 --- a/tests/integration/ha_tests/test_upgrade_from_stable.py +++ b/tests/integration/ha_tests/test_upgrade_from_stable.py @@ -26,11 +26,16 @@ @pytest.mark.abort_on_fail async def test_deploy_stable(ops_test: OpsTest) -> None: """Simple test to ensure that the PostgreSQL and application charms get deployed.""" - await ops_test.model.deploy( + await ops_test.juju( + "deploy", DATABASE_APP_NAME, - num_units=3, + "-n", + 3, # TODO move to stable once we release - channel="16/beta", + "--channel", + "16/beta/multiple-storages", + "--base", + "ubuntu@24.04", ) await ops_test.model.deploy( APPLICATION_NAME, diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index e3fead648b..81997334f1 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -35,7 +35,7 @@ CHARM_BASE = "ubuntu@22.04" METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) DATABASE_APP_NAME = METADATA["name"] -STORAGE_PATH = METADATA["storage"]["pgdata"]["location"] +STORAGE_PATH = METADATA["storage"]["data"]["location"] APPLICATION_NAME = "postgresql-test-app" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 9bcaeade96..48c22b6a74 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -133,7 +133,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"] == DATABASE_APP_NAME - assert settings["data_directory"] == f"{STORAGE_PATH}/var/lib/postgresql" + 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 new file mode 100644 index 0000000000..5033a3e7a1 --- /dev/null +++ b/tests/integration/test_storage.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +import logging + +import pytest +from pytest_operator.plugin import OpsTest + +from .helpers import CHARM_BASE, DATABASE_APP_NAME + +logger = logging.getLogger(__name__) + + +@pytest.mark.abort_on_fail +async def test_storage(ops_test: OpsTest, charm): + """Build and deploy the charm and check its storage list.""" + async with ops_test.fast_forward(): + await ops_test.model.deploy( + charm, + num_units=1, + base=CHARM_BASE, + config={"profile": "testing"}, + ) + await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") + + logger.info("Checking charm storages") + expected_storages = ["archive", "data", "logs", "temp"] + storages = await ops_test.model.list_storage() + assert len(storages) == 4, f"Expected 4 storages, got: {len(storages)}" + for index, storage in enumerate(storages): + assert ( + storage["attachments"]["unit-postgresql-0"].__dict__["storage_tag"] + == f"storage-{expected_storages[index]}-{index}" + ), f"Storage {expected_storages[index]} not found" diff --git a/tests/spread/test_storage.py/task.yaml b/tests/spread/test_storage.py/task.yaml new file mode 100644 index 0000000000..3d71c6f199 --- /dev/null +++ b/tests/spread/test_storage.py/task.yaml @@ -0,0 +1,7 @@ +summary: test_storage.py +environment: + TEST_MODULE: test_storage.py +execute: | + tox run -e integration -- "tests/integration/$TEST_MODULE" --model testing --alluredir="$SPREAD_TASK/allure-results" +artifacts: + - allure-results diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index 982302941d..3d85040b42 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -1,6 +1,5 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -from pathlib import PosixPath from subprocess import CompletedProcess, TimeoutExpired from unittest.mock import ANY, MagicMock, PropertyMock, call, mock_open, patch @@ -64,7 +63,7 @@ def test_tls_ca_chain_filename(harness): ) assert ( harness.charm.backup._tls_ca_chain_filename - == "/var/snap/charmed-postgresql/common/pgbackrest-tls-ca-chain.crt" + == "/var/snap/charmed-postgresql/current/etc/pgbackrest/pgbackrest-tls-ca-chain.crt" ) @@ -332,7 +331,7 @@ def test_construct_endpoint(harness): @pytest.mark.parametrize( "tls_ca_chain_filename", - ["", "/var/snap/charmed-postgresql/common/pgbackrest-tls-ca-chain.crt"], + ["", "/var/snap/charmed-postgresql/current/etc/pgbackrest/pgbackrest-tls-ca-chain.crt"], ) def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): with ( @@ -453,6 +452,10 @@ def test_create_bucket_if_not_exists(harness, tls_ca_chain_filename): def test_empty_data_files(harness): with ( patch("shutil.rmtree") as _rmtree, + patch("os.path.isdir", return_value=True) as _isdir, + patch("os.path.islink", return_value=False) as _islink, + patch("os.path.isfile", return_value=False) as _isfile, + patch("os.listdir", return_value=["test_file.txt"]) as _listdir, patch("pathlib.Path.is_dir") as _is_dir, patch("pathlib.Path.exists") as _exists, ): @@ -462,18 +465,24 @@ def test_empty_data_files(harness): _rmtree.assert_not_called() # Test when the removal of the data files fails. - path = PosixPath("/var/snap/charmed-postgresql/common/var/lib/postgresql") _exists.return_value = True _is_dir.return_value = True _rmtree.side_effect = OSError assert not harness.charm.backup._empty_data_files() - _rmtree.assert_called_once_with(path) + _rmtree.assert_called_once_with( + "/var/snap/charmed-postgresql/common/data/archive/test_file.txt" + ) # Test when data files are successfully removed. _rmtree.reset_mock() _rmtree.side_effect = None assert harness.charm.backup._empty_data_files() - _rmtree.assert_called_once_with(path) + _rmtree.assert_has_calls([ + call("/var/snap/charmed-postgresql/common/data/archive/test_file.txt"), + call("/var/snap/charmed-postgresql/common/var/lib/postgresql/test_file.txt"), + call("/var/snap/charmed-postgresql/common/data/logs/test_file.txt"), + call("/var/snap/charmed-postgresql/common/data/temp/test_file.txt"), + ]) def test_change_connectivity_to_database(harness): @@ -507,7 +516,7 @@ def test_execute_command(harness): patch("pwd.getpwnam") as _getpwnam, ): # Test when the command fails. - command = ["rm", "-r", "/var/lib/postgresql/data/pgdata"] + command = ["rm", "-r", "/var/snap/charmed-postgresql/common/data/db"] _run.return_value = CompletedProcess(command, 1, b"", b"fake stderr") assert harness.charm.backup._execute_command(command) == (1, "", "fake stderr") _run.assert_called_once_with( @@ -1691,7 +1700,7 @@ def test_pre_restore_checks(harness): @pytest.mark.parametrize( "tls_ca_chain_filename", - ["", "/var/snap/charmed-postgresql/common/pgbackrest-tls-ca-chain.crt"], + ["", "/var/snap/charmed-postgresql/current/etc/pgbackrest/pgbackrest-tls-ca-chain.crt"], ) def test_render_pgbackrest_conf_file(harness, tls_ca_chain_filename): with ( @@ -1953,7 +1962,7 @@ def test_start_stop_pgbackrest_service(harness): @pytest.mark.parametrize( "tls_ca_chain_filename", - ["", "/var/snap/charmed-postgresql/common/pgbackrest-tls-ca-chain.crt"], + ["", "/var/snap/charmed-postgresql/current/etc/pgbackrest/pgbackrest-tls-ca-chain.crt"], ) def test_upload_content_to_s3(harness, tls_ca_chain_filename): with (