@danwinship: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

/sig network
/priority important-longterm

/cc @robscott

All addresses in the slice must be of the same type, do you mean creating an Slice with different address types in the Addresses field?

I'm not sure if/how kube-proxy should handle an endpoint with more than 1 address. I think the goal of kube-proxy is to give each endpoint an equivalent probability of being selected. To extend that logic here, I think we'd need to take the probability assigned to an endpoint and divide it by the number of addresses attached to that endpoint. So if an endpoint has a 50% probability of being selected, and it provides 50 addresses, each should end up with a 1% probability of being selected. Of course with the way we need to do descending probabilities, the logic may get a bit complicated.

This isn't as clear as it should be, but I don't think kube-proxy or any consumer of the EndpointSlice API needs to commit to supporting every value specified by that API. For example, many may filter by address type, Service, or some other means. I think it's also valid for a consumer to say they only support a single address per family per endpoint.

So all of that to say, I'd support either documenting that kube-proxy only selects the first address in an endpoint, or updating kube-proxy to support more than one address per endpoint. Given the number of changes currently in flight for kube-proxy, documentation seems like a safer initial approach to me.

/assign

can I have this？

editing only the first address(Address[0]) in Endpoint.Address to be used. The use of an array in this field is based on historical reasons. Isn't that right ？

@cyclinder thanks for volunteering to help with this one! I know @danwinship has some big kube-proxy changes/fixes in flight right now, so it would be good to be sure you don't conflict with him. Otherwise, as long as @danwinship isn't already working on this, I think you're free to have it.

editing only the first address(Address[0]) in Endpoint.Address to be used. The use of an array in this field is based on historical reasons. Isn't that right ？

I don't think we agreed that the array is for historical reasons only. We aren't currently using it, but we aren't ready to say that we won't use it in the future at this point.

So, the docs should warn that kube-proxy currently ignores additional addresses, but it shouldn't imply that it's actually wrong to have additional addresses

Otherwise, as long as @danwinship isn't already working on this, I think you're free to have it.

I'm not

thanks @danwinship @robscott

i will work on this

It was added for a reason, but I think that reason was mixed-family slices (before we decided to break it up into single-family slices).

What would it mean to have multiple addresses here? Are they assumed to be equivalent? Or do they have different context (like a split-horizon)? If the latter, there isn't any metadata here (i.e. it's not a struct like podIPs to allow new fields).

I am inclined to believe it was meant as equivalent, and as such it's probably OK to document that assumption clearly. That said, I don't see how anyone could use multiple addresses in a meaningful way - they are either assumed equivalent or they lack enough metadata to distinguish their differences.

??

If we do document the equivalence, it should be generically worded. E.g. "These addresses are assumed to be equivalent and some consumers may choose to ignore all but the first value."

What would it mean to have multiple addresses here? Are they assumed to be equivalent?

if it was meant as equivalent, why put multiple same addresses into Addresses? I don't understand what the purpose of this is.

The first version of the API had both IP families in a single slice, so it made sense to have multiple addresses. We changed that, though we didn't immediately remove the old support. So my best guess is that we should have converted this to a singular value when we did that, but we missed it.
…
On Wed, Nov 24, 2021, 5:45 PM Cyclinder @.***> wrote: What would it mean to have multiple addresses here? Are they assumed to be equivalent? if it was meant as equivalent, why put multiple same addresses into Addresses? what is the purpose of this? I don't understand what the purpose of this is. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#106267 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKWAVHO5EOMOKCAXCXHT3TUNWIMXANCNFSM5HVQ6CJQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

so we should change the API doc? Or It's just that documenting it will only use the first address in the pkg/proxy/endpointslicecache.go, like you said above

I'd like some of the others to comment, but it's this a US holiday, so it might be a few days..

I don't see how anyone could use multiple addresses in a meaningful way - they are either assumed equivalent or they lack enough metadata to distinguish their differences.

So maybe we should just declare that the array is essentially a bug, and only the first address is used? Maybe something should warn if you ever set additional addresses?

(FTR, the context that I noticed this in was that I was adding unit tests, and was creating EndpointSlices with a single Endpoint and multiple Addresses, which doesn't work; you need multiple Endpoints with a single Address each.)

I think the simplest and most compat approach is to say "these are all assumed to be fungible and clients may choose to only use the first element".

Thoughts?

I think the simplest and most compat approach is to say "these are all assumed to be fungible and clients may choose to only use the first element".

Big +1 to that approach. I don't think it's necessarily invalid to set multiple addresses here, there may actually be some use cases for it, but adding this kind of statement should help clear things up significantly.

hmm as we push for multi homed pod this will make a lot of sense. There might also be a chance for community to provide a drop-in replacement for proxy that does much more than we do now. I think we should update the docs and don't modify anything for now.

hmm as we push for multi homed pod this will make a lot of sense

🤔

I think the simplest and most compat approach is to say "these are all assumed to be fungible and clients may choose to only use the first element".

If they're fungible, then "clients may choose to use only one of the elements" (or "only a subset"?). No reason to call out the first one specifically.

I would like to add to this issue that it would makes sense to have kube-proxy reduce the subset of endpoints in the endpointslice based on the topology.kubernetes.io/zone label of the node and the zone & conditions attributes of the endpoint. Taking only into consideration the first endpoint which is in the same zone of the node or has no zone dependency and is ready for serving.

@thockin & @cyclinder: Would it make sense to reopen this issue?

I would like to add to this issue that it would makes sense to have kube-proxy reduce the subset of endpoints in the endpointslice based on the topology.kubernetes.io/zone label of the node and the zone & conditions attributes of the endpoint. Taking only into consideration the first endpoint which is in the same zone of the node or has no zone dependency and is ready for serving.

@thockin & @cyclinder: Would it make sense to reopen this issue?

I don't follow entirely what you are suggesting, can you elaborate?

@aojea

I don't follow entirely what you are suggesting, can you elaborate?

kube-proxy should understand which zone a node is in and prefer endpoints in the same zone.

@aojea

I don't follow entirely what you are suggesting, can you elaborate?

kube-proxy should understand which zone a node is in and prefer endpoints in the same zone.

but that is unrelated to this issue, topology is considered

See KEPs related, topology-aware-routing was replaced by topology-aware-hints

https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/536-topology-aware-routing
https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/2433-topology-aware-hints

@aojea
thank you for clarifying. It seems ambiguous to me to have a zone attribute and a hint attribute.
I fail to implement the hinting on a service in v1.23.8, but this is urelated to this issue.

@aojea thank you for clarifying. It seems ambiguous to me to have a zone attribute and a hint attribute. I fail to implement the hinting on a service in v1.23.8, but this is urelated to this issue.

heh, don't worry, this topic is complex and we went through different solutions, you have more historical context on the KEPs I linked above and this thread in the mailing list https://groups.google.com/g/kubernetes-sig-network/c/wXd1D_fKjqU/m/SEwjsOfpAAAJ