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: Local internalTrafficPolicy support for ClusterIP advertisements with BGP Control Plane #31389

Closed
rastislavs opened this issue Mar 14, 2024 · 2 comments · Fixed by #31442
Assignees
Labels
area/bgp help-wanted Please volunteer for this by adding yourself as an assignee! kind/feature This introduces new functionality. sig/agent Cilium agent related.

Comments

@rastislavs
Copy link
Contributor

rastislavs commented Mar 14, 2024

Cilium Feature Proposal

Add support for .spec.internalTrafficPolicy: Local in ClusterIP service advertisements with BGP Control Plane.

Is your proposed feature related to a problem?

In PR #30963 (issue #30875) we added support for ClusterIP service advertisements with BGP Control Plane.

At the moment, the ClusterIP service prefixes are advertised from each BGP-enabled cilium node.

As the ClusterIP services can have .spec.internalTrafficPolicy set to Local, advertising a ClusterIP from a node which has no local endpoints may cause unreachability of the advertised service if the traffic is routed to that node:

For pods on nodes with no endpoints for a given Service, the Service behaves as if it has zero endpoints (for Pods on this node) even if the service does have endpoints on other nodes.

(source)

Describe the feature you'd like

If .spec.internalTrafficPolicy of a service is set to Local, advertise its ClusterIP over BGP only if it has non-zero local endpoints.

(Optional) Describe your proposed solution

BGP control plane already supports similar handling of spec.externalTrafficPolicy: Local for LoadBalancer services, so ideally the existing logic needs to be just extended to handle .spec.internalTrafficPolicy: Local as well on few places in the service reconciler (e.g. in svcDesiredRoutes handling).

@rastislavs rastislavs added help-wanted Please volunteer for this by adding yourself as an assignee! kind/feature This introduces new functionality. area/bgp labels Mar 14, 2024
@rastislavs
Copy link
Contributor Author

cc @YutaroHayakawa , @chaunceyjiang

@dylandreimerink dylandreimerink added the sig/agent Cilium agent related. label Mar 14, 2024
@chaunceyjiang
Copy link
Member

@rastislavs OK

Please assign to me.

chaunceyjiang added a commit to chaunceyjiang/cilium that referenced this issue Mar 22, 2024
Fixes: cilium#31389

As the ClusterIP services can have .spec.internalTrafficPolicy set to Local, advertising a ClusterIP from a node which has no local endpoints may cause unreachability of the advertised service if the traffic is routed to that node.

So if .spec.internalTrafficPolicy of a service is set to Local, advertise its ClusterIP over BGP only if it has non-zero local endpoints.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
github-merge-queue bot pushed a commit that referenced this issue Mar 25, 2024
Fixes: #31389

As the ClusterIP services can have .spec.internalTrafficPolicy set to Local, advertising a ClusterIP from a node which has no local endpoints may cause unreachability of the advertised service if the traffic is routed to that node.

So if .spec.internalTrafficPolicy of a service is set to Local, advertise its ClusterIP over BGP only if it has non-zero local endpoints.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
rzdebskiy pushed a commit to rzdebskiy/cilium that referenced this issue Apr 3, 2024
Fixes: cilium#31389

As the ClusterIP services can have .spec.internalTrafficPolicy set to Local, advertising a ClusterIP from a node which has no local endpoints may cause unreachability of the advertised service if the traffic is routed to that node.

So if .spec.internalTrafficPolicy of a service is set to Local, advertise its ClusterIP over BGP only if it has non-zero local endpoints.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp help-wanted Please volunteer for this by adding yourself as an assignee! kind/feature This introduces new functionality. sig/agent Cilium agent related.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants