add: auto restore option for backups #17
Conversation
…ctories lots of debugging to get auto restore working fixed auto restore loop
|
Thanks a lot for th PR. I will have a look as soon as I have time and test it. In the meantime, you could have a look at the tests and fix the linting errors 😉 |
|
Hey @silfreed, please excuse the late reply. I have overhauled Checks and some other stuff, too. Could you sync your branch with the current master version so the checks can run? Other than that, there only are some minor things i would like to add/change before merging this feature. which I will elaborate after the checks have run 🙃 |
mleutenegger
changed the title
| stat: | ||
| path: "{{ restic_auto_restore_dir }}/{{ restic_backups_loop.name }}.state" | ||
| register: restic_restore_state_file | ||
| when: restic_backups_loop.auto_restore is defined |
There was a problem hiding this comment.
I think this would fire even if auto_restore: false
| when: restic_backups_loop.auto_restore is defined | |
| when: | |
| - restic_backups_loop.auto_restore is defined | |
| - restic_backups_loop |
|
After looking through the changes again, I am a bit concerned about the implications such an automatic restore feature could cause. I think that it might even include the danger of loosing data when an empty repository is restored over a full directory (for example at setup time). In order to be working as expected, it must be ensured that an auto restore only runs exactly when it needs to, and not if:
Both of these conditions are very difficult to check for consistently, especially with us simply using the restic binary to interact with the repository and allowing the user to specify any version. Stdout of commands can change with versions, so catching just the changed state of repository initialization is already a challenge. The fact that the integrity of the data to be backuped depends on such checks makes (imho) for a pretty strong case against an automatically executed restore feature... @sbaerlocher what is your opinion on this? |
|
Coming back to this after almost a year, this still seems like something that may introduce a valid concern when the restore depends on the existence of a file. I do however still like the idea to have an easy-to-use restore script. I think we should include the script so a user can execute it at will, but not let the role run it automatically. @silfreed what would you think? |
1b0655d
I pulled out the auto restore options from gilesw's PR #13
I tried to stay within similar style as the current role but was still heavily learning Ansible so feel free to suggest improvements.
One area I'm concerned about is always restoring to / - I believe this is the right behavior but would like a 2nd set of eyes.
When I allowed the restore script to restore to $BACKUP_SOURCE, my src directory of /home/homeassistant was restored to /home/homeassistant/home/homeassistant.