-
Notifications
You must be signed in to change notification settings - Fork 201
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
Revert PR 449 (LP: #2078009) #518
Conversation
CC @alfonsosanchezbeato would this revert break any of your usecases? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. It matches the code from before #449 except for cases where it evolved over time, such as the replacement for netifaces
and unused is_composite_member
method, in addition to the new udevadm control --reload
& udevadm trigger --action=move --subsystem-match=net --settle
handling.
Let's give Alfonso some time to chime in, before merging.
I'm fine with this, but just to double check, will this restart/reload config for networkd/network-manager as a minimum in the same cases as before? We have had many issues related to this in the past and I'd like to make sure that things like moving from some config to no config, etc. work as expected. |
Yes, it's essentially reverting the behavior to what it was before until we come up with a better solution. I think you have a local patch that will restart networkd? I suppose it should continue to work. |
I have a patch for core22, but not for core24 where some networkd bug was not present anymore. |
I think it will not change things for you then. I still think we should have a command line parameter to force the restart of backends so wouldn't need to maintain a patch for that. We probably should consider adding one. |
Description
This PR essentially reverts #449
The only conflict was due to an import that was removed (netifaces).
On #449
netplan apply
was changed to only apply if there were changes in the backend config files.While this is a nice approach to avoid unnecessary network disruptions, backend configuration can be generated without
apply
be aware of it (via the generator (such as when daemon-reload is called) ornetplan generate
).In this cases (if the generator or
netplan generate
runs first), whenapply
compares the previous configuration with the new one, there will be no changes.See LP: #2078009. Here, when a new interface is added to the container, a
daemon-reload
happens (twice apparently). Because of that, the netplan generator is called, new backend configuration is generated andnetplan apply
wouldn't do any action after that.Journal from the container at the moment the interface is plugged in:
I created a PPA with these patches for testing: https://launchpad.net/~danilogondolfo/+archive/ubuntu/bugfixes
Autopkgtests on this PPA.
https://autopkgtest.ubuntu.com/results/autopkgtest-oracular-danilogondolfo-bugfixes/oracular/arm64/n/netplan.io/20240916_144524_6b1ba@/log.gz
https://autopkgtest.ubuntu.com/results/autopkgtest-oracular-danilogondolfo-bugfixes/oracular/ppc64el/n/netplan.io/20240916_144805_5f233@/log.gz
https://autopkgtest.ubuntu.com/results/autopkgtest-oracular-danilogondolfo-bugfixes/oracular/s390x/n/netplan.io/20240916_142627_dc92a@/log.gz
https://autopkgtest.ubuntu.com/results/autopkgtest-oracular-danilogondolfo-bugfixes/oracular/armhf/n/netplan.io/20240916_145550_526ff@/log.gz
https://autopkgtest.ubuntu.com/results/autopkgtest-oracular-danilogondolfo-bugfixes/oracular/amd64/n/netplan.io/20240916_145623_713d6@/log.gz
Checklist
make check
successfully.make check-coverage
).