feat(conformance): Add HTTPRouteMultipleGatewaysDifferentPools test #838

SinaChavoshi · 2025-05-15T17:54:46Z

This PR introduces a new conformance test, HTTPRouteMultipleGatewaysDifferentPools, which validates a scenario where two distinct HTTPRoute resources, parented by different Gateway resources, successfully reference and route traffic to separate InferencePool backends.

local run results: ( Ran on commit be03841 )

go test -v ./conformance -args -debug     -gateway-class gke-l7-regional-external-managed     -cleanup-base-resources=false     -run-test HTTPRouteMultipleGatewaysDifferentPools
...
=== NAME  TestConformance
    suite.go:451: 2025-06-20T18:48:11.435814261Z: Sleeping 0s for test isolation
=== RUN   TestConformance/InferencePoolResolvedRefsCondition
    conformance.go:68: Skipping InferencePoolResolvedRefsCondition: test explicitly skipped
--- PASS: TestConformance (54.85s)
    --- SKIP: TestConformance/HTTPRouteInvalidInferencePoolRef (0.00s)
    --- PASS: TestConformance/HTTPRouteMultipleGatewaysDifferentPools (47.73s)
        --- PASS: TestConformance/HTTPRouteMultipleGatewaysDifferentPools/Primary_HTTPRoute,_InferencePool,_and_Gateway_path:_verify_status_and_traffic (32.54s)
        --- PASS: TestConformance/HTTPRouteMultipleGatewaysDifferentPools/Secondary_HTTPRoute,_InferencePool,_and_Gateway_path:_verify_status_and_traffic (14.56s)
    --- SKIP: TestConformance/InferencePoolAccepted (0.00s)
    --- SKIP: TestConformance/InferencePoolHTTPRoutePortValidation (0.00s)
    --- SKIP: TestConformance/InferencePoolInvalidEPPService (0.00s)
    --- SKIP: TestConformance/HTTPRouteMultipleRulesDifferentPools (0.00s)
    --- SKIP: TestConformance/InferencePoolResolvedRefsCondition (0.00s)
PASS
ok      sigs.k8s.io/gateway-api-inference-extension/conformance 55.055s

netlify · 2025-05-15T17:54:52Z

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

Name	Link
🔨 Latest commit	`be03841`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/6855ade714d6310008324bae
😎 Deploy Preview	https://deploy-preview-838--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.

k8s-ci-robot · 2025-05-15T17:54:56Z

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.

spencerhance · 2025-05-16T18:16:20Z

/cc

spencerhance

/lgtm

conformance/conformance.go

spencerhance · 2025-05-19T18:22:08Z

/lgtm

ahg-g · 2025-05-20T15:14:53Z

/ok-to-test

danehans · 2025-06-18T22:32:53Z

https://github.com/kubernetes-sigs/gateway-api-inference-extension/pull/838/files#r2150419700 needs to be resolved before merging.

/hold

conformance/resources/manifests/manifests.yaml

danehans · 2025-06-19T23:31:44Z

Implementation-specific resources should not be in conformance tests (#838 (comment)).

/hold

SinaChavoshi · 2025-06-20T18:54:03Z

https://github.com/kubernetes-sigs/gateway-api-inference-extension/pull/838/files#r2150419700 needs to be resolved before merging.

/hold

sorry was waiting for #982 to merge, this makes the code much simpler as all tests now share the EPP and inferencePool definitions from manifests.yaml

robscott

Thanks @SinaChavoshi! This mostly LGTM

robscott · 2025-06-20T22:46:22Z

conformance/utils/config/timing.go

+	// TODO - Remove this once the upstream timeout has been changed.
+	modifiedConfig := gatewayconfig.DefaultTimeoutConfig()
+	modifiedConfig.HTTPRouteMustHaveCondition = 300 * time.Second


We should override this via flag when running tests, but should not have a hardcoded override like this.

@robscott I actually did similar thing in https://github.com/kubernetes-sigs/gateway-api-inference-extension/pull/961/files#diff-a3410329c9007a0eb0b2c451387bbf6bf97821df458a0fdf6eb669f9565f0956 . I feel for the inference-gateway conformance tests we need to have a separate setting of timeout. And I feel here DefaultInferenceExtensionTimeoutConfig is a good place to put it in.

Flag is good but I assume we don't want to change the timeout a lot. So maybe hard code here is fine?

Yeah, I like that idea of having a separate set of defaults for this project. Just want to override something like this where we're hardcoding a value in a way that would make it impossible to override via flag.

robscott · 2025-06-20T22:47:34Z

conformance/utils/traffic/traffic.go

+
+	const (
+		eppSelectionHeader = "test-epp-endpoint-selection"
+		backendPort        = 3000


It seems like we should make IP:Port the input here instead of hardcoding port in this function.

We don't need to provide the port at all. EPP will use the port configured in inferencePool.

robscott · 2025-06-20T22:50:53Z

conformance/tests/basic/httproute_multiple_gateways_different_pools.go

+
+			primaryGwAddr := k8sutils.GetGatewayEndpoint(t, s.Client, s.TimeoutConfig, primaryGatewayNN)
+			primarySelector := labels.SelectorFromSet(labels.Set{backendAppLabelKey: primaryBackendLabel})
+			primaryPod := k8sutils.GetPod(t, s.Client, appBackendNamespace, primarySelector, s.TimeoutConfig.RequestTimeout)


Does this mean that each InferencePool needs to have exactly 1 Pod running? I thought that in other tests we were able to just check the prefix of a Pod name or something similar?

zetxqx · 2025-06-20T23:23:44Z

conformance/tests/basic/httproute_multiple_gateways_different_pools.yaml

+    - group: inference.networking.x-k8s.io
+      kind: InferencePool
+      name: primary-inference-pool
+      port: 80


nit: this is not needed any more based on #918

zetxqx · 2025-06-20T23:24:10Z

conformance/tests/basic/httproute_multiple_gateways_different_pools.yaml

+    - group: inference.networking.x-k8s.io
+      kind: InferencePool
+      name: secondary-inference-pool
+      port: 80


Same here, we can remove it

zetxqx · 2025-06-20T23:33:49Z

conformance/utils/config/timing.go

+	// TODO - Remove this once the upstream timeout has been changed.
+	modifiedConfig := gatewayconfig.DefaultTimeoutConfig()
+	modifiedConfig.HTTPRouteMustHaveCondition = 300 * time.Second


@robscott I actually did similar thing in https://github.com/kubernetes-sigs/gateway-api-inference-extension/pull/961/files#diff-a3410329c9007a0eb0b2c451387bbf6bf97821df458a0fdf6eb669f9565f0956 . I feel for the inference-gateway conformance tests we need to have a separate setting of timeout. And I feel here DefaultInferenceExtensionTimeoutConfig is a good place to put it in.

Flag is good but I assume we don't want to change the timeout a lot. So maybe hard code here is fine?

zetxqx · 2025-06-20T23:35:53Z

conformance/utils/traffic/traffic.go

+
+	const (
+		eppSelectionHeader = "test-epp-endpoint-selection"
+		backendPort        = 3000


We don't need to provide the port at all. EPP will use the port configured in inferencePool.

zetxqx · 2025-06-20T23:36:21Z

conformance/utils/traffic/traffic.go

+			Path: path,
+			Host: hostname,
+			Headers: map[string]string{
+				eppSelectionHeader: fmt.Sprintf("%s:%d", targetPod.Status.PodIP, backendPort),


only podIP is needed here.

k8s-ci-robot · 2025-06-21T03:27:06Z

PR needs rebase.

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.

SinaChavoshi added 8 commits May 14, 2025 20:30

copy of accepted inference pool test to start from.

87d1756

add yaml file for the test

a9dcb62

update time out

c36e5f8

update the yaml file to add port 9002

aaf23e2

read timeout config from local repo

2a67e7d

remove excess comments

8ac7d72

copy from existing test to start with

c859896

update test and add gateway to main manifest

fe540a7

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

k8s-ci-robot requested review from nirrozenbaum and robscott May 15, 2025 17:54

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 15, 2025

k8s-ci-robot requested a review from spencerhance May 16, 2025 18:16

spencerhance approved these changes May 16, 2025

View reviewed changes

conformance/conformance.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned spencerhance May 16, 2025

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

fix spelling error

63fedab

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2025

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

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 May 20, 2025

SinaChavoshi added 3 commits May 20, 2025 17:02

correct spelling for scenarios

66f325d

check route condition on RouteConditionResolvedRefs

fd0fa10

check RouteConditionResolvedRefs instead of Reconciled

68812d7

SinaChavoshi added 5 commits June 17, 2025 00:17

remove health check

daeb8e6

Merge remote-tracking branch 'upstream/main'

79b3015

Merge remote-tracking branch 'upstream/main' into inf_pool_tests_11

f1ab79f

add todo for combining getpod methods

81562a2

Merge branch 'inf_pool_tests_11' into inf_pool_tests_12

871d4d6

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

SinaChavoshi added 2 commits June 17, 2025 21:46

Merge remote-tracking branch 'upstream'

ae33369

Merge branch 'main' into inf_pool_tests_12

43c1fa1

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 17, 2025

remove duplicate methods

182ee70

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 18, 2025

danehans reviewed Jun 19, 2025

View reviewed changes

conformance/resources/manifests/manifests.yaml Outdated Show resolved Hide resolved

conformance/resources/manifests/manifests.yaml Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main' into inf_pool_tests_12

2afea17

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

SinaChavoshi added 5 commits June 20, 2025 17:37

revert changes to conformance.go

948956c

revert chagnes to conformance.go

eae8202

revert changes to helpers.go

e88bf77

use epp and pool from shared manifest

89869d7

add todo to factor out common constants

be03841

SinaChavoshi mentioned this pull request Jun 20, 2025

Conformance: Drop the resource type as a suffix from resource names #1030

Open

robscott reviewed Jun 20, 2025

View reviewed changes

zetxqx reviewed Jun 20, 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 Jun 21, 2025

feat(conformance): Add HTTPRouteMultipleGatewaysDifferentPools test #838

Are you sure you want to change the base?

feat(conformance): Add HTTPRouteMultipleGatewaysDifferentPools test #838

Uh oh!

Conversation

SinaChavoshi commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented May 15, 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!

k8s-ci-robot commented May 15, 2025

Uh oh!

spencerhance commented May 16, 2025

Uh oh!

spencerhance left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spencerhance commented May 19, 2025

Uh oh!

ahg-g commented May 20, 2025

Uh oh!

danehans commented Jun 18, 2025

Uh oh!

Uh oh!

Uh oh!

danehans commented Jun 19, 2025

Uh oh!

SinaChavoshi commented Jun 20, 2025

Uh oh!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jun 21, 2025

Uh oh!

Uh oh!

SinaChavoshi commented May 15, 2025 •

edited

Loading

netlify bot commented May 15, 2025 •

edited

Loading