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
eds: skip unhealthy endpoints #3137
Conversation
Unknown and healthy are both considered healthy.
clab1.addLocality(testSubZones[0], 1, testEndpointAddrs[:3], &addLocalityOptions{ | ||
health: []corepb.HealthStatus{ | ||
corepb.HealthStatus_HEALTHY, | ||
corepb.HealthStatus_UNHEALTHY, |
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.
Looks like there is also DRAINING, TIMEOUT, and DEGRADED. We should ideally include those as well.
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.
Done
623a434
to
1669f79
Compare
1669f79
to
a8bba8f
Compare
for i := 0; i < 4; i++ { | ||
addr := <-cc.newSubConnAddrsCh | ||
if addr[0].Addr != wantNewSubConnAddrStrs[i] { |
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.
Will they always happen in order?
Optional assuming they are always ordered for now: would a map[string]bool
work better for wantNewSubConnAddrStrs
? It could be more future-proof in case the order changes later. To ensure it doesn't do the same address four times, you could delete
the entries from the map as they are pulled in from the channel.
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.
Now with sort and cmp.Equal
Unknown and healthy are both considered healthy.
https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/core/health_check.proto#envoy-api-enum-core-healthstatus