test: add unit tests for pgdata management, async replication, and patroni slots#1256
Draft
marceloneppel wants to merge 36 commits into
Draft
test: add unit tests for pgdata management, async replication, and patroni slots#1256marceloneppel wants to merge 36 commits into
marceloneppel wants to merge 36 commits into
Conversation
Migrate storage mount paths from /var/lib/postgresql/* to /var/lib/pg/* and configure PostgreSQL 16 data directories to use the 16/main subfolder structure. Path changes: - Archive: /var/lib/postgresql/archive → /var/lib/pg/archive - Data: /var/lib/postgresql/data/pgdata → /var/lib/pg/data/16/main - Logs: /var/lib/postgresql/logs → /var/lib/pg/logs/16/main (WAL) - Temp: /var/lib/postgresql/temp → /var/lib/pg/temp/16/main Key changes: - Update metadata.yaml storage mount locations - Update POSTGRESQL_DATA_PATH constant - Configure pgdata_path, waldir, and temp tablespace paths - Add make_parents=True to Pebble make_dir for nested directory creation - Update pgbackrest.conf.j2 with new data path - Update patroni.yml.j2 with new data_dir and waldir - Update async_replication.py cleanup paths - Update authorisation_rules_observer.py Patroni config path Note: Archive path (/var/lib/pg/archive) intentionally does not use the 16/main subfolder as it is not used in Patroni or PostgreSQL configuration. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…ix_pod Patch the OCI image health-check script (/scripts/self-signed-checker.py) to use the charm's storage path for the peer CA file instead of the hardcoded /var/lib/postgresql/data/peer_ca.pem. Make the operation idempotent and non-blocking, and call it from pebble-ready and early pod-fix flows to avoid race conditions with Pebble layer installation. Add _fix_health_check_script(container) to read, replace, and write the script. Log and handle pull/push errors; on push failure attempt an in-container sed fallback (exec ["sh","-c","sed -i 's|OLD|NEW|g' ..."]). Invoke the fixer from _on_postgresql_pebble_ready (after _create_pgdata) and from _fix_pod (early) so the script is corrected as early as possible. Update unit tests to cover the patching behaviour and the exec fallback. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
- Replace broad try/except/pass patterns with contextlib.suppress where appropriate (fixes SIM105 and S110 warnings). - Ensure the _fix_health_check_script fallback exec wait is wrapped with contextlib.suppress(Exception) to avoid swallowing exceptions without intent. - Use contextlib.suppress to defensively guard _fix_pod health-check fixes while still logging inner failures. - Re-order and tidy imports to satisfy ruff import ordering rules. - Apply ruff format across src/, tests/, and scripts/ to normalize formatting. Notes: - These are non-functional changes (linting / formatting); behavior unchanged. - Formatting touched multiple test and helper files; no logic was altered. - Tests were not executed as part of this change; please run the unit/integration suites locally (tox -e unit / tox -e integration) before pushing if desired. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
- Apply ruff/auto-formatting to integration test files to satisfy lint/format checks: - tests/integration/ha_tests/conftest.py - tests/integration/ha_tests/helpers.py - tests/integration/test_invalid_database.py - tests/integration/test_invalid_extra_user_roles.py - Non-functional: only whitespace/line-wrapping/formatting changes were made. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Temporarily disable the upgrade-related integration tests by constraining their spread task configurations so they will not be scheduled in CI. Changes: - Added a `systems:` block to the following spread task files to prevent these tasks from running across the broader CI fleet: - tests/spread/test_upgrade.py/task.yaml - tests/spread/test_upgrade_skip_pre_upgrade_check.py/task.yaml - tests/spread/test_upgrade_from_stable.py/task.yaml - tests/spread/test_async_replication_upgrade.py/task.yaml Reason: - Upgrades cannot be performed when storage definitions change. These tests fail in situations where storage definitions are modified, so they are being disabled until the upgrade path for storage changes is resolved. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
- Add PGDATA_PATH = f"{STORAGE_PATH}/16/main"
- Update tests/integration/test_storage.py and tests/integration/test_charm.py to use PGDATA_PATH
- Fix import ordering and lint issues
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
* Update canonical/data-platform-workflows action to v41 (#1211) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * feat(storage): add symlink from default PostgreSQL data path Create a symlink from /var/lib/postgresql/16/main to /var/lib/pg/data/16/main to provide compatibility with the standard PostgreSQL data directory location. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> --------- Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…ql-k8s-operator into change-storage-mounts Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Update POSTGRESQL_DATA_PATH and pgdata_path to use the symlink path (/var/lib/postgresql/16/main) instead of the actual storage path (/var/lib/pg/data/16/main) for Patroni, pgbackrest, and tests. This ensures all components reference the canonical symlink path that PostgreSQL and Patroni expect. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Non-leader units in a standby cluster now clear pgdata only after the standby leader is confirmed running, preventing system ID mismatch errors during pg_basebackup. Also fixes disk usage check to use actual pgdata path and handles Patroni API unavailability gracefully during slot sync. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…erations Update pgbackrest config template to use the proper POSTGRESQL_DATA_PATH constant and fix _empty_data_files to remove the actual directory contents rather than just the symlink, ensuring backup and restore operations work correctly. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
- Update data directory paths to match current structure (/var/lib/pg/data/16/main, etc.) - Replace rm -rf with find -delete to preserve mount point directories Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
… compatibility Clear archive, data, logs, and temp directories during backup restore preparation. This ensures pg_basebackup can use --waldir and --tablespace options when new replicas join after a restore, as these options require empty directories. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Remove the logic that cleared logs and temp directories when a non-leader unit rejoined with an empty data directory. This cleanup was intended for pg_basebackup during restore operations but is no longer needed. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Clear WAL and tablespace directories when a replica joins an initialized cluster, as PersistentVolumes may retain data from previous pods and pg_basebackup requires empty target directories. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…n standby When a new unit joins an existing standby cluster, check if the database is already running before clearing pgdata. Previously, _clear_pgdata() was called unconditionally before the member_started check, which could delete the data directory while PostgreSQL was running - causing it to crash and leaving the unit stuck in "awaiting for member to start". Now the order is: 1. If member is already started, return early (unit is working) 2. If member is not started, clear pgdata if needed and continue setup Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The `_fix_health_check_script` method was a workaround for an incorrect hardcoded certificate path in the OCI image. This is no longer needed as the upstream image has been fixed. Update the OCI image reference accordingly. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…s are cleared on retry The OCI image ships /var/lib/postgresql/16/main as a real directory, which prevents the symlink from being created on replicas. Always recreate the symlink (removing the real directory first if needed) instead of skipping when the path already exists. Also change stale storage directory cleanup to repeat until pgdata is fully populated (PG_VERSION exists), rather than running only once per unit lifecycle. This ensures retries after a failed pg_basebackup find clean directories. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Use exact pg_wal and pgsql_tmp subdirectories for --waldir and temp tablespace paths instead of their parent directories. Also defer replica startup until its endpoint is present in the members list, preventing pg_basebackup rejections that trigger remove_data_directory() calls racing with _create_pgdata(). Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Extract storage mount paths (/var/lib/pg/{archive,data,logs,temp}) into
named constants in constants.py and use them consistently across
backups, async replication, Patroni config rendering, and charm logic.
Also fix shell injection risk by passing command arguments as lists
instead of split strings, guard rotate_logs service start behind a
service existence check, and log failed replication slot syncs instead
of silently ignoring the error.
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…troni slots Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
github-actions
Bot
added
the
Libraries: OK
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (69.54%) is below the target coverage (70.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## change-storage-mounts #1256 +/- ##
=========================================================
+ Coverage 68.56% 69.54% +0.98%
=========================================================
Files 16 16
Lines 3871 3871
Branches 589 589
=========================================================
+ Hits 2654 2692 +38
+ Misses 1010 973 -37
+ Partials 207 206 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Solution
Checklist