diff --git a/src/backups.py b/src/backups.py index ed78995353..c6fb6f2243 100644 --- a/src/backups.py +++ b/src/backups.py @@ -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) @@ -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]: @@ -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: diff --git a/tests/integration/test_backups.py b/tests/integration/test_backups.py index 8c61e14384..9c4141021c 100644 --- a/tests/integration/test_backups.py +++ b/tests/integration/test_backups.py @@ -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. @@ -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. diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index d41360bf15..5be4fbf9de 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -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""" ) @@ -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"}