-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
systemd: Don't order systemd-user-sessions.service after network.target #61098
Conversation
Instead of patching the upstream unit which causes an unecessarry mass rebuild, we can override it within the systemd nixos module nixpkgs/nixos/modules/system/boot/systemd.nix Line 921 in 15d6b77
|
Madness, all madness! 😆 . This all seems like we've made things a bit of a mess? I apologize for any misunderstanding, but here's what it looks like:
I suppose I'm inclined to say we just revert the other change and.... add a dependency on network-online,target from cloud-init? (err surely it's not that simple? sorry) Thanks for working on this, hopefully we can get this sorted soon. See also #60954 , I'm working on related issues as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really not in favour of increasing our amount of patches on top of systemd.
Reading up on #60900 and #44524 (comment):
It seems adding dhcpcd.service
to network.target
is wrong, it should be requiredBy=network-online.target
(only when dhcpcd is enabled globally), and cloud-init.service
should set Wants/After=network-online.target
. systemd provides (three) network-specific targets, and we should use them:
Units that strictly require a configured network connection should pull in network-online.target (via a Wants= type dependency) and order themselves after it. This target unit is intended to pull in a service that delays further execution until the network is sufficiently set up. What precisely this requires is left to the implementation of the network managing service.
As described in #44524 (comment), dhcpcd's fork behaviour might become a bit unpredictable in dualstacked configurations. But I hope this won't become too much of an headache, given it only affects defaults (and most people configure their networks somehow), and we'd like to switch over to networkd anyways, which allows much more fine-grade control on when to reach network-online.target
.
Unfortunately, this doesn't appear to be possible for dependency configuration. All nix-specified unit options are placed in a drop-in file:
But drop-ins are unable to remove dependencies, only add them, as explained in the small print at the end of
I suppose we ought to find a better way to deal with this.
but while it works in
Is that expected? It seems strange that there's no error at all. |
I gazed long into the bug, and the bug also gazed into me :)
That's quite alright, I'm just glad anyone is interested!
Re-reading #44524, I think this is pretty much it, actually.
Haha! If I understand correctly (certainly this is how it's documented - see the manpage), This is why I referred to the upstream change as a hack - it only happens to work for SSH because disabling logins with PAM will forcibly close the inbound SSH sessions. Instead, systemd should have a principled way to terminate those sessions either 1) when It's true, I think, that killing user logins will kill any services running under users who are not set to linger. However,
It waits to acquire a lease, so that depends how fast your DHCP server is ;) it may take several seconds even if nothing is wrong. By default,
I think this particular fix is already in place.
Cheers mate, keep fighting the good fight! |
Hah, yes, I see what you mean. Though unless I'm missing something, the existing ones are Debian's patches, not ours. Running (Actually, I don't understand this at all. If we are maintaining our own branches of systemd, why are we pulling patches from other people??)
Agreed, for sure! However... I would like to clarify that this change is not merely a workaround for a bad interaction with |
These branches add some NixOS-specific things on top of systemd, but while doing so, also accidentially break systemd-tmpfiles - see #56265 (comment) for that discussion
Yes, please engage upstream in the discussion, that's where these things should be solved :-) |
@gloaming can this be rebased and rechecked across current master (contains systemd 242)? Did you check back with upstream on how this should be done? |
Revert upstream commit 8c85680 This was causing delayed logins at boot waiting on dhcpcd. Fixes NixOS#60900
6961847
to
3b0a8c6
Compare
Rebased and tested locally using @flokli I haven't contacted upstream yet, I'll try to get that done in the next couple of days. I realised why the overrideDerivation in |
I filed a PR at NixOS/systemd#28, which seems more logical than adding a patch in Nixpkgs. |
@gloaming we should contact upstream (github.com/systemd/systemd) first, before adding custom patches on our fork of systemd. |
Upstream issue filed at systemd/systemd#12853 |
@gloaming reading into We fixed the cloud-init problem descibed at 48f7778 in 57840db by setting Can you check if changing |
@flokli Yes, we should do that for sure. I still think upstream should fix the other ordering, but once we fix I've retested that ordering against this morning's |
I'm personally fine with how upstream did their ordering, and how they define `network.target`:
network.target has very little meaning during start-up. It only indicates that the network management stack is up after it has been reached. Whether any network interfaces are already configured when it is reached is undefined. Its primary purpose is for ordering things properly at shutdown: since the shutdown ordering of units in systemd is the reverse of the startup ordering, any unit that is order After=network.target can be sure that it is stopped before the network is shut down if the system is powered off. This allows services to cleanly terminate connections before going down, instead of abruptly losing connectivity for ongoing connections, leaving them in an undefined state.
We just have dhcpcd being part of the wrong target, so yes, please file a PR fixing this against master :-)
|
Closed in favour of #64621. |
Fix delayed startups waiting for DHCP, see #60900
The startup time issue seems much more annoying to me than the original SSH session issue, so I think we should merge this now. If others disagree, I guess we wait until we have a proper fix for the latter.
Motivation for this change
Improve boot times for local NixOS users.
Things done
I tested by overriding the package in
configuration.nix
:sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)