Skip to content

Commit 776608f

Browse files
elevranBenjaminBraunDev
elevran
authored and
BenjaminBraunDev
committed
Introduce plugins.TypedName to be used for Plugin base implementation (kubernetes-sigs#1086)
* introduce TypedName to reduce boilerplate, modify plugins Signed-off-by: Etai Lev Ran <[email protected]> * implement GetTypedName() Signed-off-by: Etai Lev Ran <[email protected]> * Remove Type() and Name() from Plugin interface Signed-off-by: Etai Lev Ran <[email protected]> * use TypedName as private field, not embedded Signed-off-by: Etai Lev Ran <[email protected]> --------- Signed-off-by: Etai Lev Ran <[email protected]>
1 parent 0338c5a commit 776608f

File tree

21 files changed

+249
-229
lines changed

21 files changed

+249
-229
lines changed

‎conformance/testing-epp/plugins/filter/request_header_based_filter.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"strings"
2222

23+
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins"
2324
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling/framework"
2425
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling/types"
2526
)
@@ -38,20 +39,19 @@ var _ framework.Filter = &HeaderBasedTestingFilter{}
3839
// NewHeaderBasedTestingFilter initializes a new HeaderBasedTestingFilter.
3940
// This should only be used for testing purposes.
4041
func NewHeaderBasedTestingFilter() *HeaderBasedTestingFilter {
41-
return &HeaderBasedTestingFilter{}
42+
return &HeaderBasedTestingFilter{
43+
tn: plugins.TypedName{Type: "header-based-testing", Name: "header-based-testing-filter"},
44+
}
4245
}
4346

