[DPE-10625] select valid backup entry in restore integration test PG14#1840
Draft
taurus-forever wants to merge 1 commit into
Draft
[DPE-10625] select valid backup entry in restore integration test PG14#1840taurus-forever wants to merge 1 commit into
taurus-forever wants to merge 1 commit into
Conversation
test_restore_on_new_cluster selected a backup ID from the list-backups
output by positional index (backups.split("\n")[-3]), assuming the last
two entries are always restore/timeline records. When the output
contains a different number of such entries, the selected line is
itself a timeline entry, which fails with "Cannot restore to the
timeline without restore-to-time parameter".
Parse the pipe-delimited output instead and select only lines whose
action column contains "backup" (full/differential/incremental),
and include action.results in the restore assertion message.
Backport of canonical/postgresql-k8s-operator#1552.
Assisted-by: Claude:claude-5-fable
2 tasks
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
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.
Backport of canonical/postgresql-k8s-operator#1552.
Issue
test_restore_on_new_clusterintest_backups_gcp.pywas failing withAssertionError: restore hasn't succeededbecause the charm loggedCannot restore to the timeline without restore-to-time parameter. The test used a hardcoded positional index (backups.split("\n")[-3]) to select a backup ID, assuming the last two entries are always restore/timeline records. When the output contains a different number of such entries, the selected line is itself a timeline entry — invalid for a plain restore.Solution
Replace the positional index with a semantic parse of the pipe-delimited
list-backupsoutput. The action column (second|-delimited field) is"full backup","differential backup", or"incremental backup"for real backups and"restore"for timeline entries. Filter to lines where that column contains"backup":Also improves the restore assertion message to include
action.resultsfor easier debugging.Other backup helpers reviewed and unchanged:
backup_helpers.pylines 177/222: positional indices are safe — called before any restores exist, guarded by a strict line-count assertion._get_most_recent_backup: intentionally returns the most recent entry including timeline records; used by PITR tests to verify new timelines were created.Checklist
Original prompt
A GitHub Actions job is failing in repository
canonical/postgresql-k8s-operatorfor the integration testtests/integration/test_backups_gcp.py::test_restore_on_new_cluster.Failing job URL: https://github.com/canonical/postgresql-k8s-operator/actions/runs/26546494621/job/78200057802
Workflow file:
.github/workflows/ci.yamlRelevant test file:
tests/integration/test_backups_gcp.pyFailing commit/ref:
0a0c5cd996bd36c237475a582b2d4d938f38b51aObserved failure from logs:
restoreaction.Restore failed: Cannot restore to the timeline without restore-to-time parameterAssertionError: restore hasn't succeededand atenacity.RetryError.The likely cause is fragile parsing of the
list-backupsaction output intest_restore_on_new_cluster. The current test assumes the third line from the end is always a valid backup:This positional assumption appears incorrect when the backup listing contains additional restore/timeline-related entries or formatting differences, causing the test to select an invalid backup ID that requires
restore-to-time.Please create a pull request that fixes the test by making backup selection robust.
Requirements:
tests/integration/test_backups_gcp.py.backups.split("\n")[-3]logic with parsing that:restore-to-time,action.resultsin the assertion failure).list-backupsoutput and could trigger timeline restores withoutrestore-to-time.Please open a PR with a concise title reflecting the fix.
The following is the prior conversation context from the user's chat exploration (may be truncated):
User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.
Assistant: The failure is in
test_restore_on_new_clusterand the actionable fix is in the test logic, not the workflow.Root cause
The restore action is being called with a backup entry that requires
restore-to-time, but the test assumes the third line from the end is always a valid full backup:tests/integration/test_backups_gcp.pyThe logs show the restore repeatedly failing with:
Restore failed: Cannot restore to the timeline without restore-to-time parameterAssertionError: restore hasn't succeededThat matches this fragile selection logic:
If the output format changes, contains extra blank lines, or has more restore-related entries than expected,
[-3]can still select an invalid entry.Solution
Parse the
list-backupsoutput and choose a real backup entry explicitly, excluding restore/timeline entries instead of relying on positional indexing.Suggested code change
Replace lines 136–138 with filtered parsing like this:
Better hardening
If the backup list format is structured enough, prefer selecting only entries that match a known backup type, for example full/differential/incremental backups, instead of using substring exclusions. For example: