Skip to content

nixos/nix-daemon: workaround NixOS/nix#6285#164644

Merged
vcunat merged 1 commit intoNixOS:stagingfrom
flokli:workaround-nix-daemon-socket
Mar 21, 2022
Merged

nixos/nix-daemon: workaround NixOS/nix#6285#164644
vcunat merged 1 commit intoNixOS:stagingfrom
flokli:workaround-nix-daemon-socket

Conversation

@flokli
Copy link
Member

@flokli flokli commented Mar 17, 2022

nix-daemon.socket is used to socket-activate nix-daemon.service when
/nix/var/nix/daemon-socket/socket is accessed.

Having a ConditionPathIsReadWrite on the /nix/var/nix/daemon-socket
directory will cause systemd to just skip if it's not present yet.

[ 237.187747] systemd[1]: Nix Daemon Socket was skipped because of a failed condition check (ConditionPathIsReadWrite=/nix/var/nix/daemon-socket).

As it's the nix-daemon itself that creates this directory, we're in a
chicken-and-egg problem - as long as the folder isn't created,
nix-daemon won't start (as it's only socket-activated), and the socket
unit will get skipped, as the directory doesn't exist yet.

I think we don't actually want to skip starting the socket unit when the
directory doesn't exist yet.

This has surfaced in the systemd 250 bump (which probably did apply some
more rigid checks), where tests with containers that bind-mount
/nix/var/nix/daemon-socket started to fail.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@flokli flokli requested review from edolstra, vcunat and zimbatm March 17, 2022 22:11
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Mar 17, 2022
@flokli flokli mentioned this pull request Mar 17, 2022
13 tasks
@ofborg ofborg 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 Mar 17, 2022
@flokli flokli requested review from lukegb and sternenseemann March 18, 2022 08:58
@flokli
Copy link
Member Author

flokli commented Mar 18, 2022

Based on the feedback in NixOS/nix#6282 (comment), this has been updated.

@flokli flokli force-pushed the workaround-nix-daemon-socket branch 3 times, most recently from 01e25e5 to 5ec8821 Compare March 18, 2022 15:23
@flokli flokli marked this pull request as draft March 18, 2022 15:24
@flokli flokli force-pushed the workaround-nix-daemon-socket branch from 5ec8821 to f84473a Compare March 21, 2022 10:38
@flokli flokli changed the title nixos/nix-daemon: workaround NixOS/nix#6282 nixos/nix-daemon: workaround NixOS/nix#6285 Mar 21, 2022
@flokli flokli force-pushed the workaround-nix-daemon-socket branch from f84473a to 8b24f7b Compare March 21, 2022 10:39
@flokli flokli changed the base branch from staging to staging-next March 21, 2022 10:40
@flokli
Copy link
Member Author

flokli commented Mar 21, 2022

I updated this PR. It now targets staging-next, and contains a simple tmpfiles rule addition, instead of patching the Nix package:

commit 8b24f7b5914d6b56b76566d340de89de78d42549
Author: Florian Klink <flokli@flokli.de>
Date:   Fri Mar 18 16:22:38 2022 +0100

    nixos/nix-daemon: workaround NixOS/nix#6285
    
    The Nix-provided `nix-daemon.socket` file has a
    
    > ConditionPathIsReadWrite=/nix/var/nix/daemon-socket/socket
    
    line, to skip that unit if /nix/var/nix/daemon-socket/socket is
    read-only (which is the case in some nixos-containers with that folder
    bind-ro-mounted from the host).
    
    In these cases, the unit was skipped.
    
    Systemd 250 (rightfully) started to also skip in these cases:
    
    > [ 237.187747] systemd[1]: Nix Daemon Socket was skipped because of a failed condition check (ConditionPathIsReadWrite=/nix/var/nix/daemon-socket).

    However, systemd < 250 didn't skip if /nix/var/nix/daemon-socket/socket
    didn't /exist at all/, and we were relying on this bug in the case for
    fresh NixOS systems, to have /nix/var/nix/daemon-socket/socket created
    initially.

    Move the creation of that folder to systemd-tmpfiles, by shipping an
    appropriate file in `${nixPackage}/lib/tmpfiles.d/nix-daemon.conf`
    (NixOS/nix#6285).

    In the meantime, set a systemd tmpfiles rule manually in NixOS.

    This has been tested to still work with read-only bind-mounted
    /nix/var/nix/daemon-socket/socket in containers, it'll keep them
    read-only ;-)

This is ready for another review.

@flokli flokli marked this pull request as ready for review March 21, 2022 10:41
@flokli flokli requested a review from Ericson2314 March 21, 2022 10:42
@flokli flokli added the 1.severity: channel blocker Blocks a channel label Mar 21, 2022
The Nix-provided `nix-daemon.socket` file has a

> ConditionPathIsReadWrite=/nix/var/nix/daemon-socket/socket

line, to skip that unit if /nix/var/nix/daemon-socket/socket is
read-only (which is the case in some nixos-containers with that folder
bind-ro-mounted from the host).

In these cases, the unit was skipped.

Systemd 250 (rightfully) started to also skip in these cases:

> [ 237.187747] systemd[1]: Nix Daemon Socket was skipped because of a failed condition check (ConditionPathIsReadWrite=/nix/var/nix/daemon-socket).

However, systemd < 250 didn't skip if /nix/var/nix/daemon-socket/socket
didn't /exist at all/, and we were relying on this bug in the case for
fresh NixOS systems, to have /nix/var/nix/daemon-socket/socket created
initially.

Move the creation of that folder to systemd-tmpfiles, by shipping an
appropriate file in `${nixPackage}/lib/tmpfiles.d/nix-daemon.conf`
(NixOS/nix#6285).

In the meantime, set a systemd tmpfiles rule manually in NixOS.

This has been tested to still work with read-only bind-mounted
/nix/var/nix/daemon-socket/socket in containers, it'll keep them
read-only ;-)
@vcunat
Copy link
Member

vcunat commented Mar 21, 2022

This did not fix nixosTests.containers-ip for me.

@flokli
Copy link
Member Author

flokli commented Mar 21, 2022

It does build for me:

❯ nix-build -A nixosTests.containers-ip
/nix/store/391vynhri11j8p6fr4736zid3kp6vml4-vm-test-run-containers-ipv4-ipv6

@vcunat
Copy link
Member

vcunat commented Mar 21, 2022

Ah, I'm sorry, probably mistake of that non-NixOS machine. It does work for me on another one with NixOS.

@vcunat vcunat changed the base branch from staging-next to staging March 21, 2022 18:52
@vcunat vcunat merged commit 6facca0 into NixOS:staging Mar 21, 2022
@vcunat
Copy link
Member

vcunat commented Mar 21, 2022

The systemd update is not in staging-next yet, so I merged to staging.

@flokli
Copy link
Member Author

flokli commented Mar 22, 2022

@vcunat I checked out staging with this fix (6facca0) and successfully built nixosTests.containers-ip overnight:

❯ nix-build -A nixosTests.containers-ip
/nix/store/r7y65rkmdaz0p1j1lgbi4bq2m5phd8y5-vm-test-run-containers-ipv4-ipv6

@flokli flokli deleted the workaround-nix-daemon-socket branch March 22, 2022 09:41
@vcunat
Copy link
Member

vcunat commented Mar 22, 2022

And thanks for fixing this regression promptly.

@flokli
Copy link
Member Author

flokli commented Mar 22, 2022

Thanks! It'd still be nice if this can be properly fixed upstream: NixOS/nix#6285

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

Labels

1.severity: channel blocker Blocks a channel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 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 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants