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

overlay/fcos: pin legacy ifnames on existing machines #490

Merged
merged 1 commit into from
Jun 24, 2020

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Jun 24, 2020

This introduces a coreos-keep-legacy-ifnames service to pin
network interface names to the legacy scheme on machines
where 99-default.link is missing.
This is in order to ensure that those machines do not jump
into the new scheme via auto-updates as soon as FCOS starts
shipping such unit in the OS.

Ref: coreos/fedora-coreos-tracker#484

fi

# Otherwise, make sure we stay on legacy ifnames.
sed -e "/^options / s/$/ net.ifnames=0/" -i "$f"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jlebon please double-check the my knowledge here: if injected this way, the net.ifnames=0 will be retained even if the next update payload does not have it in its embedded kargs. Correct?

Copy link
Member

Choose a reason for hiding this comment

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

The update payload doesn't have kargs. Supporting that is ostreedev/ostree#479

So yes it will be retained because there's nothing to override it.

overlay.d/15fcos/usr/libexec/coreos-keep-legacy-ifnames Outdated Show resolved Hide resolved
fi

# Otherwise, make sure we stay on legacy ifnames.
sed -e "/^options / s/$/ net.ifnames=0/" -i "$f"
Copy link
Member

Choose a reason for hiding this comment

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

The update payload doesn't have kargs. Supporting that is ostreedev/ostree#479

So yes it will be retained because there's nothing to override it.

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.

Cool, thanks! One optional tweak, but LGTM as is too.

overlay.d/15fcos/usr/libexec/coreos-keep-legacy-ifnames Outdated Show resolved Hide resolved
@lucab
Copy link
Contributor Author

lucab commented Jun 24, 2020

Rebased. The PR to drop this logic and get back to systemd default persistent names is at #491.

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

overlay.d/15fcos/usr/libexec/coreos-keep-legacy-ifnames Outdated Show resolved Hide resolved
@cgwalters
Copy link
Member

Writing an ext test for this probably wouldn't be too hard (not a blocker though)

@lucab
Copy link
Contributor Author

lucab commented Jun 24, 2020

@cgwalters do you mean literally on this logic, or are you maybe thinking of #491? I was planning to have it on the latter, as this one requires a reboot to be really tested and the old behavior will be gone from CI in a couple of releases.

This introduces a `coreos-keep-legacy-ifnames` service to pin
network interface names to the legacy scheme on machines
where `99-default.link` is missing.
This is in order to ensure that those machines do not jump
into the new scheme via auto-updates as soon as FCOS starts
shipping such unit in the OS.
@lucab
Copy link
Contributor Author

lucab commented Jun 24, 2020

Rebased to fix the typos too.

@dustymabe
Copy link
Member

will merge this in a bit unless anyone has anything else

@dustymabe dustymabe merged commit d67f11e into coreos:testing-devel Jun 24, 2020
@lucab lucab deleted the ups/pin-legacy-ifnames branch June 25, 2020 08:00
@cgwalters
Copy link
Member

FTR, this service fails on the live ISO because /boot is read-only:

Jul 01 11:59:18 localhost systemd[1]: Starting CoreOS Keep Legacy Network Interface Names...
Jul 01 11:59:18 localhost coreos-keep-legacy-ifnames[956]: sed: couldn't open temporary file /boot/loader/entries/sedTIEkyv: Read-only file system
Jul 01 11:59:18 localhost systemd[1]: coreos-keep-legacy-ifnames.service: Main process exited, code=exited, status=4/NOPERMISSION
Jul 01 11:59:18 localhost systemd[1]: coreos-keep-legacy-ifnames.service: Failed with result 'exit-code'.
Jul 01 11:59:18 localhost systemd[1]: Failed to start CoreOS Keep Legacy Network Interface Names.

@jlebon
Copy link
Member

jlebon commented Jul 2, 2020

Ahh of course. This should've had something like ConditionPathExists=!/run/ostree-live.

We should add some basic test of the live path as part of fcosKola since it's so radically different in setup from qemu. I guess coreos/coreos-assembler#1571 will make that easier too.

dustymabe referenced this pull request in dustymabe/fedora-coreos-config Jul 7, 2020
This introduces a `coreos-keep-legacy-ifnames` service to pin
network interface names to the legacy scheme on machines
where `99-default.link` is missing.
This is in order to ensure that those machines do not jump
into the new scheme via auto-updates as soon as FCOS starts
shipping such unit in the OS.

(cherry picked from commit d67f11e)
@tmartin-git
Copy link

(dustymabe@aaf6867#commitcomment-40293518)

(Just a quit note about this - someone may encouter the same issue I had)

I noticed that all kernel configurations are impacted by this change and as a consequence it actually kind of "corrupts" your rollback.

As an example, I had setup a /etc/systemd/network/99-default.link configuration file (not /usr/lib/systemd/network/99-default.link as expected) to avoid conflict with package systemd-dev.
When the update took place to FCOS v>=32, it also added net.ifnames="0" to my previous kernel-attached configuration deployement.

Then I did a rollback (to my previously working FCOS 31) and unfortunatly, my system was broken (i.e. I had legacy names on my network interfaces).

@cgwalters
Copy link
Member

As an example, I had setup a /etc/systemd/network/99-default.link configuration file

In other words, you'd manually enabled the new names, and our change reverted it?

Yes...we clearly should have been checking for that. I suspect a relatively small number of people have done this, and those who have can drop this override and use the new names going forward.

If we get more reports of this we may need to try to adjust the code to detect this case. Sorry about that, and thank you for following up here!

@tmartin-git
Copy link

I was more concerned about the changes applied to all bootable kernel cmdline parameters. (by the way don't really know if this can be done a different way)
As a result, a rollback did not work - and as far as I understand, rpm-ostree rollback should always work, shouldn't it ?

c4rt0 pushed a commit to c4rt0/fedora-coreos-config that referenced this pull request Mar 27, 2023
ci/build-test: Do a cosa build + test
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.

5 participants