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

Istio locality load balancing custom labels (Kubernetes) #19114

Closed
johscheuer opened this issue Nov 21, 2019 · 5 comments · Fixed by #19192
Closed

Istio locality load balancing custom labels (Kubernetes) #19114

johscheuer opened this issue Nov 21, 2019 · 5 comments · Fixed by #19192

Comments

@johscheuer
Copy link
Member

johscheuer commented Nov 21, 2019

Describe the feature request

I wanted to ask if it's feasible to add custom labels for the locality based load balancing. In the current setup Pilot supports the well-kown topology labels: https://istio.io/docs/ops/traffic-management/locality-load-balancing/#requirements. I would argue that it can make sense to to allow a user to overwrite these label keys with custom label keys e.g. failure-domain.myname/rack. In our concrete use case we are running on bare-metal and we set the region and zone labels (but all nodes have the same value since they are located in the same datacenter and we don't span Kubernetes across the datacenters). If we could overwrite the default region and zone label we could benefit from the locality load balancing (and fail-over).

Describe alternatives you've considered

As an alternative we could introduce the sub-zone in Kubernetes setups and allow user to define their custom label key to be used for thee sub-zone (if empty we would ignore it). IMHO this would be the better/easier alternative instead of overwriting the region and zone default label keys (since these are well-kown labels).

Obviously some one could argue "just set the right values to the region and zone labels", this would also be possible but In our case (and from our point of view) misusing the region and zone label for e.g. rack topology would be semantically incorrect.

Additional context
If we agree on a possible implementation I'm happy to contribute the according patch.

@howardjohn
Copy link
Member

As an alternative we could introduce the sub-zone in Kubernetes setups and allow user to define their custom label key to be used for thee sub-zone (if empty we would ignore it). IMHO this would be the better/easier alternative instead of overwriting the region and zone default label keys (since these are well-kown labels).

I was just thinking this would be trivial and useful change to make, so I definitely agree. Basically add a new topology.kubernetes.io/subZone annotation that people can add. Or maybe topology.istio.io/subZone, not sure how bad an idea it is to "steal" the kubernetes namespace.

Is there any use cases where the first approach (overriding the labels) is more useful? If not I say lets go ahead with that, should be a pretty simple change

@johscheuer
Copy link
Member Author

Personally I think it would be more useful to allow users to define the subzone label key because most users will have their nodes already labeled in some way. Allowing a user to use a custom label key for the subzone would have the benefit that he can reuse already existing label e.g. failure-domain.myname/rack. As a default value for the label key we could propose topology.istio.io/subZone and if a user wants he can overwrite it for his use case.

Also we should prevent hijacking the kubernetes.io label namespace where possible who know if this label will be added some time in the future.

@brianwolfe
Copy link
Member

Personally, I would prefer a fixed annotation from Istio (using topology.istio.io/subzone) to be more in line with the remaining topology domain specifications. If kubernetes standardizes a similar topology domain, it would make sense to deprecate the istio one, reading both in the meantime. Adding a flexible key selector seems harder to deprecate in the future and may overcomplicate the API.

An istio-specific annotation seems simpler, clearer in intent, and easier to deprecate if kubernetes standardizes an equivalent, more granular, topology domain.

@howardjohn
Copy link
Member

I agree with everything @brianwolfe said. I'll send a PR out next week when I am back in the office or anyone else can feel free to

@johscheuer
Copy link
Member Author

@howardjohn I don't if you already started working on it but if it's okay for you I would/could also implement this feature this week.

I also agree with you that a fixed label topology.istio.io/subzone makes sense to be domain specific and prevent to get over complicated.

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

Successfully merging a pull request may close this issue.

4 participants