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

Process network kickstart in stage 2 #2870

Merged

Conversation

rvykydal
Copy link
Contributor

@rvykydal rvykydal commented Sep 25, 2020

The first comit is already on f33-devel.
For the second there is a PR for f33-devel.
I will remove them after merging them into master with f33-devel.

All the rest The PR is only for master (so we catch fallouts before getting into Fedora 34) where we move from:

A: Network kickstart parsed in initramfs into ifcfg files based on which NM started after switch root creates and activates the respective connections.

to:

B: Network kickstart parsed in Anaconda after switch root which creates and activates NM connections via NM API.

The B: is what we already used to do for networking configured in %pre section so it has already got some usage and testing. We just needed to add a few more complex cases (eg vlan over bond).

The change from A to B was made possible by NM replacing dracut network module in initramfs.

The PR makes the code fully independent of the configuration format NM is using (ifcfg file of keyfile). Also network configuration (and perhaps Anaconda as whole) is no more dependent on fedora-import-service (passing configuration files from initramfs to rootfs), which used to cause issues like rhbz#1537977. Also it allows to get rid of hacks around ONBOOT value used for activating/not activating ifcfgs created in initramfs by NM.

The only thing that is changing with this PR is that NM default autoconnections are no more blocked from being created by ifcfgs with ONBOOT=no created in initramfs.

@rvykydal rvykydal added the master Please, use the `f39` label instead. label Sep 25, 2020
@pep8speaks
Copy link

pep8speaks commented Sep 25, 2020

Hello @rvykydal! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-10-05 11:34:05 UTC

@rvykydal rvykydal force-pushed the process-network-kickstart-in-stage-2 branch from 4344042 to 7e53198 Compare September 25, 2020 12:24
Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

It looks great! Thank you!

pyanaconda/modules/network/nm_client.py Outdated Show resolved Hide resolved
pyanaconda/modules/network/initialization.py Show resolved Hide resolved
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me as far as I can tell :).

@jkonecny12
Copy link
Member

/packit build

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me, as far as I can tell. Thank you! Massive removal is massive.

No more needed with kickstart processed in stage 2.
No more needed with NetworkManager in initramfs - taking connections over seems
to be seamless.
…ection

It is possible that the mechanism for device renaming via DEVICE and HWADDR
values in ifcfg files will be abandoned, but for now we need to keep it
supported.

With kickstart not applied in intiramfs we need to take it into account when
updating a connection from initramfs for a device renamed via ifname=.
Kickstart will be applied after switch root by the mechanism which so far
served for network configuration via %pre.
@rvykydal rvykydal force-pushed the process-network-kickstart-in-stage-2 branch from 7e53198 to 0da378c Compare October 5, 2020 11:33
@rvykydal
Copy link
Contributor Author

rvykydal commented Oct 5, 2020

I've replaced the first two commits and rebased the branch to the current master.

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@rvykydal rvykydal merged commit f7fa024 into rhinstaller:master Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master Please, use the `f39` label instead.
5 participants