Skip to content

cilium: per service algorithm follow-ups #36204

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 7 commits into from
Nov 29, 2024
Merged

cilium: per service algorithm follow-ups #36204

merged 7 commits into from
Nov 29, 2024

Conversation

borkmann
Copy link
Member

@borkmann borkmann commented Nov 27, 2024

(See commit desc)

Fixes: #36112

@borkmann borkmann added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. labels Nov 27, 2024
@borkmann borkmann force-pushed the pr/persvcalg branch 9 times, most recently from dfb5580 to d70f4c3 Compare November 28, 2024 15:38
Extend host port docs to clarify that it reuses default service
handling behavior.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Small cleanups and code style tweaks.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
We cannot extend structs based on defines to enlarge them as this compromises
the ongoing work to achieve a clang-free Cilium runtime image, or at least
makes it significantly harder. Therefore use the upper 8 bits of the affinity
timeout field in the service master entry.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: #35735 (comment)
We have several variants in the agent code: Alg, Algo, Algorithm. Streamline
this to Algorithm consistently.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 29, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 29, 2024
1) It's not needed since only the master service entry needs this, and
2) It will become buggy when we move the LB algorithm into the affinity
   timeout since that field is a union. For backend entries in the
   service map this contains the backend IDs. If this stays as-is we
   end up corrupting them.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Add a corresponding Helm option to enable the per-service load balancing
algorithm annotation.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Add docs and usage examples.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
@borkmann
Copy link
Member Author

/test

@borkmann borkmann marked this pull request as ready for review November 29, 2024 14:00
@borkmann borkmann requested review from a team as code owners November 29, 2024 14:00
@borkmann borkmann requested review from ti-mo and a user November 29, 2024 14:00
Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

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

Changes look good to me 🚀

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

docs good

Copy link
Contributor

@aspsk aspsk left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

Trivial ack for Helm changes.

@borkmann borkmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 29, 2024
@borkmann borkmann merged commit f3235ab into main Nov 29, 2024
309 checks passed
@borkmann borkmann deleted the pr/persvcalg branch November 29, 2024 16:07
@maintainer-s-little-helper maintainer-s-little-helper bot removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cilium svc handling followups before 1.17
5 participants