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

CFP: align Cilium nodeipam node selection with kubernetes cloud-providers #31356

Closed
MrFreezeex opened this issue Mar 12, 2024 · 0 comments · Fixed by #31406
Closed

CFP: align Cilium nodeipam node selection with kubernetes cloud-providers #31356

MrFreezeex opened this issue Mar 12, 2024 · 0 comments · Fixed by #31406
Labels
kind/feature This introduces new functionality. sig/agent Cilium agent related.

Comments

@MrFreezeex
Copy link
Member

MrFreezeex commented Mar 12, 2024

Cilium Feature Proposal

Is your proposed feature related to a problem?
The current code of nodeipam look at the related EndpointSlices of the Service LoadBalancer to decide which nodes should be "advertised". This is a problem when Service LoadBalancers are created with dummy endpoints which is the case for Cilium Ingress/GatewayAPI for instance.

Describe the feature you'd like
We should be more aligned with how a cloud-provider controller decides which nodes are related to a service load balancer (see the code here: https://github.com/kubernetes/cloud-provider/blob/master/controllers/service/controller.go#L1027). Which actually never look at the backing EndpointSlice even with externalTrafficPolicy=Local it seems.

(Optional) Describe your proposed solution
By default/on ETP=Cluster we should not look at the related EndpointSlices and consider all nodes with what a cloud provider usually selects. We should probably provide a way to have a node selector annotation as well (similarly to what was done here: #31081) or else this will become quickly unusable for anything that is not a really small cluster.

On ETP=Local, I believe we should retain the current behavior of looking at the EndpointSlices content. On cloud-provider IIUC ETP=local is done with healthcheck and here we don't have those kind of things implemented and not sure if we should do that now (although it could probably be interesting for some future improvements of the nodeipam).

@MrFreezeex MrFreezeex added the kind/feature This introduces new functionality. label Mar 12, 2024
@MrFreezeex MrFreezeex changed the title CFP: align Cilium nodeipam node selection behaviors with a kubernetes cloud-provider CFP: align Cilium nodeipam node selection with what kubernetes cloud-providers do Mar 12, 2024
@MrFreezeex MrFreezeex changed the title CFP: align Cilium nodeipam node selection with what kubernetes cloud-providers do CFP: align Cilium nodeipam node selection with kubernetes cloud-providers Mar 12, 2024
@dylandreimerink dylandreimerink added the sig/agent Cilium agent related. label Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature This introduces new functionality. sig/agent Cilium agent related.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants