Skip to content
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

nixos/tinc: remove ordering dependency on network.target #60231

Merged
merged 1 commit into from May 14, 2019

Conversation

lheckemann
Copy link
Member

This allows configuring IP addresses on a tinc interface using
networking.interfaces."tinc.${n}".ipv[46].addresses.

Previously, this would fail with timeouts, because of the dependency
chain

tinc.${netname}.service
--after--> network.target
--after--> network-addresses-tinc.${n}.service (and network-link-…)
--after--> sys-subsystem-net-devices-tinc.${n}.device

But the network interface doesn't exist until tinc creates it! So
systemd waits in vain for the interface to appear, and by then the
network-addresses-* and network-link-* units have failed. This leads
to the network link not being brought up and the network addresses not
being assigned, which in turn stops tinc from actually working.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @volth frequent contributor to tinc module

This allows configuring IP addresses on a tinc interface using
networking.interfaces."tinc.${n}".ipv[46].addresses.

Previously, this would fail with timeouts, because of the dependency
chain
tinc.${netname}.service
--after--> network.target
--after--> network-addresses-tinc.${n}.service (and network-link-…)
--after--> sys-subsystem-net-devices-tinc.${n}.device

But the network interface doesn't exist until tinc creates it! So
systemd waits in vain for the interface to appear, and by then the
network-addresses-* and network-link-* units have failed. This leads
to the network link not being brought up and the network addresses not
being assigned, which in turn stops tinc from actually working.
@lheckemann
Copy link
Member Author

Are there any disadvantages to this solution though?

@lheckemann
Copy link
Member Author

I'll just go ahead and merge this if I don't hear anything by Monday, given that this is for master and we're still early in the 19.09 release cycle.

@nh2
Copy link
Contributor

nh2 commented May 8, 2019

I've tested this shortly in my staging deployment, seems to work OK (though I didn't analyse it in great detail).

So merging is fine by me.

@lheckemann
Copy link
Member Author

@volth are you planning to test this? If not I'd just go ahead and merge it.

@lheckemann lheckemann merged commit 2b13c29 into NixOS:master May 14, 2019
@lheckemann lheckemann deleted the tinc-allow-networking-interfaces branch May 14, 2019 15:51
@lheckemann
Copy link
Member Author

I've added

{
        wantedBy = lib.mkForce ["sys-subsystem-net-devices-tinc.lugnet.device"];
        before = lib.mkForce [];
        partOf = ["sys-subsystem-net-devices-tinc.lugnet.device"];
        after = ["sys-subsystem-net-devices-tinc.lugnet.device"];
        serviceConfig.Type = lib.mkForce "simple";
        serviceConfig.Restart = "on-failure";
        serviceConfig.RestartSec = 10;
}

to systemd.services.network-{addresses,link}."tinc.foo" in my own config, which seems to work. but might include some unnecessary bits…

@minijackson
Copy link
Member

For me that did the trick:

systemd.services."tinc.${meshName}".after = [ "tinc.${meshName}-netdev.service" ];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants