garethgeorge / backrest

Backrest is a web UI and orchestrator for restic backup.
GNU General Public License v3.0
787 stars 31 forks source link

Restore failure + recovery leaves plan in a failed state #340

Open Thinkscape opened 2 weeks ago

Thinkscape commented 2 weeks ago

Describe the bug

If restore fails once, but succeeds after, the plan is marked as failed.

To Reproduce Steps to reproduce the behavior:

  1. Create a plan.
  2. Back up once (create snapshot).
  3. Restore a file using Browse and restore files ... into a non-existent directory or ro fs, causing failure.
  4. Restore fails. Plan is marked as failed and runs failure hooks.
  5. Restore the same file to a valid directory.

Expected behavior

Restore succeeds. The plan is now marked as successful.

Actual

Restore succeeds. Plan continues to be in "failed" state.

Screenshots

image

Platform Info

garethgeorge commented 1 week ago

Hey, this is works as intended though I can see it's somewhat unexpected here.

The status on the left hand bar reports whether any error was encountered in the last set of grouped operations in the history.

Thinkscape commented 13 hours ago

The status on the left hand bar reports whether any error was encountered in the last set of grouped operations in the history.

But the last set of grouped operations is green, like seen on the screenshots...

Thinkscape commented 13 hours ago

In other words:

  1. 🔴 🔴 🟢 Individual operations - some failed, latest one succeeds (right column).
  2. 🟢 I expect that to be reduced to the last operation set to be successful (middle column).
  3. 🟢 Then I expect that to be reduced to global nav to be successful (left nav bar).

What happens now:

  1. 🔴 🔴 🟢 Individual operations - some failed, latest one succeeds (right column)
  2. 🔴 It gets reduced to unsuccessful (middle column)
  3. 🔴 It gets reduced to unsuccessful in global nav (left nav bar)
Thinkscape commented 13 hours ago

I guess that's because of how the right-column-actions are handled and interpreted 🤷 (the little links and buttons on the right) as compared to the repo actions (top toolbar).

Alternative approach would be to not have those in the right column, and instead have a proper [ RESTORE ] button in top toolbar, which shows a dropdown of available snapshots and that triggers creation of new separate "action group".

Would make it a bit more logical to boot, navigation wise...

garethgeorge commented 11 hours ago

In other words:

  1. 🔴 🔴 🟢 Individual operations - some failed, latest one succeeds (right column).
  2. 🟢 I expect that to be reduced to the last operation set to be successful (middle column).
  3. 🟢 Then I expect that to be reduced to global nav to be successful (left nav bar).

What happens now:

  1. 🔴 🔴 🟢 Individual operations - some failed, latest one succeeds (right column)
  2. 🔴 It gets reduced to unsuccessful (middle column)
  3. 🔴 It gets reduced to unsuccessful in global nav (left nav bar)

This behavior could hide issues from the user e.g. a backup fails but the forget operation that runs after it succeeds hiding that the operation actually failed. I think for an operation set to be considered successful, every operation in the set must succeed. Similarly, for the plan / repo status the last group of operations must be entirely successful.

I guess that's because of how the right-column-actions are handled and interpreted 🤷 (the little links and buttons on the right) as compared to the repo actions (top toolbar).

Alternative approach would be to not have those in the right column, and instead have a proper [ RESTORE ] button in top toolbar, which shows a dropdown of available snapshots and that triggers creation of new separate "action group".

Would make it a bit more logical to boot, navigation wise...

I can see the argument for splitting out restore operations rather than including them in the same flow as the snapshot that's being restored from. I'll probably go ahead with this as I think it's unintuitive that it can be hard to go back and find what restores you've done without clicking through recent backups.