Open BenjaminSchubert opened 2 years ago
cc: @buildpacks/implementation-maintainers @buildpacks/buildpack-authors-tooling-contributors
On Paketo, this has impacted us in different ways. For the language families, I help to support, it's been less about reaping processes and more about proper handling of signals so that your application can gracefully shut down.
For Paketo Java, we've chosen to not do anything. The JVM runs as PID1. It handles signals, so graceful shutdown works and we've not had any complaints about reaping subprocesses. Either users aren't complaining or there's not a lot of forking of subprocesses in Java apps.
For the Paketo Rust buildpack, it was a little more of a problem. Rust apps don't handle signals out-of-the-box so graceful shutdown didn't work. What we did for Rust was to add the tini
binary by default. Process types are generated with tini
starting the Rust binary, so that signal handling and, while it wasn't a concern for users, process reaping also works.
If we got complaints from Java users, I suspect that we'd probably add tini
in there too, but probably opt-in unless there was a stronger case for making it the default.
I know that we also have a tini
buildpack, which I think some of our other language families are using. @ForestEckhardt @sophiewigmore @fg-j Is there anything you could share about how we handle PID1 in other Paketo language families?
In regards to your question, I think my $0.02 would be that it's a legit concern. Probably one that's being overlooked by at least some portion of buildpack authors. I would like to see some sort of shared solution that buildpack authors can use, but not one that's required (or not able to be opted out of). As a buildpack author, I'd like to decide when I start something as PID1 or when I use a wrapper to handle the PID1 responsibilities. That would also leave the option to delegate that decision to a user, if for example they prefer using something like pause
or docker run --init
to handle PID1.
I also don't think that we should invent our own PID1 binary. If we're going to do this, we should use something that exists already and is vetted, like tini
.
Possibly off-topic, but a challenge that we have with a buildpack like the Paketo tini
buildpack is that it cannot be self-contained. It can be used to install the tini
binary, but what you really want is for some other buildpack to emit the process types and for the tini
buildpack to run and wrap those process types with tini + <whatever the other buildpack set>
. I'm under the impression that is not possible (correct me if I'm wrong), which I think makes having an elegant solution at the buildpack level difficult.
AFAIK, we don't actually use tini anywhere for graceful shutdown anymore. We ran into some issues with it – my recollection is thin; maybe other Paketo maintainers remember better.
In some buildpacks/for some cases, bash
is PID1. For instance, in the Nodejs buildpack when a user has their source code in a nested subdirectory. bash
obviously isn't ideal from a signal handling perspective.
Otherwise, in Go and for some .NET apps, the compiled binary of the app is PID1.
For some Node.js apps, node
is PID1. Similarly, for some .NET apps, dotnet
is PID1. As you said, @dmikusa-pivotal, I have spent more time thinking about the signal handling aspect of the PID1 than the zombie reaping. Feedback about the PID1 set by any given Paketo buildpack would be welcomed.
I don't have strong opinions about whether CNB should handle this somehow upstream.
Some thoughts:
I agree with @dmikusa-pivotal that
Instead of finding a way to allow a buildpack to modify or reference other buildpacks I would like to design a way that buildpacks could modify the container entrypoint and inject it upstream of the launcher. Therefore the entrypoint could be something like tini - v -- /cnb/process/web
.
Maybe we could add a mechanism to launch.toml
?
entrypoint = ["/path/to/tini", "-v", "--", "$(ENTRYPOINT)"]
The problem here becomes if you want to set environment variables to affect the behavior of the entrypoint. The launcher typically applies buildpack provided modifications to the environment. Maybe we could:
Problem
Background
PID1 on Unix has special responsibilities. Mainly, it needs to take parentship of orphan children, and reap zombie processes.
One important thing to note is that the number of PIDs on a machine is limited, and using all PIDs will prevent new processes from getting created. Reaping zombies helps ensuring that no PIDs are used for nothing.
When working with containers and/or pods, since they usually have their own namespace, the pod/container's PID1 is responsible of this handling.
However, how this is done depends on the platform to which the container is deployed and how it is deployed.
There are mainly two ways to do this:
pause
container will be PID1 and handle those responsibilitiesWhat happens if this is not done?
Failing to do so can lead a misbehaving application to own all the PIDs on a node and make this node unsuable/crash.
There are ways of mitigating this, e.g. ensuring each application is limited on the number of PIDs it can create. However this is not ideal.
Our needs
We currently need to deploy to platforms where 1. is not possible for us, and we therefore need to ensure 2. works.
Currently, ensuring this with Buildpacks is tricky:
init
as PID1.Question
Would CNB want to, and if so, how could we enable owners of buildpack builders to ensure that buildpack built images have a valid PID1 so they can safely be deployed to any platform regardless of how it is configured?
Potential solutions
launch
in the lifecycle install itself as PID1 and reaping processes automatically