-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Conversation
dfb5580
to
d70f4c3
Compare
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>
293742e
to
9aa5c10
Compare
a3a1ad0
to
25adac0
Compare
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>
25adac0
to
1c86330
Compare
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>
a271490
to
b7ca0b3
Compare
/test |
There was a problem hiding this 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 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this 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.
(See commit desc)
Fixes: #36112