Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POST_COMMAND_FAILURE triggering if backup completes #133

Closed
sumnerboy12 opened this issue May 12, 2022 · 8 comments · Fixed by #139
Closed

POST_COMMAND_FAILURE triggering if backup completes #133

sumnerboy12 opened this issue May 12, 2022 · 8 comments · Fixed by #139
Labels

Comments

@sumnerboy12
Copy link

I only have a notification setup for POST_COMMAND_FAILURE, not the new _INCOMPLETE hook.

However I am seeing the following in my logs (and getting a notification);

Starting Backup at 2022-05-12 04:00:00
using parent snapshot 7df93e77
error: lstat /data/openhab/userdata/jsondb/backup/1652285002376--org.eclipse.smarthome.core.thing.Thing.json: no such file or directory
error: lstat /data/openhab/userdata/jsondb/backup/1652285006119--org.eclipse.smarthome.core.thing.Thing.json: no such file or directory
Files:         331 new,   243 changed, 45348 unmodified
Dirs:          184 new,  2651 changed, 18748 unmodified
Added to the repo: 1.323 GiB
processed 45922 files, 14.358 GiB in 5:38
snapshot d1fb0fda saved
Warning: at least one source file could not be read
curl -X POST --data "{\"title\": \"[restic-rest] Backup failed\", \"body\": \"Backup of docker swarm data volumes to local backup server (backup.home) failed\"}" http://notify:5000

It appears as tho the _FAILURE hook is being called for non-critical failures, i.e. missing file. The backup completes and the snapshot is created so I am not expecting to see any failure notification in this instance.

@djmaze
Copy link
Owner

djmaze commented May 22, 2022

I experienced this myself, but I find it very hard to create a reproducible test case. The code looks correct and the test works but does not simulate exactly the no such file or directory error.

It almost seems to me that there is a restic bug in here.

@djmaze djmaze added the bug label May 22, 2022
@djmaze
Copy link
Owner

djmaze commented May 22, 2022

Related upstream issue: restic/restic#2165

@sumnerboy12
Copy link
Author

Thanks for following up - will watch that restic issue for updates.

@djmaze
Copy link
Owner

djmaze commented May 22, 2022

Well, in the examples in that issue the error code 3 is correctly returned. They are asking for an additional option to completely ignore "not found" errors.

I can't find an exact matching issue upstream, so maybe this is a rather a problem with our container or the script still.

@sumnerboy12
Copy link
Author

I think this is the problem;

if [ $rc -ne 0 ]; then
  if [ $rc -eq 3 ] && [ -n "${POST_COMMANDS_INCOMPLETE:-}" ]; then
    run_commands "${POST_COMMANDS_INCOMPLETE:-}"
  else
    run_commands "${POST_COMMANDS_FAILURE:-}"
  fi
fi

If a file is missing restic returns 3. But I don't have POST_COMMANDS_INCOMPLETE set to anything. Therefore this check will fall thru to the _FAILURE hook for every non-zero rc.

Would this work?

if [ $rc -ne 0 ]; then
  if [ $rc -eq 3 ]; then
    run_commands "${POST_COMMANDS_INCOMPLETE:-}"
  else
    run_commands "${POST_COMMANDS_FAILURE:-}"
  fi
fi

Assuming your run_commands handles an empty hook.

@djmaze
Copy link
Owner

djmaze commented May 26, 2022

Oh, well, thanks! You spotted the right thing. The current logic is not comprehensive.

AFAICS, there are 2 use cases for notifications:

  1. I want to be notified of success, incomplete backup or backup failure. So I need to configure POST_COMMANDS_SUCCESS, POST_COMMANDS_FAILURE as well as POST_COMMANDS_INCOMPLETE.
  2. I just want to be notified of success or total failure. I want to ignore incomplete backups. So I need to configure POST_COMMANDS_SUCCESS and POST_COMMANDS_FAILURE. In case of an incomplete backup, the success command will be executed as well.

If you agree, then the solution would still be a bit different than what you proposed. I can do a PR for that (unless you want to make the change to your existing PR).

@djmaze
Copy link
Owner

djmaze commented May 26, 2022

Since that would be a breaking change for users who don't have POST_COMMANDS_INCOMPLETE set, I am wondering if we need to make this implicit and have a separate setting for ignoring incomplete backups instead. Something like SUCCESS_ON_INCOMPLETE_BACKUP, defaulting to false.

@sumnerboy12
Copy link
Author

I think it would be best to leave this to you :).

Happy with whatever you decide, my use-case is pretty simple, I just want to be notified when a backup fails and no snapshot is created. Otherwise silence is bliss :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants