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

Changed backup so only old backup files are deleted #215

Merged
merged 7 commits into from
Jan 31, 2024

Conversation

Dashboy1998
Copy link
Contributor

@Dashboy1998 Dashboy1998 commented Jan 30, 2024

Context

  • Currently anything older than OLD_BACKUP_DAYS in the backup folder is deleted including directories or non backup files
  • Changes here ensure only only backup files are deleted

Choices

  • Added additional flag to delete only files
  • Added additional flag so only backups which have not had the names modified are deleted
  • Added checks to verify OLD_BACKUP_DAYS is defined and is an integer.

Test instructions

  1. Start container with DELETE_OLD_BACKUPS=true
  2. Enter container: docker exec -it palworld-server bash
  3. Create at least 4 backups
  4. Go to the backup folder: cd /palworld/backups
  5. Change of the modified date the backups: find /palworld/backups/ -exec touch -d "9001 days ago" {} \;
  6. Create 2 directories: mkdir keep empty
  7. Move a backup to keep: mv palworld-save-2024-01-.... keep/
  8. Rename one backup: mv palworld-save-2024-01-.... important_palworld-save-2024-01-....
  9. Run backup
  10. Verify that empty and keep directories still exists
  11. Verify the backup in keep still exists
  12. Verify that the backup which you renamed still exists
  13. Start the container with DELETE_OLD_BACKUPS= and run a backup.
  14. Verify backup command outputs Unable to deleted old backups, OLD_BACKUP_DAYS is empty.
  15. Start the container with DELETE_OLD_BACKUPS=30b and run a backup.
  16. Verify backup command outputs Unable to deleted old backups, OLD_BACKUP_DAYS is not an integer. OLD_BACKUP_DAYS=30b

Checklist before requesting a review

  • I have performed a self-review of my code
  • I've added documentation about this change to the README.
  • I've not introduced breaking changes.

scripts/backup.sh Outdated Show resolved Hide resolved
@Dashboy1998 Dashboy1998 changed the title DRAFT: Changed back so only files are deleted, only backup files, and added su. Changed back so only files are deleted, only backup files, and added su. Jan 30, 2024
@Dashboy1998 Dashboy1998 changed the title Changed back so only files are deleted, only backup files, and added su. Changed backup so only old backup files are deleted Jan 30, 2024
Dockerfile Show resolved Hide resolved
scripts/backup.sh Show resolved Hide resolved
@thijsvanloef thijsvanloef added the waiting for feedback Waiting until author responds label Jan 31, 2024
Copy link
Owner

@thijsvanloef thijsvanloef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, ready to merge?

@Dashboy1998
Copy link
Contributor Author

LGTM, ready to merge?

Yes.

@thijsvanloef thijsvanloef merged commit 689674b into thijsvanloef:main Jan 31, 2024
4 checks passed
MusclePr pushed a commit to MusclePr/palworld-server-docker that referenced this pull request Jun 19, 2024
Changed backup so only old backup files are deleted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Waiting until author responds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants