Skip to content

networkd: make s-n-wait-online wait for at least one routable interface #482

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

Merged
merged 6 commits into from
Jul 4, 2024

Conversation

Image for: Conversation
Copy link
Collaborator

slyon commented Jun 24, 2024

Description

Image for: Description

wait-online: wait for 'routable' state, if corresponding IPs are defined

Generally, we should be waiting on "routable" state (instead of "degraded") when we have static IP/dhcp4/dhcp6/RA.

Our spec defines "at least one interface MUST be up on the link layer and have received layer 3 (IP) configuration" (ignoring link-local).

We now start two separate systemd-networkd-wait-online processes. One waiting for "carrier" & "degraded" interfaces (including the "routable" ones), not using "--any". Another one waiting just for "routable" interfaces, combined with "--any". Both blocking "network-online.target".

This is an extension of #456

FR-7838

Checklist

Image for: 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.
slyon requested review from daniloegea and enr0n June 24, 2024 15:57
Copy link

enr0n left a comment

Choose a reason for hiding this comment

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

Nice! LGTM.

slyon force-pushed the s-n-wait-online-routable branch from 1f5c78d to ebb14e4 Compare June 25, 2024 07:54
slyon added 2 commits June 27, 2024 15:58
Generally, we should be waiting on "routable" state (instead of "degraded") when we have static IP/dhcp4/dhcp6/RA.

Our spec defines "at least one interface MUST be up on the link layer and have received layer 3 (IP) configuration" (ignoring link-local).

We now start two separate "systemd-networkd-wait-online" processes. One waiting for "carrier" & "degraded" interfaces, not using "--any". Another one waiting just for "routable" interfaces, combined with "--any". Both blocking "network-online.target".
slyon force-pushed the s-n-wait-online-routable branch 3 times, most recently from a119c15 to ba03349 Compare June 27, 2024 15:00
slyon force-pushed the s-n-wait-online-routable branch from ba03349 to 56b149c Compare June 27, 2024 15:10
Copy link
Contributor

daniloegea left a comment

Choose a reason for hiding this comment

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

It seems to be working as intended.

I just left one thought about bonds. I'm not sure if we should wait for carrier on all the members by default. The system doesn't need to have carrier on all the bond members to be online.

But to have it waiting for any member we'd need to create groups of members for each bond and have one call to wait-online --any for each group... maybe we could consider that in the future. Maybe we should recommend that bond members should be optional.

Copy link
Collaborator Author

slyon commented Jul 3, 2024

I'm not sure if we should wait for carrier on all the members by default. The system doesn't need to have carrier on all the bond members to be online.

Right, I think this edge cases was not considered in the spec. But issuing a log message about it is probably a good first step. Further improvements can then be done as follow-up PRs.

PTAL, I added two new isolated commits on top of the current branch (the 2nd begin some small, unrelated refactoring).

Copy link
Contributor

daniloegea left a comment

Choose a reason for hiding this comment

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

Looks good. I just left a suggestion to make the log message a bit more informative.

slyon requested a review from daniloegea July 3, 2024 15:31
Copy link
Contributor

daniloegea left a comment

Choose a reason for hiding this comment

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

lgtm

slyon merged commit 039ea1f into canonical:main Jul 4, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants