-
-
Notifications
You must be signed in to change notification settings - Fork 305
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
Add cron command for DockerFile #101
Conversation
Thanks for the PR, I will check it once I get home from work! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested this on my local machine, it does not seem to work. I also found some major issues with the code
Dockerfile
Outdated
@@ -5,12 +5,17 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ | |||
xdg-user-dirs=0.17-2 \ | |||
procps=2:3.3.17-5 \ | |||
wget=1.21-1+deb11u1 \ | |||
&& apt-get install -y cron \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already a list of packages to install, please just use cron=<version>
Dockerfile
Outdated
TZ=UTC | ||
TZ=UTC \ | ||
BACKUP_ENABLED=true \ | ||
DAYS_TO_KEEP=7 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would preferably set this to "BACKUPS_TO_KEEP" and not days
docker-compose.yml
Outdated
@@ -17,6 +17,9 @@ services: | |||
- TZ=UTC | |||
- ADMIN_PASSWORD="adminPasswordHere" | |||
- COMMUNITY=false # Enable this if you want your server to show up in the community servers tab, USE WITH SERVER_PASSWORD! | |||
- BACKUP_ENABLED=false | |||
- DAYS_TO_KEEP=7 | |||
- BACKUP_CRON_EXPRESSION=0 0 * * * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be quoted
docker-compose.yml
Outdated
@@ -17,6 +17,9 @@ services: | |||
- TZ=UTC | |||
- ADMIN_PASSWORD="adminPasswordHere" | |||
- COMMUNITY=false # Enable this if you want your server to show up in the community servers tab, USE WITH SERVER_PASSWORD! | |||
- BACKUP_ENABLED=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backups are enabled by default in the Dockerfile. Here you set it to false, what is the preferred status of this setting?
scripts/start.sh
Outdated
@@ -67,6 +67,21 @@ if [ -n "${RCON_PORT}" ]; then | |||
sed -i "s/RCONPort=[0-9]*/RCONPort=$RCON_PORT/" /palworld/Pal/Saved/Config/LinuxServer/PalWorldSettings.ini | |||
fi | |||
|
|||
if [[ -n "${BACKUP_ENABLED}" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be if BACKUP_ENABLED = true, now it turns backups on even if the status is false
scripts/start.sh
Outdated
echo "BACKUP_CRON_EXPRESSION=${BACKUP_CRON_EXPRESSION}" | ||
fi | ||
|
||
echo "${BACKUP_CRON_EXPRESSION} root bash /usr/local/bin/backup" > /etc/cron.d/backups-cron |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be in the "else" clause, this will fail if BACKUP_CRON_EXPRESSION is empty
scripts/backup.sh
Outdated
FILE_PATH="${DESTINATION_PATH}/backup_palworld_${DATE}.tar.gz" | ||
|
||
if [ ! -f ${FILE_PATH} ]; then | ||
echo "\e[0;32m***** CREATING BACKUPS FOLDER *****\e[0m\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace echo with printf
scripts/start.sh
Outdated
echo "BACKUP_CRON_EXPRESSION=${BACKUP_CRON_EXPRESSION}" | ||
fi | ||
|
||
echo "${BACKUP_CRON_EXPRESSION} root bash /usr/local/bin/backup" > /etc/cron.d/backups-cron |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you do this as the steam
user instead?
00468ac
to
4ba38be
Compare
Context
Setting Up Automatic Backups with Cron
Adding 3 new environment variables to automate the execution of backups on the server."
Environment Variables
DAYS_TO_KEEP:
Description: This environment variable represents the number of days to retain backup files. It is used in the cleanup process to remove backup files older than the specified duration. Example Usage: If set to 7, backup files older than 7 days will be deleted during the cleanup.
BACKUP_CRON_EXPRESSION:
Description: This environment variable defines the cron expression for scheduling automatic backups. It determines when the backup script should run at specific intervals. Example Usage: If set to 0 2 * * *, the backup script will run every day at 2:00 AM.
BACKUP_ENABLED:
Description: This environment variable indicates whether automatic backups are enabled or not. If set to a non-empty value, automatic backups will be enabled; otherwise, they will be disabled. Example Usage: If set to any non-empty string (e.g., true or 1), the backup script will be configured and executed as per the specified cron schedule.
Test instructions
You can try to set up these variables with this value if you want to save backups every minute.
Checklist before requesting a review