[DPE-6052] Allow --restore-to-time=latest without a backup-id (#787)

* Allow --restore-to-time=latest without a backup-id Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Fix timestamp parse Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Add error message for missing base backup Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Fix empty list argument Signed-off-by: Marcelo Henrique Neppel <[email protected]> --------- Signed-off-by: Marcelo Henrique Neppel <[email protected]>
canonical · Nov 26, 2024 · 1a33052 · 1a33052
1 parent fdf66d4
commit 1a33052
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 9 deletions.
diff --git a/src/backups.py b/src/backups.py
@@ -479,6 +479,8 @@ def _get_nearest_timeline(self, timestamp: str) -> tuple[str, str] | None:
             (stanza, timeline) of the nearest timeline or backup. None, if there are no matches.
         """
         timelines = self._list_backups(show_failed=False) | self._list_timelines()
+        if timestamp == "latest":
+            return max(timelines.items())[1] if len(timelines) > 0 else None
         filtered_timelines = [
             (timeline_key, timeline_object)
             for timeline_key, timeline_object in timelines.items()
@@ -967,6 +969,17 @@ def _on_restore_action(self, event):  # noqa: C901
         elif is_backup_id_timeline:
             restore_stanza_timeline = timelines[backup_id]
         else:
+            backups_list = list(self._list_backups(show_failed=False).values())
+            timelines_list = self._list_timelines()
+            if (
+                restore_to_time == "latest"
+                and timelines_list is not None
+                and max(timelines_list.values() or [backups_list[0]]) not in backups_list
+            ):
+                error_message = "There is no base backup created from the latest timeline"
+                logger.error(f"Restore failed: {error_message}")
+                event.fail(error_message)
+                return
             restore_stanza_timeline = self._get_nearest_timeline(restore_to_time)
             if not restore_stanza_timeline:
                 error_message = f"Can't find the nearest timeline before timestamp {restore_to_time} to restore"
@@ -1100,11 +1113,8 @@ def _pre_restore_checks(self, event: ActionEvent) -> bool:
             event.fail(validation_message)
             return False
 
-        if not event.params.get("backup-id") and event.params.get("restore-to-time") in (
-            None,
-            "latest",
-        ):
-            error_message = "Missing backup-id or non-latest restore-to-time parameter to be able to do restore"
+        if not event.params.get("backup-id") and event.params.get("restore-to-time") is None:
+            error_message = "Either backup-id or restore-to-time parameters need to be provided to be able to do restore"
             logger.error(f"Restore failed: {error_message}")
             event.fail(error_message)
             return False

diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py
@@ -640,6 +640,10 @@ def test_get_nearest_timeline(harness):
         _list_timelines.return_value = dict[str, tuple[str, str]]({
             "2023-02-24T05:00:00Z": ("test-stanza", "2")
         })
+        assert harness.charm.backup._get_nearest_timeline("latest") == tuple[str, str]((
+            "test-stanza",
+            "2",
+        ))
         assert harness.charm.backup._get_nearest_timeline("2025-01-01 00:00:00") == tuple[
             str, str
         ](("test-stanza", "2"))
@@ -1614,6 +1618,55 @@ def test_on_restore_action(harness):
         mock_event.fail.assert_not_called()
         mock_event.set_results.assert_called_once_with({"restore-status": "restore started"})
 
+        # Test a failed PITR with only the restore-to-time parameter equal to latest
+        # (it should fail when there is no base backup created from the latest timeline).
+        mock_event.reset_mock()
+        _empty_data_files.reset_mock()
+        with harness.hooks_disabled():
+            harness.update_relation_data(
+                peer_rel_id,
+                harness.charm.app.name,
+                {
+                    "restore-timeline": "",
+                    "restore-to-time": "",
+                    "restore-stanza": "",
+                },
+            )
+        _create_pgdata.reset_mock()
+        _update_config.reset_mock()
+        _start.reset_mock()
+        mock_event.params = {"restore-to-time": "latest"}
+        harness.charm.backup._on_restore_action(mock_event)
+        _empty_data_files.assert_not_called()
+        _restart_database.assert_not_called()
+        assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {}
+        _create_pgdata.assert_not_called()
+        _update_config.assert_not_called()
+        _start.assert_not_called()
+        mock_event.set_results.assert_not_called()
+        mock_event.fail.assert_called_once()
+
+        # Test a successful PITR with only the restore-to-time parameter equal to latest.
+        mock_event.reset_mock()
+        mock_event.params = {"restore-to-time": "latest"}
+        _list_backups.return_value = {
+            "2023-01-01T09:00:00Z": (harness.charm.backup.stanza_name, "1"),
+            "2024-02-24T05:00:00Z": (harness.charm.backup.stanza_name, "2"),
+        }
+        harness.charm.backup._on_restore_action(mock_event)
+        _empty_data_files.assert_called_once()
+        _restart_database.assert_not_called()
+        assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {
+            "restore-timeline": "2",
+            "restore-to-time": "latest",
+            "restore-stanza": f"{harness.charm.model.name}.{harness.charm.cluster_name}",
+        }
+        _create_pgdata.assert_called_once()
+        _update_config.assert_called_once()
+        _start.assert_called_once_with("postgresql")
+        mock_event.fail.assert_not_called()
+        mock_event.set_results.assert_called_once_with({"restore-status": "restore started"})
+
 
 def test_pre_restore_checks(harness):
     with (
@@ -1680,13 +1733,13 @@ def test_pre_restore_checks(harness):
         assert harness.charm.backup._pre_restore_checks(mock_event) is True
         mock_event.fail.assert_not_called()
 
-        # Test with single (bad) restore-to-time=latest parameter
+        # Test with single restore-to-time=latest parameter
         mock_event.reset_mock()
         mock_event.params = {"restore-to-time": "latest"}
-        assert harness.charm.backup._pre_restore_checks(mock_event) is False
-        mock_event.fail.assert_called_once()
+        assert harness.charm.backup._pre_restore_checks(mock_event) is True
+        mock_event.fail.assert_not_called()
 
-        # Test with good restore-to-time=latest parameter
+        # Test with both backup-id and restore-to-time=latest parameters
         mock_event.reset_mock()
         mock_event.params = {"backup-id": "2023-01-01T09:00:00Z", "restore-to-time": "latest"}
         assert harness.charm.backup._pre_restore_checks(mock_event) is True