Refactor EPP initialization for unified integration testing by zetxqx · Pull Request #2351 · kubernetes-sigs/gateway-api-inference-extension

zetxqx · 2026-02-17T07:34:20Z

What type of PR is this?

/kind test

What this PR does / why we need it:

Runner Refactoring (cmd/epp/runner):
- Extracted Setup() from Run(). This method initializes the Manager, Datastore, and ExtProcServerRunner but yields them before starting, allowing tests to inspect state or inject dependencies.
- Added IsIntegrationTest to Options. When enabled,
  * this injects a FakePodMetricsClient
  * bypasses k8s controller name validation.
- Fix: Corrected the argument order for setupDatastore, swapping PoolName and PoolNamespace to match the function signature in EPP startup setupDatastore is passing in wrong parameter in standalone mode #2367
Integration Test Harness (test/integration):
- Rewrote NewTestHarness to use eppRunner.Setup() instead of manually constructing the server stack.
- Updated hermetic_test.go to configure tests via server.Options, ensuring tests run with a configuration path identical to the main binary.
Utilities:
- Refactored StartExtProcServer and extracted ExtProcServerClient to separate server execution from client connection management.

Which issue(s) this PR fixes:

Fixes partially #2332 , further refactor is needed, current implementation still rely on a IsIntegrationTest flag in opts.

This flag is now used in limited two places:

replace the metrics client with a fake one.
for k8s controller skipNameValidation to true.

Fixes #2367

Does this PR introduce a user-facing change?:

…eModelRewrites is being watched.

k8s-ci-robot · 2026-02-17T07:34:23Z

@zetxqx: The label(s) kind/test cannot be applied, because the repository doesn't have them.

Details

In response to this:

What type of PR is this?

/kind test

What this PR does / why we need it:

Integration test for the fix in #2300

small refactor: Split Runner.Run in cmd/epp/runner/runner.go into Run and Setup. The new Setup method returns the ctrl.Manager and datastore.Datastore instances, enabling testability in integration testing.

Added TestCRDWatchers. it verifies that the EPP correctly watches InferenceModelRewrite and InferenceObjective resources and update the dataStore.

Which issue(s) this PR fixes:

Fixes partially #2332

Does this PR introduce a user-facing change?:

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 · 2026-02-17T07:34:26Z

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

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

zetxqx · 2026-02-18T07:23:06Z

Note: the new commit introduces opts.IsIntegrationTest in the prod code path. But I think it's an acceptable state before more refactoring. Given opts.IsIntegrationTest is only used for two places.

inject the fake metrics client.
bypass the k8s name validation check.

Open to any comment for further refactoring.

nirrozenbaum · 2026-02-18T08:36:34Z

Note: the new commit introduces opts.IsIntegrationTest in the prod code path. But I think it's an acceptable state before more refactoring. Given opts.IsIntegrationTest is only used for two places.

inject the fake metrics client.

bypass the k8s name validation check.

Open to any comment for further refactoring.

I'm not in favor of adding IsIntegrationTest flag in production. this is error prone and could lead to wrong deployments.

I propose to try an alternative approach:
inside cmd/epp, we can have additional test_runner.go file.
we need to expose setting some of the things in runner (add private fields in runner, add private functions to set them, e.g., function called withPodMetricsClient).
so these functions cannot be called outside of epp/runner package.
then in runner.go, we add conditionals, for example if podMetricsClient is already set, don't create it but use whatever was set.

then tests can use test_runner.go.
what do you think?

zetxqx · 2026-02-18T09:12:35Z

Note: the new commit introduces opts.IsIntegrationTest in the prod code path. But I think it's an acceptable state before more refactoring. Given opts.IsIntegrationTest is only used for two places.

inject the fake metrics client.

bypass the k8s name validation check.

Open to any comment for further refactoring.

I'm not in favor of adding IsIntegrationTest flag in production. this is error prone and could lead to wrong deployments.

I propose to try an alternative approach: inside cmd/epp, we can have additional test_runner.go file. we need to expose setting some of the things in runner (add private fields in runner, add private functions to set them, e.g., function called withPodMetricsClient). so these functions cannot be called outside of epp/runner package. then in runner.go, we add conditionals, for example if podMetricsClient is already set, don't create it but use whatever was set.

then tests can use test_runner.go. what do you think?

Great suggestion, I moved those custom method for integration test to separate test_runner.go under cmd/epp in the new commit 22808d1 . However since the integration test is under different folder, it's not possible to make them private. Should we consider move the current integration test test/integration/epp/hermetic_test.go under cmd/epp ?

nirrozenbaum · 2026-02-18T10:25:51Z

Great suggestion, I moved those custom method for integration test to separate test_runner.go under cmd/epp in the new commit 22808d1 . However since the integration test is under different folder, it's not possible to make them private. Should we consider move the current integration test test/integration/epp/hermetic_test.go under cmd/epp ?

would adding a NewTestRunner function help?
that should return NewRunner().WithTestPodMetricsClient...

zetxqx · 2026-02-18T17:44:28Z

Great suggestion, I moved those custom method for integration test to separate test_runner.go under cmd/epp in the new commit 22808d1 . However since the integration test is under different folder, it's not possible to make them private. Should we consider move the current integration test test/integration/epp/hermetic_test.go under cmd/epp ?

would adding a NewTestRunner function help? that should return NewRunner().WithTestPodMetricsClient...

Good call, updated. Refactored more, now we only need to set withSkipNameValidation before call setup(). this can be further eliminated by not share testEnv for every test cases. But that may make the test run slower.

ahg-g

Great!

…r.setup, remove serverRunner in testHarness and setUp return

ahg-g · 2026-02-19T02:50:22Z

 	testEnv       *envtest.Environment
 	testScheme    = runtime.NewScheme()
-	logger        = zap.New(zap.UseDevMode(true), zap.Level(zapcore.Level(logutil.DEFAULT)))
+	logger        = zap.New(zap.UseDevMode(true), zap.Level(-1*zapcore.Level(logutil.DEFAULT)))


what does the -1 mean?

I found there is no log while debugging and found this zapcore actually needs to multiply -1 to take effect

ahg-g · 2026-02-19T02:51:56Z

