Skip to content

networkmanager: network-online --wants--> NetworkManager-wait-online #60954

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 31, 2019
Merged

networkmanager: network-online --wants--> NetworkManager-wait-online #60954

merged 1 commit into from
May 31, 2019
+4 −0

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented May 5, 2019

Motivation for this change

Services relying on network-online.target were running before
the network was ready, every time.

Looking at why this was happening, it seems our network-online
had no actual dependency on the appropriate connectivity check
when using networkmanager, namely NetworkManager-wait-online.service.

(we get this right for configurations using networkd, I believe)

Looking at networkmanager's provided service
etc/systemd/system/NetworkManager-wait-online.service there's
a WantedBy in [Install] that should be accounted for.

Running this command before/after the included commit should
help see the difference:

(this is after:)

$ systemctl show -p Wants,After network-online.target
Wants=NetworkManager-wait-online.service
After=network.target NetworkManager-wait-online.service

Now my services don't all complain when trying to run
just a tiny bit too early in my boot :).

If there's agreement this is the correct configuration
please 'backport' to at least 19.03.

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.

Sorry, something went wrong.

@dtzWill
networkmanager: network-online --wants--> NetworkManager-wait-online

Partially verified

This commit is signed with the committer’s verified signature.
domenkozar’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
4588661
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 5, 2019
etu added a commit to etu/nixconfig that referenced this pull request May 5, 2019
@etu
hosts/kodi: Fix mount of NFS on boot

Verified

This commit was signed with the committer’s verified signature.
domenkozar Domen Kožar
GPG key ID: C2FFBCAFD2C24246
Verified
Learn about vigilant mode
a2923cc 
Apply this suggestion to fix my NFS mount on boot:
NixOS/nixpkgs#60954
@etu
Copy link
Contributor

etu commented May 5, 2019 •
edited
Loading

@dtzWill I've just tried this on my 19.03 system that didn't run NetworkManager and had this very same issue. Now it runs network manager with this wait online target added and it works like a charm. So I'm very much in favor of pulling this in, not sure about @NixOS/backports though.

Also, this same issue does exists with the regular networking.dhcpcd.enable, but yeah, that's another issue.

@samueldr
Copy link
Member

samueldr commented May 5, 2019

I feel backporting should be fine, considering this is a bugfix, unless it is expected that adding this dependency could break setups.

I'm not familiar enough with systemd to say for sure if this could cause issues for dependency chains. Though from what I know it feels alright.

@samueldr samueldr added the 9.needs: port to stable A PR needs a backport to the stable release. label May 5, 2019
@dtzWill
Copy link
Member Author

dtzWill commented May 6, 2019

(requesting review from networkmanager maintainers)

@dtzWill dtzWill mentioned this pull request May 8, 2019
Closed
10 tasks
@dtzWill
Copy link
Member Author

dtzWill commented May 21, 2019

Beep boop, is lack of feedback approval or a request for more time?

@dtzWill
Copy link
Member Author

dtzWill commented May 31, 2019

Welp, here we go. Hopefully it fixes more than it breaks overall. LMK if you have questions/problems.

@dtzWill dtzWill merged commit a72d6f9 into NixOS:master May 31, 2019
@dtzWill dtzWill deleted the fix/network-online-actually-online-with-networkmanager branch May 31, 2019 04:06
@fpletz
Copy link
Member

fpletz commented Jun 6, 2019

Thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etu etu

@domenkozar domenkozar

@obadz obadz

@Phreedom Phreedom

@rickynils rickynils

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dtzWill @etu @samueldr @fpletz