mailcow / mailcow-dockerized

mailcow: dockerized - 🐮 + 🐋 = 💕
https://mailcow.email
GNU General Public License v3.0
8.86k stars 1.18k forks source link

Fix possible incorrect behaviour of backup_and_restore.sh with --delete-days parameter #4559

Open selivan opened 2 years ago

selivan commented 2 years ago

Summary

backup_and_restore.sh has --delete-days parameter that works very straightforward:

find ${BACKUP_LOCATION}/mailcow-* -maxdepth 0 -mmin +$((${1}*60*24)) -exec rm -rvf {} \;

Also the script does rotation without checking if the backup was successful or not.

This is a pitfall for two possible very bad scenarios:

The two scenarios can combine: server goes offline for N+M days, than it goes offline but now docker stops working: /var/lib/docker failed to mount. Backup volume however mounted correctly, so now the cron job creates incorrect backup and deletes all good backups.

I suggest:

Motivation

Users will be less likely to find themselves without correct backup.

Also, if the backup script can provide exit code indication backup success or failure, it can be integrated with monitoring.

Additional context

When I was just starting to work in IT, I've learned the idea about backup rotation only after checking that taken backup is correct the hard way. Let's keep everybody else from that experience :)

iskrant commented 1 year ago

+1

dodedodo commented 1 year ago

A discussion on implementing existing backup software like borgbackup or restic might be appropriate here. This is basic functionality that has been implemented at least a dozen times. Why reinvent the wheel?