feat(conformance): Add test for invalid EPP service reference #959

SinaChavoshi · 2025-06-10T22:56:52Z

This PR introduces a new conformance test, InferencePoolInvalidEPPService, to validate controller behavior when an InferencePool is misconfigured.

Local validation results :

go test -v ./conformance -args -debug     -gateway-class gke-l7-regional-external-managed     -cleanup-base-resources=false     -run-test InferencePoolInvalidEPPService
...
=== NAME  TestConformance/InferencePoolInvalidEPPService
    apply.go:279: 2025-06-10T21:34:16.558081817Z: Deleting httproute-for-pool-non-existent-epp HTTPRoute
    apply.go:279: 2025-06-10T21:34:16.655140703Z: Deleting pool-non-existent-epp InferencePool
=== NAME  TestConformance
    suite.go:451: 2025-06-10T21:34:16.778401071Z: Sleeping 0s for test isolation
=== RUN   TestConformance/InferencePoolResolvedRefsCondition
    conformance.go:68: Skipping InferencePoolResolvedRefsCondition: test explicitly skipped
--- PASS: TestConformance (116.45s)
    --- SKIP: TestConformance/HTTPRouteInvalidInferencePoolRef (0.00s)
    --- SKIP: TestConformance/InferencePoolAccepted (0.00s)
    --- PASS: TestConformance/InferencePoolInvalidEPPService (112.70s)
        --- PASS: TestConformance/InferencePoolInvalidEPPService/Verify_Gateway_reports_Programmed:Invalid_backend_reference (112.05s)
    --- SKIP: TestConformance/InferencePoolResolvedRefsCondition (0.00s)
PASS
ok

k8s-ci-robot · 2025-06-10T22:57:02Z

Hi @SinaChavoshi. 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-06-10T22:57:04Z

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

Name	Link
🔨 Latest commit	`4413509`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/684a1ed9c0959d0008d044ab
😎 Deploy Preview	https://deploy-preview-959--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 project configuration.

robscott

Thanks @SinaChavoshi!

conformance/tests/basic/inferencepool_invalid_epp_service.go

robscott

Thanks @SinaChavoshi!

robscott · 2025-06-13T19:51:07Z

conformance/tests/basic/inferencepool_invalid_epp_service.go

