Update interface names based on suggestions

Author: rahulgurnani

pkg/epp/requestcontrol/director.go

@@ -119,7 +119,7 @@ func (d *Director) resolveTargetModel(reqCtx *handlers.RequestContext) (*handler
 	var ok bool
 	reqCtx.IncomingModelName, ok = requestBodyMap["model"].(string)
 	if !ok {
-		return nil, errutil.Error{Code: errutil.BadRequest, Msg: "model not found in request body"}
+		return reqCtx, errutil.Error{Code: errutil.BadRequest, Msg: "model not found in request body"}
 	}
 	if reqCtx.TargetModelName == "" {
 		// Default to incoming model name
@@ -347,14 +347,14 @@ func (d *Director) runPreRequestPlugins(ctx context.Context, request *scheduling
 	}
 }
 
-// prepareData runs the PrepareData plugin with retries and timeout.
-func prepareData(plugin PrepareData, ctx context.Context, request *schedulingtypes.LLMRequest, pods []types.Pod) {
+// prepareData executes the PrepareRequestData plugins with retries and timeout.
+func prepareData(plugin DataProducer, ctx context.Context, request *schedulingtypes.LLMRequest, pods []types.Pod) {
 	currentTimeout := prepareDataTimeout
 	for i := 0; i <= prepareDataMaxRetries; i++ {
 		done := make(chan struct{})
 		go func() {
 			defer close(done)
-			plugin.PrepareData(ctx, request, pods)
+			plugin.PrepareRequestData(ctx, request, pods)
 		}()
 
 		select {
@@ -377,10 +377,10 @@ func (d *Director) runPrepareDataPlugins(ctx context.Context,
 	// Parallely execute PrepareData for all the plugins. Some plugins might take time to prepare data e.g. latency predictor.
 	// Failure in any prepareData doesn't block the request processing.
 	var wg sync.WaitGroup
-	for _, plugin := range d.requestControlPlugins.prepareDataPlugins {
+	for _, plugin := range d.requestControlPlugins.dataProducerPlugins {
 		loggerDebug.Info("Running PrepareData plugin", "plugin", plugin.TypedName())
 		wg.Add(1)
-		go func(p PrepareData) {
+		go func(p DataProducer) {
 			defer wg.Done()
 			prepareData(p, ctx, request, pods)
 		}(plugin)
@@ -393,8 +393,8 @@ func (d *Director) runAdmitRequestPlugins(ctx context.Context,
 	loggerDebug := log.FromContext(ctx).V(logutil.DEBUG)
 	for _, plugin := range d.requestControlPlugins.admitRequestPlugins {
 		loggerDebug.Info("Running AdmitRequest plugin", "plugin", plugin.TypedName())
-		if denyReason := plugin.Admit(ctx, request, pods); denyReason != "" {
-			loggerDebug.Info("AdmitRequest plugin denied the request", "plugin", plugin.TypedName(), "reason", denyReason)
+		if denyReason := plugin.AdmitRequest(ctx, request, pods); denyReason != nil {
+			loggerDebug.Info("AdmitRequest plugin denied the request", "plugin", plugin.TypedName(), "reason", denyReason.Error())
 			return false
 		}
 		loggerDebug.Info("Completed running AdmitRequest plugin successfully", "plugin", plugin.TypedName())

pkg/epp/requestcontrol/director_test.go

@@ -93,23 +93,27 @@ func (ds *mockDatastore) PodList(predicate func(backendmetrics.PodMetrics) bool)
 	return res
 }
 
-type mockPrepareRequestDataPlugin struct {
-	tn                plugins.TypedName
-	prepareDataCalled bool
+type mockDataProducerPlugin struct {
+	tn                       plugins.TypedName
+	prepareRequestDataCalled bool
 }
 
-func newmockPrepareRequestDataPlugin(name string) *mockPrepareRequestDataPlugin {
-	return &mockPrepareRequestDataPlugin{
+func newmockDataProducerPlugin(name string) *mockDataProducerPlugin {
+	return &mockDataProducerPlugin{
 		tn: plugins.TypedName{Type: "mock-prepare-request-data", Name: name},
 	}
 }
 
-func (m *mockPrepareRequestDataPlugin) TypedName() plugins.TypedName {
+func (m *mockDataProducerPlugin) TypedName() plugins.TypedName {
 	return m.tn
 }
 
-func (m *mockPrepareRequestDataPlugin) PrepareData(ctx context.Context, request *schedulingtypes.LLMRequest, pods []schedulingtypes.Pod) {
-	m.prepareDataCalled = true
+func (m *mockDataProducerPlugin) Produces() map[string]any {
+	return map[string]any{}
+}
+
+func (m *mockDataProducerPlugin) PrepareRequestData(ctx context.Context, request *schedulingtypes.LLMRequest, pods []schedulingtypes.Pod) {
+	m.prepareRequestDataCalled = true
 }
 
 type mockAdmitRequestPlugins struct {
@@ -127,9 +131,9 @@ func (m *mockAdmitRequestPlugins) TypedName() plugins.TypedName {
 	return m.tn
 }
 
-func (m *mockAdmitRequestPlugins) Admit(ctx context.Context, request *schedulingtypes.LLMRequest, pods []schedulingtypes.Pod) string {
+func (m *mockAdmitRequestPlugins) AdmitRequest(ctx context.Context, request *schedulingtypes.LLMRequest, pods []schedulingtypes.Pod) error {
 	m.admitRequestCalled = true
-	return ""
+	return nil
 }
 
 func TestDirector_HandleRequest(t *testing.T) {
@@ -241,19 +245,19 @@ func TestDirector_HandleRequest(t *testing.T) {
 	}
 
 	tests := []struct {
-		name                    string
-		reqBodyMap              map[string]any
-		mockAdmissionController *mockAdmissionController
-		inferenceObjectiveName  string
-		schedulerMockSetup      func(m *mockScheduler)
-		wantErrCode             string                   // Expected errutil code string
-		wantReqCtx              *handlers.RequestContext // Fields to check in the returned RequestContext
-		wantMutatedBodyModel    string                   // Expected model in reqCtx.Request.Body after PostDispatch
-		targetModelName         string                   // Expected model name after target model resolution
-		prepareDataCalled       bool
-		admitRequestCalled      bool
-		prepareDataPlugins      *mockPrepareRequestDataPlugin
-		admitRequestPlugins     *mockAdmitRequestPlugins
+		name                     string
+		reqBodyMap               map[string]any
+		mockAdmissionController  *mockAdmissionController
+		inferenceObjectiveName   string
+		schedulerMockSetup       func(m *mockScheduler)
+		wantErrCode              string                   // Expected errutil code string
+		wantReqCtx               *handlers.RequestContext // Fields to check in the returned RequestContext
+		wantMutatedBodyModel     string                   // Expected model in reqCtx.Request.Body after PostDispatch
+		targetModelName          string                   // Expected model name after target model resolution
+		prepareRequestDataCalled bool
+		admitRequestCalled       bool
+		dataProducerPlugins      *mockDataProducerPlugin
+		admitRequestPlugins      *mockAdmitRequestPlugins
 	}{
 		{
 			name: "successful completions request",
@@ -333,10 +337,10 @@ func TestDirector_HandleRequest(t *testing.T) {
 				},
 				TargetEndpoint: "192.168.1.100:8000,192.168.2.100:8000,192.168.4.100:8000",
 			},
-			wantMutatedBodyModel: model,
-			targetModelName:      model,
-			prepareDataCalled:    true,
-			prepareDataPlugins:   newmockPrepareRequestDataPlugin("test-plugin"),
+			wantMutatedBodyModel:     model,
+			targetModelName:          model,
+			prepareRequestDataCalled: true,
+			dataProducerPlugins:      newmockDataProducerPlugin("test-plugin"),
 		},
 		{
 			name: "successful chat completions request with admit request plugins",
@@ -518,8 +522,8 @@ func TestDirector_HandleRequest(t *testing.T) {
 				test.schedulerMockSetup(mockSched)
 			}
 			config := NewConfig()
-			if test.prepareDataPlugins != nil {
-				config = config.WithPrepareDataPlugins(test.prepareDataPlugins)
+			if test.dataProducerPlugins != nil {
+				config = config.WithDataProducers(test.dataProducerPlugins)
 			}
 			if test.admitRequestPlugins != nil {
 				config = config.WithAdmitRequestPlugins(test.admitRequestPlugins)
@@ -569,10 +573,10 @@ func TestDirector_HandleRequest(t *testing.T) {
 					"Mutated reqCtx.Request.Body model mismatch")
 			}
 			if test.admitRequestPlugins != nil {
-				assert.True(t, test.admitRequestPlugins.admitRequestCalled, "AdmitRequestPlugins not called")
+				assert.True(t, test.admitRequestPlugins.admitRequestCalled, "AdmitRequest not called")
 			}
-			if test.prepareDataPlugins != nil {
-				assert.True(t, test.prepareDataPlugins.prepareDataCalled, "PrepareDataPlugins not called")
+			if test.dataProducerPlugins != nil {
+				assert.True(t, test.dataProducerPlugins.prepareRequestDataCalled, "PrepareRequestData not called")
 			}
 		})
 	}

pkg/epp/requestcontrol/plugins.go

@@ -58,17 +58,19 @@ type ResponseComplete interface {
 	ResponseComplete(ctx context.Context, request *types.LLMRequest, response *Response, targetPod *backend.Pod)
 }
 
-// PrepareData is called by the director before scheduling requests.
-// PrepareData plugin is implemented by data producers which produce data from different sources.
-type PrepareData interface {
-	plugins.Plugin
-	PrepareData(ctx context.Context, request *types.LLMRequest, pods []types.Pod)
+// PrepareRequestData is called by the director before scheduling requests.
+// DataProducer plugin is implemented by data producers which produce data from different sources.
+type DataProducer interface {
+	plugins.ProducerPlugin
+	PrepareRequestData(ctx context.Context, request *types.LLMRequest, pods []types.Pod)
 }
 
-// AdmitRequest is called by the director after the PrepareData plugins and before scheduling.
-// AdmitRequest plugin is implemented by plugins for admission control. These plugins need to implement Admit method.
-type AdmitRequest interface {
+// AdmissionPlugin is called by the director after the prepare data phase and before scheduling.
+// When a request has to go through multiple AdmissionPlugin,
+// the request is admitted only if all plugins say that the request should be admitted.
+type AdmissionPlugin interface {
 	plugins.Plugin
-	// Admit returns the denial reason if the request is denied. If the request is allowed, it returns an empty string.
-	Admit(ctx context.Context, request *types.LLMRequest, pods []types.Pod) string
+	// AdmitRequest returns the denial reason, wrapped as error if the request is denied.
+	// If the request is allowed, it returns nil.
+	AdmitRequest(ctx context.Context, request *types.LLMRequest, pods []types.Pod) error
 }

pkg/epp/requestcontrol/request_control_config.go

@@ -23,8 +23,8 @@ import (
 // NewConfig creates a new Config object and returns its pointer.
 func NewConfig() *Config {
 	return &Config{
-		admitRequestPlugins:      []AdmitRequest{},
-		prepareDataPlugins:       []PrepareData{},
+		admitRequestPlugins:      []AdmissionPlugin{},
+		dataProducerPlugins:      []DataProducer{},
 		preRequestPlugins:        []PreRequest{},
 		responseReceivedPlugins:  []ResponseReceived{},
 		responseStreamingPlugins: []ResponseStreaming{},
@@ -34,8 +34,8 @@ func NewConfig() *Config {
 
 // Config provides a configuration for the requestcontrol plugins.
 type Config struct {
-	admitRequestPlugins      []AdmitRequest
-	prepareDataPlugins       []PrepareData
+	admitRequestPlugins      []AdmissionPlugin
+	dataProducerPlugins      []DataProducer
 	preRequestPlugins        []PreRequest
 	responseReceivedPlugins  []ResponseReceived
 	responseStreamingPlugins []ResponseStreaming
@@ -71,13 +71,13 @@ func (c *Config) WithResponseCompletePlugins(plugins ...ResponseComplete) *Confi
 }
 
 // WithPrepareDataPlugins sets the given plugins as the PrepareData plugins.
-func (c *Config) WithPrepareDataPlugins(plugins ...PrepareData) *Config {
-	c.prepareDataPlugins = plugins
+func (c *Config) WithDataProducers(plugins ...DataProducer) *Config {
+	c.dataProducerPlugins = plugins
 	return c
 }
 
 // WithAdmitRequestPlugins sets the given plugins as the AdmitRequest plugins.
-func (c *Config) WithAdmitRequestPlugins(plugins ...AdmitRequest) *Config {
+func (c *Config) WithAdmitRequestPlugins(plugins ...AdmissionPlugin) *Config {
 	c.admitRequestPlugins = plugins
 	return c
 }