Amend the endpoint picker protocol to support multiple fallback endpoints #761

wbpcode · 2025-04-30T13:30:54Z

Part of #414

This PR extend value format of x-gateway-destination-endpoint to support multiple endpoints.
The new format could support mutliple fallback endpoints.
The new format is compatible with previous format becase the previous format could be treat a speical case of new format.
This will simplify the proxy implementation. The proxy needn't to handle different endpoint sources.
This will simplify the EPP implementation because the EPP needn't to set different endpoint sources.
It's possible to specify the fallback endpoints by HTTP header now.

linux-foundation-easycla · 2025-04-30T13:30:58Z

The committers listed above are authorized under a signed CLA.

✅ login: wbpcode / name: code (c62d3b3, c204cf7, 6a3c8f6, 1047cb6, f7d96a8)

k8s-ci-robot · 2025-04-30T13:31:02Z

Welcome @wbpcode!

It looks like this is your first PR to kubernetes-sigs/gateway-api-inference-extension 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gateway-api-inference-extension has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-04-30T13:31:03Z

Hi @wbpcode. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

netlify · 2025-04-30T13:32:53Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`6a3c8f6`
🔍 Latest deploy log	https://app.netlify.com/sites/gateway-api-inference-extension/deploys/6822c0369eba680009f39f78
😎 Deploy Preview	https://deploy-preview-761--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

…ints Signed-off-by: wangbaiping(wbpcode) <[email protected]>

wbpcode · 2025-04-30T13:46:07Z

When I am reviewing the related Envoy implementation of this EPP (envoyproxy/envoy#38757), I found the current design only support single fallback endpoint. That inspire me to create this PR.

And if we want to support multiple fallback endpoints, the EPP must send a list of addresses to proxy. If there must be a list of addresses, i think it would be better to make the x-gateway-destination-endpoint a list directly. Then we needn't break the list into different two headers or metadata entries.

And from the proxy's pespective, I will prefer only one source to get the external specfied endpoints. And the proxy could iterate the endpoints list in order and select the first valid one. If the retrying happens, the proxy could continue the iteration from the position where it stop last time.

cc @yanavlasov

ahg-g · 2025-04-30T20:38:17Z

/assign @LiorLieberman for review

/hold

ahg-g · 2025-04-30T20:38:32Z

/ok-to-test

LiorLieberman · 2025-05-01T00:42:08Z

/cc

LiorLieberman

Thanks @wbpcode!

Question - how would it work with an implementation that leverage envoy subsets?

For ref - see the istio implementation of this API

wbpcode · 2025-05-01T01:31:18Z

Thanks @wbpcode!

Question - how would it work with an implementation that leverage envoy subsets?

For ref - see the istio implementation of this API

Sorry，I am not allowed to access the doc. But I think it is impossible for subset lb based solution to support multiple fallback endpoints. That's say, if the istio's implementation is used, then the EPP couldn't support multiple fallback endpoints anyway and the legacy format should be used. And I believe the subset lb based solution couldn't support retrying also. (The problem is similar with the original dst based solution.)

I will also suggest the istio to update the implementation to use the new override host lb.

From the reference document, seems it's an experimental implementation and hasn't implemented the fallback mechanism, so I think it's possible to take the retrying and multiple fallback endpoints into account at this timepoint.

wbpcode · 2025-05-01T01:41:17Z

I think another alternative is to define a new key like x-gateway-destination-endpoints for both primary host and multiple fallback endpoints. But anyway, the EPP may need additional switch to enable fallback endpoints or not, it is possible to keep compatibility with the implementation detail of istio.

The istio haven't release this feature and I think it's possible to switch subset lb to the override host lb in the future.

danehans · 2025-05-01T16:33:25Z

docs/proposals/004-endpoint-picker-protocol/README.md

+}
+```
+
+Single fallback endpoint also CAN be set using the key `x-gateway-destination-endpoint-fallback` in the same metadata namespace as one used for `x-gateway-destination-endpoint` as follows:


Should the EPP still support x-gateway-destination-endpoint-fallback if x-gateway-destination-endpoint now supports endpoint fallback by listing multiple addresses? cc: @ahg-g @kfswain

Is the API break change acceptable or is there a way to deprecate the x-gateway-destination-endpoint-fallback? Ideally, the EPP should has a flag to enable the new format or use the legacy format. And then we can deprecate the legacy format gradually.

danehans · 2025-05-01T16:44:03Z

docs/proposals/004-endpoint-picker-protocol/README.md

 - Setting different value leads to unpredictable behavior because proxies aren't guaranteed to support both paths, and so this protocol does not define what takes precedence.

 ### Destination endpoint fallback
-A single fallback endpoint CAN be set using the key `x-gateway-destination-endpoint-fallback` in the same metadata namespace as one used for `x-gateway-destination-endpoint` as follows:
+
+For each HTTP request, if destination endpoint fallback is necessary or possible, the EPP CAN set the `x-gateway-destination-endpoint` HTTP header or metadata entry with multiple addresses in `<ip:port>,<ip:port>,...` format. Multiple addresses are separated by commas. The first valid endpoint in the addresses list will be used as the primary endpoint. And if retrying is happening, the proxy will try the endpoints after the selected endpoint in order.


Should we continue to have this subsection or update the ## Destination Endpoint section to state that one address MUST be set or multiple addresses CAN be set for fallback purposes? Thoughts @ahg-g @kfswain?

I have updated content of the ## Destination Endpoint section.

The value of the header or metadata entry MUST contains at least one endpoint in `<ip:port>` format or multiple endpoints in `<ip:port>,<ip:port>,...` format. Multiple endpoints are separated by commas. The first valid endpoint in the value will be used. And if retrying is happening, the proxy will try the endpoints after the previously selected endpoint in order.

The x-gateway-destination-endpoint-fallback is still be kept for backward compatibility.

AndresGuedez · 2025-05-01T18:14:25Z

docs/proposals/004-endpoint-picker-protocol/README.md

 - Setting different value leads to unpredictable behavior because proxies aren't guaranteed to support both paths, and so this protocol does not define what takes precedence.

 ### Destination endpoint fallback
-A single fallback endpoint CAN be set using the key `x-gateway-destination-endpoint-fallback` in the same metadata namespace as one used for `x-gateway-destination-endpoint` as follows:
+
+For each HTTP request, if destination endpoint fallback is necessary or possible, the EPP CAN set the `x-gateway-destination-endpoint` HTTP header or metadata entry with multiple addresses in `<ip:port>,<ip:port>,...` format. Multiple addresses are separated by commas. The first valid endpoint in the addresses list will be used as the primary endpoint. And if retrying is happening, the proxy will try the endpoints after the selected endpoint in order.


Note that this is a breaking change that will impact the Istio implementation at least, which is based on Envoy LB subsetting and requires a single value to be specified.

Adding @LiorLieberman for feedback.

From API's perspective, this is not a break change. But if take the implementation detail into account, this actually may break the istio implementation. But ideally, we should have a flag to enable the new format or use legacy format in the EPP.

left a comment below

LiorLieberman

The istio haven't release this feature and I think it's possible to switch subset lb to the override host lb in the future.

Its definitely possible to switch. I assume you are talking about envoyproxy/envoy#38757 ?

Will need to to catch up with this PR its been a month or two since I last look at it.

This PR is not even merged yet though - whats the timeline to get it a non experimental API in envoy?

wbpcode · 2025-05-06T02:15:11Z

The istio haven't release this feature and I think it's possible to switch subset lb to the override host lb in the future.

Its definitely possible to switch. I assume you are talking about envoyproxy/envoy#38757 ?

Will need to to catch up with this PR its been a month or two since I last look at it.

This PR is not even merged yet though - whats the timeline to get it a non experimental API in envoy?

We hope the envoyproxy/envoy#38757 could be used to support EPP with multiple fallback endpionts and retrying support.

If we can get agreement with the multiple addresses header/metadata format (<ip>:<port>,<ip>:<port>,..., no matter which key is used), I think it will be pretty quick for Envoy to merge that (may be only one or two weeks, I will discuss that with @yanavlasov)

wbpcode · 2025-05-09T02:58:22Z

cc @LiorLieberman The override host lb is merged and will be released at next envoy major version. :)

With the new override host lb, both the legacy format and the new format could be supported. With following configuration, all the endpoints will be joined as an endpoints list. The first valid endpoint will be used. And if retrying is happening, the proxy will try the endpoints after the previously selected endpoint in order.

By this way, multiple fallback endpoints, retrying could be supported. And it's compatible to the legacy format (every header/metadata contains only one endpoint).

override_host_sources:
- header: "x-gateway-destination-endpoint"
- metadata:
    key: "envoy.lb"
    path:
    - key: "x-gateway-destination-endpoint"
- header: "x-gateway-destination-endpoint-fallback"
- metadata:
    key: "envoy.lb"
    path:
    - key: "x-gateway-destination-endpoint-fallback"

or

override_host_sources:
- header: "x-gateway-destination-endpoint"
- header: "x-gateway-destination-endpoint-fallback"

or

override_host_sources:
- header: "x-gateway-destination-endpoint"

Signed-off-by: wangbaiping(wbpcode) <[email protected]>

LiorLieberman · 2025-05-09T04:00:18Z

Thanks, will take a deeper look tmrw. FTR I opened istio/istio#56230 and cc'd you earlier today.

Also https://docs.google.com/document/d/1F82G_o3ENM94Imx5pYKG0x-m508_FSJyfbu1d8J8CCk/edit which is still in early stages (you will need to be either istio or dev@Kubernetes to view it)

LiorLieberman

I am going to take this proposal with other istio folks, hopefully we can converge on override_host_lb, but would like to get the feedback first.

@wbpcode To clarify - what is the benefit of supporting multiple keys in x-gateway-destination-endpoint as opposed to x-gateway-destination-endpoint-fallback or -fallback*s*?

This will simplify the proxy implementation. The proxy needn't to handle different endpoint sources.

This will simplify the EPP implementation because the EPP needn't to set different endpoint sources.

It's possible to specify the fallback endpoints by HTTP header now.

I dont see any major simplification by moving the fallbacks to x-gateway-destination-endpoint. I would in fact prefer to leave x-gateway-destination-endpoint a single value now, and in the future maybe evolve it to support some key:value map or list formatted values. For example one random idea in mind, x-gateway-destination-endpoint: zone-a:<ip:port>,zone-b:<ip-port> to support some version of topology. Is this too crazy?

wbpcode · 2025-05-10T08:40:58Z

To clarify - what is the benefit of supporting multiple keys in x-gateway-destination-endpoint as opposed to x-gateway-destination-endpoint-fallback or -fallbacks

The key benefit is the multiple fallback endpoints support. In the current proposal, only one primary endpoint and an optional fallback endpoint could be selected.

With the new format, the EPP server could select a list of endpoints and the proxy could try them one by one. And from the perspective of the API, this is a compatible change.

I dont see any major simplification by moving the fallbacks to x-gateway-destination-endpoint.

At the initial design of the override host lb, we designed a little complex API to support fallback endpoints. This is why I think moving the fallbacks to x-gateway-destination-endpoint could simplify the proxy implementation. But finally we resolved it in the new design.

But anyway, I still think single/clear/explicit endpoints source would be helpful for both proxy and EPP server.

I would in fact prefer to leave x-gateway-destination-endpoint a single value now, and in the future maybe evolve it to support some key:value map or list formatted values. For example one random idea in mind, x-gateway-destination-endpoint: zone-a:ip:port,zone-b: to support some version of topology. Is this too crazy?

If there is reasonable requirement, I think it's fine to support more complex format in the future. But considering the fact the <ip:port> basically is the most fine-grained unit, I didn't get why the zone make sense here?

howardjohn

why do we need 2 lists of endpoints? I would expect either a single endpoint + fallback OR a list of endpoints, but not 2 lists?

docs/proposals/004-endpoint-picker-protocol/README.md

danehans · 2025-05-12T15:46:18Z

docs/proposals/004-endpoint-picker-protocol/README.md

@@ -58,7 +60,16 @@ dynamicMetadata: {
 }
 ```

-### Why envoy.lb namespace as a default? 
+The endpoints specified in `x-gateway-destination-endpoint-fallback` MAY be tried after the endpoints specified in `x-gateway-destination-endpoint` if all the endpoints specified in `x-gateway-destination-endpoint` are unavailable.


When x-gateway-destination-endpoint defines more than one IP:Port, the additional entries act as a fallback, correct? If so, why is x-gateway-destination-endpoint-fallback still needed?

it's unnecessary, but not sure if it's ok or not to remove it directly from the proposal. I will removed it first.

wbpcode · 2025-05-13T03:37:13Z

why do we need 2 lists of endpoints? I would expect either a single endpoint + fallback OR a list of endpoints, but not 2 lists?

Yeah. I think we all prefer the only one list of endpoints. In the format, Only the x-gateway-destination-endpoint could be a list of endpoints and the x-gateway-destination-endpoint-fallback is only be kept for API backward compatibility.

I have removed the x-gateway-destination-endpoint-fallback according to comments from @danehans @howardjohn

Signed-off-by: wangbaiping(wbpcode) <[email protected]>

danehans · 2025-05-13T19:39:09Z

Only the x-gateway-destination-endpoint could be a list of endpoints and the x-gateway-destination-endpoint-fallback is only be kept for API backward compatibility.

@LiorLieberman, can you confirm whether x-gateway-destination-endpoint-fallback is still required by the Istio implementation? If so, @wbpcode, we can mark it as deprecated and remove it in a future release.

LiorLieberman

If there is reasonable requirement, I think it's fine to support more complex format in the future. But considering the fact the ip:port basically is the most fine-grained unit, I didn't get why the zone make sense here?

Theoretically the API can evolve to support something like "this is the best endpoint in this zone" (or any other topology key). Thats what I was referring to.

@LiorLieberman, can you confirm whether x-gateway-destination-endpoint-fallback is still required by the Istio implementation? If so, @wbpcode, we can mark it as deprecated and remove it in a future release.

Istio does not support this key currently, so it is safe from this perspective. But would be good to check with all implementations of this API, last time we had a breaking change in the EPP istio release could not function.

If you all in agreement that one list is better than two, I am fine with that. I thought a different key is slightly cleaner and leaves more room for expansion but this opinion is not very strong.

k8s-ci-robot · 2025-05-13T22:49:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiorLieberman, wbpcode
Once this PR has been reviewed and has the lgtm label, please assign jeffwan for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

danehans · 2025-05-14T16:47:33Z

@ahg-g, since you applied the hold, PTAL now that @LiorLieberman provided feedback.

cc: @arkodg and @kfswain to confirm whether Envoy Gateway uses x-gateway-destination-endpoint-fallback.

wbpcode · 2025-05-19T13:59:49Z

friendly ping @ahg-g

k8s-ci-robot requested review from ahg-g and danehans April 30, 2025 13:30

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 30, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 30, 2025

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 30, 2025

Amend the endpoint picker protocol to support multiple fallback endpo…

c62d3b3

…ints Signed-off-by: wangbaiping(wbpcode) <[email protected]>

wbpcode force-pushed the dev-multiple-fallback-support branch from 77fbe07 to c62d3b3 Compare April 30, 2025 13:34

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Apr 30, 2025

wbpcode mentioned this pull request Apr 30, 2025

Dynamic forwarding load balancing policy envoyproxy/envoy#38757

Merged

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 30, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 30, 2025

k8s-ci-robot requested a review from LiorLieberman May 1, 2025 00:42

LiorLieberman reviewed May 1, 2025

View reviewed changes

danehans reviewed May 1, 2025

View reviewed changes

AndresGuedez reviewed May 1, 2025

View reviewed changes

danehans mentioned this pull request May 1, 2025

Add Initial Gateway API Inference Extension Support kgateway-dev/kgateway#10411

Open

25 tasks

LiorLieberman reviewed May 3, 2025

View reviewed changes

LiorLieberman mentioned this pull request May 8, 2025

Support new override_host_lb envoy load balancing policy istio/istio#56230

Open

address comments

1047cb6

Signed-off-by: wangbaiping(wbpcode) <[email protected]>

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 9, 2025

add optionally

f7d96a8

Signed-off-by: wangbaiping(wbpcode) <[email protected]>

LiorLieberman reviewed May 9, 2025

View reviewed changes

howardjohn reviewed May 11, 2025

View reviewed changes