-
Notifications
You must be signed in to change notification settings - Fork 283
test: add hermetic coverage for standalone mode #2175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,13 +96,25 @@ func TestMain(m *testing.M) { | |
| } | ||
|
|
||
| func TestFullDuplexStreamed_KubeInferenceObjectiveRequest(t *testing.T) { | ||
| // executionModes defines the permutations of EPP deployment modes to test. | ||
| executionModes := []struct { | ||
| name string | ||
| standalone bool | ||
| }{ | ||
| {name: "Standard", standalone: false}, | ||
| {name: "Standalone", standalone: true}, | ||
| } | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| requests []*extProcPb.ProcessingRequest | ||
| pods []podState | ||
| wantResponses []*extProcPb.ProcessingResponse | ||
| wantMetrics map[string]string | ||
| waitForModel string | ||
| // requiresCRDs indicates that this test case relies on specific Gateway API CRD features (like | ||
| // InferenceModelRewrite) which are not available in Standalone mode. | ||
| requiresCRDs bool | ||
| }{ | ||
| // --- Standard Routing Logic --- | ||
| { | ||
|
|
@@ -277,6 +289,7 @@ func TestFullDuplexStreamed_KubeInferenceObjectiveRequest(t *testing.T) { | |
| wantMetrics: map[string]string{ | ||
| "inference_objective_request_total": cleanMetric(metricReqTotal(modelToBeWritten, modelAfterRewrite)), | ||
| }, | ||
| requiresCRDs: true, | ||
| }, | ||
| { | ||
| name: "protocol: simple GET (header only)", | ||
|
|
@@ -371,40 +384,49 @@ func TestFullDuplexStreamed_KubeInferenceObjectiveRequest(t *testing.T) { | |
| }, | ||
| } | ||
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| // Resolve Model to Sync. | ||
| modelToSync := tc.waitForModel | ||
| if modelToSync == "" { | ||
| modelToSync = modelMyModel | ||
| } | ||
| for _, mode := range executionModes { | ||
| t.Run(mode.name, func(t *testing.T) { | ||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| if mode.standalone && tc.requiresCRDs { | ||
| t.Skipf("Skipping test %q: requires CRDs, but running in Standalone mode", tc.name) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @capri-xiyue I expect to support standalone with inference CRDs, standalone can have two modes of operation, one with the Inference* CRDs (a richer mode of operation that allows using InfObj for example) and one without (using pod selector and completely decoupled from all CRDs). The stable thing in standalone is running without the Gateway API dependency.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We can add a third harness for standalone w/ CRDs.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. I agree that we can add a third harness for standalone w/ CRDs.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, @capri-xiyue can you pls add that in a follow up PR?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. |
||
| } | ||
|
|
||
| h := NewTestHarness(t, context.Background()). | ||
| WithBaseResources(). | ||
| WithPods(tc.pods). | ||
| WaitForSync(len(tc.pods), modelToSync) | ||
| var h *TestHarness | ||
| if mode.standalone { | ||
| h = NewTestHarness(t, context.Background(), WithStandaloneMode()) | ||
| } else { | ||
| h = NewTestHarness(t, context.Background()).WithBaseResources() | ||
| } | ||
|
|
||
| // Wait for metrics to settle to avoid race conditions where Datastore has pods but Scheduler/Metrics collector | ||
| // hasn't processed them yet (causing random scheduling or missing metrics). | ||
| if len(tc.pods) > 0 { | ||
| h.WaitForReadyPodsMetric(len(tc.pods)) | ||
| } | ||
| // In Standalone mode, we cannot wait for an Objective CRD to sync as it doesn't exist. | ||
| // We only wait for Pod discovery. | ||
| modelToSync := tc.waitForModel | ||
| if modelToSync == "" { | ||
| modelToSync = modelMyModel | ||
| } | ||
|
|
||
| responses, err := integration.StreamedRequest(t, h.Client, tc.requests, len(tc.wantResponses)) | ||
| h.WithPods(tc.pods).WaitForSync(len(tc.pods), modelToSync) | ||
| if len(tc.pods) > 0 { | ||
| h.WaitForReadyPodsMetric(len(tc.pods)) | ||
| } | ||
|
|
||
| require.NoError(t, err) | ||
| responses, err := integration.StreamedRequest(t, h.Client, tc.requests, len(tc.wantResponses)) | ||
| require.NoError(t, err) | ||
|
|
||
| if diff := cmp.Diff(tc.wantResponses, responses, | ||
| protocmp.Transform(), | ||
| protocmp.SortRepeated(func(a, b *configPb.HeaderValueOption) bool { | ||
| return a.GetHeader().GetKey() < b.GetHeader().GetKey() | ||
| }), | ||
| ); diff != "" { | ||
| t.Errorf("Response mismatch (-want +got): %v", diff) | ||
| } | ||
| if diff := cmp.Diff(tc.wantResponses, responses, | ||
| protocmp.Transform(), | ||
| protocmp.SortRepeated(func(a, b *configPb.HeaderValueOption) bool { | ||
| return a.GetHeader().GetKey() < b.GetHeader().GetKey() | ||
| }), | ||
| ); diff != "" { | ||
| t.Errorf("Response mismatch (-want +got): %v", diff) | ||
| } | ||
|
|
||
| if len(tc.wantMetrics) > 0 { | ||
| h.ExpectMetrics(tc.wantMetrics) | ||
| if len(tc.wantMetrics) > 0 { | ||
| h.ExpectMetrics(tc.wantMetrics) | ||
| } | ||
| }) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what triggered adding this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure actually. I ran
go mod tidyand this continuously gets applied, but I do not think this should be triggered by my delta.