-	// 7. Register Cleanup
 	t.Cleanup(func() {
-		serverCancel()
+		// serverCancel()


how do we cancel the server now? don't we need to create a context with cancel, and pass the context to NewTestRunnerSetup above and call cancel here?

Removed the comment. The server was ext_proc server started manually in the harness. Now since we are using setup we only needs to cancel the k8s manager, ext_proc is one of the runners managed by the manger.

zetxqx · 2026-02-19T20:03:53Z

/retest

ahg-g · 2026-02-19T21:48:38Z

You didn't push a commit, also the failed test is a lint error: https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_gateway-api-inference-extension/2351/pull-gateway-api-inference-extension-verify-main/2024575702841430016

pass in the context

ahg-g · 2026-02-20T11:45:30Z

/lgtm
/approve

k8s-ci-robot · 2026-02-20T11:45:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, zetxqx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [ahg-g]

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

capri-xiyue · 2026-02-20T17:28:18Z

/retest

…es-sigs#2351) * add integration test for checking the inferenceObjective and inferenceModelRewrites is being watched. * continue refactor integration * not use opts.IsIntegrationTest. * inject PodMetricsClient via SetUp * remove unneeded function, rename skipNameValidation, unexported runner.setup, remove serverRunner in testHarness and setUp return * encapsulate more to harness * Update test/integration/epp/hermetic_test.go pass in the context --------- Co-authored-by: Abdullah Gharaibeh <[email protected]>

…es-sigs/gateway-api-inference-extension#2351) * add integration test for checking the inferenceObjective and inferenceModelRewrites is being watched. * continue refactor integration * not use opts.IsIntegrationTest. * inject PodMetricsClient via SetUp * remove unneeded function, rename skipNameValidation, unexported runner.setup, remove serverRunner in testHarness and setUp return * encapsulate more to harness * Update test/integration/epp/hermetic_test.go pass in the context --------- Co-authored-by: Abdullah Gharaibeh <[email protected]>

add integration test for checking the inferenceObjective and inferenc…

db39204

…eModelRewrites is being watched.

k8s-ci-robot requested review from danehans and nirrozenbaum February 17, 2026 07:34

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 17, 2026

ahg-g reviewed Feb 17, 2026

View reviewed changes

Comment thread cmd/epp/runner/runner.go Outdated

ahg-g reviewed Feb 17, 2026

View reviewed changes

Comment thread test/integration/epp/hermetic_test.go Outdated

continue refactor integration

68fce88

zetxqx force-pushed the integrationtest-crdwatcher branch from b6d63bc to 68fce88 Compare February 18, 2026 06:48

zetxqx changed the title ~~adds integration test validation for CRD watchers and small refactor on EPP runner~~ Refactor EPP initialization for unified integration testing Feb 18, 2026

not use opts.IsIntegrationTest.

22808d1

ahg-g reviewed Feb 18, 2026

View reviewed changes

Comment thread cmd/epp/runner/runner.go Outdated

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 18, 2026

inject PodMetricsClient via SetUp

f4fd95c

zetxqx force-pushed the integrationtest-crdwatcher branch from 9682cce to f4fd95c Compare February 18, 2026 17:51

ahg-g reviewed Feb 18, 2026

View reviewed changes

Comment thread cmd/epp/runner/runner.go Outdated

Comment thread cmd/epp/runner/runner.go Outdated

Comment thread cmd/epp/runner/test_runner.go Outdated

Comment thread pkg/epp/server/controller_manager.go Outdated

Comment thread cmd/epp/runner/runner.go Outdated

nirrozenbaum reviewed Feb 18, 2026

View reviewed changes

Comment thread cmd/epp/runner/runner.go Outdated

remove unneeded function, rename skipNameValidation, unexported runne…

e3c8ccc

…r.setup, remove serverRunner in testHarness and setUp return

ahg-g reviewed Feb 19, 2026

View reviewed changes

ahg-g mentioned this pull request Feb 19, 2026

Add hermetic test for epp with inferencepool #2277

Merged

encapsulate more to harness

4a56d94

zetxqx force-pushed the integrationtest-crdwatcher branch from 1034b2f to 4a56d94 Compare February 20, 2026 00:33

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 20, 2026

ahg-g reviewed Feb 20, 2026

View reviewed changes

Comment thread test/integration/epp/hermetic_test.go Outdated

Update test/integration/epp/hermetic_test.go

b2ce70d

pass in the context

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 20, 2026

k8s-ci-robot assigned ahg-g Feb 20, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2026

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 20, 2026

k8s-ci-robot merged commit 688c0cc into kubernetes-sigs:main Feb 20, 2026
11 checks passed

zetxqx deleted the integrationtest-crdwatcher branch February 20, 2026 18:09

Conversation

zetxqx commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Feb 17, 2026

Uh oh!

netlify Bot commented Feb 17, 2026 • 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!

Uh oh!

Uh oh!

zetxqx commented Feb 18, 2026

Uh oh!

nirrozenbaum commented Feb 18, 2026

Uh oh!

zetxqx commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nirrozenbaum commented Feb 18, 2026

Uh oh!

Uh oh!

zetxqx commented Feb 18, 2026

Uh oh!

ahg-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahg-g Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

zetxqx Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

ahg-g Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

zetxqx Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zetxqx commented Feb 19, 2026

Uh oh!

ahg-g commented Feb 19, 2026

Uh oh!

Uh oh!

ahg-g commented Feb 20, 2026

Uh oh!

k8s-ci-robot commented Feb 20, 2026

Uh oh!

capri-xiyue commented Feb 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zetxqx commented Feb 17, 2026 •

edited

Loading

netlify Bot commented Feb 17, 2026 •

edited

Loading

zetxqx commented Feb 18, 2026 •

edited

Loading