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

Migrate CriticalConnection (deprecated) to KeepConfiguration (LP: #1896799) #424

Merged
merged 2 commits into from Nov 29, 2023

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Nov 27, 2023

Description

With systemd-v243 (which is the default since focal), the "CriticalConnection" attribute was
revamped into "KeepConfiguration" [1], addressing several issues, especially in high-availability
environents [2][3].

This PR is preparatory work for #409

If I understand the code correctly, it still uses "CriticalConnection" when setting "critical: true" in the netplan configuration.

Closes: https://bugs.launchpad.net/netplan/+bug/1896799

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • [] New/changed keys in YAML format are documented.
  • [] (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad. LP#1896799

@slyon
Copy link
Collaborator Author

slyon commented Nov 27, 2023

@chr4 Are you OK with getting this landed first before we continue with the extension of KeepConfiguration= as described in #409?

The extension requires some consensus on new Netplan YAML schema, so it takes a bit longer, but the few lines from this PR should already fix the issue, while providing a (limited) KeepConfiguration= setting.

Let me know what you think!

PS: I attributed the changes to your name.

@chr4
Copy link
Contributor

chr4 commented Nov 27, 2023

No issues with taking the long road here, I agree that this is more consistent (and solves a few issues that came up in the discussion of #409).

PS: I attributed the changes to your name.

Thanks, much apprechiated!

@slyon slyon merged commit c97c476 into canonical:main Nov 29, 2023
14 of 15 checks passed
@slyon slyon added the community This PR has been proposed by somebody outside of the Netplan team and roadmap commitments. label Nov 29, 2023
@@ -482,7 +482,7 @@ def test_dhcp_critical_true(self):
LinkLocalAddressing=ipv6

[DHCP]
CriticalConnection=true
KeepConfiguration=true
Copy link

Choose a reason for hiding this comment

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

They put it into Network section, though it still modifies DHCPv4 behaviour.
DHCP.CriticalConnection / DHCPv4.CriticalConnection => Network.KeepConfiguration
https://www.freedesktop.org/software/systemd/man/latest/systemd.network.html#KeepConfiguration=

Copy link
Collaborator Author

@slyon slyon Dec 5, 2023

Choose a reason for hiding this comment

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

Very good catch, thanks a lot! Fixed in 76e896a

slyon added a commit that referenced this pull request Dec 5, 2023
daniloegea pushed a commit that referenced this pull request Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community This PR has been proposed by somebody outside of the Netplan team and roadmap commitments.
Projects
None yet
3 participants