NixOS-specific systemd patch makes tmpfiles.d unreliable

[jameysharp]

Many NixOS modules now rely on systemd.tmpfiles.rules to set up files and directories needed for their service units to start. There used to be documentation advising against doing so, but that documentation was removed in 2017 in 43404d9, and in a quick search I found quite a few recent merged pull requests adding more uses of tmpfiles.d.

However, a 2014 patch against systemd by @edolstra (NixOS/systemd@91702db) makes tmpfiles.d unreliable for this purpose.

Normal systemd service units automatically depend on sysinit.target being reached Before the service. Upstream's systemd-tmpfiles-setup.service also explicitly declares that it must finish Before the sysinit.target unit. So normally services can rely on their tmpfiles having been created before the service starts.

In the NixOS fork of systemd, systemd-tmpfiles-setup.service no longer has a Before dependency declared on sysinit.target, which means that regular services can start before systemd-tmpfiles has finished. So there's a race condition that may sometimes prevent a service from starting.

In practice, systemd-tmpfiles is pretty fast, and even if a service fails to start systemd will probably retry it, so observing this bug in the wild is probably pretty unlikely. Still, I'd be a lot happier if this race condition were closed entirely.

The original commit message said that this patch was to avoid an indirect dependency from sshd.service on local-fs.target, because that interferes somehow with the NixOps send-keys feature. But it didn't explain why that was a problem, so I can't tell whether it's still an issue today. If it's still important, there are probably better ways to fix it, like perhaps setting DefaultDependencies=no on sshd.service and explicitly listing its true dependencies, but I can't be sure without an explanation of the underlying NixOps issue.

I'd encourage reverting NixOS/systemd@91702db and its rebased equivalents in any branches that are still used by supported versions of NixOS. I'm happy to help brainstorm alternative fixes that don't have such widespread side effects, given some pointers on what problem this patch was actually trying to solve.

[Mic92]

Here is the send-keys feature for reference: https://github.com/NixOS/nixops/blob/master/nix/keys.nix#L249

[jameysharp]

@Mic92 Thanks for the pointer. I also needed to see the other half of the implementation (e.g. https://github.com/NixOS/nixops/blob/v1.6.1/nixops/backends/__init__.py#L209). It seems to me that the best fix for that is to override sshd.service with these options:

DefaultDependencies=no
Conflicts=shutdown.target
Before=shutdown.target multi-user.target

That drops sshd.service's After dependencies on basic.target and sysinit.target but should keep all the other dependencies it has today.

I'd apply this override in nixops/nix/keys.nix so it doesn't affect any NixOS configurations that aren't deployed using NixOps. You could even limit it to !config.deployment.storeKeysOnMachine && config.deployment.keys != {}.

@volth True, but switch-to-configuration re-runs systemd-tmpfiles before (re)starting units, as of NixOS 17.03 (4e4161c). So the issue I described is the only one I'm aware of at this point.

[andir]

This could be resolved with the work on #61321.

The workarounds added for nixops send-keys is a hack. With #61321 and adding the mount option _netdev to the crypto volumes provide the same feature level. Problem there is that (currently) without _netdev the system will fail to mount the local-fs.target.

The nixops configuration I used to verify this on 19.03 and my systemd v242 PR is the following:

{
  server = {
    deployment.targetEnv = "libvirtd";
    fileSystems."/secret" = {
      device = "/dev/mapper/secretdisk";
      options = [ "_netdev" ];
    };
    deployment.autoLuks = {
      secretdisk = {
        device = "/dev/vda";
        passphrase = "foobar";
        autoFormat = true;
      };
    };
  };
}

(I had to manually add the volume via virt-manager)

See also #56265 (comment) for a related discussion (cc @flokli).

[jameysharp]

The _netdev mount option sounds like a great fix to me! Does that require people to change their existing NixOps configurations, or is there a reasonable way to automatically set that flag?

If you specify that mount option, is the systemd upgrade even necessary? Even systemd 239 from NixOS 19.03 documents that mount option in systemd.mount(5).

[andir]

The system upgrade is not required but I am working on this as part of the upgrade (and an ongoing effort to remove more custom patches from systemd). So the behavior will likely only change with 19.09 (or the unstable equivalent).

Right now I am trying to figure out how to make the migration without breaking everyone's NixOps deployments.

We had a bit of a discussion on #nixos-dev last night and adding an additional (required) option to the autoLuks modules seems like the best approach so far. I am going to work on a NixOps PR today. Will crosslink once it is ready.

The issue with automagically adding the mount option to all the fstab entries is that we can not tell which of them depends on an autoLuks managed device. NixOS usually uses UUIDs for disks and there can be many layers of indirections we don't know of (LUKS, RAID, ZFS, …).

With a new mandatory autoLuks.<name>.mountPoint option we could ensure that people add the respective mountpoints to the autoLuks module and THEN we can (with higher chances of success) add the _netdev option to that path.

[flokli]

@jameysharp systemd 242 was merged to master, with patches mentioned in the OP reverted. deployment.autoLuks is discussed in #62211, but I guess the original issue here is fixed 🎉

[jameysharp]

Hooray, thank you!