4447
// HeaderBasedTestingFilter filters Pods based on an address specified in the "test-epp-endpoint-selection" request header.
45-
type HeaderBasedTestingFilter struct{}
46-
47-
// Type returns the type of the filter.
48-
func (f *HeaderBasedTestingFilter) Type() string {
49-
return "header-based-testing"
48+
type HeaderBasedTestingFilter struct {
49+
tn plugins.TypedName
5050
}
5151

52-
// Name returns the type of the filter.
53-
func (f *HeaderBasedTestingFilter) Name() string {
54-
return "header-based-testing-filter"
52+
// TypedName returns the type and name tuple of this plugin instance.
53+
func (f *HeaderBasedTestingFilter) TypedName() plugins.TypedName {
54+
return f.tn
5555
}
5656

5757
// Filter selects pods that match the IP addresses specified in the request header.

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

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -555,15 +555,18 @@ schedulingProfiles:
555555
var _ framework.Filter = &test1{}
556556

557557
type test1 struct {
558+
tn plugins.TypedName
558559
Threshold int `json:"threshold"`
559560
}
560561

561-
func (f *test1) Type() string {
562-
return test1Type
562+
func newTest1() *test1 {
563+
return &test1{
564+
tn: plugins.TypedName{Type: test1Type, Name: "test-1"},
565+
}
563566
}
564567

565-
func (f *test1) Name() string {
566-
return "test-1"
568+
func (f *test1) TypedName() plugins.TypedName {
569+
return f.tn
567570
}
568571

569572
// Filter filters out pods that doesn't meet the filter criteria.
@@ -575,14 +578,18 @@ func (f *test1) Filter(_ context.Context, _ *types.CycleState, _ *types.LLMReque
575578
var _ framework.Scorer = &test2{}
576579
var _ framework.PostCycle = &test2{}
577580

578-
type test2 struct{}
581+
type test2 struct {
582+
tn plugins.TypedName
583+
}
579584

580-
func (f *test2) Type() string {
581-
return test2Type
585+
func newTest2() *test2 {
586+
return &test2{
587+
tn: plugins.TypedName{Type: test2Type, Name: "test-2"},
588+
}
582589
}
583590

584-
func (f *test2) Name() string {
585-
return "test-2"
591+
func (m *test2) TypedName() plugins.TypedName {
592+
return m.tn
586593
}
587594

588595
func (m *test2) Score(_ context.Context, _ *types.CycleState, _ *types.LLMRequest, _ []types.Pod) map[types.Pod]float64 {
@@ -594,14 +601,18 @@ func (m *test2) PostCycle(_ context.Context, _ *types.CycleState, _ *types.Profi
594601
// compile-time type validation
595602
var _ framework.Picker = &testPicker{}
596603

597-
type testPicker struct{}
604+
type testPicker struct {
605+
tn plugins.TypedName
606+
}
598607

599-
func (p *testPicker) Type() string {
600-
return testPickerType
608+
func newTestPicker() *testPicker {
609+
return &testPicker{
610+
tn: plugins.TypedName{Type: testPickerType, Name: "test-picker"},
611+
}
601612
}
602613

603-
func (p *testPicker) Name() string {
604-
return "test-picker"
614+
func (p *testPicker) TypedName() plugins.TypedName {
615+
return p.tn
605616
}
606617

607618
func (p *testPicker) Pick(_ context.Context, _ *types.CycleState, _ []*types.ScoredPod) *types.ProfileRunResult {
@@ -611,14 +622,18 @@ func (p *testPicker) Pick(_ context.Context, _ *types.CycleState, _ []*types.Sco
611622
// compile-time type validation
612623
var _ framework.ProfileHandler = &testProfileHandler{}
613624

614-
type testProfileHandler struct{}
625+
type testProfileHandler struct {
626+
tn plugins.TypedName
627+
}
615628

616-
func (p *testProfileHandler) Type() string {
617-
return testProfileHandlerType
629+
func newTestProfileHandler() *testProfileHandler {
630+
return &testProfileHandler{
631+
tn: plugins.TypedName{Type: testProfileHandlerType, Name: "test-profile-handler"},
632+
}
618633
}
619634

620-
func (p *testProfileHandler) Name() string {
621-
return "test-profile-handler"
635+
func (p *testProfileHandler) TypedName() plugins.TypedName {
636+
return p.tn
622637
}
623638

624639
func (p *testProfileHandler) Pick(_ context.Context, _ *types.CycleState, _ *types.LLMRequest, _ map[string]*framework.SchedulerProfile, _ map[string]*types.ProfileRunResult) map[string]*framework.SchedulerProfile {
@@ -631,28 +646,28 @@ func (p *testProfileHandler) ProcessResults(_ context.Context, _ *types.CycleSta
631646

632647
func registerTestPlugins() {
633648
plugins.Register(test1Type,
634-
func(name string, parameters json.RawMessage, handle plugins.Handle) (plugins.Plugin, error) {
635-
result := test1{}
636-
err := json.Unmarshal(parameters, &result)
637-
return &result, err
649+
func(_ string, parameters json.RawMessage, _ plugins.Handle) (plugins.Plugin, error) {
650+
result := newTest1()
651+
err := json.Unmarshal(parameters, result)
652+
return result, err
638653
},
639654
)
640655

641656
plugins.Register(test2Type,
642-
func(name string, parameters json.RawMessage, handle plugins.Handle) (plugins.Plugin, error) {
643-
return &test2{}, nil
657+
func(_ string, _ json.RawMessage, _ plugins.Handle) (plugins.Plugin, error) {
658+
return newTest2(), nil
644659
},
645660
)
646661

647662
plugins.Register(testPickerType,
648-
func(name string, parameters json.RawMessage, handle plugins.Handle) (plugins.Plugin, error) {
649-
return &testPicker{}, nil
663+
func(_ string, _ json.RawMessage, _ plugins.Handle) (plugins.Plugin, error) {
664+
return newTestPicker(), nil
650665
},
651666
)
652667

653668
plugins.Register(testProfileHandlerType,
654-
func(name string, parameters json.RawMessage, handle plugins.Handle) (plugins.Plugin, error) {
655-
return &testProfileHandler{}, nil
669+
func(_ string, _ json.RawMessage, _ plugins.Handle) (plugins.Plugin, error) {
670+
return newTestProfileHandler(), nil
656671
},
657672
)
658673
}

‎pkg/epp/plugins/plugins.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@ import (
2424
// Plugin defines the interface for a plugin.
2525
// This interface should be embedded in all plugins across the code.
2626
type Plugin interface {
27-
// Type returns the type of the plugin.
28-
Type() string
29-
// Name returns the name of this plugin instance.
30-
Name() string
27+
// TypedName returns the type and name tuple of this plugin instance.
28+
TypedName() TypedName
3129
}
3230

3331
// Handle provides plugins a set of standard data and tools to work with

‎pkg/epp/plugins/typedname.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package plugins
18+
19+
// TypedName is a utility struct providing a type and a name
20+
// to plugins.
21+
// It implements the Plugin interface and can be embedded in
22+
// plugins across the code to reduce boilerplate.
23+
type TypedName struct {
24+
// Type returns the type of the plugin.
25+
Type string
26+
// Name returns the name of this plugin instance.
27+
Name string
28+
}
29+
30+
const (
31+
Separator = "/"
32+
)
33+
34+
// String returns the type and name rendered as
35+
// "<type>/<name>".
36+
func (tn *TypedName) String() string {
37+
return tn.Type + Separator + tn.Name
38+
}

‎pkg/epp/requestcontrol/director.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,19 +650,19 @@ func RandomWeightedDraw(logger logr.Logger, model *v1alpha2.InferenceModel, seed
650650
func (d *Director) runPreRequestPlugins(ctx context.Context, request *schedulingtypes.LLMRequest, schedulingResult *schedulingtypes.SchedulingResult,
651651
targetPort int) {
652652
for _, plugin := range d.preRequestPlugins {
653-
log.FromContext(ctx).V(logutil.DEBUG).Info("Running pre-request plugin", "plugin", plugin.Type())
653+
log.FromContext(ctx).V(logutil.DEBUG).Info("Running pre-request plugin", "plugin", plugin.TypedName().Type)
654654
before := time.Now()
655655
plugin.PreRequest(ctx, request, schedulingResult, targetPort)
656-
metrics.RecordRequestControlPluginProcessingLatency(PreRequestPluginType, plugin.Type(), time.Since(before))
656+
metrics.RecordRequestControlPluginProcessingLatency(PreRequestPluginType, plugin.TypedName().Type, time.Since(before))
657657
}
658658
}
659659

660660
func (d *Director) runPostResponsePlugins(ctx context.Context, request *schedulingtypes.LLMRequest, response *Response, targetPod *backend.Pod) {
661661
for _, plugin := range d.postResponsePlugins {
662-
log.FromContext(ctx).V(logutil.DEBUG).Info("Running post-response plugin", "plugin", plugin.Type())
662+
log.FromContext(ctx).V(logutil.DEBUG).Info("Running post-response plugin", "plugin", plugin.TypedName().Type)
663663
before := time.Now()
664664
plugin.PostResponse(ctx, request, response, targetPod)
665-
metrics.RecordRequestControlPluginProcessingLatency(PostResponsePluginType, plugin.Type(), time.Since(before))
665+
metrics.RecordRequestControlPluginProcessingLatency(PostResponsePluginType, plugin.TypedName().Type, time.Since(before))
666666
}
667667
}
668668

‎pkg/epp/requestcontrol/director_test.go‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datastore"
4141
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/handlers"
4242
latencypredictor "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/latencypredictorasync"
43+
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins"
4344
schedulingtypes "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling/types"
4445
errutil "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/util/error"
4546
logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/util/logging"
@@ -894,9 +895,7 @@ func pointer(v int32) *int32 {
894895
}
895896

896897
func TestDirector_HandleResponse(t *testing.T) {
897-
pr1 := &testPostResponse{
898-
TypeRes: "pr1",
899-
}
898+
pr1 := newTestPostResponse("pr1")
900899

901900
ctx := logutil.NewTestLoggerIntoContext(context.Background())
902901
ds := datastore.NewDatastore(t.Context(), nil)
@@ -932,14 +931,25 @@ func TestDirector_HandleResponse(t *testing.T) {
932931
}
933932
}
934933

934+
const (
935+
testPostResponseType = "test-post-response"
936+
)
937+
935938
type testPostResponse struct {
936-
TypeRes string
939+
tn plugins.TypedName
937940
lastRespOnResponse *Response
938941
lastTargetPodOnResponse string
939942
}
940943

941-
func (p *testPostResponse) Type() string { return p.TypeRes }
942-
func (p *testPostResponse) Name() string { return "test-post-response" }
944+
func newTestPostResponse(name string) *testPostResponse {
945+
return &testPostResponse{
946+
tn: plugins.TypedName{Type: testPostResponseType, Name: name},
947+
}
948+
}
949+
950+
func (p *testPostResponse) TypedName() plugins.TypedName {
951+
return p.tn
952+
}
943953

944954
func (p *testPostResponse) PostResponse(_ context.Context, _ *schedulingtypes.LLMRequest, response *Response, targetPod *backend.Pod) {
945955
p.lastRespOnResponse = response

‎pkg/epp/scheduling/framework/plugins/filter/decision_tree_filter.go‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ const (
3636
// compile-time type assertion
3737
var _ framework.Filter = &DecisionTreeFilter{}
3838

39-
// DecisionTreeFilter applies current fitler, and then recursively applies next filters
39+
// DecisionTreeFilter applies current filter, and then recursively applies next filters
4040
// depending success or failure of the current filter.
4141
// It can be used to construct a flow chart algorithm.
42+
// Since a DecisionTreeFilter takes on the type and name of the current filter,
43+
// it is not embedding a fixed plugins.TypeName.
4244
type DecisionTreeFilter struct {
4345
Current framework.Filter
4446
// NextOnSuccess filter will be applied after successfully applying the current filter.
@@ -131,20 +133,14 @@ func loadDecisionTreeEntry(entry *decisionTreeFilterEntry, handle plugins.Handle
131133
return nil, errors.New("either pluginRef or decisionTree must be specified")
132134
}
133135

134-
// Type returns the type of the filter.
135-
func (f *DecisionTreeFilter) Type() string {
136+
func (f *DecisionTreeFilter) TypedName() plugins.TypedName {
136137
if f == nil {
137-
return "nil"
138+
// TODO: this keeps the previous behavior ("nil"/"") - not sure
139+
// why done this way.
140+
// Change to empty TypedName or some more meaningful values?
141+
return plugins.TypedName{Type: "nil", Name: ""}
138142
}
139-
return f.Current.Type()
140-
}
141-
142-
// Name returns the name of the filter.
143-
func (f *DecisionTreeFilter) Name() string {
144-
if f == nil {
145-
return ""
146-
}
147-
return f.Current.Name()
143+
return f.Current.TypedName()
148144
}
149145

150146
// Filter filters out pods that doesn't meet the filter criteria.
@@ -161,7 +157,7 @@ func (f *DecisionTreeFilter) Filter(ctx context.Context, cycleState *types.Cycle
161157
if f.NextOnSuccess != nil {
162158
next = f.NextOnSuccess
163159
}
164-
loggerTrace.Info("Filter succeeded", "filter", f.Type(), "next", next.Type(), "filteredPodCount", len(filteredPod))
160+
loggerTrace.Info("Filter succeeded", "filter", f.TypedName(), "next", next.TypedName(), "filteredPodCount", len(filteredPod))
165161
// On success, pass the filtered result to the next filter.
166162
return next.Filter(ctx, cycleState, request, filteredPod)
167163
} else {
@@ -172,7 +168,7 @@ func (f *DecisionTreeFilter) Filter(ctx context.Context, cycleState *types.Cycle
172168
if f.NextOnFailure != nil {
173169
next = f.NextOnFailure
174170
}
175-
loggerTrace.Info("Filter failed", "filter", f.Type(), "next", next.Type())
171+
loggerTrace.Info("Filter failed", "filter", f.TypedName(), "next", next.TypedName())
176172
// On failure, pass the initial set of pods to the next filter.
177173
return next.Filter(ctx, cycleState, request, pods)
178174
}

0 commit comments

Comments
 (0)