-
Notifications
You must be signed in to change notification settings - Fork 592
add priority to workload entry #3619
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
|
😊 Welcome @ramaraochavali! This is either your first contribution to the Istio api repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
howardjohn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this relate to the many many other 'priority' mechanisms we have? what if its priority=3 but in another region with locality LB enabled?
This seems like it could be done today with DR failoverPriority? https://istio.io/latest/docs/reference/config/networking/destination-rule/#LocalityLoadBalancerSetting-failover_priority
I do not think this can be solved by DR. This is for endpoints matched by a DNS cluster. So they do not have k8s labels associated.
The priority specified here would be taken in to consideration for DNS endpoints specified overriding the locality LB specified in DR. As I mentioned, this is mainly useful for workloadentries specified via DNS resolution to implement failover across two DNS clusters. |
What do you mean? Its a WorkloadEntry, it has labels? |
|
It is the same workload entry that has different DNS as endpoints. It is used in the context of ServiceEntry as two different endpoints |
1 similar comment
|
It is the same workload entry that has different DNS as endpoints. It is used in the context of ServiceEntry as two different endpoints |
|
how can 1 workload entry have multiple DNS addresses? maybe showing an example would help me understand |
|
Sorry. I meant two workload entries used in the context of ServiceEntry as two different DNS end points. We could create synthetic labels for each to identify priority but that sounds very indirect way of doing it for external endpoints. |
|
Here is the Service Entry I want to create - with two external endpoints. My goal is to have google.com endpoints have priority 0 and salesforce.com endpoints have priority 1. I want to configure a DR so that when google.com endpoints become unavailable, traffic goes through salesforce.com endpoints. Here is the DR I tried. But priorities wont change based on failoverPriority because the endpoints are resolved by Envoy. Am I missing some thing here that we can make this setup work with existing priority settings? |
|
Even you set priority in the WLE, you cannot achieve auto failover by priority.
This is because we donot suport DNS cluster failover yet. Haven't tried if envoy works in this case |
It works. I have validated it if we set the correct priority to each LocalityLbEndpoint. |
|
BTW, The DR I created to just validate failOverPriority for DNS does not work today. If we add priority to WLE and set directly, we do not need DR changes |
IMO this is a bug. There is no reason for this other than the implementation in the control plane is wrong because we pass nil here (https://github.com/istio/istio/blob/2eae57b36d1740f8e44e54c0efde07bfb9c706c5/pilot/pkg/networking/core/cluster_traffic_policy.go#L329) but depend on it in the locality code |
Yes. This can be fixed (though it is not as straight forward as EDS). But the point I am making is, for DNS type of Endpoints defined via Service Entries, it seems creating synthetic labels to specify priority seems not intuitive (or indirect way) than specifying the priority directly when defining the WLE. We can argue if we need a different way to set priority than relying on failoverPriority in DR but IMO it seems simpler to specify priority directly instead of having manage labels especially when there are more than two endpoints in the SE. |
|
The main concern is that WorkloadEntry can also be used separate object other than endpoints inlined in SE. If we support priority in the spec, it seems lack of flexibility when used separately. Two options may compromise:
|
I agree with this. I think it has partly to do with how Resolution is there in SE and reusing WorkloadEntry as is there. We can say if priority is set for WLE, it is considered and rest of the priority calculations are not done for DNS type of SEs. Other option is to have If we decide not to add any api option, we can always suggest DR option after fixing the bug (But I need to check how complicated the config will be if there are more than two endpoints) |
Why not for all WorkloadEntry? Why not for Pod? why not for EndpointSlice? We have a way to do priority, I don't think we should have multiple conflicting ways. How do we explain to users which to use? What happens when both are set? |
It is mainly because for the above it can be determined by k8s labels (location, network etc) auto set by some controllers which we have control over and it is not needed to set priority for each endpoint in the slice (other than group of endpoints). But for DNS, the priority can be at each endpoint and can vary based on many constraints like if you are using LLM as DNS endpoint, cost or some preferred LLM for certain workloads etc. It may not always be possible to express them as labels.
We can say explicit priority is set that is honoured. Any way, I am also in parallel trying to fix the failOverPriority for DNS endpoints and see if that helps |
Adds priority to workload entry. This is especially useful to setup active/passive between two clusters defined by two different DNS based clusters. When endpoints in priority(0), becomes unhealthy - traffic automatically moves to priority1.