Skip to content

Commit

Permalink
[DPE-4464] Add Incremental backup support + proper type naming (#487)
Browse files Browse the repository at this point in the history
* add incremental backups + name fix

* fix tests

* update naming convention to reflect spec
  • Loading branch information
lucasgameiroborges authored May 29, 2024
1 parent ed4620a commit 1e7b7a3
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 12 deletions.
3 changes: 2 additions & 1 deletion actions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ create-backup:
description: The backup type, the default value is 'full'.
Full backup is a full copy of all data.
Differential backup is a copy only of changed data since the last full backup.
Possible values - full, differential.
Incremental backup is a copy only of changed data since the last backup (any type).
Possible values - full, differential, incremental.
get-primary:
description: Get the unit with is the primary/leader in the replication.
get-password:
Expand Down
23 changes: 13 additions & 10 deletions src/backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from ops.pebble import ChangeError, ExecError
from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed

from constants import BACKUP_USER, WORKLOAD_OS_GROUP, WORKLOAD_OS_USER
from constants import BACKUP_TYPE_OVERRIDES, BACKUP_USER, WORKLOAD_OS_GROUP, WORKLOAD_OS_USER
from relations.async_replication import ASYNC_PRIMARY_RELATION, ASYNC_REPLICA_RELATION

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -474,16 +474,14 @@ def _on_s3_credential_changed(self, event: CredentialsChangedEvent):

def _on_create_backup_action(self, event) -> None: # noqa: C901
"""Request that pgBackRest creates a backup."""
backup_type = event.params.get("type", "full").lower()[:4]
if backup_type not in ["full", "diff"]:
error_message = (
f"Invalid backup type: {backup_type}. Possible values: full, differential."
)
backup_type = event.params.get("type", "full")
if backup_type not in BACKUP_TYPE_OVERRIDES:
error_message = f"Invalid backup type: {backup_type}. Possible values: {', '.join(BACKUP_TYPE_OVERRIDES.keys())}."
logger.error(f"Backup failed: {error_message}")
event.fail(error_message)
return

logger.info(f"A {backup_type} backup has been requested on unit")
logger.info(f"A {backup_type} backup has been requested on unit")
can_unit_perform_backup, validation_message = self._can_unit_perform_backup()
if not can_unit_perform_backup:
logger.error(f"Backup failed: {validation_message}")
Expand Down Expand Up @@ -530,7 +528,7 @@ def _on_create_backup_action(self, event) -> None: # noqa: C901
"pgbackrest",
f"--stanza={self.stanza_name}",
"--log-level-console=debug",
f"--type={backup_type}",
f"--type={BACKUP_TYPE_OVERRIDES[backup_type]}",
"backup",
]
if self.charm.is_primary:
Expand Down Expand Up @@ -705,9 +703,9 @@ def _on_restore_action(self, event):

def _generate_fake_backup_id(self, backup_type: str) -> str:
"""Creates a backup id for failed backup operations (to store log file)."""
if backup_type == "F":
if backup_type == "full":
return datetime.strftime(datetime.now(), "%Y%m%d-%H%M%SF")
if backup_type == "D":
if backup_type == "differential":
backups = self._list_backups(show_failed=False, parse=False).keys()
last_full_backup = None
for label in backups[::-1]:
Expand All @@ -718,6 +716,11 @@ def _generate_fake_backup_id(self, backup_type: str) -> str:
if last_full_backup is None:
raise TypeError("Differential backup requested but no previous full backup")
return f'{last_full_backup}_{datetime.strftime(datetime.now(), "%Y%m%d-%H%M%SD")}'
if backup_type == "incremental":
backups = self._list_backups(show_failed=False, parse=False).keys()
if not backups:
raise TypeError("Incremental backup requested but no previous successful backup")
return f'{backups[-1]}_{datetime.strftime(datetime.now(), "%Y%m%d-%H%M%SI")}'

def _fetch_backup_from_id(self, backup_id: str) -> str:
"""Fetches backup's pgbackrest label from backup id."""
Expand Down
1 change: 1 addition & 0 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@
UNIT_SCOPE = "unit"

SECRET_KEY_OVERRIDES = {"ca": "cauth"}
BACKUP_TYPE_OVERRIDES = {"full": "full", "differential": "diff", "incremental": "incr"}
2 changes: 1 addition & 1 deletion tests/integration/test_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ async def test_backup_and_restore(ops_test: OpsTest, cloud_configs: Tuple[Dict,
# Run the "create backup" action.
logger.info("creating a backup")
action = await ops_test.model.units.get(replica).run_action(
"create-backup", **{"type": "diff"}
"create-backup", **{"type": "differential"}
)
await action.wait()
backup_status = action.results.get("backup-status")
Expand Down

0 comments on commit 1e7b7a3

Please sign in to comment.