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

nixos/oci-containers: support rootless containers & healthchecks #368565

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Dec 27, 2024

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 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)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

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)
@Ma27 Ma27 requested review from adisbladis and fpletz December 27, 2024 15:25
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` labels Dec 27, 2024
type = types.enum [
"conmon"
"healthy"
"container"
Copy link
Member Author

@Ma27 Ma27 Dec 27, 2024

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.

@nix-owners nix-owners bot requested a review from roberth December 27, 2024 15:27
) "network-online.target";
wants =
lib.optional (container.imageFile == null && container.imageStream == null) "network-online.target"
++ lib.optional dependOnLingerService "linger-users.service";
Copy link
Member Author

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.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 27, 2024
@nixos-discourse
Copy link

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

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
4 participants