Merge ~rafaeldtinoco/ubuntu/+source/systemd:lp1815101-eoan into ubuntu/+source/systemd:ubuntu/eoan-devel

@xnox,

would you mind telling me if you see this as a SRU for Disco and Bionic as well ? looks like sustaining want a fix for networkd restarts dropping HA aliases from nics. more robust HA software (like pacemaker) have monitors on VIP resources and restart virtual IPs when networkd is restarted and wipes virtual aliases, but, some others, like keepalived or ha-proxy, don't: thats why I want to consider KeepConfiguration= as a "fix" and not as a feature, if you agree.

(This will help us out not pursing all HA software and checking which of them are broken because networkd wipes the aliases, and consider this as 1 fix for all).

comments ?

thx a lot!

Overall, the patchset looks mostly good (one comment farther down in the patches), but I'm quite concerned about handling of dhcp addresses on stop, start, and restart. I don't think it's even entirely worked out correctly/fully upstream yet. That's not to say that the dhcp handling behavior in our SRU releases is correct, or that upstream is correct, it's just complex and I feel like the KeepConfiguration param makes it more complex.

Since HA and the other software that need this only uses manually-added static addresses/routes (right?), I'd much prefer something smaller, less complex, that only causes networkd to ignore foreign static addresses/routes.

But I don't currently have any suggestions on how to do that :-)

Dan,

I agree with you 100% there. I made sure to review the code being added to this SRU to check if, at least, it would keep default behavior without KeepConfiguration= being set, and that is true.

Other behaviors could be considered "broken" for Eoan/Disco/Bionic (possibly upstream ?) so I don't think, like you said, we should care too much (specially when one of them brakes DHCP spec, by not ever releasing the lease, on purpose).

The summary for this change is pretty much:

- [KEEP_CONFIGURATION_NO] = "no",
- [KEEP_CONFIGURATION_DHCP] = "dhcp",
- [KEEP_CONFIGURATION_STATIC] = "static",
- [KEEP_CONFIGURATION_YES] = "yes",

+ [KEEP_CONFIGURATION_NO] = "no",
+ [KEEP_CONFIGURATION_DHCP_ON_STOP] = "dhcp-on-stop",
+ [KEEP_CONFIGURATION_DHCP] = "dhcp",
+ [KEEP_CONFIGURATION_STATIC] = "static",
+ [KEEP_CONFIGURATION_YES] = "yes",

where:

"dhcp-stop": will not drop dhcp addresses and routes on stopping daemon (fixing restart issues)
"dhcp": will NEVER drop dhcp addresses (breaking dhcp spec on purpose, just like upstream)

so, as long as we default to "dhcp-stop" we have the same behavior as the previous CriticalConnection= setting, even if it is still used. So, I'm not removing or breaking existing configs.

Perhaps, a suggestion to your point, would be to have "static" only option, but then I would have to backport the feature not being 1:1 on upstream commits, harder to maintain and a worse delta in the next merge :\.

No free lunch I guess...

I have reviewed the patches and they LGTM as well.
The complexity discussion is lost as soon as you consider what it already does and I appreciate that we fix it with whatever upstream did in later versions instead of coming up with our own version of it - that way at least only one kind of complexity exists :-)

This definitely needs a review of someone skilled in systemd and I appreciate that ddstreet already looked over it and would also wait and see what xnox says. We might subscribe rbalint as well, I saw him do a lot of systemd recently.

Also on upload this maybe deserves a few extra days in proposed and given the time you most likely need to modify this to be an SRU. I guess early 20.04 will get systemd 243 and you then need to modify this to become 242-7ubuntu2.1 for eoan.

Thank you Christian, subscribed rbalint also.

> Thank you Christian, subscribed rbalint also.

Err, you already did. Ok.

Ok, +1 from me (not that my approval is needed :)

We'll get the complexity this param adds in 20.04 anyway, and I don't see any other way better than this to handle the problem.

I'll include this in my upload to Eoan unless I hear any objections.

Thanks!

This looks like it has migrated as 242-7ubuntu3.2 for systemd in eoan-updates (main).
https://launchpad.net/ubuntu/+source/systemd/242-7ubuntu3.2