adds integration test validation for CRD watchers and small refactor on EPP runner#2351
adds integration test validation for CRD watchers and small refactor on EPP runner#2351zetxqx wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
…eModelRewrites is being watched.
|
@zetxqx: The label(s) DetailsIn response to this:
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. |
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zetxqx The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| return nil | ||
| } | ||
|
|
||
| func (r *Runner) Setup(ctx context.Context, cfg *rest.Config, opts *runserver.Options) (ctrl.Manager, datastore.Datastore, error) { |
There was a problem hiding this comment.
Pls add a comment here that we split the set up into a separate function to enable integration testing of the set up.
| opts.GRPCHealthPort = healthPort | ||
| opts.EndpointTargetPorts = []int{8000} | ||
|
|
||
| mgr, dataStore, err := eppRunner.NewRunner().Setup(ctx, testEnv.Config, opts) |
There was a problem hiding this comment.
Can we have this test invoke NewTestHarness, and have NewTestHarness call runner.Setup?
What type of PR is this?
/kind test
What this PR does / why we need it:
Integration test for the fix in #2300
Runner.Runincmd/epp/runner/runner.gointoRunandSetup. The newSetupmethod returns thectrl.Manageranddatastore.Datastoreinstances, enabling testability in integration testing.TestCRDWatchers. it verifies that the EPP correctly watchesInferenceModelRewriteandInferenceObjectiveresources and update the dataStore.Which issue(s) this PR fixes:
Fixes partially #2332
Does this PR introduce a user-facing change?: