-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
nixos/oci-containers: support rootless containers & healthchecks #368565
base: master
Are you sure you want to change the base?
Conversation
Closes NixOS#259770 Closes NixOS#207050 The motivation for the former is to not execute the container as root, so you don't have to `sudo -i` to perform podman management tasks. The idea behind healthchecks is to be able to keep the unit in the activating state until the container is healthy, only then then unit is marked as active. The following changes were necessary: * Move the ctr-id into `/run/${containerName}` to make podman can actually write to it since it's now in its RuntimeDirectory. * Make `sdnotify` option configurable (`healthy` for healthchecks that must pass, default remains `conmon`). * Set Delegate=yes for `sdnotify=healthy` to make sure a rootless container can actually talk to sd_notify[1]. * Add a warning that lingering must be enabled to have a `systemd --user` instance running which is required for the cgroup support to work properly. * Added a testcase for rootless containers with both conmon and healthchecks. [1] containers/podman#20573 (comment)
type = types.enum [ | ||
"conmon" | ||
"healthy" | ||
"container" |
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.
Note: this probably requires some testing before merging. I haven't really verified if all the settings are correct for this case.
) "network-online.target"; | ||
wants = | ||
lib.optional (container.imageFile == null && container.imageStream == null) "network-online.target" | ||
++ lib.optional dependOnLingerService "linger-users.service"; |
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.
Still kinda amazing, how this is the result of an RFC that has
We want a unified Nix code style that's consistent with itself, easily readable, accessible and results in readable diffs.
(emphasis mine) as explicit goal.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/podman-rootless-with-systemd/23536/13 |
Closes #259770
Closes #207050
The motivation for the former is to not execute the container as root, so you don't have to
sudo -i
to perform podman management tasks.The idea behind healthchecks is to be able to keep the unit in the activating state until the container is healthy, only then then unit is marked as active.
The following changes were necessary:
Move the ctr-id into
/run/${containerName}
to make podman can actually write to it since it's now in its RuntimeDirectory.Make
sdnotify
option configurable (healthy
for healthchecks that must pass, default remainsconmon
).Set Delegate=yes for
sdnotify=healthy
to make sure a rootless container can actually talk to sd_notify[1].Add a warning that lingering must be enabled to have a
systemd --user
instance running which is required for the cgroup support to work properly.Added a testcase for rootless containers with both conmon and healthchecks.
[1] containers/podman#20573 (comment)
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.