Skip to content

Commit

Permalink
[DPE-4479][DPE-4625] Make backups behavior correspond to specification (
Browse files Browse the repository at this point in the history 
#542)

* add list-backups new format

* add create backup check + unit test

* fix integration test
  • Loading branch information
@lucasgameiroborges
lucasgameiroborges authored Jul 3, 2024
1 parent b29c81a commit 2827474
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 35 deletions.
71 changes: 66 additions & 5 deletions src/backups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,43 @@ def _execute_command(

def _format_backup_list(self, backup_list) -> str:
"""Formats provided list of backups as a table."""
backups = ["{:<21s} | {:<12s} | {:s}".format("backup-id", "backup-type", "backup-status")]
backups.append("-" * len(backups[0]))
for backup_id, backup_type, backup_status in backup_list:
s3_parameters, _ = self._retrieve_s3_parameters()
backups = [
"Storage bucket name: {:s}".format(s3_parameters["bucket"]),
"Backups base path: {:s}/backup/\n".format(s3_parameters["path"]),
"{:<20s} | {:<12s} | {:<8s} | {:<20s} | {:<23s} | {:<20s} | {:<20s} | {:s}".format(
"backup-id",
"type",
"status",
"reference-backup-id",
"LSN start/stop",
"start-time",
"finish-time",
"backup-path",
),
]
backups.append("-" * len(backups[2]))
for (
backup_id,
backup_type,
backup_status,
reference,
lsn_start_stop,
start,
stop,
path,
) in backup_list:
backups.append(
"{:<21s} | {:<12s} | {:s}".format(backup_id, backup_type, backup_status)
"{:<20s} | {:<12s} | {:<8s} | {:<20s} | {:<23s} | {:<20s} | {:<20s} | {:s}".format(
backup_id,
backup_type,
backup_status,
reference,
lsn_start_stop,
start,
stop,
path,
)
)
return "\n".join(backups)

Expand All @@ -284,11 +316,29 @@ def _generate_backup_list_output(self) -> str:
backups = json.loads(output)[0]["backup"]
for backup in backups:
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])
lsn_start_stop = f'{backup["lsn"]["start"]} / {backup["lsn"]["stop"]}'
time_start, time_stop = (
datetime.strftime(datetime.fromtimestamp(stamp), "%Y-%m-%dT%H:%M:%SZ")
for stamp in backup["timestamp"].values()
)
backup_path = f'/{self.stanza_name}/{backup["label"]}'
error = backup["error"]
backup_status = "finished"
if error:
backup_status = f"failed: {error}"
backup_list.append((backup_id, backup_type, backup_status))
backup_list.append((
backup_id,
backup_type,
backup_status,
backup_reference,
lsn_start_stop,
time_start,
time_stop,
backup_path,
))
return self._format_backup_list(backup_list)

def _list_backups(self, show_failed: bool, parse=True) -> OrderedDict[str, str]:
Expand Down Expand Up @@ -514,6 +564,17 @@ def _on_create_backup_action(self, event) -> None: # noqa: C901
event.fail(error_message)
return

