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

Jump to bottom

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

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

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.

etu added a commit to etu/nixconfig that referenced this pull request May 5, 2019
@etu
hosts/kodi: Fix mount of NFS on boot
a2923cc 
Apply this suggestion to fix my NFS mount on boot:
NixOS/nixpkgs#60954
@etu
Copy link
Contributor

etu commented May 5, 2019 •
edited

@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 etu approved these changes

@domenkozar domenkozar Awaiting requested review from domenkozar

@obadz obadz Awaiting requested review from obadz

@Phreedom Phreedom Awaiting requested review from Phreedom

@rickynils rickynils Awaiting requested review from rickynils

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

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