inference: initial removal of Envoy deployer#12689
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes support for InferencePool with the kgateway class (deprecated in v2.1), while maintaining support through the agentgateway class. The changes include removing the Envoy deployer infrastructure, cleaning up inference extension test code, and updating configurations to use agentgateway instead of kgateway.
Key Changes
- Removed the Envoy deployer for InferencePool, including the controller, Helm charts, and plugin infrastructure
- Deleted endpoint picker validation, status management, IR, and collection code
- Updated test gateway configuration to use
agentgatewayclass
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/kubernetes/e2e/features/inferenceextension/testdata/gateway.yaml | Updated gatewayClassName from kgateway to agentgateway |
| pkg/deployer/deployer_test.go | Removed InferencePool endpoint picker deployment test |
| internal/kgateway/wellknown/controller.go | Removed InferencePoolFinalizer constant |
| internal/kgateway/translator/httproute/translate_httproute_test.go | Removed InferencePool backend translation tests |
| internal/kgateway/krtcollections/policy_test.go | Removed InferencePool backend lookup and port override tests |
| internal/kgateway/krtcollections/policy.go | Removed InferencePool port normalization logic |
| internal/kgateway/helm/inference-extension/* | Removed entire inference extension Helm chart |
| internal/kgateway/helm/embed.go | Removed InferenceExtensionHelmChart embed |
| internal/kgateway/extensions2/plugins/inferenceextension/endpointpicker/* | Removed all endpoint picker plugin files |
| internal/kgateway/deployer/inference_extension.go | Removed InferenceExtension deployer |
| internal/kgateway/deployer/gateway_parameters_test.go | Removed InferencePool GVK watch test |
| internal/kgateway/deployer/deployer_factory.go | Removed NewInferencePoolDeployer factory |
| internal/kgateway/controller/start.go | Removed InferencePool controller initialization |
| internal/kgateway/controller/inferencepool_controller* | Removed InferencePool controller and tests |
| internal/kgateway/controller/controller.go | Removed InferencePool watch and index setup |
| Makefile | Changed conformance gateway class from kgateway to agentgateway |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
6a9e0ee to
367eb98
Compare
b681004 to
e2df76c
Compare
timflannagan
left a comment
There was a problem hiding this comment.
Overall LGTM. There's some krtcollection code that could be cleaned up (afaik, agw doesn't use that code?) and I think we can remove the GIE envoy-based plugin as well.
timflannagan
left a comment
There was a problem hiding this comment.
Plugin needed for status'.
\# Description Reimplements the existing Controller Ginkgo suite test as a Go/Testify suite. It fixes the following issues found in the previous implementation: - Use of global suite context in objects initialized per test. This breaks tests run in parallel. - Use of hacky context cancellation to stop the controller from within tests. - Incorrectly stopping and restarting the controller from within tests to work around specific race conditions. A suite testing the core controller should not stop it to avoid test races. Stopping the controller can mask bugs in both tests and the controller code. - Fragile assertions in the metrics tests that rely on approximations that break when the test or reconciler is udpated. - Bugs in test cleanup, which results in stale envtest files and zombie processes lingering after the test has completed. It addresses the above issues, and in addition does the following: - Consolidates all controller specific tests into a single Testify unit test suite. This means the controller is shared for all tests and doesn't require to be initialized per test. This speeds up tests and avoids test hacks to stop/start the controller. - Ports each Ginkgo test to stdlib Go test within the suite. - Uses Patch() instead of Update() to update the Gateway Service status in the test, as Update silently discards the status update without an error when the Deployer concurrently writes the same Service. - Preemptively removes Inference controller tests that are being removed in PR kgateway-dev#12689 instead of porting them. \# Change Type ``` /kind cleanup /kind flake ``` \# Changelog ```release-note NONE ``` Signed-off-by: Shashank Ram <[email protected]>
\# Description Reimplements the existing Controller Ginkgo suite test as a Go/Testify suite. It fixes the following issues found in the previous implementation: - Use of global suite context in objects initialized per test. This breaks tests run in parallel. - Use of hacky context cancellation to stop the controller from within tests. - Incorrectly stopping and restarting the controller from within tests to work around specific race conditions. A suite testing the core controller should not stop it to avoid test races. Stopping the controller can mask bugs in both tests and the controller code. - Fragile assertions in the metrics tests that rely on approximations that break when the test or reconciler is udpated. - Bugs in test cleanup, which results in stale envtest files and zombie processes lingering after the test has completed. It addresses the above issues, and in addition does the following: - Consolidates all controller specific tests into a single Testify unit test suite. This means the controller is shared for all tests and doesn't require to be initialized per test. This speeds up tests and avoids test hacks to stop/start the controller. - Ports each Ginkgo test to stdlib Go test within the suite. - Uses Patch() instead of Update() to update the Gateway Service status in the test, as Update silently discards the status update without an error when the Deployer concurrently writes the same Service. - Preemptively removes Inference controller tests that are being removed in PR kgateway-dev#12689 instead of porting them. \# Change Type ``` /kind cleanup /kind flake ``` \# Changelog ```release-note NONE ``` Signed-off-by: Shashank Ram <[email protected]>
\# Description Reimplements the existing Controller Ginkgo suite test as a Go/Testify suite. It fixes the following issues found in the previous implementation: - Use of global suite context in objects initialized per test. This breaks tests run in parallel. - Use of hacky context cancellation to stop the controller from within tests. - Incorrectly stopping and restarting the controller from within tests to work around specific race conditions. A suite testing the core controller should not stop it to avoid test races. Stopping the controller can mask bugs in both tests and the controller code. - Fragile assertions in the metrics tests that rely on approximations that break when the test or reconciler is udpated. - Bugs in test cleanup, which results in stale envtest files and zombie processes lingering after the test has completed. It addresses the above issues, and in addition does the following: - Consolidates all controller specific tests into a single Testify unit test suite. This means the controller is shared for all tests and doesn't require to be initialized per test. This speeds up tests and avoids test hacks to stop/start the controller. - Ports each Ginkgo test to stdlib Go test within the suite. - Uses Patch() instead of Update() to update the Gateway Service status in the test, as Update silently discards the status update without an error when the Deployer concurrently writes the same Service. - Preemptively removes Inference controller tests that are being removed in PR kgateway-dev#12689 instead of porting them. \# Change Type ``` /kind cleanup /kind flake ``` \# Changelog ```release-note NONE ``` Signed-off-by: Shashank Ram <[email protected]>
e2df76c to
a5dac9d
Compare
|
|
||
| // Inferencepool backing for routes | ||
| backingPool *inf.InferencePool |
There was a problem hiding this comment.
This should be removed when the legacy Inference extension plugin, e.g. internal/kgateway/extensions2/plugins/inferenceextension/endpointpicker is removed, not when the deployer is removed.
There was a problem hiding this comment.
Tests will be added as a part of Agw translation
| }) | ||
| }) | ||
|
|
||
| Context("HTTPRoute resource routing with InferencePool backendRef", func() { |
There was a problem hiding this comment.
|
TODOs:
|
\# Description Removes the Envoy-based controller and deployer for the Inference extension. \# Change Type ``` /kind breaking_change /kind cleanup ``` \# Changelog ```release-note Support for InferencePool with the `kgateway` class which was deprecated in v2.1 has been removed. Support is available with the `agentgateway` class. ``` \# Additional Notes Fixes kgateway-dev#12619 Signed-off-by: Shashank Ram <[email protected]>
a5dac9d to
0b08707
Compare
|
0b08707 removes the inferencce extension plugin that is responsible for managing InferencePool status. Cherry-pick danehans@bf21eb2 and e2e/conformance tests will pass. |
Signed-off-by: Daneyon Hansen <[email protected]>
Signed-off-by: Shashank Ram <[email protected]> Signed-off-by: Daneyon Hansen <[email protected]> Co-authored-by: Daneyon Hansen <[email protected]>
…outing kgateway v2.2 removed InferencePool routing from its Envoy data plane (PR kgateway-dev/kgateway#12689, deprecated in v2.1). The Gateway API Inference Extension support that AICR uses for CNCF AI Conformance Advanced Ingress moved entirely to the separate agentgateway project, which ships its own Helm charts, GatewayClass, controller, and AgentgatewayParameters CRD. AICR's only kgateway consumer was the inference Gateway resource — zero HTTPRoutes, no TrafficPolicy/BackendConfigPolicy/etc. — so this PR replaces kgateway entirely rather than running both side-by-side. Changes: - registry + mixin: kgateway/kgateway-crds (v2.0.0, cr.kgateway.dev) -> agentgateway/agentgateway-crds (v2.2.1, cr.agentgateway.dev) - inference-gateway.yaml: GatewayParameters (gateway.kgateway.dev) -> AgentgatewayParameters (agentgateway.dev/v1alpha1, strategic-merge patch on Deployment spec); gatewayClassName: kgateway -> agentgateway; namespace kgateway-system -> agentgateway-system - health check + component dirs renamed under git mv to preserve history - conformance validator (validators/conformance/inference_gateway_check.go) updated to gate on `agentgateway` component and query the new GatewayClass / namespace / Deployment names - evidence collector script + requirement title updated - GPU H100 inference workflow path filter follows the renamed component directories so PRs touching only agentgateway* still trigger CI - Go test fixtures, undeploy.sh.tmpl + golden files, chainsaw assertions, UAT recipe snapshots regenerated against the new component graph - docs/user (component-catalog, container-images, api-reference, cli-reference) refreshed; container-images.md regenerated via `make bom-docs` - demos/cuj2-* + demos/query.md + demos/images/meta.md updated so kubectl commands and architecture diagrams match the new namespace and component names Conformance-equivalence: AICR's `ai_inference` requirement is data-plane agnostic — it asserts a GatewayClass is Accepted, a Gateway is Programmed, and the InferencePool CRDs exist. Swapping to agentgateway preserves all five evidence checks; only the names in the captured output change. Historical evidence snapshots under docs/conformance/cncf/v1.35/nim-eks/, demos/examples/CUJ2-Test-Report.md, design doc 005, and CHANGELOG entries are intentionally left referencing kgateway as frozen records. Closes follow-up #1 from issue NVIDIA#698. Validation: - make qualify (Go unit tests, golangci-lint, yamllint, 20/20 chainsaw including cli-bundle-agentgateway-templates, vulnerability scan, license headers): all green - Bundle generation verified end-to-end (recipe -> bundle -> deploy.sh layout under /tmp); rendered post chart confirmed to emit AgentgatewayParameters + Gateway with class agentgateway in namespace agentgateway-system - validators/conformance package tests pass (22.6s)
Description
Fixes #12618
Change Type
/kind cleanup
Changelog
Additional Notes
I am 90% sure we need to port the status writing over to Agentgateway