/test

While this is definitely an improvement (and does solve the immediate problem!), would it make more sense to enumerate EndpointSlice.endpoints.conditions and EndpointSlice.endpoints.nodeName to utilize the same logic the service controller does when determining if traffic should be directed to a node/pod?

Edit:
A quick test of a pod with a misconfigured readinessProbe shows that it won't be considered a backend pod, so the above might be overkill.

Hi @bewing, I am not sure if I understand your concern.

would it make more sense to enumerate EndpointSlice.endpoints.conditions and EndpointSlice.endpoints.nodeName to utilize the same logic the service controller does when determining if traffic should be directed to a node/pod?

We are using cilium internal Endpoints representation. Do you have some concern that we should be using an alternative approach ?

My concern was that endpoint didn't have the same data available as EndpointSlice, which has conditions.ready explicitly tracking the readiness of the endpoint to receive traffic.

ready
ready indicates that this endpoint is prepared to receive traffic, according
to whatever system is managing the endpoint. A nil value indicates an
unknown state. In most cases consumers should interpret this unknown state
as ready. For compatibility reasons, ready should never be "true" for
terminating endpoints, except when the normal readiness behavior is being
explicitly overridden, for example when the associated Service has set the
publishNotReadyAddresses flag.

However, given that testing shows that a pod failing its readiness check doesn't contribute to a node's BGP advertisement, the two approaches are probably functionally identical

@bewing I see what you mean. I believe terminating setting of Cilium ep backend is only set when conditions.terminating is set. So, there might be some other scenarios in which backend is not in terminating state but also not ready to receive traffic. If there is such a case, we should expand Cilium endpoint backend to expose ready field.

I think we can continue with this approach at this time, since there is clear case of excluding terminating pod from backend selection in case of eTP=local BGP advertisements. We can revisit if we need to add ready bool in backend structure of cilium ep.

Edit: If we check here, I think terminating state should cover all cases.