+					Path: routePath,
+				},
+				Response: conformancehttp.Response{
+					StatusCode: http.StatusInternalServerError,


Mind including a change to the corresponding API spec for this?

https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/api/v1alpha2/inferencepool_types.go#L75

I'd recommend adding something along the lines of:

"When an ExtensionRef is invalid, such as pointing to a Service that does not exist, 500 status codes MUST be returned for requests that would have otherwise been routed to the InferencePool"

This is similar to https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1/httproute_types.go#L265C5-L266C73.

cc @danehans @shaneutt

@SinaChavoshi @robscott I pushed #991 to resolve this comment, PTAL.

robscott · 2025-06-13T19:52:00Z

/ok-to-test

danehans · 2025-06-16T18:28:18Z

/lgtm
/approve

k8s-ci-robot · 2025-06-16T18:28:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danehans, robscott, SinaChavoshi

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~~ [danehans]

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

…etes-sigs#959) * fix boilerplate header * add tests for InferencePoolInvalidEPPService * change to expect error on httproute refcond

…e it easier to add plugins (#881) * configuration implementation (after rebase...) Signed-off-by: Shmuel Kallner <[email protected]> * Moved plugin registry back to pkg/epp/plugins Signed-off-by: Shmuel Kallner <[email protected]> * Removed unneeded 'forced imports' of scorers Signed-off-by: Shmuel Kallner <[email protected]> * Changed 'profilepicker' to 'profilehandler' in new and old code Signed-off-by: Shmuel Kallner <[email protected]> * Pass the configured SchedulingProfiles to LoadSchedulerConfig Signed-off-by: Shmuel Kallner <[email protected]> * Ensure that both the configText and configFile flags are not specified Signed-off-by: Shmuel Kallner <[email protected]> * Load RequestControl plugins from the configuration Signed-off-by: Shmuel Kallner <[email protected]> * Register all plugin factories Signed-off-by: Shmuel Kallner <[email protected]> * Review fixes Signed-off-by: Shmuel Kallner <[email protected]> * Reverted unneeded change Signed-off-by: Shmuel Kallner <[email protected]> * Updates from review comments Signed-off-by: Shmuel Kallner <[email protected]> * Added a stub interface for plugins to get data from the EPP Signed-off-by: Shmuel Kallner <[email protected]> * Added a temporary implementation of plugins.Handle Signed-off-by: Shmuel Kallner <[email protected]> * Added pluginName and plugins.Handle to plugin factory interface Signed-off-by: Shmuel Kallner <[email protected]> * Updated plugin factory signatures to reflect new API Signed-off-by: Shmuel Kallner <[email protected]> * Updated plugin instantiation to reflect new API Signed-off-by: Shmuel Kallner <[email protected]> * Updated plugin instantiation to reflect new API Signed-off-by: Shmuel Kallner <[email protected]> * Updated tests to reflect new API Signed-off-by: Shmuel Kallner <[email protected]> * Do not rename the imported package Signed-off-by: Shmuel Kallner <[email protected]> * Only upper layer of code should log errors Signed-off-by: Shmuel Kallner <[email protected]> * Only pass what is needed to instantiate the plugins Signed-off-by: Shmuel Kallner <[email protected]> * Review updates Signed-off-by: Shmuel Kallner <[email protected]> * Review update Signed-off-by: Shmuel Kallner <[email protected]> * Review update. Make more clear that the code only checks for already defined names Signed-off-by: Shmuel Kallner <[email protected]> * fixed e2e doc in makefile (does not require GPUs) (#976) Signed-off-by: Nir Rozenbaum <[email protected]> * API: Adds 5xx Status Code for Invalid ExtRef (#991) Signed-off-by: Daneyon Hansen <[email protected]> * feat(conformance): Add test for invalid EPP service reference (#959) * fix boilerplate header * add tests for InferencePoolInvalidEPPService * change to expect error on httproute refcond * moved the creation of the context to main.go. (#995) this is useful when writing a different main like llm-d, allowing to propogate the same context to the whole system. Signed-off-by: Nir Rozenbaum <[email protected]> * fix dead links (#989) * feat: add health check for epp cluster (#966) * feat: add health check for epp cluster Signed-off-by: zhengkezhou1 <[email protected]> * remove tls Signed-off-by: zhengkezhou1 <[email protected]> * don't use tls Signed-off-by: zhengkezhou1 <[email protected]> * health checking flag Signed-off-by: zhengkezhou1 <[email protected]> * fix import Signed-off-by: zhengkezhou1 <[email protected]> * add tls options Signed-off-by: zhengkezhou1 <[email protected]> --------- Signed-off-by: zhengkezhou1 <[email protected]> * Server unit test and utility to help with such tests (#820) Signed-off-by: Ira <[email protected]> * Update dynamic-lora-sidecar to expose metrics to track loaded adapters (#980) * Add a metrics to track loaded adapters * Update the sample manifests * Add explanation of metrics from dyanmic LoRA adapter sidecar * Add explanation of metrics from dyanmic LoRA adapter sidecar (take 2) * Update metrics.md based on feedback * refactor: Replace prefix cache structure with golang-lru (#928) * refactor: Replace prefix cache structure with golang-lru Signed-off-by: Kfir Toledo <[email protected]> Co-authored-by: Maroon Ayoub <[email protected]> * fix: rename prefix scorer parameters and convert test to benchmark test Signed-off-by: Kfir Toledo <[email protected]> * feat: Add per server LRU capacity Signed-off-by: Kfir Toledo <[email protected]> * fix: Fix typos and error handle Signed-off-by: Kfir Toledo <[email protected]> * fix: add safety check for LRUCapacityPerServer Signed-off-by: Kfir Toledo <[email protected]> --------- Signed-off-by: Kfir Toledo <[email protected]> Co-authored-by: Maroon Ayoub <[email protected]> * feat(conformance): Add HTTPRouteMultipleRulesDifferentPools test (#834) * copy of accepted inference pool test to start from. * add yaml file for the test * update time out * update the yaml file to add port 9002 * read timeout config from local repo * remove excess comments * correct spelling for scenarios * check route condition on RouteConditionResolvedRefs * remove empty lines in yaml * set optional/defaulted fields as unspecified * fix timeout * fix boilerplate header * change varialbe names to use primary secondary consistently. * remove extra comments * factor out common code * Add actual http traffic validation using echo-basic * remove extra comments from manifest * remove modifiedTimeoutConfig.HTTPRouteMustHaveCondition per review comment. * intermediate update * fix the test run * factor out common code * move epp def to shared manifest * remove extra comments * revert back to two epps * add to do for epp image * switch to GeneralMustHaveConditionTimeout * undo gateway version changes * remove unused HTTPRouteMustHaveConditions * update doc string for GetPod * update docstring * Remove resource type from names in manifests. * remove type from name * remove health check * add todo for combining getpod methods * configuration implementation (after rebase...) Signed-off-by: Shmuel Kallner <[email protected]> * After review, made code more obvious Signed-off-by: Shmuel Kallner <[email protected]> * Fixed merge issues Signed-off-by: Shmuel Kallner <[email protected]> --------- Signed-off-by: Shmuel Kallner <[email protected]> Signed-off-by: Nir Rozenbaum <[email protected]> Signed-off-by: Daneyon Hansen <[email protected]> Signed-off-by: zhengkezhou1 <[email protected]> Signed-off-by: Ira <[email protected]> Signed-off-by: Kfir Toledo <[email protected]> Co-authored-by: Nir Rozenbaum <[email protected]> Co-authored-by: Daneyon Hansen <[email protected]> Co-authored-by: sina chavoshi <[email protected]> Co-authored-by: Xudong Wang <[email protected]> Co-authored-by: Zhengke Zhou <[email protected]> Co-authored-by: Ira Rosen <[email protected]> Co-authored-by: Shotaro Kohama <[email protected]> Co-authored-by: Kfir Toledo <[email protected]> Co-authored-by: Maroon Ayoub <[email protected]>

SinaChavoshi added 7 commits May 28, 2025 21:59

fix boilerplate header

bb257fd

Merge remote-tracking branch 'upstream/main'

fc2a410

Merge remote-tracking branch 'upstream/main'

d34457a

Merge remote-tracking branch 'upstream/main'

753917a

Merge remote-tracking branch 'upstream/main'

d5530dc

Merge remote-tracking branch 'upstream/main'

b57fb8a

add tests for InferencePoolInvalidEPPService

f7d8cbd

k8s-ci-robot requested a review from kfswain June 10, 2025 22:56

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

k8s-ci-robot requested a review from nirrozenbaum June 10, 2025 22:56

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 Jun 10, 2025

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 10, 2025

robscott reviewed Jun 11, 2025

View reviewed changes

conformance/tests/basic/inferencepool_invalid_epp_service.go Outdated Show resolved Hide resolved

change to expect error on httproute refcond

4413509

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 12, 2025

robscott approved these changes Jun 13, 2025

View reviewed changes

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 Jun 13, 2025

k8s-ci-robot assigned danehans Jun 16, 2025

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

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

k8s-ci-robot merged commit ea5f667 into kubernetes-sigs:main Jun 16, 2025
9 checks passed

zetxqx mentioned this pull request Jun 19, 2025

Conformance Tests for v0.5 Release #977

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(conformance): Add test for invalid EPP service reference #959

feat(conformance): Add test for invalid EPP service reference #959

Uh oh!

SinaChavoshi commented Jun 10, 2025

Uh oh!

k8s-ci-robot commented Jun 10, 2025

Uh oh!

netlify bot commented Jun 10, 2025 •

edited

Loading

Uh oh!

robscott left a comment

Uh oh!

Uh oh!

robscott left a comment

Uh oh!

robscott Jun 13, 2025

Uh oh!

danehans Jun 16, 2025

Uh oh!

robscott commented Jun 13, 2025

Uh oh!

danehans commented Jun 16, 2025

Uh oh!

k8s-ci-robot commented Jun 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat(conformance): Add test for invalid EPP service reference #959

feat(conformance): Add test for invalid EPP service reference #959

Uh oh!

Conversation

SinaChavoshi commented Jun 10, 2025

Uh oh!

k8s-ci-robot commented Jun 10, 2025

Uh oh!

netlify bot commented Jun 10, 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!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

robscott Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

danehans Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

robscott commented Jun 13, 2025

Uh oh!

danehans commented Jun 16, 2025

Uh oh!

k8s-ci-robot commented Jun 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

netlify bot commented Jun 10, 2025 •

edited

Loading