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.

Sign up for GitHub

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

Jump to bottom

overlay.d/14NetworkManager-plugins: Explicitly disable ifcfg-rh plugin #1991

Closed
wants to merge 1 commit into from
Closed

overlay.d/14NetworkManager-plugins: Explicitly disable ifcfg-rh plugin #1991

wants to merge 1 commit into from

Conversation

LorbusChris
Copy link
Contributor

The NM issue [1] where compiled-in default plugins could not be disabled with the -= syntax has been fixed upstream, so move to explicitly disabling the ifcfg-rh plugin.

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/491

@LorbusChris
overlay.d/14NetworkManager-plugins: Explicitly disable ifcfg-rh plugin
e80302b 
The NM issue [1] where compiled-in default plugins could not be disabled
with the `-=`  syntax has been fixed upstream, so move to explicitly
disabling the ifcfg-rh plugin.

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/491
@LorbusChris LorbusChris mentioned this pull request Sep 28, 2022
Merged
@dustymabe
Copy link
Member

dustymabe commented Sep 28, 2022 •
edited
Loading

Aside: Just wondering.. How did you happen upon this?

Now.. on to the strategy here...

It turns out (with some encouragement from us) the NM team moved the ifcfg-rh plugin to a subpackage in Fedora 36 (i.e. no longer included by default). Instead of what you propose here I propose we:

  • remove this 14NetworkManager-plugins overlay completely
  • add NetworkManager-initscripts-ifcfg-rh to the list of packages to make sure we never include (for FCOS)
  • add a test that verifies the plugin exists or not based on the OS (i.e. RHCOS 8 would want to verify it exists and Fedora* would want to verify it doesn't exist). For RHCOS 9, not sure cc @travier.

@travier
Copy link
Member

travier commented Sep 28, 2022

ifcfg files are no longer the default in RHEL 9: https://www.redhat.com/en/blog/rhel-9-networking-say-goodbye-ifcfg-files-and-hello-keyfiles

@travier
Copy link
Member

travier commented Sep 28, 2022

We currently have this: https://github.com/openshift/os/blob/master/overlay.d/05rhcos/usr/lib/NetworkManager/conf.d/10-disable-default-plugins.conf

Maybe we could remove it for RHEL 9 but not sure it's urgent.

@travier
Copy link
Member

travier commented Sep 28, 2022 •
edited
Loading

(so overall, changes here do not impact RHCOS)

@dustymabe
Copy link
Member

We currently have this: https://github.com/openshift/os/blob/master/overlay.d/05rhcos/usr/lib/NetworkManager/conf.d/10-disable-default-plugins.conf

Maybe we could remove it for RHEL 9 but not sure it's urgent.

I think you'll want to make whatever changes we make here (i.e. making sure the NetworkManager-initscripts-ifcfg-rh is in the exclude list) for RHCOS 9 as soon as possible so any early testing doesn't yield false-positive feedback.

@travier travier mentioned this pull request Sep 29, 2022
Open
@travier
Copy link
Member

travier commented Sep 29, 2022

Filed openshift/os#1022 to discuss that.

@LorbusChris
Copy link
Contributor Author

@dustymabe we stumbled over this when adding the 05rhcos overlay to okd-machine-os (there was an error because the file now exists twice, in two different overlays), and I thought doing the cleanup couldn't hurt. I think your proposal makes sense, though.
Closing this so the discussion can be focused in openshift/os#1022

/close

@dustymabe
Copy link
Member

any volunteers to implement #1991 (comment) for FCOS?

@travier travier mentioned this pull request Sep 30, 2022
Merged
@travier
Copy link
Member

travier commented Sep 30, 2022

Did your suggestion in #1993

@travier travier closed this Sep 30, 2022
travier added a commit to travier/fedora-coreos-config that referenced this pull request Sep 30, 2022
@travier
manifest: Exclude support for NM legacy ifcfg config format
297b2e6 
Since Fedora 36, the ifcfg-rh plugin implementing support for reading
network configuration files in the legacy ifcfg format has been split
out into its own RPM subpackage.

We can now remove our configuration workaround and instead ensure that
we do not ship this plugin at all.

See: coreos#1991
travier added a commit to travier/fedora-coreos-config that referenced this pull request Sep 30, 2022
@travier
manifest: Exclude support for NM legacy ifcfg config format
17d11d6 
Since Fedora 36, the ifcfg-rh plugin implementing support for reading
network configuration files in the legacy ifcfg format has been split
out into its own RPM subpackage.

We can now remove our configuration workaround and instead ensure that
we do not ship this plugin at all.

See: coreos#1991
dustymabe pushed a commit that referenced this pull request Sep 30, 2022
@travier @dustymabe
manifest: Exclude support for NM legacy ifcfg config format
f706a4f 
Since Fedora 36, the ifcfg-rh plugin implementing support for reading
network configuration files in the legacy ifcfg format has been split
out into its own RPM subpackage.

We can now remove our configuration workaround and instead ensure that
we do not ship this plugin at all.

See: #1991
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Sep 30, 2022
@dustymabe
tests/kola: add ext.config.networking.nm-ifcfg-rh-plugin test
db9fa35 
This test checks that the ifcfg-rh NetworkManager plugin is in
the state we expect it to be in. Some more context in:

- coreos#1993
- coreos#1991
- openshift/os#1022
@dustymabe dustymabe mentioned this pull request Sep 30, 2022
Merged
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Mar 13, 2023
@dustymabe
tests/kola: add ext.config.networking.nm-ifcfg-rh-plugin test
b336013 
This test checks that the ifcfg-rh NetworkManager plugin is in
the state we expect it to be in. Some more context in:

- coreos#1993
- coreos#1991
- openshift/os#1022
dustymabe added a commit that referenced this pull request Mar 14, 2023
@dustymabe
tests/kola: add ext.config.networking.nm-ifcfg-rh-plugin test
c357ecd 
This test checks that the ifcfg-rh NetworkManager plugin is in
the state we expect it to be in. Some more context in:

- #1993
- #1991
- openshift/os#1022
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023
@travier @HuijingHei
manifest: Exclude support for NM legacy ifcfg config format
6a682b8 
Since Fedora 36, the ifcfg-rh plugin implementing support for reading
network configuration files in the legacy ifcfg format has been split
out into its own RPM subpackage.

We can now remove our configuration workaround and instead ensure that
we do not ship this plugin at all.

See: coreos#1991
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023
@dustymabe @HuijingHei
tests/kola: add ext.config.networking.nm-ifcfg-rh-plugin test
69f630f 
This test checks that the ifcfg-rh NetworkManager plugin is in
the state we expect it to be in. Some more context in:

- coreos#1993
- coreos#1991
- openshift/os#1022
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023
@travier @HuijingHei
manifest: Exclude support for NM legacy ifcfg config format
505e676 
Since Fedora 36, the ifcfg-rh plugin implementing support for reading
network configuration files in the legacy ifcfg format has been split
out into its own RPM subpackage.

We can now remove our configuration workaround and instead ensure that
we do not ship this plugin at all.

See: coreos#1991
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023
@dustymabe @HuijingHei
tests/kola: add ext.config.networking.nm-ifcfg-rh-plugin test
1c46632 
This test checks that the ifcfg-rh NetworkManager plugin is in
the state we expect it to be in. Some more context in:

- coreos#1993
- coreos#1991
- openshift/os#1022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LorbusChris @dustymabe @travier