Adds InferenceExtension to GatewayParameters #10601

danehans · 2025-02-10T18:36:09Z

Description

Introduces initial Gateway API Inference Extension support to GatewayParameters by adding a list of EndpointPicker extensions. Each EndpointPicker extension references an InferencePool that can be used as a backendRef for managed HTTPRoutes.

Note: Until #10594 is resolved, the inferenceExtension field is only supported when GatewayParameters is referenced by a Gateway.

Supports #10411

API changes

Adds InferenceExtension to KubernetesProxyConfig.

Code changes

All code changes are from make generate-all

CI changes

N/A

Docs changes

The generate-crd-reference-docs target is broken so this PR doe snot include CRD doc changes. @artberger does this target now live in a separate repo?

Context

See #10411 for additional context.

Interesting decisions

N/A

Testing steps

make go-test

Notes for reviewers

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

danehans · 2025-02-10T18:47:34Z

api/v1alpha1/gateway_parameters_types.go

+	// InferenceExtension defines the desired state of the Gateway API inference extension.
+	// For additional details, see: https://gateway-api-inference-extension.sigs.k8s.io/.
+	//
+	// InferenceExtension can only be specified when GatewayParameters is referenced by a Gateway.


Note: The inferenceExtension field is only supported when the GatewayParameters is referenced by a Gateway until #10594 is resolved.

if we specify on a gateway class, why can't a gateway "inherit" it?

@yuval-k thanks for the review and feedback. Since multiple Gateways can reference a GatewayClass the EndpointPickerExtension API would need to associate a Gateway to an InferencePool. This can be achieved using a list map keyed on the Gateway namespaced name. For example:

type InferenceExtension struct { // +listType=map // +listMapKey=gatewayNsName // +kubebuilder:validation:MaxItems=8 EndpointPickers []EndpointPickerExtension `json:"endpointPickers"` }

Each EndpointPickerExtension must now define gatewayNsName: <GW_NS>/<GW_NAME>. This is acceptable when GatewayParameters is referenced by a GatewayClass but is overly verbose when GatewayParameters is referenced by a Gateway.

why do we need to associate Gateway to an InferencePool? i thought InferencePool represents the upstream?

An extension is associated to an InferencePool:

apiVersion: apps/v1 kind: Deployment metadata: name: inference-gateway-ext-proc ... containers: - name: inference-gateway-ext-proc image: us-central1-docker.pkg.dev/k8s-staging-images/gateway-api-inference-extension/epp:main args: - -poolName - "test-infer-pool"

InferencePool does represent the upstream. The implementation can reconcile HTTPRoutes with an InferencePool backendRef and use the connection configuration from the InferencePool to configure the parentRefs Gateways. I was trying to be explicit with the initial implementation, e.g. associate an extension with a Gateway. I'll work through the implementation without the additional verbosity and see if I hit any roadblocks.

danehans · 2025-02-10T18:49:46Z

api/v1alpha1/gateway_parameters_types.go

@@ -612,6 +620,52 @@ type AiExtension struct {
 	Stats *AiExtensionStats `json:"stats,omitempty"`
 }

+// InferenceExtension defines the desired state of the Gateway API inference extension.
+// For additional details, see: https://gateway-api-inference-extension.sigs.k8s.io/.
+type InferenceExtension struct {


Note: Additional extensions will be introduced in future releases of Gateway API inference extension.

timflannagan · 2025-02-10T18:59:32Z

Do we need to add this field to the default GW params object in install/helm/kgateway?

jenshu · 2025-02-10T18:57:42Z

api/v1alpha1/gateway_parameters_types.go

+	// EndpointPickers defines a list of EndpointPicker extensions.
+	//
+	// +kubebuilder:validation:MaxItems=8
+	EndpointPickers []EndpointPickerExtension `json:"endpointPickers"`


omitempty should only be used with optional fields and this field is required (xref).

jenshu · 2025-02-10T18:57:56Z

api/v1alpha1/gateway_parameters_types.go

+	// this GatewayParameters.
+	//
+	// +kubebuilder:validation:Required
+	PoolRef InferencePoolObjRef `json:"poolRef"`


should this be a pointer?

also, omitempty?

Pointers should only be used for optional fields (xref).

Just wanted to clarify for myself. Optional fields in our apis have to have:

a pointer type in the Go definition or have a built-in nil value (so maps and lists are fine, ie. Priorities []Priority json:"priorities,omitempty"` is okay).

omitempty set

How is the They have the +optional comment tag in Go. used? https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#optional-vs-required Do we want to start adding the +optional comment tag to non-required fields?

And then for required fields, do we want to use // +kubebuilder:validation:Required or just:

Make the field not have the omitempty

Make it not a pointer

Yes, it's good to have our APIs follow k8s API conventions and use +optional for optional fields.

+kubebuilder:validation:Required can be used for required fields but just to be explicit since this is default behavior.

jenshu · 2025-02-10T18:59:53Z

api/v1alpha1/gateway_parameters_types.go

+	// +kubebuilder:default="inference.networking.x-k8s.io"
+	// +kubebuilder:validation:MaxLength=253
+	// +kubebuilder:validation:Pattern=`^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
+	Group string `json:"group,omitempty"`


can this accept arbitrary gvks, or only InferencePools?

Only InferencePools but it's idiomatic to use Group/Kind (with defaults) for xReference APIs. This approach also provides the option (if needed) to use a kgateway CRD to represent the Endpoint Picker extension.

timflannagan · 2025-02-10T20:49:26Z

The generate-crd-reference-docs target is broken so this PR doe snot include CRD doc changes. @artberger does this target now live in a separate repo?

Known issue that's being tracked in #10568.

yuval-k · 2025-02-11T14:29:03Z

api/v1alpha1/gateway_parameters_types.go

+	// this GatewayParameters.
+	//
+	// +kubebuilder:validation:Required
+	PoolRef InferencePoolObjRef `json:"poolRef"`


I thought InferencePool is meant to be used as a Backend on an http route? why do we need it in the gateway params?

The EndpointPicker represents a single instance of the Endpoint Picker extension. The Gateway uses the PoolRef to discover the extension's configuration, e.g. how to connect to it.

I'm still not sure why this is on the gateway params? what's the difference between the PoolRef here and the one on the HttpRoute?

See #10601 (comment).

danehans · 2025-02-11T18:49:47Z

Do we need to add this field to the default GW params object in install/helm/kgateway?

Yes, but the Helm install will be a separate PR. I wanted this PR to focus purely on the GatewayParameter changes required to introduce Inference Extension.

danehans · 2025-02-11T19:28:11Z

@yuval-k @jenshu @timflannagan I rebased and responded to your comments, PTAL.

danehans · 2025-02-12T17:18:46Z

Marking as "draft" as I work through the initial implementation based on #10601 (comment).

danehans · 2025-02-18T18:43:36Z

@yuval-k commit 593313a refactors the API to enable/disable the EPP extension.

Signed-off-by: Daneyon Hansen <[email protected]>

danehans · 2025-02-22T19:34:31Z

/hold since #10676 does not require any input to enable inference extension functionality. Instead, the controller and deployer are enabled if the InferencePool CRD exists.

Co-authored-by: Nathan Fudenberg <[email protected]> Co-authored-by: changelog-bot <changelog-bot>

danehans commented Feb 10, 2025

View reviewed changes

jenshu reviewed Feb 10, 2025

View reviewed changes

yuval-k reviewed Feb 11, 2025

View reviewed changes

danehans force-pushed the issue_10411_part1 branch from 5ed7c30 to 110b220 Compare February 11, 2025 19:18

danehans mentioned this pull request Feb 12, 2025

Add Initial Gateway API Inference Extension Support #10411

Open

16 tasks

danehans marked this pull request as draft February 12, 2025 17:18

danehans force-pushed the issue_10411_part1 branch from 110b220 to 593313a Compare February 18, 2025 18:40

danehans marked this pull request as ready for review February 18, 2025 18:41

danehans force-pushed the issue_10411_part1 branch from 593313a to 0c5bfa7 Compare February 18, 2025 22:11

Adds Initial InferencePool Support to GatewayParameters

698b433

Signed-off-by: Daneyon Hansen <[email protected]>

danehans force-pushed the issue_10411_part1 branch from 0c5bfa7 to 698b433 Compare February 18, 2025 22:26

danehans marked this pull request as draft February 22, 2025 19:34

stevenctl pushed a commit to stevenctl/gloo that referenced this pull request Feb 24, 2025

initial TLSRoute support (kgateway-dev#10601)

c03f3c5

Co-authored-by: Nathan Fudenberg <[email protected]> Co-authored-by: changelog-bot <changelog-bot>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds InferenceExtension to GatewayParameters #10601

Adds InferenceExtension to GatewayParameters #10601

danehans commented Feb 10, 2025

danehans Feb 10, 2025

yuval-k Feb 11, 2025

danehans Feb 11, 2025

yuval-k Feb 12, 2025 •

edited

Loading

danehans Feb 12, 2025

danehans Feb 10, 2025

timflannagan commented Feb 10, 2025

jenshu Feb 10, 2025

danehans Feb 11, 2025 •

edited

Loading

jenshu Feb 10, 2025

danehans Feb 11, 2025

npolshakova Feb 19, 2025 •

edited

Loading

npolshakova Feb 19, 2025

danehans Feb 22, 2025 •

edited

Loading

jenshu Feb 10, 2025

danehans Feb 11, 2025

timflannagan commented Feb 10, 2025

yuval-k Feb 11, 2025

danehans Feb 11, 2025

yuval-k Feb 12, 2025

danehans Feb 12, 2025

danehans commented Feb 11, 2025

danehans commented Feb 11, 2025

danehans commented Feb 12, 2025

danehans commented Feb 18, 2025

danehans commented Feb 22, 2025

Adds InferenceExtension to GatewayParameters #10601

Are you sure you want to change the base?

Adds InferenceExtension to GatewayParameters #10601

Conversation

danehans commented Feb 10, 2025

Description

API changes

Code changes

CI changes

Docs changes

Context

Interesting decisions

Testing steps

Notes for reviewers

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuval-k Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timflannagan commented Feb 10, 2025

Choose a reason for hiding this comment

danehans Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npolshakova Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danehans Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timflannagan commented Feb 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danehans commented Feb 11, 2025

danehans commented Feb 11, 2025

danehans commented Feb 12, 2025

danehans commented Feb 18, 2025

danehans commented Feb 22, 2025

yuval-k Feb 12, 2025 •

edited

Loading

danehans Feb 11, 2025 •

edited

Loading

npolshakova Feb 19, 2025 •

edited

Loading

danehans Feb 22, 2025 •

edited

Loading