Skip to content

Conversation

lucasgameiroborges
Copy link

Issue

Two backup components where divergent from their related specification here: list-backups action should output more info than just id, type and status, and create-backup action should raise an error if a type diff/incr is requested but there's no previous (full) backup to reference.

Solution

  • Fetch the additional info desired from pgbackrest info command + some context about the bucket and path where the backups are stored. New list-backups action output:
backups: |-
  Storage bucket name: test-bucket
  Backups base path: /test-path/backup/

  backup-id            | type         | status   | reference-backup-id  | LSN start/stop          | start-time           | finish-time          | backup-path
  -----------------------------------------------------------------------------------------------------------------------------------------------------------
  2024-07-01T22:17:07Z | full         | finished | None                 | 0/21000028 / 0/21000100 | 2024-07-01T22:17:07Z | 2024-07-01T22:17:10Z | /dev.patroni-postgresql-k8s/20240701-221707F
  2024-07-01T22:18:53Z | differential | finished | 2024-07-01T22:17:07Z | 0/23000028 / 0/23000100 | 2024-07-01T22:18:53Z | 2024-07-01T22:18:55Z | /dev.patroni-postgresql-k8s/20240701-221707F_20240701-221853D
  2024-07-01T22:23:33Z | incremental  | finished | 2024-07-01T22:18:53Z | 0/25000028 / 0/25000100 | 2024-07-01T22:23:33Z | 2024-07-01T22:23:35Z | /dev.patroni-postgresql-k8s/20240701-221707F_20240701-222333I
  • Check if there are any previous backups created before creating a new diff/incr type backup. If not, return fail like below:
Action id 134 failed: Invalid backup type: incremental. No previous full backup to reference.
  • Increment unit tests.

Copy link

codecov bot commented Jul 1, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.25%. Comparing base (ea373d7) to head (8ae0815).
Report is 1 commits behind head on main.

Files Patch % Lines
src/backups.py 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #542      +/-   ##
==========================================
+ Coverage   70.23%   70.25%   +0.02%     
==========================================
  Files          10       10              
  Lines        2785     2797      +12     
  Branches      518      521       +3     
==========================================
+ Hits         1956     1965       +9     
- Misses        731      732       +1     
- Partials       98      100       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

backup_id, backup_type = self._parse_backup_id(backup["label"])
backup_reference = "None"
if backup["reference"]:
backup_reference, _ = self._parse_backup_id(backup["reference"][-1])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because pgbackrest returns a list of references, we pick only the most recent one to represent.
For example, if an incremental backup A depends on the previous differential backup B, which depends on a previous full backup C, the reference list for A will be [C, B] --> last one, most recent B will be shown

Copy link
Member

@marceloneppel marceloneppel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants