I've a doubt about the koha-run-backups script that it deletes its backup files if the backup destination directory is empty.
First findings: and a correction, if there are some other files in the backup destination directory, script also counts them and this makes the problem. In the remove part of script, sed "s:^$name ... part removes the instance name and expects only related backup files, but if somehow there are some other files, backups are being deleted It could be better to find older files with find command maybe.
I also have this issue, backups are fixed to 2 days even harrcoding days variable from 2 to 7
I do not understand why it is validated that a complete pair of sql.gz and tar.gz exists before deleting the pair. As if it would be especially desirable to preserve backups that may have failed before being completed indefinitely. I suggest that we don't do this. We can select files for deletion with the `find' command. We can use the modification time instead of the date in the filename. These should normally be the same. We can also use a stricter pattern for matching files to avoid deleting files that have not been created by the backup script. For instance: find $backupdir -maxdepth 1 \( -mtime +$days -or -mtime $days \) \ -name $name-'[1-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9].???'.gz \ \( -name \*.sql.gz -or -name \*.tar.gz \) -print0 | xargs -0 -r rm
Created attachment 158533 [details] [review] Bug 30627: Verify --days parameter and use find command to select old backups for deletion Test plan * Create some old fake backups: backuproot=/var/spool/koha instance=kohadev backupdir="$backuproot"/"$instance" for i in 1 2 3 4 ; do for j in sql tar xxx ; do file="$backupdir"/"$instance"-$(date -I -d "- $i day").${j}.gz if ! test -e "$file" ; then touch -t "$(date +%Y%m%d%H%M -d "- $i day")" "$file" fi done done * Verify that --days parameter is validated sudo koha-run-backups --days 0 sudo koha-run-backups --days foo * Run backup sudo koha-run-backups --days 3 * Verify that backups from 3 days have been preserved and older backups have been deleted * Verify that filenames that do not match the pattern (the .xxx.gz files) are preserved
Added assignee.
Created attachment 159931 [details] [review] Bug 30627: Verify --days parameter and use find command to select old backups for deletion Test plan * Create some old fake backups: backuproot=/var/spool/koha instance=kohadev backupdir="$backuproot"/"$instance" for i in 1 2 3 4 ; do for j in sql tar xxx ; do file="$backupdir"/"$instance"-$(date -I -d "- $i day").${j}.gz if ! test -e "$file" ; then touch -t "$(date +%Y%m%d%H%M -d "- $i day")" "$file" fi done done * Verify that --days parameter is validated sudo koha-run-backups --days 0 sudo koha-run-backups --days foo * Run backup sudo koha-run-backups --days 3 * Verify that backups from 3 days have been preserved and older backups have been deleted * Verify that filenames that do not match the pattern (the .xxx.gz files) are preserved Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
I don't understand the code enough to fully QA this but here are some findings. why is there `\( -name \*.sql.gz -or -name \*.tar.gz \)` for the pattern? Doesn't that negates the use of the preceding more strict pattern? Wait, is that just to constrain the ??? to be either sql or tar? (just found that there is an implicit AND (-a) when there is no operator between tests) ----- About switching to using file metadata instead of file name to get the date. There should be at least a mention in the documentation of --days and a mention in the release notes ("Text to go in the release notes"). To lessen the chances of mistakes due to eventual divergences there. So if there is one, we have less to worry about and keep the proposed approach to use metadata. Speaking of divergences, is file metadata something we can count on? I don't know enough about managing servers and backups to reliably say if that's reliable or not. Here are some thoughts - if another backup system backups koha backup files, then we can expect date to be preserved - if the files are moved via ssh or something else to a new machine to restore a koha backup (hardware upgrade for a koha server), then at worse date should be reset to current date, so backups would be deleted later, that doesn't look like much of an issue - zip archives preserve the date - copying on a FAT32 USB drive also - if the place where the koha backup files are mounted is via some network protocol, is that possible (and not that rare enough) that it doesn't support date metadata? I recall with a phone using MTP (not a network protocol and MTP has tons of very bad implementation) that all the files I copied to my PC got their date set to 1970... So that's the only real worry. Does date being reset to 1970 is a thing that can happen enough to compromise the use of date metadata vs name?
(In reply to Victor Grousset/tuxayo from comment #7) > So that's the only real worry. Does date being reset to 1970 is a thing that > can happen enough to compromise the use of date metadata vs name? LOL It happens all the time :)
Created attachment 161494 [details] [review] Bug 30627: Verify --days parameter and use find command to select old backups for deletion Test plan * Create some old fake backups: backuproot=/var/spool/koha instance=kohadev backupdir="$backuproot"/"$instance" for i in 1 2 3 4 ; do for j in sql tar xxx ; do file="$backupdir"/"$instance"-$(date -I -d "- $i day").${j}.gz if ! test -e "$file" ; then touch -t "$(date +%Y%m%d%H%M -d "- $i day")" "$file" fi done done * Verify that --days parameter is validated sudo koha-run-backups --days 0 sudo koha-run-backups --days foo * Run backup sudo koha-run-backups --days 3 * Verify that backups from 3 days have been preserved and older backups have been deleted * Verify that filenames that do not match the pattern (the .xxx.gz files) are preserved Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Great simplification and fix, Andreas. Victor: Can understand your concerns, but should not really be an issue imo in daily operation.
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.03
(In reply to Marcel de Rooy from comment #10) > Victor: Can understand your concerns, but should not really be an issue imo > in daily operation. Oh great :D I though we were doomed when you said "LOL It happens all the time :)" T_T But no! Thanks for your insight that this shouldn't be a problem in practice ^^
Backported to 23.05.x for upcoming 23.05.09
*** Bug 23943 has been marked as a duplicate of this bug. ***