fixes an issue where unhealthy endpoints for service entries are not removed from routing#14222
Conversation
gateway-bot
added
kind/fix
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Add readiness-awareness to ServiceEntry-derived endpoints so NotReady pod-backed workloads are excluded, matching the EndpointSlice-based Service behavior and enabling locality failover when all local pods are NotReady.
Changes:
- Add a
Readyfield tokrtcollections.LocalityPodand include it in equality comparisons. - Populate
Readyfor pod-backed workloads and defaultReady=truefor WorkloadEntry/inline endpoints. - Filter out
!Readyworkloads when computing ServiceEntry endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/krtcollections/pods.go | Introduces LocalityPod.Ready and sets it for pod-backed workloads. |
| pkg/kgateway/extensions2/plugins/serviceentry/endpoints.go | Skips NotReady workloads when building ServiceEntry endpoints. |
| pkg/kgateway/extensions2/plugins/serviceentry/collections.go | Marks WorkloadEntry/inline endpoints as always ready. |
| Locality ir.PodLocality | ||
| AugmentedLabels map[string]string | ||
| Addresses []string | ||
| // Ready reflects the pod's Kubernetes readiness (the Ready condition is True). | ||
| // It is used to exclude NotReady pods from ServiceEntry-derived endpoints, | ||
| // mirroring the EndpointSlice-based Service path which skips NotReady endpoints. | ||
| // Synthetic LocalityPods built from WorkloadEntries / inline endpoints set this | ||
| // to true, since pod readiness does not apply to them. | ||
| Ready bool |
| // Skip NotReady pod-backed workloads so that ingress (and any other | ||
| // ServiceEntry consumer) does not send traffic to pods that Kubernetes has | ||
| // marked NotReady. This mirrors the EndpointSlice-based Service path, which | ||
| // skips endpoints whose Ready condition is not true, and lets locality | ||
| // failover promote endpoints in a peer cluster when all local pods are | ||
| // NotReady. WorkloadEntry and inline endpoints set Ready=true. | ||
| if !workload.Ready { | ||
| continue | ||
| } |
| AugmentedLabels: labels, | ||
| Locality: l, | ||
| Addresses: extractPodIPs(pod), | ||
| Ready: isPodReadyConditionTrue(pod.Status), | ||
| } |
98b3a99 to
3107aff
Compare
…removed from routing Signed-off-by: Nick Nellis <[email protected]>
Signed-off-by: Nick Nellis <[email protected]>
3107aff to
42dc3cf
Compare
ymesika
left a comment
ymesika
left a comment
There was a problem hiding this comment.
Changes make sense to me and aligned with the Service behavior.
Filtering on Ready covers graceful termination (kubelet marks draining pods NotReady). Terminal state check would be redundant since it's a subset of !Ready. Can you just confirm the repro shows the draining pod reporting NotReady, so we know the signal fires on termination and not only on a manually failed probe?
Could you phrase the description and release-note in terms of this repo construct this actually operates on - ServiceEntry clusters with workloadSelector-backed pod endpoints respecting pod readiness rather than global services?
|
Thanks @ymesika. Agreed - filtering on the pod On the repro: the draining pod reports I've also reframed the description and release-note around the actual construct this operates on - ServiceEntry clusters with I also fixed the unit test failure: the setup-test fixture pods ( |
nmnellis
changed the title
readiness filter on ServiceEntry-derived endpoints dropped all pod-backed endpoints and TestServiceEntry/se-backendref failed. Add a Ready=True condition to the reviews pods to reflect their intended serving state. Signed-off-by: Nick Nellis <[email protected]>
1ea888c to
ab9f44d
Compare
|
/retest |
…removed from routing (#14222) Signed-off-by: Nick Nellis <[email protected]> Signed-off-by: David Jumani <[email protected]>
Description
ServiceEntry clusters that select pod endpoints via
workloadSelectornow respect pod readiness: pod-backed endpoints whose KubernetesReadycondition is notTrueare excluded from the cluster's EDS, mirroring the EndpointSlice-based Service path. This lets locality failover promote endpoints in a peer cluster when all local pods for a selected workload go NotReady (e.g. during graceful termination, where kubelet flips the pod'sReadycondition toFalse).Scope:
workloadSelector-backed pod endpoints are affected.WorkloadEntryand inlineServiceEntryendpoints are treated as always Ready and are never filtered, since pod readiness does not apply to them (their health is managed by the remote/owning system).Change Type
/kind fix
Changelog