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

WIP: Run NM via systemd unit, don't depend on ip=dhcp kargs #321

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

See https://github.com/coreos/ignition-dracut/issues/94
and coreos/ignition#948

Needs pairing with a cosa PR to drop the default ip=dhcp kargs.

And yes we really want to upstream this into NM by default or so.

@dustymabe
Copy link
Member

I do really want NM in the initrd and I've talked with the NM team about it (see the RFE they asked me to open), but I think it's risky to move away from the current NM initrd implementation today. I'd prefer if we kept the initrd implementation to the defaults (and hopefully NM upstream will move to NM started by systemd in the initrd soon).

An alternative way to achieve the goal of booting a live ISO without networking is to dynamically add some dracut networking args at runtime if some specific conditions are met. The POC is in #326. This way will allow us to keep the same NM initrd implementation that is the default in NM upstream.

@jlebon
Copy link
Member

jlebon commented Mar 30, 2020

coreos/ignition-dracut#164 works on top of this by only activating network.target if the Ignition config requires it.

Hmm so actually we can delete the nm-online.conf dropin from here.

@cgwalters
Copy link
Member Author

but I think it's risky to move away from the current NM initrd implementation today.

That's a reasonable position. OTOH, this approach is still running NM as a oneshot unit - we're not trying to run as a daemon, which would definitely be a bigger change.

IOW, we're just lifting the execution out of the abandoned swap of the initqueue into the shiny modern glass office tower of systemd units.

@cgwalters
Copy link
Member Author

Another highly important thing going on here is that we can drop the default kargs from coreos-assembler. Hopefully everyone agress with that.

But, doing so will have a tricky interlock with ensuring RHCOS keeps building. This gets back into the features.yaml thing so we can "ratchet" changes safely.


install() {
# We're forcibly overriding NM to be run as a service
install_and_enable_unit coreos-NetworkManager.service network-online.target
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be network.target, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm...debatable. See https://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/
I think what Ignition wants is much closer to network-online.target.

Comment on lines 20 to 26
fetchd="$initdir/$systemdsystemunitdir/ignition-fetch.service.d/"
mkdir -p "${fetchd}"
cat >${fetchd}/nm-online.conf << EOF
[Unit]
After=network-online.target
Wants=network-online.target
EOF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we drop this part? With the separate ignition-fetch-offline.service, we can make ignition-fetch.service hard require network.target (the tricky bit is still the "systemd always starts dependencies of units", but I'll work around that there).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, feel free to push whatever changes you want to this PR if it helps you iterate!

See https://github.com/coreos/ignition-dracut/issues/94
and coreos/ignition#948

Needs pairing with a cosa PR to drop the default `ip=dhcp` kargs.

And yes we really want to upstream this into NM by default or so.

Co-Authored-By: Dusty Mabe <dusty@dustymabe.com>
jlebon added a commit to jlebon/coreos-assembler that referenced this pull request Mar 30, 2020
We want to move to a model where networking isn't unconditionally
brought up, but instead only if Ignition requires it.

Works with:
coreos/fedora-coreos-config#321
coreos/ignition-dracut#164
coreos/ignition#956
jlebon added a commit to jlebon/coreos-assembler that referenced this pull request Mar 30, 2020
We want to move to a model where networking isn't unconditionally
brought up, but instead only if Ignition requires it.

But I think we'll still have to keep supporting
`ignition_network_kcmdline` since it's kind of part of our API now that
it can be overridden via `ignition.firstboot`.

Works with:
coreos/fedora-coreos-config#321
coreos/ignition-dracut#164
coreos/ignition#956
@jlebon
Copy link
Member

jlebon commented Mar 30, 2020

I cherry-pick the key hunks from #326 we need for the live ISO and opened the corresponding cosa patch: coreos/coreos-assembler#1298

[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/libexec/nm-initrd-generator rd.neednet=1 ip=dhcp,dhcp6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is going to work. I fear this is going to miss:

  • custom kargs for static networking (e.g. the ones injected by coreos-installer)
  • runtime kargs from /etc/cmdline.d. (e.g. the ones forwarded from vmware guestinfo)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom kargs for static networking (e.g. the ones injected by coreos-installer)

Right, we probably need to make this an ExecStartPre= that looks at /proc/cmdline and defers to that if any relevant arguments exist.

runtime kargs from /etc/cmdline.d. (e.g. the ones forwarded from vmware guestinfo)

This only exists in coreos/afterburn#379 right?

But hmm...either way, we should order After=dracut-cmdline.service and parse.../tmp/networking_opts it looks like?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the vmware guestinfo bits specifically are only in a not-yet-merged PR, but generally I think the cmdline.d primitive is a useful one to keep.

@cgwalters
Copy link
Member Author

Most of this is obsolete with the current afterburn handling; the core idea of running NM as a unit still stands, but yeah, better done upstream.

@cgwalters cgwalters closed this May 22, 2020
dustymabe pushed a commit to jbtrystram/fedora-coreos-config that referenced this pull request Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants