Reintroduce Plugin.Name() (#1057)

* rename Factory function as FactoryFunc

Signed-off-by: Etai Lev Ran <[email protected]>

* Refactor field name from PluginName to Type

JSON tags are not changed yet as it would impact all test files/sample YAML.
Will be in a follow up commit.

Signed-off-by: Etai Lev Ran <[email protected]>

* Review test cases

Added comments describing test beyond the already descriptive name
(e.g., field Plugins was initially called PluginReference so names
can be a little misleading when using PluginRef - it's not the
scheduling profile's references).
Removed redundant test case (coverage is still 100%).

Signed-off-by: Etai Lev Ran <[email protected]>

* Change pluginName to type in encoding

Change the JSON tag on type
Change test files and test cases' text to use type instead of pluginName

Signed-off-by: Etai Lev Ran <[email protected]>

* implement Name() for all plugins

Signed-off-by: Etai Lev Ran <[email protected]>

* Added WithName() to all plugins

Plugins (except testing only plugins) now allow calling WithName() to assign their name.
Facory functions work by calling New() to avoid code duplication and centralize parameter checking.
When constructing an unnamed Plugin, its name defaults to Type()

Signed-off-by: Etai Lev Ran <[email protected]>

* implement review comments

Set plugin name to its type explicitly in New()
Remove empty name check from WithName()
Refine error message in config loading (added plugin name for missing type)

Signed-off-by: Etai Lev Ran <[email protected]>

* change Name() comment

Signed-off-by: Etai Lev Ran <[email protected]>

---------

Signed-off-by: Etai Lev Ran <[email protected]>

Author: elevran

api/config/v1alpha1/defaults.go

@@ -23,7 +23,7 @@ package v1alpha1
 func SetDefaults_EndpointPickerConfig(cfg *EndpointPickerConfig) {
 	for idx, pluginConfig := range cfg.Plugins {
 		if pluginConfig.Name == "" {
-			cfg.Plugins[idx].Name = pluginConfig.PluginName
+			cfg.Plugins[idx].Name = pluginConfig.Type
 		}
 	}
 }

api/config/v1alpha1/endpointpickerconfig_types.go

@@ -46,13 +46,13 @@ type EndpointPickerConfig struct {
 type PluginSpec struct {
 	// +optional
 	// Name provides a name for plugin entries to reference. If
-	// omitted, the value of the PluginName field will be used.
+	// omitted, the value of the Plugin's Type field will be used.
 	Name string `json:"name"`
 
 	// +required
 	// +kubebuilder:validation:Required
-	// PluginName specifies the plugin to be instantiated.
-	PluginName string `json:"pluginName"`
+	// Type specifies the plugin type to be instantiated.
+	Type string `json:"type"`
 
 	// +optional
 	// Parameters are the set of parameters to be passed to the plugin's

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

@@ -49,6 +49,11 @@ func (f *HeaderBasedTestingFilter) Type() string {
 	return "header-based-testing"
 }
 
+// Name returns the type of the filter.
+func (f *HeaderBasedTestingFilter) Name() string {
+	return "header-based-testing-filter"
+}
+
 // Filter selects pods that match the IP addresses specified in the request header.
 func (f *HeaderBasedTestingFilter) Filter(_ context.Context, _ *types.CycleState, request *types.LLMRequest, pods []types.Pod) []types.Pod {
 	headerValue, ok := request.Headers[headerTestEppEndPointSelectionKey]

pkg/epp/common/config/loader/configloader.go

@@ -120,13 +120,13 @@ func LoadSchedulerConfig(configProfiles []v1alpha1.SchedulingProfile, handle plu
 }
 
 func instantiatePlugin(pluginSpec configapi.PluginSpec, handle plugins.Handle) (plugins.Plugin, error) {
-	factory, ok := plugins.Registry[pluginSpec.PluginName]
+	factory, ok := plugins.Registry[pluginSpec.Type]
 	if !ok {
-		return nil, fmt.Errorf("failed to instantiate the plugin. plugin %s not found", pluginSpec.PluginName)
+		return nil, fmt.Errorf("failed to instantiate the plugin. plugin type %s not found", pluginSpec.Type)
 	}
 	thePlugin, err := factory(pluginSpec.Name, pluginSpec.Parameters, handle)
 	if err != nil {
-		return nil, fmt.Errorf("failed to instantiate the plugin %s. Error: %s", pluginSpec.PluginName, err)
+		return nil, fmt.Errorf("failed to instantiate the plugin type %s. Error: %s", pluginSpec.Type, err)
 	}
 	return thePlugin, err
 }
@@ -135,18 +135,18 @@ func validateConfiguration(theConfig *configapi.EndpointPickerConfig) error {
 	names := make(map[string]struct{})
 
 	for _, pluginConfig := range theConfig.Plugins {
-		if pluginConfig.PluginName == "" {
-			return errors.New("plugin reference definition missing a plugin name")
+		if pluginConfig.Type == "" {
+			return fmt.Errorf("plugin definition for %s is missing a type", pluginConfig.Name)
 		}
 
 		if _, ok := names[pluginConfig.Name]; ok {
-			return fmt.Errorf("the name %s has been specified for more than one plugin", pluginConfig.Name)
+			return fmt.Errorf("plugin name %s used more than once", pluginConfig.Name)
 		}
 		names[pluginConfig.Name] = struct{}{}
 
-		_, ok := plugins.Registry[pluginConfig.PluginName]
+		_, ok := plugins.Registry[pluginConfig.Type]
 		if !ok {
-			return fmt.Errorf("plugin %s is not found", pluginConfig.PluginName)
+			return fmt.Errorf("plugin type %s is not found", pluginConfig.Type)
 		}
 	}