Skip to content

Commit c593dfa

Browse files
LukeAVanDrieLukeAVanDrie
committed
feat: improve config loading coverage
1 parent 4d75e80 commit c593dfa

File tree

6 files changed

+267
-52
lines changed

6 files changed

+267
-52
lines changed

‎apix/config/v1alpha1/endpointpickerconfig_types.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,11 +306,11 @@ func (fcc *FlowControlConfig) String() string {
306306
var parts []string
307307

308308
if fcc.SaturationDetectorRef != "" {
309-
parts = append(parts, fmt.Sprintf("SaturationDetectorRef: %s", fcc.SaturationDetectorRef))
309+
parts = append(parts, "SaturationDetectorRef: "+fcc.SaturationDetectorRef)
310310
}
311311

312312
if fcc.MaxBytes != nil {
313-
parts = append(parts, fmt.Sprintf("MaxBytes: %d", fcc.MaxBytes.Value()))
313+
parts = append(parts, "MaxBytes: "+fcc.MaxBytes.String())
314314
} else {
315315
parts = append(parts, "MaxBytes: unlimited")
316316
}

‎apix/config/v1alpha1/endpointpickerconfig_types_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,6 @@ func TestStringers(t *testing.T) {
5050
},
5151
want: "{PluginRef: test-ref, Weight: 2.50}",
5252
},
53-
{
54-
name: "SaturationDetector",
55-
obj: &SaturationDetector{
56-
QueueDepthThreshold: 10,
57-
KVCacheUtilThreshold: 0.8,
58-
MetricsStalenessThreshold: metav1.Duration{Duration: 100 * time.Millisecond},
59-
},
60-
want: "{QueueDepthThreshold: 10, KVCacheUtilThreshold: 0.80, MetricsStalenessThreshold: 100ms}",
61-
},
6253
{
6354
name: "FlowControlConfig",
6455
obj: &FlowControlConfig{

‎pkg/epp/config/loader/configloader_test.go‎

Lines changed: 143 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,13 @@ import (
5353
// Define constants for test plugins.
5454
// Constants must match those used in testdata_test.go.
5555
const (
56-
testPluginType = "test-plugin"
57-
testPickerType = "test-picker"
58-
testScorerType = "test-scorer"
59-
testProfileHandler = "test-profile-handler"
60-
testSourceType = "test-source"
61-
testExtractorType = "test-extractor"
56+
testPluginType = "test-plugin"
57+
testPickerType = "test-picker"
58+
testScorerType = "test-scorer"
59+
testProfileHandler = "test-profile-handler"
60+
testSourceType = "test-source"
61+
testExtractorType = "test-extractor"
62+
testSaturationDetector = "test-saturation-detector"
6263
)
6364

6465
// --- Test: Phase 1 (Raw Loading & Static Defaults) ---
@@ -177,6 +178,88 @@ func TestLoadRawConfiguration(t *testing.T) {
177178
},
178179
wantErr: false,
179180
},
181+
{
182+
name: "Success - SaturationDetector correctly configured",
183+
configText: successSaturationDetectorConfigText,
184+
want: &configapi.EndpointPickerConfig{
185+
TypeMeta: metav1.TypeMeta{
186+
APIVersion: "inference.networking.x-k8s.io/v1alpha1",
187+
Kind: "EndpointPickerConfig",
188+
},
189+
Plugins: []configapi.PluginSpec{
190+
{Name: "maxScore", Type: "max-score-picker"},
191+
{Name: "my-detector", Type: utilizationdetector.UtilizationDetectorType},
192+
},
193+
SchedulingProfiles: []configapi.SchedulingProfile{
194+
{
195+
Name: "default",
196+
Plugins: []configapi.SchedulingPlugin{
197+
{PluginRef: "maxScore"},
198+
},
199+
},
200+
},
201+
FlowControl: &configapi.FlowControlConfig{
202+
SaturationDetectorRef: "my-detector",
203+
},
204+
FeatureGates: configapi.FeatureGates{
205+
flowcontrol.FeatureGate,
206+
},
207+
},
208+
wantErr: false,
209+
},
210+
{
211+
name: "Success - SaturationDetector default configuration",
212+
configText: successSaturationDetectorDefaultConfigText,
213+
want: &configapi.EndpointPickerConfig{
214+
TypeMeta: metav1.TypeMeta{
215+
APIVersion: "inference.networking.x-k8s.io/v1alpha1",
216+
Kind: "EndpointPickerConfig",
217+
},
218+
Plugins: []configapi.PluginSpec{
219+
{Name: "maxScore", Type: "max-score-picker"},
220+
},
221+
SchedulingProfiles: []configapi.SchedulingProfile{
222+
{
223+
Name: "default",
224+
Plugins: []configapi.SchedulingPlugin{
225+
{PluginRef: "maxScore"},
226+
},
227+
},
228+
},
229+
FlowControl: nil,
230+
FeatureGates: configapi.FeatureGates{
231+
flowcontrol.FeatureGate,
232+
},
233+
},
234+
wantErr: false,
235+
},
236+
{
237+
name: "Success - SaturationDetector with flowControl feature gate disabled",
238+
configText: successSaturationDetectorFlowControlDisabledConfigText,
239+
want: &configapi.EndpointPickerConfig{
240+
TypeMeta: metav1.TypeMeta{
241+
APIVersion: "inference.networking.x-k8s.io/v1alpha1",
242+
Kind: "EndpointPickerConfig",
243+
},
244+
Plugins: []configapi.PluginSpec{
245+
{Name: "maxScore", Type: "max-score-picker"},
246+
{Name: "my-detector", Type: utilizationdetector.UtilizationDetectorType},
247+
},
248+
SchedulingProfiles: []configapi.SchedulingProfile{
249+
{
250+
Name: "default",
251+
Plugins: []configapi.SchedulingPlugin{
252+
{PluginRef: "maxScore"},
253+
},
254+
},
255+
},
256+
FeatureGates: configapi.FeatureGates{},
257+
FlowControl: &configapi.FlowControlConfig{
258+
SaturationDetectorRef: "my-detector",
259+
},
260+
},
261+
wantErr: false,
262+
},
180263
{
181264
name: "Error - Invalid YAML",
182265
configText: errorBadYamlText,
@@ -356,6 +439,39 @@ func TestInstantiateAndConfigure(t *testing.T) {
356439
"Should be GlobalStrict type")
357440
},
358441
},
442+
{
443+
name: "Success - SaturationDetector default configuration",
444+
configText: successSaturationDetectorDefaultConfigText,
445+
wantErr: false,
446+
validate: func(t *testing.T, handle fwkplugin.Handle, rawCfg *configapi.EndpointPickerConfig, cfg *config.Config) {
447+
require.NotNil(t, rawCfg.FlowControl, "FlowControl should be defaulted")
448+
require.Equal(t, utilizationdetector.UtilizationDetectorType, rawCfg.FlowControl.SaturationDetectorRef)
449+
require.NotNil(t, cfg.SaturationDetector, "SaturationDetector should be instantiated")
450+
require.Equal(t, utilizationdetector.UtilizationDetectorType, cfg.SaturationDetector.TypedName().Name)
451+
},
452+
},
453+
{
454+
name: "Success - SaturationDetector explicit configuration",
455+
configText: successSaturationDetectorConfigText,
456+
wantErr: false,
457+
validate: func(t *testing.T, handle fwkplugin.Handle, rawCfg *configapi.EndpointPickerConfig, cfg *config.Config) {
458+
require.NotNil(t, rawCfg.FlowControl)
459+
require.Equal(t, "my-detector", rawCfg.FlowControl.SaturationDetectorRef)
460+
require.NotNil(t, cfg.SaturationDetector)
461+
require.Equal(t, "my-detector", cfg.SaturationDetector.TypedName().Name)
462+
},
463+
},
464+
{
465+
name: "Success - SaturationDetector with flowControl feature gate disabled",
466+
configText: successSaturationDetectorFlowControlDisabledConfigText,
467+
wantErr: false,
468+
validate: func(t *testing.T, handle fwkplugin.Handle, rawCfg *configapi.EndpointPickerConfig, cfg *config.Config) {
469+
// Even if FG is disabled, we still instantiate SaturationDetector if it's in the config.
470+
// This is because SaturationDetector is used by the main picker logic, independent of FlowControl queueing.
471+
require.NotNil(t, cfg.SaturationDetector)
472+
require.Equal(t, "my-detector", cfg.SaturationDetector.TypedName().Name)
473+
},
474+
},
359475
{
360476
name: "Success - Parser Config",
361477
configText: successParserConfigText,
@@ -493,6 +609,16 @@ func TestInstantiateAndConfigure(t *testing.T) {
493609
configText: errorParserWrongPluginNameText,
494610
wantErr: true,
495611
},
612+
{
613+
name: "Error - SaturationDetector wrong plugin type",
614+
configText: errorSaturationDetectorWrongPluginTypeText,
615+
wantErr: true,
616+
},
617+
{
618+
name: "Error (SaturationDetector) - Missing Plugin",
619+
configText: errorSaturationDetectorMissingPluginText,
620+
wantErr: true,
621+
},
496622
}
497623

498624
for _, tc := range tests {
@@ -616,6 +742,13 @@ func (m *mockExtractor) Extract(ctx context.Context, data any, ep fwkdl.Endpoint
616742
return nil
617743
}
618744

745+
// Mock SaturationDetector
746+
type mockSaturationDetector struct{ mockPlugin }
747+
748+
func (m *mockSaturationDetector) Saturation(ctx context.Context, candidatePods []fwkdl.Endpoint) float64 {
749+
return 0.0
750+
}
751+
619752
func registerTestPlugins(t *testing.T) {
620753
t.Helper()
621754

@@ -662,6 +795,10 @@ func registerTestPlugins(t *testing.T) {
662795
return &mockExtractor{mockPlugin{t: fwkplugin.TypedName{Name: name, Type: testExtractorType}}}, nil
663796
})
664797

798+
fwkplugin.Register(testSaturationDetector, func(name string, _ json.RawMessage, _ fwkplugin.Handle) (fwkplugin.Plugin, error) {
799+
return &mockSaturationDetector{mockPlugin{t: fwkplugin.TypedName{Name: name, Type: testSaturationDetector}}}, nil
800+
})
801+
665802
fwkplugin.Register(fairness.GlobalStrictFairnessPolicyType, func(name string, _ json.RawMessage, _ fwkplugin.Handle) (fwkplugin.Plugin, error) {
666803
return &flowcontrolmocks.MockFairnessPolicy{
667804
TypedNameV: fwkplugin.TypedName{Name: name, Type: fairness.GlobalStrictFairnessPolicyType},

‎pkg/epp/config/loader/defaults.go‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,6 @@ func applySystemDefaults(cfg *configapi.EndpointPickerConfig, handle fwkplugin.H
112112
if err := ensureParser(cfg, handle, allPlugins); err != nil {
113113
return fmt.Errorf("failed to apply parser defaults: %w", err)
114114
}
115-
if err := ensureSaturationDetector(cfg, handle, allPlugins); err != nil {
116-
return fmt.Errorf("failed to apply saturation detector defaults: %w", err)
117-
}
118115
return nil
119116
}
120117

@@ -207,7 +204,22 @@ func ensureFlowControlLayer(
207204
}
208205
}
209206
if _, ok := allPlugins[registry.DefaultFairnessPolicyRef]; !ok {
210-
return registerDefaultPlugin(cfg, handle, registry.DefaultFairnessPolicyRef)
207+
if err := registerDefaultPlugin(cfg, handle, registry.DefaultFairnessPolicyRef); err != nil {
208+
return err
209+
}
210+
}
211+
212+
if cfg.FlowControl == nil {
213+
cfg.FlowControl = &configapi.FlowControlConfig{}
214+
}
215+
if cfg.FlowControl.SaturationDetectorRef == "" {
216+
cfg.FlowControl.SaturationDetectorRef = utilizationdetector.UtilizationDetectorType
217+
}
218+
219+
if _, ok := allPlugins[cfg.FlowControl.SaturationDetectorRef]; !ok {
220+
if err := registerDefaultPlugin(cfg, handle, utilizationdetector.UtilizationDetectorType); err != nil {
221+
return err
222+
}
211223
}
212224
return nil
213225
}
@@ -235,25 +247,6 @@ func ensureParser(
235247
return nil
236248
}
237249

238-
func ensureSaturationDetector(
239-
cfg *configapi.EndpointPickerConfig,
240-
handle fwkplugin.Handle,
241-
allPlugins map[string]fwkplugin.Plugin,
242-
) error {
243-
if cfg.FlowControl == nil {
244-
cfg.FlowControl = &configapi.FlowControlConfig{}
245-
}
246-
if cfg.FlowControl.SaturationDetectorRef == "" {
247-
cfg.FlowControl.SaturationDetectorRef = utilizationdetector.UtilizationDetectorType
248-
}
249-
if _, ok := allPlugins[cfg.FlowControl.SaturationDetectorRef]; !ok {
250-
if err := registerDefaultPlugin(cfg, handle, utilizationdetector.UtilizationDetectorType); err != nil {
251-
return err
252-
}
253-
}
254-
return nil
255-
}
256-
257250
// registerDefaultPlugin instantiates a plugin with empty configuration (defaults) and adds it to both the handle and
258251
// the config spec.
259252
func registerDefaultPlugin(

‎pkg/epp/config/loader/testdata_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,40 @@ parser:
243243
pluginRef: openaiParser
244244
`
245245

246+
// successSaturationDetectorConfigText tests saturation detector instantiation.
247+
const successSaturationDetectorConfigText = `
248+
apiVersion: inference.networking.x-k8s.io/v1alpha1
249+
kind: EndpointPickerConfig
250+
featureGates:
251+
- flowControl
252+
plugins:
253+
- name: maxScore
254+
type: max-score-picker
255+
- name: my-detector
256+
type: utilization-detector
257+
schedulingProfiles:
258+
- name: default
259+
plugins:
260+
- pluginRef: maxScore
261+
flowControl:
262+
saturationDetectorRef: my-detector
263+
`
264+
265+
// successSaturationDetectorDefaultConfigText tests saturation detector defaulting.
266+
const successSaturationDetectorDefaultConfigText = `
267+
apiVersion: inference.networking.x-k8s.io/v1alpha1
268+
kind: EndpointPickerConfig
269+
plugins:
270+
- name: maxScore
271+
type: max-score-picker
272+
schedulingProfiles:
273+
- name: default
274+
plugins:
275+
- pluginRef: maxScore
276+
featureGates:
277+
- flowControl
278+
`
279+
246280
// --- Invalid Configurations (Syntax/Structure) ---
247281

248282
// errorBadYamlText contains invalid YAML syntax.
@@ -599,3 +633,57 @@ schedulingProfiles:
599633
parser:
600634
pluginRef: wrongParser # Wrong names
601635
`
636+
637+
// errorSaturationDetectorWrongPluginTypeText references a plugin of the wrong type.
638+
const errorSaturationDetectorWrongPluginTypeText = `
639+
apiVersion: inference.networking.x-k8s.io/v1alpha1
640+
kind: EndpointPickerConfig
641+
plugins:
642+
- name: maxScore
643+
type: max-score-picker
644+
- name: testScorer
645+
type: test-scorer
646+
schedulingProfiles:
647+
- name: default
648+
plugins:
649+
- pluginRef: maxScore
650+
featureGates:
651+
- flowControl
652+
flowControl:
653+
saturationDetectorRef: testScorer # Wrong type
654+
`
655+
656+
// errorSaturationDetectorMissingPluginText references a non-existent plugin.
657+
const errorSaturationDetectorMissingPluginText = `
658+
apiVersion: inference.networking.x-k8s.io/v1alpha1
659+
kind: EndpointPickerConfig
660+
plugins:
661+
- name: maxScore
662+
type: max-score-picker
663+
schedulingProfiles:
664+
- name: default
665+
plugins:
666+
- pluginRef: maxScore
667+
featureGates:
668+
- flowControl
669+
flowControl:
670+
saturationDetectorRef: non-existent
671+
`
672+
673+
// successSaturationDetectorFlowControlDisabledConfigText tests that saturation detector is initialized even if flowControl feature gate is disabled.
674+
const successSaturationDetectorFlowControlDisabledConfigText = `
675+
apiVersion: inference.networking.x-k8s.io/v1alpha1
676+
kind: EndpointPickerConfig
677+
plugins:
678+
- name: maxScore
679+
type: max-score-picker
680+
- name: my-detector
681+
type: utilization-detector
682+
schedulingProfiles:
683+
- name: default
684+
plugins:
685+
- pluginRef: maxScore
686+
featureGates: []
687+
flowControl:
688+
saturationDetectorRef: my-detector
689+
`

0 commit comments

Comments
 (0)