Skip to content

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Mar 18, 2022

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

In container usecases, sometimes /nix/var/nix/daemon-socket is
bind-mounted read-only into the container.

In these cases, we want to skip starting nix-daemon.socket.

However, since systemd 250, ConditionPathIsReadWrite is also not met
if /nix/var/nix/daemon-socket doesn't exist at all. This means, a
regular NixOS system will skip starting nix-daemon.socket:

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

To prevent this from happening, ship a tmpfiles file that'll cause the
directory to be created if it doesn't exist already.

In the case of NixOS, we can just add Nix to systemd.tmpfiles.packages
and have these files picked up automatically.

…ectory

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

In container usecases, sometimes /nix/var/nix/daemon-socket is
bind-mounted read-only into the container.

In these cases, we want to skip starting nix-daemon.socket.

However, since systemd 250, `ConditionPathIsReadWrite` is also not met
if /nix/var/nix/daemon-socket doesn't exist at all. This means, a
regular NixOS system will skip starting nix-daemon.socket:

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

To prevent this from happening, ship a tmpfiles file that'll cause the
directory to be created if it doesn't exist already.

In the case of NixOS, we can just add Nix to `systemd.tmpfiles.packages`
and have these files picked up automatically.
@Ericson2314
Copy link
Member

@flokli this line of thinking makes sense to me, but I am never up to date on the systemd details I don't take my own judgement here very seriously :).

@Ericson2314
Copy link
Member

As the the container stuff, I like how this ignores it.

It would be good to reach out the systemd people on this, but I think there ought to be something more explicit about when a service inside the container is to be provided by outside the container. E.g. perhaps the inner one should have a different .service file that says "host delegated" or similar? (I am also unclear whether running systemd inside containers is standard practice or not.)

@flokli
Copy link
Contributor Author

flokli commented Mar 18, 2022

It would be good to reach out the systemd people on this, but I think there ought to be something more explicit about when a service inside the container is to be provided by outside the container. E.g. perhaps the inner one should have a different .service file that says "host delegated" or similar? (I am also unclear whether running systemd inside containers is standard practice or not.)

nixos-container create foo; nixos-container start foo; machinectl shell foo will get you into a NixOS container, with its own systemd. That's nothing new, and what the ConditionPathIsReadWrite= check was also used for - in the case of a bind-ro directory (what nixos-container start foo does), the nix-daemon startup was skipped. This also didn't regress, so we don't need to involve systemd into that.

What regressed was that the unit file is now getting skipped on the host, because ConditionPathIsReadWrite= now will also skip it the folder doesn't exist at all (which is reasonable, it previously just worked by accident).

The tmpfiles file here will now make sure a directory is present on the host, and will still properly skip as intended inside the container (and leave the read-only bind mount alone)

@Ericson2314
Copy link
Member

@flokli If the container has its own systemd, shouldn't it also have its own unit files (or "active" unit files?)? I would want the inner systemd to one of:

  1. Not know about the nix-daemon service at all. The socket just exists, and socket activation will stil lwork.
  2. Know it is provided by the external systemd, so in addition to the above, there could be a limited protocol also for the inside being able to request that the outside systemd start the service, recreate the socket, etc.

@flokli
Copy link
Contributor Author

flokli commented Mar 18, 2022 via email

@Ericson2314
Copy link
Member

Yeah agreed it is orthogonal. I leave it to @edolstra to review this, but this seems like a good thing to do. I made it so it is possible to disable the nix daemon in NixOS, and that should probably be used instead until systemd has the extra magic.

flokli added a commit to flokli/nixpkgs that referenced this pull request 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 ;-)
@flokli
Copy link
Contributor Author

flokli commented Mar 21, 2022

I made it so it is possible to disable the nix daemon in NixOS, and that should probably be used instead until systemd has the extra magic.

systemd doesn't need any extra magic for this. We just need to have something creating that folder, other than the .socket unit. For now, I moved this to a downstream systemd.tmpfile.rules in NixOS/nixpkgs#164644 to unblock the channels.

Shipping a tmpfiles file via lib/tmpfiles.d/nix-daemon.conf in Nix itself is the more canonical way to describe this, though, especially when interacting with other distros - so we should move to that once its in.

vcunat added a commit to NixOS/nixpkgs that referenced this pull request Mar 21, 2022
@flokli
Copy link
Contributor Author

flokli commented Mar 22, 2022

@edolstra can you review this? Something equivalent ended up downstream in nixpkgs, but other distros packaging Nix might run into the same issues if they use our provided nix-daemon.service and bump to systemd 250.

@edolstra
Copy link
Member

Should install-systemd-multi-user.sh be updated to install this file?

@flokli
Copy link
Contributor Author

flokli commented Mar 22, 2022

I wasn't aware of that script. I think so, yes. We should install it to /etc/tmpfiles.d/.

@flokli
Copy link
Contributor Author

flokli commented Mar 22, 2022

No, I read install-multi-user.sh, which executes install-systemd-multi-user.sh.

Its create_directories() function already does create the daemon folder, so we don't need to extend it.

@edolstra
Copy link
Member

Shouldn't it run systemd-tmpfiles --create to create the socket directory?

@Ericson2314
Copy link
Member

Ericson2314 commented Mar 22, 2022

@flokli I think @edolstra means, the systemd multi user installer does systemctl link "/nix/var/nix/profiles/default$SERVICE_SRC", but will that install the this new file too? Or just the service file?

@flokli
Copy link
Contributor Author

flokli commented Mar 22, 2022

install-multi-user.sh's create_directories() already creates the folder.

However, we can duplicate this in install-systemd-multi-user.sh too.

I made an attempt, assuming $out from Nix ends up in /nix/var/nix/profiles/default, and pushed it to this PR.

I lack the tooling to test this, review welcome.

…emon.conf, too

While `create_directories()` from install-multi-user.sh seems to already
create parts of the directory structure, it's marked as deprecated, and
it won't hurt also copying over the tmpfiles config and have it execute
once.
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

If the tempfiles thing is good (which I am still fundamentally not qualified to answer, but I think it is) this seems like the right way to do it.

@flokli
Copy link
Contributor Author

flokli commented Mar 24, 2022

@edolstra can you make a call here?

@edolstra edolstra merged commit c9148f4 into NixOS:master Mar 24, 2022
@flokli flokli deleted the add-tmpfile branch March 24, 2022 20:46
@flokli
Copy link
Contributor Author

flokli commented Mar 24, 2022

Thanks! I assume this should be backported to the *-maintenance branches?

What's the process here? Can you do a cherry-pick, or do I open individual PRs?

@edolstra
Copy link
Member

Before backporting, it would be great if somebody can test the installer with --daemon.

ln -sfn /nix/var/nix/profiles/default/$TMPFILES_SRC $TMPFILES_DEST

_sudo "to run systemd-tmpfiles once to pick that path up" \
sytemd-tmpfiles create --prefix=/nix/var/nix
Copy link
Member

Choose a reason for hiding this comment

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

typo
fixed in #6319

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants