Skip to content

k3s: use patched util-linuxMinimal#407810

Merged
Mic92 merged 3 commits intoNixOS:masterfrom
numinit:k3s/util-linux-fixes
May 20, 2025
Merged

k3s: use patched util-linuxMinimal#407810
Mic92 merged 3 commits intoNixOS:masterfrom
numinit:k3s/util-linux-fixes

Conversation

@numinit
Copy link
Contributor

@numinit numinit commented May 17, 2025

Until #405952 is fixed, we can use our own util-linux to avoid breaking k3s in the release. Revert the last commit in this series when that happens.

CC @NixOS/k3s

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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.

numinit added 3 commits May 16, 2025 21:34
Some of the attributes fetched throw, so tryEval them. Recurse into the
attribute set to pick up all the tests.
The full version of util-linux has systemd, NLS, and ncurses support.
k3s only uses a couple utilities from it at runtime, so use the minimal
version.
Until NixOS#405952 is fixed, we can use our own util-linux to avoid breaking
k3s in the release. Revert this commit when that happens.
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: k3s Kubernates distribution (https://k3s.io/) labels May 17, 2025
@numinit
Copy link
Contributor Author

numinit commented May 17, 2025

@ofborg build k3s nixosTests.k3s.airgap-images nixosTests.k3s.auto-deploy-charts nixosTests.k3s.etcd nixosTests.k3s.multi-node nixosTests.k3s.single-node nixosTests.k3s.auto-deploy nixosTests.k3s.containerd-config nixosTests.k3s.kubelet-config

@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 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 17, 2025
@numinit
Copy link
Contributor Author

numinit commented May 17, 2025

@NixOS/k3s I don't know nearly enough about k3s but it probably doesn't need systemd, NLS, or ncurses support for the 3-4 commands it runs in util-linux right? 😉

@numinit numinit mentioned this pull request May 17, 2025
13 tasks
@Mic92
Copy link
Member

Mic92 commented May 17, 2025

@NixOS/k3s I don't know nearly enough about k3s but it probably doesn't need systemd, NLS, or ncurses support for the 3-4 commands it runs in util-linux right? 😉

Would be also my assumption.

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested in an actual cluster but changes look good to me.

k3sUtilLinux = util-linuxMinimal.overrideAttrs (prev: {
patches =
prev.patches or [ ]
++ lib.singleton (fetchpatch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit that can be disregarded but what is the advantage of syntax over just doing:

Suggested change
++ lib.singleton (fetchpatch {
++ [
(fetchpatch {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least I find this easier to read.

Copy link
Contributor

@rorosen rorosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good

@Mic92
Copy link
Member

Mic92 commented May 17, 2025

@numinit Did you tested this in an actual cluster as well? I don't know all our tests well enough to know if they test this particular bug.

@numinit
Copy link
Contributor Author

numinit commented May 17, 2025

I have not but a couple people in #405952 mentioned that it solved their problem.

patches =
prev.patches or [ ]
++ lib.singleton (fetchpatch {
url = "https://github.com/util-linux/util-linux/pull/3479.patch";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use https://github.com/util-linux/util-linux/commit/7dbfe31a83f45d5aef2b508697e9511c569ffbc8.patch instead of the pull request patch as it is already merged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Missed that. We should never point to a pull request in nixpkgs because the hash can get invalidated easily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about this change. Will make a follow up PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels May 17, 2025
@numinit
Copy link
Contributor Author

numinit commented May 17, 2025

I will try to repro the bug in the tests along with the requested change

@rorosen rorosen mentioned this pull request May 17, 2025
3 tasks
Copy link
Contributor

@jfly jfly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm this fixes things for my k3s cluster.

overrideContainerdAttrs;

# TODO (#405952): remove this patch. We had to add it to avoid a mass rebuild
# for the 25.05 release. Once the above PR is merged, switch back to plain util-linuxMinimal.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the above PR

Which PR? Is this missing from the comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's #405952 one line above.

@Mic92 Mic92 merged commit b6a5eeb into NixOS:master May 20, 2025
59 of 62 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented May 20, 2025

Successfully created backport PR for release-25.05:

@trofi
Copy link
Contributor

trofi commented May 31, 2025

Bisect says 727809f causes the following eval failures:

$ nix-instantiate -A k3s.tests
error:
       … while evaluating the attribute 'recurseForDerivations'

       … while selecting an attribute
         at pkgs/applications/networking/cluster/k3s/builder.nix:487:36:
          486|         in
          487|         lib.mapAttrs (name: value: nixosTests.k3s.${name}.${k3s_version}) nixosTests.k3s;
             |                                    ^
          488|       tests = passthru.mkTests k3sVersion;

       error: expected a set but found a Boolean: true

@numinit
Copy link
Contributor Author

numinit commented May 31, 2025

Ah, yikes - I've got a fix. That set of tests didn't even eval before, but now it's angry about the recurseForDerivations we added.

@numinit
Copy link
Contributor Author

numinit commented May 31, 2025

Corrected in #412756 (and all the tests pass).

I previously had to fix this because the tests were also pulling in all the k3s aliases which were throwing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: k3s Kubernates distribution (https://k3s.io/) 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants