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

Fixes instrumenting of cron to preserve environment variables. #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krobertson
Copy link

This changes the startup script to create a crontab with all of the
supported environment variables prepended to it to ensure they are
available when calling to arkmanager.

This fixes an issue where updating and restarting from cron would fail
to restart the server properly.

This change also includes a number of trailing whitespace trims, as well
as an update to the README where the Variables section had a typo for
the UPDATEONRESTART entry.

Fixes #13 and arkmanager/ark-server-tools#678

This changes the startup script to create a crontab with all of the
supported environment variables prepended to it to ensure they are
available when calling to arkmanager.

This fixes an issue where updating and restarting from cron would fail
to restart the server properly.

This change also includes a number of trailing whitespace trims, as well
as an update to the README where the Variables section had a typo for
the UPDATEONRESTART entry.
@changemenemo
Copy link

what's your start command of the container

@krobertson
Copy link
Author

I just use the default one. Dockerfile has it call user.sh.

@krobertson
Copy link
Author

I repushed the krobertson/ark:test image I posted on Docker Hub. If you were using it before now, you'll want to repull. I realized I hadn't re-pushed it after correcting the UPDATEONRESTART spelling. Having it wrong resulted in the crontab not loading at all because of a syntax error.

Also, worth noting the startup scripts do not have set -e where an error in the setup does not result in the container failing.

@TuRz4m
Copy link
Owner

TuRz4m commented Jan 2, 2017

Great work :) I saw your issue on Ark Server Tools this weekend but I wasn't available. If you're sure it works, I can merge it right now, but I don't have time to test it before the end of the week.

@changemenemo
Copy link

ow ow finally turzam show himself :p @krobertson Do you have look at your logs? you don't have different PID now?

@krobertson
Copy link
Author

Was going to say wait for the next update to see, but looks like 253.71 came out and our server updated and restarted just fine.

@changemenemo
Copy link

and nothingin the logs?

@changemenemo
Copy link

it's our container itself who decide to go fetch the update right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

restarting server after update not working
3 participants