Update default target-pod and inject it into response metadata #270

ahg-g · 2025-01-31T23:22:55Z

Fixes #258
Fixes #181

k8s-ci-robot · 2025-01-31T23:23:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ahg-g]

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

netlify · 2025-01-31T23:23:11Z

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

Name	Link
🔨 Latest commit	`91d1d4f`
🔍 Latest deploy log	https://app.netlify.com/sites/gateway-api-inference-extension/deploys/67a11e4221bec50008738830
😎 Deploy Preview	https://deploy-preview-270--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.

liu-cong

/hold
/lgtm

Just a few nits. Feel free to unhold.

pkg/manifests/vllm/deployment.yaml

pkg/ext-proc/handlers/request.go

liu-cong · 2025-01-31T23:59:03Z

Pls also update this

gateway-api-inference-extension/docs/proposals/003-model-server-protocol/protocol.md

Line 15 in f9d2ef9

    
           adding the `target-pod` HTTP header in the request, or otherwise return an error.

pkg/ext-proc/handlers/request.go

AndresGuedez · 2025-02-01T16:47:19Z

pkg/ext-proc/handlers/request.go

 			},
 		},
+		DynamicMetadata: &structpb.Struct{
+			Fields: map[string]*structpb.Value{


This LGTM.

@yanavlasov PTAL as well.

ahg-g · 2025-02-03T18:50:49Z

Pls also update this

gateway-api-inference-extension/docs/proposals/003-model-server-protocol/protocol.md

Line 15 in f9d2ef9

adding the `target-pod` HTTP header in the request, or otherwise return an error.

done

liu-cong · 2025-02-03T19:01:20Z

/lgtm

robscott · 2025-02-03T19:37:32Z

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


 For each HTTP request, the EPP MUST communicate to the proxy the picked model server endpoint, via
-adding the `target-pod` HTTP header in the request, or otherwise return an error.
+adding the `x-gateway-destination-endpoint` HTTP header in the request and as an unstructured entry in the [dynamic_metadata](https://github.com/envoyproxy/go-control-plane/blob/c19bf63a811c90bf9e02f8e0dc1dcef94931ebb4/envoy/service/ext_proc/v3/external_processor.pb.go#L320) field of the ext-proc response, or otherwise return an error.


Just to clarify - we're setting this endpoint information here in two places to ensure a wide variety of implementations can support this? What should an implementation do if both values are set but differ? Should one have precedent over the other?

Just to clarify - we're setting this endpoint information here in two places to ensure a wide variety of implementations can support this?

Correct.

What should an implementation do if both values are set but differ? Should one have precedent over the other?

Good point, I think we need to say that they must be not differ, and if they do, the behavior is unknown, this is because some gateway providers will always only look at one of them, wdyt?

Yeah, I think that's reasonable, just wanted to make sure we had it written out.

robscott · 2025-02-03T19:52:23Z

Thanks @ahg-g!

/lgtm

ahg-g · 2025-02-03T19:54:24Z

/hold cancel

…netes-sigs#270) * Update default target-pod and inject it into response metadata * Addressing comments round 1 * Update the endpoint picker proposal * define the behavior when the two values differ

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 31, 2025

k8s-ci-robot requested review from kfswain and liu-cong January 31, 2025 23:23

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 31, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 31, 2025

liu-cong reviewed Jan 31, 2025

View reviewed changes

pkg/manifests/vllm/deployment.yaml Outdated Show resolved Hide resolved

pkg/ext-proc/handlers/request.go Outdated Show resolved Hide resolved

pkg/ext-proc/handlers/request.go Show resolved Hide resolved

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

k8s-ci-robot assigned liu-cong Jan 31, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2025

liu-cong reviewed Feb 1, 2025

View reviewed changes

pkg/ext-proc/handlers/request.go Outdated Show resolved Hide resolved

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 1, 2025

AndresGuedez reviewed Feb 1, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2025

ahg-g added 3 commits February 3, 2025 18:51

Update default target-pod and inject it into response metadata

fbe330d

Addressing comments round 1

c86ea56

Update the endpoint picker proposal

f381644

ahg-g force-pushed the header branch from 3f53f60 to f381644 Compare February 3, 2025 18:52

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2025

robscott reviewed Feb 3, 2025

View reviewed changes

define the behavior when the two values differ

91d1d4f

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 3, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 3, 2025

k8s-ci-robot assigned robscott Feb 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2025

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

k8s-ci-robot merged commit 614199e into kubernetes-sigs:main Feb 3, 2025
7 of 8 checks passed

Update default target-pod and inject it into response metadata #270

Update default target-pod and inject it into response metadata #270

Uh oh!

Conversation

ahg-g commented Jan 31, 2025

Uh oh!

k8s-ci-robot commented Jan 31, 2025

Uh oh!

netlify bot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

liu-cong left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liu-cong commented Jan 31, 2025

Uh oh!

Uh oh!

AndresGuedez Feb 1, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g commented Feb 3, 2025

Uh oh!

liu-cong commented Feb 3, 2025

Uh oh!

robscott Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robscott Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

robscott commented Feb 3, 2025

Uh oh!

ahg-g commented Feb 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

netlify bot commented Jan 31, 2025 •

edited

Loading

ahg-g Feb 3, 2025 •

edited

Loading