Skip to content

Commit

Permalink
Revert "nixos/systemd.nix: don’t require online for multi-user.target"
Browse files Browse the repository at this point in the history 
This reverts commit 764c820.

While this is desireable in principle, some of our modules and services
fail during service startup if no network is available don't currently
properly set Wants=network-online.target.

If nothing pulls in this target anymore, systemd won't try to reach it.

We have many VM tests waiting for `network-online.target`, and after
764c820 fail with the following error
message:

```
error: unit "network-online.target" is inactive and there are no pending jobs
```

Most likely, test scripts shouldn't wait for `network-online.target` in
first place (as `network-online.target` says nothing about whether a
service has been started), but instead, the script should wait for the
network ports of the corresponding service to be open.

Let's revert this for now, and re-apply in a draft PR, fixing the tests
before merging it back in.
  • Loading branch information
@flokli
flokli committed May 1, 2020
1 parent 448db2e commit 15d761a
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions nixos/modules/system/boot/systemd.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,7 @@ in
systemd.services.systemd-journald.stopIfChanged = false;
systemd.targets.local-fs.unitConfig.X-StopOnReconfiguration = true;
systemd.targets.remote-fs.unitConfig.X-StopOnReconfiguration = true;
systemd.targets.network-online.wantedBy = [ "multi-user.target" ];
systemd.services.systemd-binfmt.wants = [ "proc-sys-fs-binfmt_misc.mount" ];

# Don't bother with certain units in containers.
Expand Down

6 comments on commit 15d761a

@RobbieGM
Copy link

Choose a reason for hiding this comment

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

This is a shame, and I think newcomers to NixOS should be made well aware of this so they can decide for themselves whether to override it for a faster boot time. In most cases it won't break anything to remove it and will just improve performance.

@SaltyKitkat
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anything still blocking this change for now?

@flokli
Copy link
Contributor Author

@flokli flokli commented on 15d761a Apr 18, 2023

Choose a reason for hiding this comment

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

I'm not sure I ever got around to open the draft PR. Any chance you could pick this up? Happy to review it.

@akho
Copy link
Contributor

@akho akho commented on 15d761a Sep 27, 2023

Choose a reason for hiding this comment

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

I'm not sure I ever got around to open the draft PR. Any chance you could pick this up? Happy to review it.

What would be involved in that? I’m somewhat interested in this, but not man-weeks interested :)

A PR removing the line is easy to submit, but I don’t think I understand the full scope of the issue — you mention VM tests failing; how would I find out which ones?

@nixos-discourse
Copy link

Choose a reason for hiding this comment

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

This commit has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/why-does-multi-user-target-depend-on-network-online-target/33565/5

@rnhmjoj
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 try to build all VM tests (in the nixosTests attrset), but it may be too much depending on your hardware and patience.
A better idea would probably be to ask someone in the infra team to set up a branch for this and let hydra run the tests.

Please sign in to comment.