apply: make sure that networkd is restarted when needed#449
apply: make sure that networkd is restarted when needed#449slyon merged 4 commits intocanonical:mainfrom
Conversation
|
|
Do you know how safe a networkd restart is? I mean, I'm trying to determine what kind of change in behavior it would introduce from the point of view of the user. I'm concerned it would cause this kind of problem: https://discuss.linuxcontainers.org/t/systemd-update-networkd-restart-breaks-routed-containers/9979 |
|
This is going backwards wrt. #200 |
Right, as it looks like networkd is not really respecting the reload/reconfigure commands, at least on 20.04. |
Restarting it should be in theory fine, this is how it was being done in the past. The issue you shared seemed to happen only on 18.04 I think. |
|
I have tested this through different UC versions, and I see:
Therefore, it would make sense to backport b9ef830 to netplan in 20/22, and remove it from this PR, but keep 8dabb54. Are there branches where the commit could be proposed for 20/22? An alternative could be to find out the patch fixing the issue in systemd-networkd and SRU to 20/22, but being systemd it might take longer than doing an SRU of netplan, I think. |
|
Thanks a lot for your continued investigation @alfonsosanchezbeato !
Do we have a clean reproducer? I could bisect systemd to find the commit fixing the behaviour. I'd prefer fixing it properly in systemd that backporting a workaround.
I very much like what you did with |
Create a UC22 image, then install network-manager snap, then run then you have the bug (if not, the interfaces should be still |
|
We had/have some similar quirks for Netplan in Focal already:
So I guess I'd be fine SRUing it for Focal/UC20 in Netplan. While for Jammy/UC22 I'd still like to find the proper fix in systemd-networkd. |
I wonder if there is any reproducer to show this issue on Ubuntu classic? |
|
An alternative (instead of changing the current default behavior of |
8dabb54 to
6874d4a
Compare
There was a problem hiding this comment.
Therefore, it would make sense to backport b9ef830 to netplan in 20/22, and remove it from this PR, but keep 8dabb54.
So I've reverted the first commit in this PR (6874d4a), but kept the "apply: restart networkd instead of reload/reconfigure" commit as suggested (ac0d615).
Additionally, I've added a small fix that was failing autopkgtests when we have no networkd configuration at all in /run/systemd/networkd/ (directory doesn't exist) before or after the generator run.
With the workaround moved to canonical/core-base#214 I think this PR (just the cleaner networkd-config comparison logic) should now be good for a (squash) merge!
@daniloegea WDYT? (CC @alfonsosanchezbeato)
Due to systemd-networkd bugs, reload/reconfigure was not working in some cases. Instead, restart the service if the configuration has changed (LP#2058976).
Copy the current networkd configuration and then call generate so we know more accurately if systemd-networkd needs to be restarted.
This reverts commit b9ef830.
0a9a9ca to
a5e9afd
Compare
|
Rebased. |
daniloegea
left a comment
There was a problem hiding this comment.
I think it looks fine.
So, effectively now we are going to continue to reload (not restart) networkd only when we detect changes in the generated configuration. It's sounds like a nice improvement.
The problem is that it doesn't fix the issue for UC and now they need to maintain a patch (PR#214) that will force the restart.
Wouldn't it help them if we introduce a way to force-restart the backends via a CLI parameter?
This is fine by me too, thank you! |
This PR fixes these two problems:
It restarts networkd instead of reload/reconfigureDue to systemd-networkd bugs, reload/reconfigure was not working in
some cases. Instead, restart the service if the configuration has
changed (LP#2058976).
Copy the current networkd configuration and then call generate so we
know more accurately if systemd-networkd needs to be restarted.