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

fix(network-manager): support teaming under NM+systemd #1547

Merged
merged 1 commit into from Jun 28, 2021

Conversation

dustymabe
Copy link
Contributor

Previously when NM was run without dbus then teaming would come
up appropriately [1], but now that dbus exists we also need to
include some supporting infrastructure to allow for it to work
again.

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/commit/d689380cfc5734a29b1302d68027190e1a606265

@dustymabe
Copy link
Contributor Author

I have tested this locally.

@github-actions github-actions bot added modules Issue tracker for all modules network-manager Issues related to the network-manager module labels Jun 24, 2021
inst_multiple -o \
"$systemdsystemunitdir"/teamd@.service \
"$dbussystem"/teamd.conf \
"$dbussystemconfdir"/teamd.conf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jpirko - the fedora rpm currently defines the dbus conf in /etc/dbus-1/system.d/teamd.conf. Should it be modified to place it in /usr/share/dbus-1/system.d/teamd.conf instead?

Here I'm picking it up from either place in case it changes.

@dustymabe
Copy link
Contributor Author

cc @bengal

@@ -38,6 +38,12 @@ install() {
inst "$dbussystem"/org.freedesktop.NetworkManager.conf
inst_multiple nmcli nm-online

# teaming support under systemd+dbus
inst_multiple -o \
"$systemdsystemunitdir"/teamd@.service \
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, NetworkManager spawns teamd directly and then waits that teamd appears on D-Bus to communicate with it.
This line above is not needed because teamd is not started as a systemd service.

The rest LGTM, I also tried the patch and it works well. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in latest push. Thanks @bengal.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM now.

Previously when NM was run without dbus then teaming would come
up appropriately [1], but now that dbus exists we also need to
include some supporting infrastructure to allow for it to work
again.

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/commit/d689380cfc5734a29b1302d68027190e1a606265
@dustymabe
Copy link
Contributor Author

@haraldh - when we merge this do you think we could get it backported to F34?

@dustymabe
Copy link
Contributor Author

LGTM now.

Thanks @bengal - any other reviewers we need to wait on or can this be merged now?

Copy link
Collaborator

@johannbg johannbg left a comment

Choose a reason for hiding this comment

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

LGTM

@johannbg johannbg enabled auto-merge (rebase) June 25, 2021 19:08
@dustymabe
Copy link
Contributor Author

Any ideas how to fix the failing packit test?

@johannbg
Copy link
Collaborator

@dustymabe packit tests passing are not a requirement so you can just ignore it. Anyways to fix a failing packit test requires us to open up an issue with the packit team, which then will look into it along with infra with high probability that it's not our PR's causing failure, not packit itself but downstream maintainer or infra making some mistake or upgrade ( this has happened before ).

@johannbg johannbg merged commit a97d2ce into dracutdevs:master Jun 28, 2021
@dustymabe dustymabe deleted the dusty-fix-nm-teaming branch June 28, 2021 16:28
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Jul 9, 2021
Contains upstream fixes needed to get NM running via systemd+dbus
in the initramfs without issues.

- dracutdevs/dracut#1547
- dracutdevs/dracut#1548
- dracutdevs/dracut#1552

Needed to get dracut unfrozen:
coreos/fedora-coreos-tracker#842 (comment)
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Jul 9, 2021
Contains upstream fixes needed to get NM running via systemd+dbus
in the initramfs without issues.

- dracutdevs/dracut#1547
- dracutdevs/dracut#1548
- dracutdevs/dracut#1552

Needed to get dracut unfrozen:
coreos/fedora-coreos-tracker#842 (comment)
dustymabe added a commit to coreos/fedora-coreos-config that referenced this pull request Jul 9, 2021
Contains upstream fixes needed to get NM running via systemd+dbus
in the initramfs without issues.

- dracutdevs/dracut#1547
- dracutdevs/dracut#1548
- dracutdevs/dracut#1552

Needed to get dracut unfrozen:
coreos/fedora-coreos-tracker#842 (comment)
stewartsmith pushed a commit to stewartsmith/fedora-rpms-dracut that referenced this pull request Jun 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules Issue tracker for all modules network-manager Issues related to the network-manager module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants