EndpointSlice to IR Route Destinations#1494
Conversation
|
this issue is blocked on #1105 which is blocked on #1492, because we would like to provide a way to fallback to ClusterIP Loadbalancing support for users who are running on systems with EndpointSlice disabled or prefer L3 Loadbalancing via ClusterIP / kube-proxy before enabling EndpointSlice by default |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
Codecov Report
@@ Coverage Diff @@
## main #1494 +/- ##
==========================================
+ Coverage 65.44% 65.47% +0.02%
==========================================
Files 88 88
Lines 13031 13097 +66
==========================================
+ Hits 8528 8575 +47
- Misses 3979 3993 +14
- Partials 524 529 +5
|
https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#weighted-least-request Relates to envoyproxy#1105 Relevant now that we are introducing multiple endpoints with envoyproxy#1494 Signed-off-by: Arko Dasgupta <[email protected]>
internal/cmd/egctl/translate.go
Outdated
There was a problem hiding this comment.
I understand that we want the routing behavior to be backward compatible, but I think using endpoints as default to route traffic allows EG to bypass kube-proxy's overhead and fully leverage Envoy's advanced capabilities: more LB algorithms, session sticky, etc.
There was a problem hiding this comment.
I have disabled this in egctl x translate, because if we enable it, it will be mandatory, and all translations will fail if EndpointSlice is not provided by the user, this config is usually generated by k8s and is not part of user config, I was planning on raising a separate GH issue on dealing with this convenience / correctness dilemma
https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#weighted-least-request Relates to #1105 Relevant now that we are introducing multiple endpoints with #1494 Signed-off-by: Arko Dasgupta <[email protected]>
zhaohuabing
left a comment
There was a problem hiding this comment.
LGTM. Just need to fix the lint and test.
Relates to envoyproxy#1256 Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
add another level of indirection in RouteDestination called DestinationSetting Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
| ds := &ir.DestinationSetting{ | ||
| Weight: ptr.To(uint32(1)), | ||
| Endpoints: []*ir.DestinationEndpoint{ir.NewDestEndpoint(host, uint32(port))}, | ||
| } |
There was a problem hiding this comment.
this can be simplified to a function
There was a problem hiding this comment.
can this refactor be done in a follow up ?
this PR is changing 255 files most of which are YAML, and will affect other PRs
Signed-off-by: Arko Dasgupta <[email protected]>
Relates to #1256