if (
backup_type in ["differential", "incremental"]
and len(self._list_backups(show_failed=False)) == 0
):
error_message = (
f"Invalid backup type: {backup_type}. No previous full backup to reference."
)
logger.error(f"Backup failed: {error_message}")
event.fail(error_message)
return

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:
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/test_backups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ async def test_backup_and_restore(ops_test: OpsTest, cloud_configs: Tuple[Dict,
action = await ops_test.model.units.get(replica).run_action("list-backups")
await action.wait()
backups = action.results.get("backups")
# 2 lines for header output, 1 backup line ==> 3 total lines
assert len(backups.split("\n")) == 3, "full backup is not outputted"
# 5 lines for header output, 1 backup line ==> 6 total lines
assert len(backups.split("\n")) == 6, "full backup is not outputted"
await ops_test.model.wait_for_idle(status="active", timeout=1000)

# Write some data.
Expand All @@ -197,8 +197,8 @@ async def test_backup_and_restore(ops_test: OpsTest, cloud_configs: Tuple[Dict,
action = await ops_test.model.units.get(replica).run_action("list-backups")
await action.wait()
backups = action.results.get("backups")
# 2 lines for header output, 2 backup lines ==> 4 total lines
assert len(backups.split("\n")) == 4, "differential backup is not outputted"
# 5 lines for header output, 2 backup lines ==> 7 total lines
assert len(backups.split("\n")) == 7, "differential backup is not outputted"
await ops_test.model.wait_for_idle(status="active", timeout=1000)

# Write some data.
Expand Down
110 changes: 84 additions & 26 deletions tests/unit/test_backups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,48 +542,97 @@ def test_execute_command(harness):


def test_format_backup_list(harness):
# Test when there are no backups.
assert (
harness.charm.backup._format_backup_list([])
== """backup-id | backup-type | backup-status
----------------------------------------------------"""
)
with patch(
"charms.data_platform_libs.v0.s3.S3Requirer.get_s3_connection_info"
) as _get_s3_connection_info:
# Test when there are no backups.
_get_s3_connection_info.return_value = {
"bucket": " /test-bucket/ ",
"access-key": " test-access-key ",
"secret-key": " test-secret-key ",
"path": " test-path/ ",
}
assert (
harness.charm.backup._format_backup_list([])
== """Storage bucket name: test-bucket
Backups base path: /test-path/backup/
# Test when there are backups.
backup_list = [
("2023-01-01T09:00:00Z", "full", "failed: fake error"),
("2023-01-01T10:00:00Z", "full", "finished"),
]
assert (
harness.charm.backup._format_backup_list(backup_list)
== """backup-id | backup-type | backup-status
----------------------------------------------------
2023-01-01T09:00:00Z | full | failed: fake error
2023-01-01T10:00:00Z | full | finished"""
)
backup-id | type | status | reference-backup-id | LSN start/stop | start-time | finish-time | backup-path
-----------------------------------------------------------------------------------------------------------------------------------------------------------"""
)

# Test when there are backups.
backup_list = [
(
"2023-01-01T09:00:00Z",
"full",
"failed: fake error",
"None",
"0/3000000 / 0/5000000",
"2023-01-01T09:00:00Z",
"2023-01-01T09:00:05Z",
"a/b/c",
),
(
"2023-01-01T10:00:00Z",
"full",
"finished",
"None",
"0/5000000 / 0/7000000",
"2023-01-01T10:00:00Z",
"2023-01-01T010:00:07Z",
"a/b/d",
),
]
assert (
harness.charm.backup._format_backup_list(backup_list)
== """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
-----------------------------------------------------------------------------------------------------------------------------------------------------------
2023-01-01T09:00:00Z | full | failed: fake error | None | 0/3000000 / 0/5000000 | 2023-01-01T09:00:00Z | 2023-01-01T09:00:05Z | a/b/c
2023-01-01T10:00:00Z | full | finished | None | 0/5000000 / 0/7000000 | 2023-01-01T10:00:00Z | 2023-01-01T010:00:07Z | a/b/d"""
)


def test_generate_backup_list_output(harness):
with patch("charm.PostgreSQLBackups._execute_command") as _execute_command:
with (
patch(
"charms.data_platform_libs.v0.s3.S3Requirer.get_s3_connection_info"
) as _get_s3_connection_info,
patch("charm.PostgreSQLBackups._execute_command") as _execute_command,
):
_get_s3_connection_info.return_value = {
"bucket": " /test-bucket/ ",
"access-key": " test-access-key ",
"secret-key": " test-secret-key ",
"path": " test-path/ ",
}
# Test when no backups are returned.
_execute_command.return_value = ('[{"backup":[]}]', None)
assert (
harness.charm.backup._generate_backup_list_output()
== """backup-id | backup-type | backup-status
----------------------------------------------------"""
== """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
-----------------------------------------------------------------------------------------------------------------------------------------------------------"""
)

# Test when backups are returned.
_execute_command.return_value = (
'[{"backup":[{"label":"20230101-090000F","error":"fake error"},{"label":"20230101-100000F","error":null}]}]',
'[{"backup":[{"label":"20230101-090000F","error":"fake error","reference":null,"lsn":{"start":"0/3000000","stop":"0/5000000"},"timestamp":{"start":1719866711,"stop":1719866714}}]}]',
None,
)
assert (
harness.charm.backup._generate_backup_list_output()
== """backup-id | backup-type | backup-status
----------------------------------------------------
2023-01-01T09:00:00Z | full | failed: fake error
2023-01-01T10:00:00Z | full | finished"""
== """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
-----------------------------------------------------------------------------------------------------------------------------------------------------------
2023-01-01T09:00:00Z | full | failed: fake error | None | 0/3000000 / 0/5000000 | 2024-07-01T20:45:11Z | 2024-07-01T20:45:14Z | /None.patroni-postgresql-k8s/20230101-090000F"""
)


Expand Down Expand Up @@ -1140,6 +1189,15 @@ def test_on_create_backup_action(harness):
mock_event.fail.assert_called_once()
mock_event.set_results.assert_not_called()

# Test when the backup is of type diff/incr when there's no previous full backup.
mock_event.reset_mock()
mock_event.params = {"type": "differential"}
_upload_content_to_s3.return_value = True
_is_primary.return_value = True
harness.charm.backup._on_create_backup_action(mock_event)
mock_event.fail.assert_called_once()
mock_event.set_results.assert_not_called()

# Test when the backup fails.
mock_event.reset_mock()
mock_event.params = {"type": "full"}
Expand Down

0 comments on commit 2827474

Please sign in to comment.