-
Notifications
You must be signed in to change notification settings - Fork 78
Allow platform operators to configure init #332
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| # Meta | ||
| [meta]: #meta | ||
| - Name: Add init configuration to builder.toml | ||
| - Start Date: 2025-05-06 | ||
| - Author(s): @AidanDelaney, @ConallCav, @MikeLaptev | ||
| - Status: Draft <!-- Acceptable values: Draft, Approved, On Hold, Superseded --> | ||
| - RFC Pull Request: (leave blank) | ||
| - CNB Pull Request: (leave blank) | ||
| - CNB Issue: (leave blank) | ||
| - Supersedes: (put "N/A" unless this replaces an existing RFC, then link to that RFC) | ||
|
|
||
| # Summary | ||
| [summary]: #summary | ||
|
|
||
| This RFC proposes the addition of an `[init]` configuration section to builder.toml files used in the Cloud Native Buildpacks ecosystem. This section will allow builders to specify the use of an init binary (such as `tini`) to properly forward signals and reap orphaned child processes, improving runtime behavior of containerized applications. | ||
|
|
||
| # Definitions | ||
| [definitions]: #definitions | ||
|
|
||
| - **PID 1**: The first process started by the Linux kernel during system boot and is responsible for initializing the user space and reaping orphaned zombie processes. PID 1 ignores default signal behaviors that terminate other processes: `SIGINT`, `SIGTERM`, `SIGHUP`, and `SIGQUIT` do not terminate PID 1 by default. It must handle SIGCHLD correctly to reap zombie processes. | ||
|
|
||
| - **subreaper**: In Linux, a process that has enabled the `PR_SET_CHILD_SUBREAPER` flag. This special process will "adopt" and reap any orphaned child processes whose parent dies, instead of having those processes re-parented to the init process (PID 1). This helps prevent zombie processes and ensures proper signal handling. | ||
|
|
||
| - **Zombie Process**: A process that has completed execution but still has an entry in the process table because its exit status hasn't been collected by its parent process. | ||
|
|
||
| - **Process Reaping**: The act of collecting exit statuses from terminated child processes, removing their entries from the process table. | ||
|
|
||
| - **Init Process**: Traditionally, the first process (PID 1) started during Linux boot. In containers, this is the main application process. Unlike a traditional init system, container entrypoints often lack proper signal handling and zombie process management. | ||
|
|
||
| - **Tini**: A lightweight but complete init system often used in containers to properly handle process signals and reap zombie processes. | ||
|
|
||
| # Motivation | ||
| [motivation]: #motivation | ||
|
|
||
| Containers typically lack a traditional init system. This can cause issues with zombie processes and unforwarded signals, especially when the entrypoint process does not manage its child processes correctly. | ||
|
|
||
| To mitigate this, container runtimes like Docker offer an `--init` flag that inserts a subreaper (commonly `tini`) as PID 1. However, this must be manually enabled, and not all deployment environments support it (eg: kubernetes). | ||
|
|
||
| Adding subreaper support to Buildpacks via configuration will allow builders to standardize process management and improve container behavior in diverse runtime environments. | ||
|
|
||
| The following motivating example is a python application that loops forever. When run as PID 1 it will ignore SIGTERM, which makes it difficult for runtime platforms (podman, docker, kubernetes) to shut down the container. Adding an `init` as PID 1 correctly handles SIGTERM. | ||
|
|
||
| ``` | ||
| import time | ||
|
|
||
| print("Starting an infinite loop. Ignores SIGTERM.") | ||
|
|
||
| while True: | ||
| time.sleep(1) | ||
| print("Still running...") | ||
| ``` | ||
|
|
||
| Given the above application we can: | ||
|
|
||
| 1. Create an image using `pack build eg1` | ||
| 2. Run using podman/docker `docker run –rm -it eg1` | ||
| 3. Notice that a `podman stop` or `docker stop -t 10` does not immediately stop the running image and resorts to a `SIGKILL` | ||
|
|
||
| The following example creates a single zombie child process. The container will exit after `60` seconds. However, until the container exits the zombie process occupies an entry in the process table. In extreme cases this can exhaust all entries in the process table. | ||
|
|
||
| ``` | ||
| import os | ||
| import time | ||
|
|
||
| pid = os.fork() | ||
|
|
||
| if pid == 0: | ||
| # Child process | ||
| print(f"Child PID {os.getpid()}...") | ||
| os._exit(0) | ||
| else: | ||
| # Parent process: sleep without calling wait() | ||
| print(f"Parent PID {os.getpid()}, child PID {pid}") | ||
| print("Not waiting for child to finish (zombie will be created)") | ||
| time.sleep(60) | ||
| ``` | ||
|
|
||
| Given the above example we can: | ||
|
|
||
| 1. Create an image using `pack build eg2` | ||
| 2. Run `eg2` using `podman` or `docker` | ||
| 3. Observe that `docker exec {container_id} ps` shows that the child process is a zombie | ||
|
|
||
| The two motivating problems of signal handling and sub-process reaping can be addressed either by pushing the respoinsiblity for signal handling and sub-process reaping back onto developers, or by providing platform operators with an option to include an init-style system. This RFC allows platoform-operators to take control of the problem and proposes a mechanism allowing them to opt-in to an init-syle system on their platform. | ||
|
|
||
| # What it is | ||
| [what-it-is]: #what-it-is | ||
|
|
||
| We propose to allow platform operators define a subreaper for all images built using their builder. The use of a subreaper is optional. The omission of a subreaper from the `builder.toml` defaults to the current behaviour. | ||
|
|
||
| We propose that the subreaper, for example `tini`, is launched as PID 1 and runs the process defined in the image. | ||
|
|
||
| # How it Works | ||
| [how-it-works]: #how-it-works | ||
|
|
||
| Assuming an `init` binary, `tini` in this example, is available on the run time, we can simulate how processes can be forked from the `init` binary: | ||
|
|
||
| `docker run --rm -it --entrypoint /bin/tini py -s -- /cnb/process/web` | ||
|
|
||
| We propose that the enablement of a subreaper is configured at the builder level. This allows platform operators to control whether or not they want to use a subreaper on their platform. | ||
|
|
||
| ``` | ||
| [init] | ||
| enabled = true | ||
| binary = /bin/tini | ||
| ``` | ||
|
|
||
| Builders for which `init` is enabled will result in the following `launch.toml` being produced. Where `init` is provided in `launch.toml` the entrypoint for the exported image will use the given init binary as PID1. | ||
|
|
||
| ``` | ||
| [[processes]] | ||
| type = "web" | ||
| init = /bin/tini | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's your thinking on putting
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The proposed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah ok, I missed the each process could change the behavior part. |
||
| ``` | ||
|
|
||
| The `init` field of a process interacts with the existing `direct` field. Where `direct = true` the init binary is used to launch the direct process. | ||
|
|
||
| ``` | ||
| [[processes]] | ||
| type = "web" | ||
| direct = true | ||
| init = /bin/tini | ||
| ``` | ||
|
|
||
| Where `direct = false`, or is unspecified, and an init binary is configured, then the init binary is used to launch the shell process that eventually forks the application process. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need to deal with direct = false? I thought this was removed, so there is only direct = true now.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The distinction between launch with shell and launch directly is still in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was specifically thinking of https://github.com/buildpacks/rfcs/blob/main/text/0093-remove-shell-processes.md |
||
|
|
||
| ``` | ||
| [[proceses]] | ||
| type = "web" | ||
| direct = false | ||
| init = /bin/tini | ||
| ``` | ||
|
|
||
| # Migration | ||
| [migration]: #migration | ||
|
|
||
| This feature does not replace an existing feature, migration is opt-in only. | ||
|
|
||
| # Drawbacks | ||
| [drawbacks]: #drawbacks | ||
| Slight increase in complexity and image size of the run image (due to inclusion of an init binary). | ||
|
|
||
| Introduces a build-time/runtime coupling that may not be desirable. The base run image now must include the init binary specified in `builder.toml`. | ||
|
|
||
| Only configurable by platform operators. | ||
|
|
||
| # Alternatives | ||
| [alternatives]: #alternatives | ||
|
|
||
| - What other designs have been considered? | ||
|
|
||
| We considered hosting the init binary on the build image and copying it to the run image. We have opted for the more simple option of referring to the init binary on the run image. | ||
|
|
||
| We also considered providing a `cnb-init` binary distributed with `lifecycle`. We have opted for platform providers to supply their own preferred init binary. | ||
|
|
||
| Augment `launcher` with `go-reaper`. It solves the issue of reaping child-processes, but not the issue of correct signal handling. | ||
|
|
||
| - What is the impact of not doing this? | ||
|
|
||
| The impact of not supporting an init-style system requires all application developers to support correct signal handling and implement a sub-reaper if they fork processes. In large-scale deployments, the lack of correct reapoing can lead to PID exhaustion on cluster nodes. | ||
|
|
||
| # Prior Art | ||
| [prior-art]: #prior-art | ||
|
|
||
| * Docker’s [--init](https://docs.docker.com/reference/cli/docker/container/run/#init) flag | ||
| * Kubernetes [sidecars using init processes](https://kubernetes.io/docs/tasks/configure-pod-container/share-process-namespace/#understanding-process-namespace-sharing) for signal forwarding | ||
|
|
||
| # Unresolved Questions | ||
| [unresolved-questions]: #unresolved-questions | ||
|
|
||
| - Do we also allow a `pack --init` flag to allow application authors to enable `init`? | ||
|
|
||
| # Spec. Changes (OPTIONAL) | ||
| [spec-changes]: #spec-changes | ||
|
|
||
| * Impacts `builder.toml` and `launch.toml` | ||
|
|
||
| # History | ||
| [history]: #history | ||
|
|
||
| <!-- | ||
| ## Amended | ||
| ### Meta | ||
| [meta-1]: #meta-1 | ||
| - Name: (fill in the amendment name: Variable Rename) | ||
| - Start Date: (fill in today's date: YYYY-MM-DD) | ||
| - Author(s): (Github usernames) | ||
| - Amendment Pull Request: (leave blank) | ||
|
|
||
| ### Summary | ||
|
|
||
| A brief description of the changes. | ||
|
|
||
| ### Motivation | ||
|
|
||
| Why was this amendment necessary? | ||
| ---> | ||
Uh oh!
There was an error while loading. Please reload this page.
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 like that the builder can provide an init process. I like that it can be any init process. Both nice features.
I don't like that whether this feature is enabled or not is at the builder/platform level. I don't think the builder/platform has a good perspective to know if an init process is actually required. There are some apps that will correctly run as PID1, so in this case the init process isn't needed. The platform is not introspecting each app so it would have no clue. It would just add the init process regardless. Not the end of the world, but one more thing running in the container.
If a platform operator did want to allow users to opt out of always having an init process, then they need to maintain two different builders. One with and one without. Then the user would need to pick which builder to use for which app, and that's kind of a pain/error prone.
I feel like there should be some way for this behavior to be overridden by a.) the buildpacks and b.) a user.
So my $0.02 would be that the builder can provide a init process and sets the default. Then buildpacks and users are able to override (in that order of precedence). It might be handy to add a label or annotation or something the operator can use to look at images and see who's overriding this. That way they can easily audit who's not using their default choice.
If that's too complicated, then my vote would be for builder default w/user override. I think that's going to give the most flexibility and accuracy.
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 think I understand your perspective. This RFC suggests a way to provide a default for a platform, and you would like to see a
--init=enableor--init=disableinpack?From my perspective, I have a strong use-case for the builder to provide a default. I think we can deal with user overrides of the default in a subsequent RFC. This allows us to control the scope of this work.
I don't yet understand how the decision of init/no-init can be made at a buildpack level, as we could have a situation where Buildpack A wants init and Buildpack B does not want it. I agree that the end-user programmer is well-placed to enable or disable init. However, our motivating examples use application written by a target end-user programmer who does not quite undersand process reaping or signal handling. So having a builder default does make sense.
Do we agree that this RFC provides enough functionality to implement
pack --init={enable|disable}in the future?Uh oh!
There was an error while loading. Please reload this page.
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.
👍
It wasn't clear to me when I first read this that the builder would set a default. It sounded like the builder would set a value and that was it. I can think of reasons why an operator would want to enforce this and not allow an override, so I wasn't certain the exact intent.
I think if you clarify that the builder supplies the default, and that out of the box we're not intending to provide any way to override that default (i.e. MVP) but that in the future we may provide ways to override, then that would be good.
Yes, that's a little fuzzier in my head. Probably would be a situation where it would be tied to the process type, so like with process types, the last buildpack to update it would win. i.e. if buildpack A runs first and says it should have an init proc, then buildpack B runs and says the same process type should not have an init proc, it would not have an init proc.
I'm totally fine not doing this for now, since it is undefined and would increase the scope a lot.
I'd really like pack to include support for overriding this out of the box. It seems incomplete to have a default and offer no way to override said default, but I totally get where you're coming from too. That's not something you particularly need.
Like I said above, I'd be 👍 so long as the RFC is written to make it clear the builder value is a default and that we've given thought to allowing a pack override in the future and that it would be possible without breaking changes to what we're defining here, because I think there's a very strong chance someone will come along later and ask for this.