pelican-eggs / yolks

Docker images designed for use with Pterodactyl's Egg system.
MIT License
89 stars 248 forks source link

Use tini for some images to fix the ^C stop bug #166

Closed QuintenQVD0 closed 10 months ago

QuintenQVD0 commented 1 year ago

Description

This PR moves some images

to use tini to start the container afbeelding

This will spawn a process with the shell specified in the shebang of the entrypoint.sh and will now if ^C is set as the stop signal send a SIGTERM as the current wings stop logic is a bit broken. It should still work if the stop command is anything else then ^C for example the stop for minecraft java SIGTERM is the default signal send by the kill command on Linux. It should force the process to exit but still gracefully.

This is a temp workaround as we still have issues with the stop logic and can not yet use exec

The -g flag forwards to the entire process group including children. what is needed on ubuntu or debian the package is located at /usr/bin/tini on alpine on /sbin/tini

NOTES

A SPECIAL THANKS TO

Test images

all images that have changed here are build in this repo: https://hub.docker.com/r/quintenqvd/pterodactyl_tini/tags If you want to test some

All Submissions:

arvitus commented 1 year ago

I just built the python 3.11 image using your PR, but I get [FATAL tini (7)] exec /entrypoint.sh failed: No such file or directory. Dunno if that's on me, but I used the files from your PR.

QuintenQVD0 commented 1 year ago

I just built the python 3.11 image using your PR, but I get [FATAL tini (7)] exec /entrypoint.sh failed: No such file or directory. Dunno if that's on me, but I used the files from your PR.

I have a look as that file is there.

QuintenQVD0 commented 1 year ago

But it must be build with hithub actions and the entrypoint.sh file must be in the same releative folder then it is here as youe local build. @arvitus

arvitus commented 1 year ago

I think, it may be because I'm building on windows. Line endings and shit. I'm currently building again and will keep you updated :)

Edit: I think the building worked now, but now I get this:

[Pterodactyl Daemon]: Finished pulling Docker container image
Python 3.11.4
:/home/container$ if [[ -d .git ]] && [[ ${AUTO_UPDATE} == "1" ]]; then git pull; fi; if [[ ! -z ${PY_PACKAGES} ]]; then pip install -U --prefix .local ${PY_PACKAGES}; fi; if [[ -f /home/container/${REQUIREMENTS_FILE} ]]; then pip install -U --prefix .local -r ${REQUIREMENTS_FILE}; fi; /usr/local/bin/python /home/container/${BOT_PY_FILE}
/usr/local/bin/python: can't find '__main__' module in '/home/container/'
container@pterodactyl~ Server marked as offline...

Edit2: Ok, it seems like that has nothing to do with your PR as the above error also happens on the default images.

Edit3: After fixing the error above, which had indeed nothing to do with the image, I can say that the image works like it's supposed to be. I was finally able to set a signal handler in my python script and intercept the SIGTERM, which wouldn't have been possible with the default images.

QuintenQVD0 commented 1 year ago

I think, it may be because I'm building on windows. Line endings and shit. I'm currently building again and will keep you updated :)

Edit: I think the building worked now, but now I get this:

[Pterodactyl Daemon]: Finished pulling Docker container image
Python 3.11.4
:/home/container$ if [[ -d .git ]] && [[ ${AUTO_UPDATE} == "1" ]]; then git pull; fi; if [[ ! -z ${PY_PACKAGES} ]]; then pip install -U --prefix .local ${PY_PACKAGES}; fi; if [[ -f /home/container/${REQUIREMENTS_FILE} ]]; then pip install -U --prefix .local -r ${REQUIREMENTS_FILE}; fi; /usr/local/bin/python /home/container/${BOT_PY_FILE}
/usr/local/bin/python: can't find '__main__' module in '/home/container/'
container@pterodactyl~ Server marked as offline...

Edit2: Ok, it seems like that has nothing to do with your PR as the above error also happens on the default images.

nice to hear that. I am building a temp repo with all those images build so you can test them directly

QuintenQVD0 commented 1 year ago

Fore those reading this I pushed a temp docker repo here: https://hub.docker.com/r/quintenqvd/pterodactyl_tini/tags

With those images build. The build is done from this repo: https://github.com/QuintenQVD0/yolks-tini

only changed images are build so no wine, proton, mono,...

QuintenQVD0 commented 1 year ago

I changed python to use SIGINIT as that is what it expect and not SIGTERM

QuintenQVD0 commented 1 year ago

I think, it may be because I'm building on windows. Line endings and shit. I'm currently building again and will keep you updated :)

Edit: I think the building worked now, but now I get this:

[Pterodactyl Daemon]: Finished pulling Docker container image
Python 3.11.4
:/home/container$ if [[ -d .git ]] && [[ ${AUTO_UPDATE} == "1" ]]; then git pull; fi; if [[ ! -z ${PY_PACKAGES} ]]; then pip install -U --prefix .local ${PY_PACKAGES}; fi; if [[ -f /home/container/${REQUIREMENTS_FILE} ]]; then pip install -U --prefix .local -r ${REQUIREMENTS_FILE}; fi; /usr/local/bin/python /home/container/${BOT_PY_FILE}
/usr/local/bin/python: can't find '__main__' module in '/home/container/'
container@pterodactyl~ Server marked as offline...

Edit2: Ok, it seems like that has nothing to do with your PR as the above error also happens on the default images.

Edit3: After fixing the error above, which had indeed nothing to do with the image, I can say that the image works like it's supposed to be. I was finally able to set a signal handler in my python script and intercept the SIGTERM, which wouldn't have been possible with the default images.

I change to SIGINT for python on my test images as apparently that is what python expect and not SIGTERM

QuintenQVD0 commented 1 year ago

Pleasd wait merging this! I have to edit it to send SIGINT as that is what most aplications expect

QuintenQVD0 commented 1 year ago

I changed everything to SIGINT as that is ctrl+c and that is what the stop button should do and not SIGTERM as for that we have the kill button.

gOOvER commented 10 months ago

Very nice decision :) Thanks for that :)

Notacod3r commented 10 months ago

Can I offer compensation for a fix to this issue for the Bedrock ARM egg? @QuintenQVD0 or anyone that are able. The community server I'm running has had some significant downtime, and I would like to rectify this.

QuintenQVD0 commented 10 months ago

Can I offer compensation for a fix to this issue for the Bedrock ARM egg? @QuintenQVD0 or anyone that are able. The community server I'm running has had some significant downtime, and I would like to rectify this.

The box64 emulator does not like tini and does not work. I can give you a test image if you like that.

QuintenQVD0 commented 10 months ago

@Notacod3r please message me on discord. quintenqvd as I did not have this.