Skip to content

Commit 1cea649

Browse files
committed
fix(config): address review feedback for saturation filter injection
- Use allPlugins parameter instead of re-fetching from handle - Add tests for ensureSaturationDetector with Filter-implementing detector (injected) vs plain detector (not injected) Signed-off-by: RishabhSaini <rishabhsaini01@gmail.com>
1 parent b6f4a48 commit 1cea649

2 files changed

Lines changed: 92 additions & 1 deletion

File tree

pkg/epp/config/loader/defaults.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func ensureSaturationDetector(
293293
}
294294
}
295295

296-
if sd, ok := handle.GetAllPluginsWithNames()[sdConfig.PluginRef]; ok {
296+
if sd, ok := allPlugins[sdConfig.PluginRef]; ok {
297297
if _, isFilter := sd.(fwksched.Filter); isFilter {
298298
injectFilterIntoProfiles(cfg.SchedulingProfiles, sdConfig.PluginRef)
299299
}

pkg/epp/config/loader/defaults_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,26 @@ import (
2424
"k8s.io/utils/ptr"
2525

2626
configapi "github.com/llm-d/llm-d-router/apix/config/v1alpha1"
27+
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
2728
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
29+
fwksched "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/scheduling"
2830
extractormetrics "github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/datalayer/extractor/metrics"
2931
sourcemetrics "github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/datalayer/source/metrics"
3032
testutils "github.com/llm-d/llm-d-router/test/utils"
3133
)
3234

35+
// mockFilterDetector implements both SaturationDetector and Filter, like the real utilization-detector.
36+
type mockFilterDetector struct{ mockPlugin }
37+
38+
var (
39+
_ fwksched.Filter = &mockFilterDetector{}
40+
)
41+
42+
func (m *mockFilterDetector) Saturation(_ context.Context, _ []fwkdl.Endpoint) float64 { return 0 }
43+
func (m *mockFilterDetector) Filter(_ context.Context, _ *fwksched.InferenceRequest, eps []fwksched.Endpoint) []fwksched.Endpoint {
44+
return eps
45+
}
46+
3347
// metricsPlugins returns an allPlugins map with mock stubs for both default metrics plugins.
3448
// Providing them prevents ensureDataLayer from calling registerDefaultPlugin (which needs the
3549
// global factory registry). The function still injects the DataLayer.Sources entries.
@@ -120,3 +134,80 @@ func TestEnsureDataLayer(t *testing.T) {
120134
})
121135

122136
}
137+
138+
func TestEnsureSaturationDetector_InjectsFilter(t *testing.T) {
139+
t.Run("detector implementing Filter is injected into profiles", func(t *testing.T) {
140+
w := 2.0
141+
cfg := &configapi.EndpointPickerConfig{
142+
SchedulingProfiles: []configapi.SchedulingProfile{
143+
{Name: "default", Plugins: []configapi.SchedulingPlugin{{PluginRef: "scorer", Weight: &w}}},
144+
},
145+
}
146+
handle := testutils.NewTestHandle(context.Background())
147+
148+
detectorName := "my-detector"
149+
detector := &mockFilterDetector{mockPlugin{t: fwkplugin.TypedName{Type: detectorName, Name: detectorName}}}
150+
handle.AddPlugin(detectorName, detector)
151+
152+
allPlugins := handle.GetAllPluginsWithNames()
153+
cfg.FlowControl = &configapi.FlowControlConfig{
154+
SaturationDetector: &configapi.SaturationDetectorConfig{PluginRef: detectorName},
155+
}
156+
157+
err := ensureSaturationDetector(cfg, handle, allPlugins)
158+
159+
require.NoError(t, err)
160+
require.Len(t, cfg.SchedulingProfiles[0].Plugins, 2)
161+
require.Equal(t, detectorName, cfg.SchedulingProfiles[0].Plugins[1].PluginRef)
162+
})
163+
164+
t.Run("detector not implementing Filter is not injected", func(t *testing.T) {
165+
w := 2.0
166+
cfg := &configapi.EndpointPickerConfig{
167+
SchedulingProfiles: []configapi.SchedulingProfile{
168+
{Name: "default", Plugins: []configapi.SchedulingPlugin{{PluginRef: "scorer", Weight: &w}}},
169+
},
170+
}
171+
handle := testutils.NewTestHandle(context.Background())
172+
173+
detectorName := "plain-detector"
174+
detector := &mockSaturationDetector{mockPlugin{t: fwkplugin.TypedName{Type: detectorName, Name: detectorName}}}
175+
handle.AddPlugin(detectorName, detector)
176+
177+
allPlugins := handle.GetAllPluginsWithNames()
178+
cfg.FlowControl = &configapi.FlowControlConfig{
179+
SaturationDetector: &configapi.SaturationDetectorConfig{PluginRef: detectorName},
180+
}
181+
182+
err := ensureSaturationDetector(cfg, handle, allPlugins)
183+
184+
require.NoError(t, err)
185+
require.Len(t, cfg.SchedulingProfiles[0].Plugins, 1, "non-filter detector should not be injected")
186+
})
187+
188+
t.Run("detector already in profile is not duplicated", func(t *testing.T) {
189+
detectorName := "my-detector"
190+
cfg := &configapi.EndpointPickerConfig{
191+
SchedulingProfiles: []configapi.SchedulingProfile{
192+
{Name: "default", Plugins: []configapi.SchedulingPlugin{
193+
{PluginRef: detectorName},
194+
{PluginRef: "picker"},
195+
}},
196+
},
197+
}
198+
handle := testutils.NewTestHandle(context.Background())
199+
200+
detector := &mockFilterDetector{mockPlugin{t: fwkplugin.TypedName{Type: detectorName, Name: detectorName}}}
201+
handle.AddPlugin(detectorName, detector)
202+
203+
allPlugins := handle.GetAllPluginsWithNames()
204+
cfg.FlowControl = &configapi.FlowControlConfig{
205+
SaturationDetector: &configapi.SaturationDetectorConfig{PluginRef: detectorName},
206+
}
207+
208+
err := ensureSaturationDetector(cfg, handle, allPlugins)
209+
210+
require.NoError(t, err)
211+
require.Len(t, cfg.SchedulingProfiles[0].Plugins, 2, "already present, no duplicate")
212+
})
213+
}

0 commit comments

Comments
 (0)