From 0729b8c55e0dfaf302af4c57546871d47a652048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= Date: Fri, 13 Mar 2020 21:47:07 +0100 Subject: [PATCH] Revert Merge #82310: nixos/systemd: apply .link ...even when networkd is disabled This reverts commit ce78f3ac701017008aa7f1db387b871b7ae65e01, reversing changes made to dc34da0755b3c36469965659c0ee4a1337e81c05. I'm sorry; Hydra has been unable to evaluate, always returning > error: unexpected EOF reading a line and I've been unable to reproduce the problem locally. Bisecting pointed to this merge, but I still can't see what exactly was wrong. --- nixos/doc/manual/release-notes/rl-2003.xml | 8 -- .../services/networking/zerotierone.nix | 17 ++-- nixos/modules/system/boot/networkd.nix | 79 ++++++++----------- nixos/tests/networking.nix | 60 +++++--------- 4 files changed, 59 insertions(+), 105 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2003.xml b/nixos/doc/manual/release-notes/rl-2003.xml index a123be2ccf4..918906d27e7 100644 --- a/nixos/doc/manual/release-notes/rl-2003.xml +++ b/nixos/doc/manual/release-notes/rl-2003.xml @@ -712,14 +712,6 @@ auth required pam_succeed_if.so uid >= 1000 quiet For further reference, please read #68953 or the corresponding discourse thread. - - - The systemd.network.links option is now respected - even when systemd-networkd is disabled. - This mirrors the behaviour of systemd - It's udev that parses .link files, - not systemd-networkd. - - diff --git a/nixos/modules/services/networking/zerotierone.nix b/nixos/modules/services/networking/zerotierone.nix index cf39ed065a7..042c4d5addd 100644 --- a/nixos/modules/services/networking/zerotierone.nix +++ b/nixos/modules/services/networking/zerotierone.nix @@ -69,14 +69,13 @@ in environment.systemPackages = [ cfg.package ]; # Prevent systemd from potentially changing the MAC address - systemd.network.links."50-zerotier" = { - matchConfig = { - OriginalName = "zt*"; - }; - linkConfig = { - AutoNegotiation = false; - MACAddressPolicy = "none"; - }; - }; + environment.etc."systemd/network/50-zerotier.link".text = '' + [Match] + OriginalName=zt* + + [Link] + AutoNegotiation=false + MACAddressPolicy=none + ''; }; } diff --git a/nixos/modules/system/boot/networkd.nix b/nixos/modules/system/boot/networkd.nix index 3078f84f6e9..6dfbe66fc64 100644 --- a/nixos/modules/system/boot/networkd.nix +++ b/nixos/modules/system/boot/networkd.nix @@ -355,14 +355,6 @@ let }; linkOptions = commonNetworkOptions // { - # overwrite enable option from above - enable = mkOption { - default = true; - type = types.bool; - description = '' - Whether to enable this .link unit. It's handled by udev no matter if systemd-networkd is enabled or not - ''; - }; linkConfig = mkOption { default = {}; @@ -1053,49 +1045,44 @@ in }; - config = mkMerge [ - # .link units are honored by udev, no matter if systemd-networkd is enabled or not. - { - systemd.network.units = mapAttrs' (n: v: nameValuePair "${n}.link" (linkToUnit n v)) cfg.links; - environment.etc = unitFiles; - } + config = mkIf config.systemd.network.enable { - (mkIf config.systemd.network.enable { + users.users.systemd-network.group = "systemd-network"; - users.users.systemd-network.group = "systemd-network"; + systemd.additionalUpstreamSystemUnits = [ + "systemd-networkd.service" "systemd-networkd-wait-online.service" + ]; - systemd.additionalUpstreamSystemUnits = [ - "systemd-networkd.service" "systemd-networkd-wait-online.service" - ]; + systemd.network.units = mapAttrs' (n: v: nameValuePair "${n}.link" (linkToUnit n v)) cfg.links + // mapAttrs' (n: v: nameValuePair "${n}.netdev" (netdevToUnit n v)) cfg.netdevs + // mapAttrs' (n: v: nameValuePair "${n}.network" (networkToUnit n v)) cfg.networks; - systemd.network.units = mapAttrs' (n: v: nameValuePair "${n}.netdev" (netdevToUnit n v)) cfg.netdevs - // mapAttrs' (n: v: nameValuePair "${n}.network" (networkToUnit n v)) cfg.networks; + environment.etc = unitFiles; - systemd.services.systemd-networkd = { - wantedBy = [ "multi-user.target" ]; - restartTriggers = attrNames unitFiles; - # prevent race condition with interface renaming (#39069) - requires = [ "systemd-udev-settle.service" ]; - after = [ "systemd-udev-settle.service" ]; + systemd.services.systemd-networkd = { + wantedBy = [ "multi-user.target" ]; + restartTriggers = attrNames unitFiles; + # prevent race condition with interface renaming (#39069) + requires = [ "systemd-udev-settle.service" ]; + after = [ "systemd-udev-settle.service" ]; + }; + + systemd.services.systemd-networkd-wait-online = { + wantedBy = [ "network-online.target" ]; + }; + + systemd.services."systemd-network-wait-online@" = { + description = "Wait for Network Interface %I to be Configured"; + conflicts = [ "shutdown.target" ]; + requisite = [ "systemd-networkd.service" ]; + after = [ "systemd-networkd.service" ]; + serviceConfig = { + Type = "oneshot"; + RemainAfterExit = true; + ExecStart = "${config.systemd.package}/lib/systemd/systemd-networkd-wait-online -i %I"; }; + }; - systemd.services.systemd-networkd-wait-online = { - wantedBy = [ "network-online.target" ]; - }; - - systemd.services."systemd-network-wait-online@" = { - description = "Wait for Network Interface %I to be Configured"; - conflicts = [ "shutdown.target" ]; - requisite = [ "systemd-networkd.service" ]; - after = [ "systemd-networkd.service" ]; - serviceConfig = { - Type = "oneshot"; - RemainAfterExit = true; - ExecStart = "${config.systemd.package}/lib/systemd/systemd-networkd-wait-online -i %I"; - }; - }; - - services.resolved.enable = mkDefault true; - }) - ]; + services.resolved.enable = mkDefault true; + }; } diff --git a/nixos/tests/networking.nix b/nixos/tests/networking.nix index a519ca06594..933a4451af9 100644 --- a/nixos/tests/networking.nix +++ b/nixos/tests/networking.nix @@ -5,10 +5,11 @@ , networkd }: with import ../lib/testing-python.nix { inherit system pkgs; }; +with pkgs.lib; let - router = { config, pkgs, lib, ... }: - with lib; + router = { config, pkgs, ... }: + with pkgs.lib; let vlanIfs = range 1 (length config.virtualisation.vlans); in { @@ -84,7 +85,7 @@ let static = { name = "Static"; nodes.router = router; - nodes.client = { pkgs, lib, ... }: with lib; { + nodes.client = { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 2 ]; networking = { useNetworkd = networkd; @@ -136,7 +137,7 @@ let dhcpSimple = { name = "SimpleDHCP"; nodes.router = router; - nodes.client = { pkgs, lib, ... }: with lib; { + nodes.client = { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 2 ]; networking = { useNetworkd = networkd; @@ -192,7 +193,7 @@ let dhcpOneIf = { name = "OneInterfaceDHCP"; nodes.router = router; - nodes.client = { pkgs, lib, ... }: with lib; { + nodes.client = { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 2 ]; networking = { useNetworkd = networkd; @@ -231,7 +232,7 @@ let ''; }; bond = let - node = address: { pkgs, lib, ... }: with lib; { + node = address: { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 2 ]; networking = { useNetworkd = networkd; @@ -267,7 +268,7 @@ let ''; }; bridge = let - node = { address, vlan }: { pkgs, lib, ... }: with lib; { + node = { address, vlan }: { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ vlan ]; networking = { useNetworkd = networkd; @@ -280,7 +281,7 @@ let name = "Bridge"; nodes.client1 = node { address = "192.168.1.2"; vlan = 1; }; nodes.client2 = node { address = "192.168.1.3"; vlan = 2; }; - nodes.router = { pkgs, lib, ... }: with lib; { + nodes.router = { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 2 ]; networking = { useNetworkd = networkd; @@ -317,7 +318,7 @@ let macvlan = { name = "MACVLAN"; nodes.router = router; - nodes.client = { pkgs, lib, ... }: with lib; { + nodes.client = { pkgs, ... }: with pkgs.lib; { environment.systemPackages = [ pkgs.iptables ]; # to debug firewall rules virtualisation.vlans = [ 1 ]; networking = { @@ -371,7 +372,7 @@ let ''; }; sit = let - node = { address4, remote, address6 }: { pkgs, lib, ... }: with lib; { + node = { address4, remote, address6 }: { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 ]; networking = { useNetworkd = networkd; @@ -413,7 +414,7 @@ let ''; }; vlan = let - node = address: { pkgs, lib, ... }: with lib; { + node = address: { pkgs, ... }: with pkgs.lib; { #virtualisation.vlans = [ 1 ]; networking = { useNetworkd = networkd; @@ -526,7 +527,7 @@ let ''; }; }; - nodes.client_with_privacy = { pkgs, lib, ... }: with lib; { + nodes.client_with_privacy = { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 ]; networking = { useNetworkd = networkd; @@ -539,7 +540,7 @@ let }; }; }; - nodes.client = { pkgs, lib, ... }: with lib; { + nodes.client = { pkgs, ... }: with pkgs.lib; { virtualisation.vlans = [ 1 ]; networking = { useNetworkd = networkd; @@ -602,9 +603,9 @@ let testScript = '' targetIPv4Table = """ - 10.0.0.0/16 proto static scope link mtu 1500 - 192.168.1.0/24 proto kernel scope link src 192.168.1.2 - 192.168.2.0/24 via 192.168.1.1 proto static + 10.0.0.0/16 proto static scope link mtu 1500 + 192.168.1.0/24 proto kernel scope link src 192.168.1.2 + 192.168.2.0/24 via 192.168.1.1 proto static """.strip() targetIPv6Table = """ @@ -654,33 +655,8 @@ let ), "The IPv6 routing table has not been properly cleaned:\n{}".format(ipv6Residue) ''; }; - # even with disabled networkd, systemd.network.links should work - # (as it's handled by udev, not networkd) - link = { - name = "Link"; - nodes.client = { pkgs, ... }: { - virtualisation.vlans = [ 1 ]; - networking = { - useNetworkd = networkd; - useDHCP = false; - }; - systemd.network.links."50-foo" = { - matchConfig = { - Name = "foo"; - Driver = "dummy"; - }; - linkConfig.MTUBytes = "1442"; - }; - }; - testScript = '' - print(client.succeed("ip l add name foo type dummy")) - print(client.succeed("stat /etc/systemd/network/50-foo.link")) - client.succeed("udevadm settle") - assert "mtu 1442" in client.succeed("ip l show dummy0") - ''; - }; }; -in pkgs.lib.mapAttrs (pkgs.lib.const (attrs: makeTest (attrs // { +in mapAttrs (const (attrs: makeTest (attrs // { name = "${attrs.name}-Networking-${if networkd then "Networkd" else "Scripted"}"; }))) testCases