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

Use serving property instead of ready to determine endpoint health #51103

Open
the-redshift opened this issue May 16, 2024 · 8 comments
Open

Comments

@the-redshift
Copy link

Change Request

I propose using serving property of endpoint in endpoint slice instead of currently used ready to determine endpoint health.
It's objectively more informative than ready.

Furthermore, you can not really use publishNotReadyAddresses in Headless service in conjunction with Istio, because if you do, and you e.g. scale STS, then traffic will go almost instantly to the initializing Pod, because it immediately registers as ready:
https://github.com/kubernetes/endpointslice/blob/c2781e49dd7910f9b4881db2f0b176dea260ca20/utils.go#L42

That behaviour could be controlled via some ENV or config property.

Alternatives you've considered

None exist that I know

Affected product area

[ ] Ambient
[ ] Docs
[ ] Dual Stack
[ ] Installation
[x] Networking
[x] Performance and Scalability
[ ] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Affected features (please put an X in all that apply)

[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane

Additional context

I'd love to contribute if you deem this change worthy.

@ramaraochavali
Copy link
Contributor

Furthermore, you can not really use publishNotReadyAddresses in Headless service in conjunction with Istio, because if you do, and you e.g. scale STS, then traffic will go almost instantly to the initializing Pod, because it immediately registers as ready:
https://github.com/kubernetes/endpointslice/blob/c2781e49dd7910f9b4881db2f0b176dea260ca20/utils.go#L42

How will serving solve this? I thought "serving" would only impact terminating pods

@the-redshift
Copy link
Author

the-redshift commented May 16, 2024

Furthermore, you can not really use publishNotReadyAddresses in Headless service in conjunction with Istio, because if you do, and you e.g. scale STS, then traffic will go almost instantly to the initializing Pod, because it immediately registers as ready:
https://github.com/kubernetes/endpointslice/blob/c2781e49dd7910f9b4881db2f0b176dea260ca20/utils.go#L42

How will serving solve this? I thought "serving" would only impact terminating pods

Facts. Now that I think about it, it probably should be healthy == serving && !termination.
This way we essentialy get what equals to ready , but without relying on ready (as it's "polluted" by publishNotReadyAddresses).

@howardjohn
Copy link
Member

Does kube-proxy do this?

@the-redshift
Copy link
Author

Does kube-proxy do this?

Good question! Looks like they don't though:
https://github.com/kubernetes/kubernetes/blob/2a003648b026abcb6371f7da00740561c393a9a5/pkg/proxy/topology.go#L46

So basically I should go talk to them first?

@howardjohn
Copy link
Member

Well we don't necessarily have to do what they do, but the people who develop EndpointSlice API and kube-proxy are the same, so they probably are following best practices. So we should find out why they do it that way and see if it applies to us?

@the-redshift
Copy link
Author

Makes sense. I'll go and initiate a conversation with them, I'll be back.

@aojea
Copy link
Contributor

aojea commented May 17, 2024

Furthermore, you can not really use publishNotReadyAddresses in Headless service in conjunction with Istio, because if you do, and you e.g. scale STS, then traffic will go almost instantly to the initializing Pod, because it immediately registers as ready:

everything is debatable, but it is important to understand the use cases, is this use case realistic, if is headless it is only about DNS, it means it does not have ClusterIP ... is Istio participating here if the request is sent directly to the PodIP?

@the-redshift
Copy link
Author

I'm back!
Here's a very good comment explaining why kube-proxy has to rely on Ready:
kubernetes/kubernetes#124914 (comment)


Furthermore, you can not really use publishNotReadyAddresses in Headless service in conjunction with Istio, because if you do, and you e.g. scale STS, then traffic will go almost instantly to the initializing Pod, because it immediately registers as ready:

everything is debatable, but it is important to understand the use cases, is this use case realistic, if is headless it is only about DNS, it means it does not have ClusterIP ... is Istio participating here if the request is sent directly to the PodIP?

That's a good question! Let me see if I can cook up a decent example after the weekend.

Thanks for being active in here guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants