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

Add tini for sigterm handling #128

Merged
merged 4 commits into from
Dec 16, 2023
Merged

Add tini for sigterm handling #128

merged 4 commits into from
Dec 16, 2023

Conversation

janw
Copy link
Contributor

@janw janw commented Dec 14, 2023

This adds tini as a minimal init to ensure that signalling (SIGTERM in particular) is handled properly in containerized environments. Specifically this will ensure a timely shutdown, for example when running docker stop … or restarting the container.

Thank you for making this project! 🙏

@luigi311
Copy link
Owner

luigi311 commented Dec 16, 2023

Interesting, ive never seen this but looking at the github page it looks like it is built into docker natively via the --init flag, is there still a reason to include this into the docker image? The project itself also looks pretty dead, is it still being supported?

@janw
Copy link
Contributor Author

janw commented Dec 16, 2023

Other container engines do not provide the init flag unfortunately, for example containerd. Personally, I'm running the service in a Kubernetes cluster, and that does not provide it either.

In that way, adding tini to the container ensures that it is compatible with the wider OCI ecosystem.

If you prefer to not add more dependencies, thankfully jellyplex is simple enough that the same result can be achieved by implementing a very simple signal handler in the application itself; something like:

import signal
import sys

signal.signal(signal.SIGTERM, lambda *_: sys.exit(0))

I'd be happy to adjust the PR if you want.

@luigi311 luigi311 changed the base branch from main to dev December 16, 2023 22:07
@luigi311 luigi311 merged commit cc73568 into luigi311:dev Dec 16, 2023
7 checks passed
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.

2 participants