Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

add coreos-teardown-initramfs-network.service #159

Merged
merged 8 commits into from
Mar 24, 2020

Conversation

dustymabe
Copy link
Member

This is a forward port of coreos-teardown-initramfs-network.service
from the spec2x branch [1] (used for RHEL CoreOS). When moving to NM
in the initrd [2] we decided that we also needed a mechanism to take down
the networking between the initramfs and the real root. While we would
like to use NetworkManager's logic to do this operation in the future
it's currently not easily achieved because NetworkManager is not running
persistently in the initramfs [3].

Additionally, we have decided to propagate initramfs networking configuration
in some scenarios [4]. The policy here is:

  • If a networking configuration was provided before this point
    (most likely via Ignition) and exists in the real root then
    we do nothing and don't propagate any initramfs networking.
  • If a user did not provide any networking configuration
    then we'll propagate the initramfs networking configuration
    into the real root.

[1] #78
[2] coreos/fedora-coreos-tracker#394
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1814038
[4] coreos/fedora-coreos-tracker#394 (comment)

@dustymabe dustymabe changed the title add coreos-teardown-initramfs.service add coreos-teardown-initramfs-network.service Mar 19, 2020
@dustymabe
Copy link
Member Author

this is working in initial tests but I need to look at things more comprehensively before we merge this.. setting to draft for now

@cgwalters
Copy link
Member

I think one reason we weren't doing this in FCOS is that we ended up with two DHCP leases or something. We need to also kill dhclient; maybe it's watching for the interface going down, but that's going to be racy because it may not have a chance to release the lease before being killed by switchroot.

@dustymabe
Copy link
Member Author

@cgwalters - I haven't seen any issues with that in initial tests with NM in the initrd. I'll watch out for it.

@jlebon
Copy link
Member

jlebon commented Mar 19, 2020

I think the double lease issue was fixed since coreos/fedora-coreos-config#82.

dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Mar 19, 2020
Moving to NetworkManager in the initrd should help us solve some
problems we've been having with Networking. For what we want to
do in Fedora CoreOS doing this right requires the network to be
torn down in the initrd and also possibly propagated forward.

Requires: coreos/ignition-dracut#159
Fixes: coreos/fedora-coreos-tracker#394
@jlebon
Copy link
Member

jlebon commented Mar 20, 2020

Could we split the tearing down part from the persisting part? Presumably once we can use NM for the former, that service would just be e.g. ExecStart=nmcli networking off, right?

@dustymabe
Copy link
Member Author

Could we split the tearing down part from the persisting part? Presumably once we can use NM for the former, that service would just be e.g. ExecStart=nmcli networking off, right?

right.

@dustymabe
Copy link
Member Author

