Replace Network-interfaces.target

[groxxda]

Motivation for this change

This is ready for review and test

Systemd upstream provides 3 targets that should be used in services to allow ordering and dependencies for networking (network, network-pre, network-online)
Nixos adds at least network-interfaces.target to the list where it is not clear what it's purpose is.
To make network setup easier to understand for module developers this is an effor to get rid of the superfluous target.
This is a follow up on the removal of up-up.target and is not yet ready for merge.
I post this merely for early review and discussion (Hijacking an unrelated or was not a good idea in the first place)

Most commits should be safe to merge (getting closer to upstream stuff)

Especially the changes to the scripted network interface setup should be reviewed with care even though all networking tests passed with them.

See also https://github.com/joachifm/nixpkgs/tree/network-interfaces

Ping @joachifm

Next steps:

• Merge joachifm's branch
• Review all changes, especially the work on network-interfaces-scripted.nix
• Cleanup the mess that accumulated in all these commits

Things done

• Tested using sandboxing
  (nix.useSandbox on NixOS,
  or option build-use-sandbox in nix.conf
  on non-NixOS)
• Built on platform(s)
  • NixOS
  • OS X
  • Linux
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

[mention-bot]

@groxxda, thanks for your PR! By analyzing the annotation information on this pull request, we identified @wkennington, @offlinehacker and @rick68 to be potential reviewers

[groxxda]

I just rebased against current master with the changes from #18319
Here's a merge conflict that looks relevant:
@fpletz changed dhcpcd.service in c58654e to Before = network.target (like raspberrypi does) while I added After = network.target (like coreos chromiumos archlinux do).

[joachifm]

Hm, network.target is supposed to mean that networking functionality is "available". To me, the intuitive choice is to order dhcpcd after network.target.

Ordering before/after causes dhcpcd to be stopped after/before network.target on shutdown, not sure if that makes any practical difference to us, however.

[fpletz]

Before or After network.target for dhcpcd are both fine by me. It doesn't really matter because dhcpcd detects if networking and interfaces are or become available and acts accordingly.

[fpletz]

The changes to scripted network interfaces look fine to me.

[joachifm]

@groxxda how are you feeling about this? Is it ready from your pov? I think it's been up long enough that we can begin thinking about integrating it.

[groxxda]

@joachifm I would be happy to see this merged. The closer we are to the next release the higher the risk will be.
Note that I mainly tested with networkd (scripted networking on my laptop doesn't really count). I hope the scripted networking doesn't break..
Would be awesome if someone would test this first on a machine that uses scripted networking and has an advanced network configuration (@fpletz maybe take a shot at one of those cosmodromes? 🚀 🙈 )

[joachifm]

I'm working on integrating this today, going to do some testing and stuff just to be sure we're not breaking the world :)

[groxxda]

Thanks @joachifm 🎉 Let me know if there's something I can do. I'm happy to help out to get these changes merged

[joachifm]

@groxxda could you run a few of the nixos tests that might be affected? In particular, stuff like misc, etcd, openssh, firewall, and any other that looks important to you.

[joachifm]

I'm having a problem actually getting tests to run locally atm, stuff is just timing out on me for no obvious reason (with and without this PR).

[groxxda]

@joachifm I'll see if I can run some tests. Last time qemu segfaulted sporadically giving false-negatives..

By the way you should be able to push directly to my branch (Allow edits from maintainers is checked). Feel free to make use of it 👍

[joachifm]

I think we'll have to accept that some regressions may be introduced by this, so it's not a complete blocker seeing as nobody has brought up any fundamental objections thus far, but it'd be nice to not break everything at once :)

[groxxda]

Tests:
misc: 17 out of 17 tests succeeded
etcd: 2 out of 2 tests succeeded
firewall: finishes
openssh: 2 out of 2 tests succeeded
initrdNetwork: finishes

[groxxda]

nix-build '<nixpkgs/nixos/tests/networking.nix>' --arg networkd false finishes

[joachifm]

Alright, let's do it.

[groxxda]

Thanks again for reviewing and pushing this forward! 🍻

Also thanks @fpletz

[Mic92]

While this is a good description, what monit does it obscures the name if on the boot screen "Pro-active monitoring utility for unix systems started" appears.

[groxxda]

That's right. I simply took it from the upstream file at https://bitbucket.org/tildeslash/monit/src/e02247e0480b19be88e6a908886da83bbb1bc6a3/system/startup/monit.service.in?at=master
Should we revert?

[joachifm]

Sorry, I don't understand the problem? Is it that it doesn't mention the name of the unit being started?

[Mic92]

yes.

[joachifm]

A potential regression: https://hydra.nixos.org/build/41566104

[groxxda]

@joachifm More likely one of these times out because hydra load is high failures..
Looks similar to https://hydra.nixos.org/build/41444005

[edolstra]

@groxxda

This PR changed several units to be pulled in by network-setup.service rather than a target (network-interfaces.target). This is wrong. On a switch to a new configuration. network-setup.service is not necessarily restarted, so any new dependencies will not be started. Target units OTOH are treated specially by switch-to-configuration and are started even when they are already running to ensure new dependencies are activated

For example, if I add the following to my configuration:

  networking.interfaces.enp0s31f6.ip4 =
    [ { address = "192.168.1.9";
        prefixLength = 24;
      }
    ];

than this will have no effect until I reboot my system or explicitly start network-addresses-enp0s31f6.service.

Probably these units should be made wanted by one of the networking targets.

[edolstra]

I'm also worried about the use of partOf = [ "network-setup.service" ] in various units, which means that stopping network-setup.service during switch-to-configuration will kill all statically configured networking interfaces. (See NixOS/nixops#640, though there the dependent units are also stopped directly.)

[groxxda]

Replacing network-setup.services by network-setup.target should fix the problems, right?

[edolstra]

Yeah that sounds good.