-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Adding support for TrafficDistribution #32678
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
@robscott the changes LGTM but there are some files that need to be regenerated accordingly the CI. |
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.
Could you also add a small doc update to the topology awareness section to document support for TrafficDistribution? https://docs.cilium.io/en/stable/network/kubernetes/kubeproxy-free/#topology-aware-hints
/test |
c6b4e0d
to
2aa69ae
Compare
Updated, let me know if I missed anything @tommyp1ckles.
Ran
Added, thanks for the idea @borkmann! |
47c53df
to
397a751
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.
Looks good to me, thank you
This commit adds support for Service Traffic Distribution, a Kubernetes feature that is on track to go beta/available-by-default in v1.31. TrafficDistribution represents the latest and hopefully final iteration in Kubernetes topology aware routing. The default logic is quite simple - route traffic within the same zone if there are any healthy endpoints in that zone. The kube-proxy implementation of that is still based on the EndpointSlice hints field, but does not strictly need to be. This KEP also comes with a more fundamental change to the implementation logic. As part of a broader goal of separating concerns, we've removed the check from Kube-Proxy that verified that a topology annotation was set before honoring hints on EndpointSlices. Now we simply check that all endpoints for an EndpointSlice have hints, and honor them if so. In this commit I've left some logic to determine if a Service is likely to have hints set, but that is now only used to determine if a Service should be reconciled again if the local node labels change. Signed-off-by: Rob Scott <robertjscott@google.com>
/test |
/ci-e2e |
@robscott unfortunately, the bot that responds to comments cannot accept multiple commands in the same message. Additionally, you will need to become an organization member in order to be able to run tests. |
/ci-e2e |
/ci-e2e-upgrade |
/ci-ingress |
Test |
Thanks for the info @joestringer! I've filed cilium/community#127 to become an organization member.
I can't think of anything in this PR that should affect the results of Ingress conformance tests. As far as I can tell, this is a common flake per #31857. |
/ci-ingress |
I wonder if we should actually bump this to |
Fine by me, I hesitated between the two options when setting the label. |
This PR adds support for Service Traffic Distribution, a Kubernetes feature that should go beta/available-by-default in v1.31. For more information, refer to the following resources:
TrafficDistribution represents the latest and hopefully final iteration in Kubernetes topology aware routing. The default logic is quite simple - route traffic within the same zone if there are any healthy endpoints in that zone. The kube-proxy implementation of that is still based on the EndpointSlice hints field, but does not strictly need to be,
This KEP also comes with a more fundamental change to the implementation logic. As part of a broader goal of separating concerns, we've removed the check from Kube-Proxy that verified that a topology annotation was set before honoring hints on EndpointSlices. Now we simply check that all endpoints for an EndpointSlice have hints, and honor them if so.
In this PR I've left some logic to determine if a Service is likely to have hints set, but that is now only used to determine if a Service should be reconciled again if the local node labels change.