though one problem with doing that is the question of who is responsible for removing /run/NetworkManager/* files. Is it the teardown or the propagate.

@dustymabe
Copy link
Member Author

ok I talked with jlebon and we are going to leave this in a single teardown script/service for now.

I did a bunch of testing on this using a script to simulate different scenarios. It seems like things are going well so far. This script is at https://dustymabe.fedorapeople.org/fcos-network-testing.sh . I need to find the right place to put it for now. I've been told that network testing in our CI is going to be somewhat limited for now because of unpriv qemu networking but maybe we can use this script as inspiration for CI tests when that is no longer a blocker.

@dustymabe dustymabe marked this pull request as ready for review March 24, 2020 03:42
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Mar 24, 2020
Moving to NetworkManager in the initrd should help us solve some
problems we've been having with Networking. For what we want to
do in Fedora CoreOS doing this right requires the network to be
torn down in the initrd and also possibly propagated forward.

Requires: coreos/ignition-dracut#159
Fixes: coreos/fedora-coreos-tracker#394
Copy link
Contributor

@darkmuggle darkmuggle left a comment

Choose a reason for hiding this comment

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

A couple of questions, possibly nits.

@dustymabe dustymabe force-pushed the dusty-network branch 2 times, most recently from 46726e4 to 0e3e188 Compare March 24, 2020 19:09
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Mar 24, 2020
Moving to NetworkManager in the initrd should help us solve some
problems we've been having with Networking. For what we want to
do in Fedora CoreOS doing this right requires the network to be
torn down in the initrd and also possibly propagated forward.

Requires: coreos/ignition-dracut#159
Fixes: coreos/fedora-coreos-tracker#394
@darkmuggle
Copy link
Contributor

darkmuggle commented Mar 24, 2020

LGTM, I guess my comments were nits.

dracut/30ignition/module-setup.sh Outdated Show resolved Hide resolved
dracut/30ignition/ignition-generator Outdated Show resolved Hide resolved
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Mar 24, 2020
Moving to NetworkManager in the initrd should help us solve some
problems we've been having with Networking. For what we want to
do in Fedora CoreOS doing this right requires the network to be
torn down in the initrd and also possibly propagated forward.

We effectively move *to* NetworkManager in the initrd (the default
in Fedora 31+) by putting back in the nm-initrd-generator, which
we were previously removing.

Requires: coreos/ignition-dracut#159
Fixes: coreos/fedora-coreos-tracker#394
This is a forward port of coreos-teardown-initramfs-network.service
from the spec2x branch [1] (used for RHEL CoreOS). When moving to NM
in the initrd [2] we decided that we also needed a mechanism to take down
the networking between the initramfs and the real root. While we would
like to use NetworkManager's logic to do this operation in the future
it's currently not easily achieved because NetworkManager is not running
persistently in the initramfs [3].

[1] coreos#78
[2] coreos/fedora-coreos-tracker#394
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1814038
This will make it easier for us to add a bit more code into this
file.
…g if desired

The policy here is:

   - If a networking configuration was provided before this point
     (most likely via Ignition) and exists in the real root then
     we do nothing and don't propagate any initramfs networking.
   - If a user did not provide any networking configuration
     then we'll propagate the initramfs networking configuration
     into the real root.

See coreos/fedora-coreos-tracker#394 (comment)
Remove the removal of the `/tmp/net.$1.did-setup` (used by
network-legacy dracut module) and start removing files under
/run/NetworkManager/ instead.
down_teams() is a placeholder for now to remind us that teams do
exist and need to be considered. If we find things that need to be
done then we'll add actions to do in the down_teams() function in
the future.
On recommendation from the NM team let's take down routes as well.
On recommendation from the NM team let's try to delete the device
first and if that doesn't work then set it to down and flush any
associated addresses. Deleting virtual devices (bonds, teams, bridges,
ip-tunnels, etc) will clean up any associated kernel resources, so
this makes the logic for bringing down these devices easier. A real
device can't be deleted so that will fail and we'll fallback to setting
it down and flushing addresses.
Since we have ignition-complete.target we don't need to use the
generator to enable the unit.
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM!

@dustymabe dustymabe merged commit f67d587 into coreos:master Mar 24, 2020
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Mar 24, 2020
Moving to NetworkManager in the initrd should help us solve some
problems we've been having with Networking. For what we want to
do in Fedora CoreOS doing this right requires the network to be
torn down in the initrd and also possibly propagated forward.

We effectively move *to* NetworkManager in the initrd (the default
in Fedora 31+) by putting back in the nm-initrd-generator, which
we were previously removing.

Requires: coreos/ignition-dracut#159
Fixes: coreos/fedora-coreos-tracker#394
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Mar 24, 2020
Needed for appropriately implementing NetworkManager support
in the initramfs. See coreos/ignition-dracut#159
@dustymabe
Copy link
Member Author

FYI: I've been testing this using an ugly script for now (better than hand testing at least 😄). It goes through and makes sure variations of initrd passed networking config and ignition passed networking config works. See coreos/fedora-coreos-config#316

dustymabe added a commit to coreos/fedora-coreos-config that referenced this pull request Mar 24, 2020
Moving to NetworkManager in the initrd should help us solve some
problems we've been having with Networking. For what we want to
do in Fedora CoreOS doing this right requires the network to be
torn down in the initrd and also possibly propagated forward.

We effectively move *to* NetworkManager in the initrd (the default
in Fedora 31+) by putting back in the nm-initrd-generator, which
we were previously removing.

Requires: coreos/ignition-dracut#159
Fixes: coreos/fedora-coreos-tracker#394
dustymabe added a commit to coreos/fedora-coreos-config that referenced this pull request Mar 24, 2020
Needed for appropriately implementing NetworkManager support
in the initramfs. See coreos/ignition-dracut#159
@dustymabe dustymabe deleted the dusty-network branch April 21, 2020 20:46
dustymabe added a commit to dustymabe/ignition-dracut that referenced this pull request Apr 22, 2020
This is a backport of the commits merged in f67d587, which was
part of coreos#159. The upstream commits persisted Networking as part
of the teardown service so we also remove `persist-ifcfg.sh`.
dustymabe added a commit to dustymabe/ignition-dracut that referenced this pull request Apr 24, 2020
This is a backport of the merge commits f67d587 (coreos#159), and
390779d (coreos#174). The upstream commits persisted networking
configuration and hostname as part of the teardown service so we
also remove `persist-ifcfg.sh`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants