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

units: use Requires in systemd-networkd-wait-online.service #6065

Merged
merged 1 commit into from Jul 3, 2017

Conversation

keszybz
Copy link
Member

@keszybz keszybz commented Jun 1, 2017

tl;dr: make it possible to start the network at any time by starting network-online.target

In the initial design, foobar-wait-online.service would have
Requisite=foobar.service, so that foobar-wait-online.service could be enabled
unconditionally, irrespective of whether foobar.service itself is enabled.
Unfortunately this doesn't work too well:

  1. the message about foobar-wait-online.service being skipped because of a
    "missing dependency" looks like there is a problem. This is mostly cosmetic,
    but it also quite confusing. We generally don't want any messages of this
    type during default boot.

  2. it is impossible to start and wait for the network in an
    implementation-agnostic way: systemctl start network-online.target, or
    Wants/After=network-online.target in a unit don't work because pulling in
    network-online.target pulls in foobar-wait-online.service, but it in turn
    does not pull in foobar.service. During startup, foobar.service is pulled in
    by multi-user.target, but not in a smaller transaction which does not
    include multi-user.target.

This change means that *-wait-online.service should be installed through
presets, so that it can be enabled/disabled at will by the administrator.
Our own systemd-networkd-wait-online.service does this already, and
similar change has been requested for NetworkManager-wait-online.service
(https://bugzilla.redhat.com/show_bug.cgi?id=1455704).

This change should by mostly backwards-compatible, unless somebody has some
wait-online.service enabled, without having the corresponding network
implementation enabled, and they are relying on it not being started. I think
that's relatively unlikely because of issue 1. above, and I'm not aware of this
being the default in any distro. And being able to start the network in an
implementation-agnostic way is pretty important, see
https://bugzilla.redhat.com/show_bug.cgi?id=1452866.

I know this could be controversial, but I think the reasons for the change are good.
If this route is accepted here, I'll post the corresponding change for NetworkManager.

In the initial design, foobar-wait-online.service would have
Requisite=foobar.service, so that foobar-wait-online.service could be enabled
unconditionally, irrespective of whether foobar.service itself is enabled.
Unfortunately this doesn't work too well:

1. the message about foobar-wait-online.service being skipped because of a
   "missing dependency" *looks* like an is problem. This is mostly cosmetic,
   but it also quite confusing. We generally don't want any messages of this
   type during default boot.

2. it is impossible to start and wait for the network in an
   implementation-agnostic way: systemctl start network-online.target, or
   Wants/After=network-online.target in a unit don't work because pulling in
   network-online.target pulls in foobar-wait-online.service, but it in turn
   does not pull in foobar.service. During startup, foobar.service is pulled in
   by multi-user.target, but not in a smaller transaction which does not
   include multi-user.target.

This change means that *-wait-online.service should be installed through
presets, so that it can be enabled/disabled at will by the administrator.
Our own systemd-networkd-wait-online.service does this already, and
similar change has been requested for NetworkManager-wait-online.service
(https://bugzilla.redhat.com/show_bug.cgi?id=1455704).

This change should by mostly backwards-compatible, unless somebody has some
wait-online.service enabled, without having the corresponding network
implementation enabled, and they are relying on it not being started.  I think
that's relatively unlikely because of issue 1. above, and I'm not aware of this
being the default in any distro. And being able to start the network in an
implementation-agnostic way is pretty important, see
https://bugzilla.redhat.com/show_bug.cgi?id=1452866.
@keszybz keszybz added the units label Jun 1, 2017
@arvidjaar
Copy link
Contributor

(https://bugzilla.redhat.com/show_bug.cgi?id=1455704).

It does not look like good justification to force this change. If some unit requires network, it should tell so in unit definition, not expect it to magically be started via unrelated unit.

And if you are doing such change, at the very least extend documentation to clearly state that network-online.target forcibly starts network.target. So far documentation makes it quite explicit that both are independent.

@keszybz
Copy link
Member Author

keszybz commented Jun 10, 2017

(https://bugzilla.redhat.com/show_bug.cgi?id=1455704).

It does not look like good justification to force this change. If some unit requires network, it should tell so in unit definition, not expect it to magically be started via unrelated unit.

That bug is not the justification. It's a follow-up to align NM with sd-networkd and the new scheme of things.

So far documentation makes it quite explicit that both are independent.

Hm, what part of the docs you have in mind?

@keszybz
Copy link
Member Author

keszybz commented Jul 2, 2017

bump

@poettering poettering merged commit 9db3078 into systemd:master Jul 3, 2017
stewartsmith pushed a commit to stewartsmith/pkg-fedora-NetworkManager that referenced this pull request Oct 25, 2021
- systemd: let NM-w-o.service require NetworkManager service (rh #1452866)
  Related to rh #1455704.
  See also: systemd/systemd#6065

- platform: really treat dsa devices as regular wired ethernet (rh #1371289)
  actually apply patch.

- in spec file, explicitly set dhclient as default (resync with upstream
  spec file).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants