Skip to content
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

Adds EP-10411: Gateway API Inference Extension Support #10420

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

danehans
Copy link
Contributor

@danehans danehans commented Jan 8, 2025

Adds Enhancement Proposal 10411 that proposes adding Gateway API Inference Extension support.

Supports #10411

@danehans
Copy link
Contributor Author

danehans commented Jan 8, 2025

cc: @EItanya

Copy link
Contributor

@EItanya EItanya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, just some detail questions

docs/enhancements/10411.md Show resolved Hide resolved
docs/enhancements/10411.md Outdated Show resolved Hide resolved
docs/enhancements/10411.md Outdated Show resolved Hide resolved
docs/enhancements/10411.md Outdated Show resolved Hide resolved
stevenctl added a commit to stevenctl/gloo that referenced this pull request Jan 15, 2025
Signed-off-by: Daneyon Hansen <[email protected]>
Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: Daneyon Hansen <[email protected]>
Co-authored-by: Yuval Kohavi <[email protected]>
Co-authored-by: Sam Heilbron <[email protected]>
Co-authored-by: Nathan Fudenberg <[email protected]>
Co-authored-by: Ryan Old <[email protected]>
Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Bernie Birnbaum <[email protected]>
Co-authored-by: Rachael Graham <[email protected]>
Co-authored-by: Ravinder Punia <[email protected]>
Co-authored-by: sheidkamp <[email protected]>
Co-authored-by: Jenny Shu <[email protected]>
Co-authored-by: Nadine Spies <[email protected]>
Co-authored-by: David Jumani <[email protected]>
Co-authored-by: Eitan Yarmush <[email protected]>
Co-authored-by: Nina Polshakova <[email protected]>
Co-authored-by: Steven Landow <[email protected]>
Co-authored-by: Ariana W. <[email protected]>
@danehans
Copy link
Contributor Author

@EItanya @yuval-k or @linsun for additional approvals.

docs/enhancements/10411.md Outdated Show resolved Hide resolved
docs/enhancements/10411.md Outdated Show resolved Hide resolved
docs/enhancements/10411.md Outdated Show resolved Hide resolved
docs/enhancements/10411.md Outdated Show resolved Hide resolved
docs/enhancements/10411.md Outdated Show resolved Hide resolved
### Configuration

* Update the [configuration](https://github.com/k8sgateway/k8sgateway/blob/main/install/helm/gloo/generate/values.go) API to enable/disable this feature.
* Update Helm charts to install/uninstall k8sgateway with this feature based on user-provided configuration.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is enabled via GatewayParameters, does it require helm charts change? Trying to understand if this is configured at install time of CP or deployment of the gateway DP?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A user should be able to set a Helm knob to enable this feature. The internal config plumbing should convert the helm know into the equivalent GatewayParameters setting. This will avoid users having to configure the GatewayParameters for GIE during runtime. Depending on the time required to implement this EP, I may defer the Helm piece as follow-up work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM thanks for the explanation!

Adds Enhancement Proposal 10411 that proposes adding Gateway API
Inference Extension support.

Signed-off-by: Daneyon Hansen <[email protected]>
Signed-off-by: Daneyon Hansen <[email protected]>
@timflannagan
Copy link
Member

@danehans Just a heads up that we moved the enhancements repository outside of the docs/ directory into https://github.com/kgateway-dev/kgateway/tree/main/design.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enhancements template was moved to design: https://github.com/kgateway-dev/kgateway/tree/main/design.


### Configuration

* Update the [configuration](https://github.com/kgateway-dev/kgateway/blob/main/install/helm/gloo/generate/values.go) API to enable/disable this feature.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We ripped out the legacy chart (and therefore the values.go generate file). See https://github.com/kgateway-dev/kgateway/tree/main/install/helm/kgateway for the new location.

* Secure the gRPC connection between Gateway and GIE implementations.
* Support kgateway upgrades when this feature is enabled.

## Implementation Details
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can update this proposal as we iterate on the initial implementation, but I think it would be nice to sketch out the helm chart / API changes, e.g. GatewayParameter, changes for historical context.


### Plugin

* Add GIE as a supported [plugin](https://github.com/kgateway-dev/kgateway/tree/main/projects/gateway2/extensions2/plugins). The plugin will manage [Endpoints](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/endpoint.html) based on the InferencePool resource specification. The Gateway implementation, e.g. Envoy proxy, will forward matching requests using the [External Processing Filter](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/ext_proc/v3/ext_proc.proto#external-processing-filter-proto) to the ES deployment. The ES is responsible for processing the request, selecting an Endpoint, and returning the selected Endpoint to Envoy for routing.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up that the projects/ directory was deflated in #10583. We're also planning on renaming the gateway2 directory in #10602 as a follow-up to that initial PR.

Copy link
Contributor

@npolshakova npolshakova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the EP from docs/enhancements/10411.md to design: https://github.com/kgateway-dev/kgateway/tree/main/design

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants