search

kmwoley / restic-windows-backup

Powershell scripts to run Restic backups on Windows
MIT License
330 stars 68 forks source link

Unsafe defaults for Snapshot Maintenance #70

Open chrestomanci opened 1 year ago

chrestomanci commented 1 year ago

I have just installed restic-windows-backup on a machine that also dual boots to Linux, and had it delete most of my backup snapshots taken from Linux.

The issue appears to be that the script run prune then forget by default the first time it is run, there are no checks for deleting an excessive number of snapshots and the default Retention Policy says "--group-by host,tags" when the path is also significant for backups taken under Linux

Please change the default configuration so that SnapshotMaintenanceEnabled is set to false and add "--dry-run" to the default SnapshotRetentionPolicy, then in the Readme.md suggest that after installation the user should:

  1. Run the backup by hand in the PowerShell by running backup.ps1 and checking the output for errors
  2. Wait a week or so for some scheduled nightly backups to run.
  3. Change SnapshotMaintenanceEnabled to true, run backup.ps1 by hand and check that the proposed retention is sensible
  4. Only then, remove the "--dry-run" flag from SnapshotRetentionPolicy

You should also add a footnote to the Readme.md to warn of this issue on machines that dual boot.

I think that these defaults would be much safer than current. It after installation the user forgets to turn on SnapshotMaintenanceEnabled, then they will end up with hundreds of snapshots, that restic will cope with and only cause a small loss of performance. The current default is dangerous.

I can prepare a PR with these proposed changes if you like.

kmwoley commented 1 year ago

Thank you for bringing this to my attention. I'm sorry this happened to you. The original (and current) intent of the script was for people who are targeting a repository that's only used by this one machine. In the short term, I'll put a warning in the readme about the behavior of the script and instructions to review the maintenance settings if someone intends to use the script with a shared repository. I don't think I'm going to go as far as to disable maintenance or set the out-of-the-box configuration to be a dry-run, given that the primary use case is to make it easy to get up and running with a Windows restic backup with minimal fuss, and part of that is to have some amount of prune/forget configured by default.

On Thu, Dec 29, 2022 at 11:52 AM David Pottage @.***> wrote:

I have just installed restic-windows-backup on a machine that also dual boots to Linux, and had it delete most of my backup snapshots taken from Linux.

The issue appears to be that the script run prune then forget by default the first time it is run, there are no checks for deleting an excessive number of snapshots and the default Retention Policy says "--group-by host,tags" when the path is also significant for backups taken under Linux

Please change the default configuration so that SnapshotMaintenanceEnabled is set to false and add "--dry-run" to the default SnapshotRetentionPolicy, then in the Readme.md suggest that after installation the user should:

  1. Run the backup by hand in the PowerShell by running backup.ps1 and checking the output for errors
  2. Wait a week or so for some scheduled nightly backups to run.
  3. Change SnapshotMaintenanceEnabled to true, run backup.ps1 by hand and check that the proposed retention is sensible
  4. Only then, remove the "--dry-run" flag from SnapshotRetentionPolicy

You should also add a footnote to the Readme.md to warn of this issue on machines that dual boot.

I think that these defaults would be much safer than current. It after installation the user forgets to turn on SnapshotMaintenanceEnabled, then they will end up with hundreds of snapshots, that restic will cope with and only cause a small loss of performance. The current default is dangerous.

I can prepare a PR with these proposed changes if you like.

— Reply to this email directly, view it on GitHub https://github.com/kmwoley/restic-windows-backup/issues/70, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOWG6ALO5V3CLPA2IHG36TWPXTYVANCNFSM6AAAAAATMLGM6E . You are receiving this because you are subscribed to this thread.Message ID: @.***>

chrestomanci commented 1 year ago

While adding a warning to the bottom of the readme is a useful step, and would have averted the problem for me because I do read them, I am still concerned that the proposed defaults are unsafe. I think that as a minimum you should change the default --group-by flag to host,tags,paths - It would make no difference in the default case of one backup taken from windows only, but would protect other backups, not just in the dual boot scenario but also if the user adds additional or more frequent backups of an important work directory.

I understand that your aim is to create a setup and forget backup solution, and it does sort of work that way in the simplest default case, but a prudent admin should not run with the default setup at first because it might not work or could do unexpected things and the log output from the windows scheduler is difficult or impossible to obtain.

My advice to a more careful sysadmin would be to skip the windows scheduler part of the setup, and also to and add "--dry-run" to the default SnapshotRetentionPolicy, and run the backup for the first time in a powershell console window monitoring the output, as the backup could easily and silently fail due to any number of miss configurations. (eg, wrong credentials, wrong destination, too much or too little data backed up).

Once that initial run has completed without anything unexpected then it would make sense to enable scheduled backups and automatic prune & forget but without that first test there are too many ways that the backup could go wrong so that it is not there when needed.

Would you agree that this is sensible advice, and adjust the Readme to reflect that ?