Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions apix/config/v1alpha1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,27 @@ package v1alpha1
//
// This naming convension is required by the defalter-gen code.
func SetDefaults_EndpointPickerConfig(cfg *EndpointPickerConfig) {
// If no name was given for the plugin, use it's type as the name
for idx, pluginConfig := range cfg.Plugins {
if pluginConfig.Name == "" {
cfg.Plugins[idx].Name = pluginConfig.Type
}
}

// If No SchedulerProfiles were specified in the confguration,
// create one named default with references to all of the
// plugins mentioned in the Plugins section of the configuration.
if len(cfg.SchedulingProfiles) == 0 {
cfg.SchedulingProfiles = make([]SchedulingProfile, 1)

thePlugins := []SchedulingPlugin{}
for _, pluginConfig := range cfg.Plugins {
thePlugins = append(thePlugins, SchedulingPlugin{PluginRef: pluginConfig.Name})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we only add the plugins that are applicable allowed in a scheduling profile? scorer, filter, picker?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point in time in the code, one doesn't know what the plugins are as they have not yet been instantiated.

If you want, I can move this code to the LoadSchedulingConfig function, with appropriate changes to the configuration validation code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend consolidating validation in this pkg, this is the practice, validation is invoked after defaulting automatically. We can do that in a followup PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the code in ScheculingProfile.AddPlugins, ignores any plugins that are of types other than filter, scorer, picker, or postCycle

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but when we default and log the config, they will show up, silently ignoring them is not ideal.

}

cfg.SchedulingProfiles[0] = SchedulingProfile{
Name: "default",
Plugins: thePlugins,
}
}
}
4 changes: 4 additions & 0 deletions pkg/epp/common/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,8 @@ const (
// DefaultQueueThresholdCritical is the default backend waiting queue size
// threshold.
DefaultQueueThresholdCritical = 5

// DefaultScorerWeight is the weight used for scorers referenced in the
// configuration without explicit weights.
DefaultScorerWeight = 1
)
31 changes: 23 additions & 8 deletions pkg/epp/common/config/loader/configloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@ import (
"k8s.io/apimachinery/pkg/util/sets"

configapi "sigs.k8s.io/gateway-api-inference-extension/apix/config/v1alpha1"
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/common/config"
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins"
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling"
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling/framework"
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling/framework/plugins/picker"
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling/framework/plugins/profile"
)

var scheme = runtime.NewScheme()
Expand Down Expand Up @@ -64,18 +67,31 @@ func LoadSchedulerConfig(configProfiles []configapi.SchedulingProfile, handle pl
profiles := map[string]*framework.SchedulerProfile{}
for _, namedProfile := range configProfiles {
profile := framework.NewSchedulerProfile()
pickerAdded := false
for _, plugin := range namedProfile.Plugins {
referencedPlugin := handle.Plugin(plugin.PluginRef)
if scorer, ok := referencedPlugin.(framework.Scorer); ok {
if plugin.Weight == nil {
return nil, fmt.Errorf("scorer '%s' is missing a weight", plugin.PluginRef)
// Set default weight if one wasn't set in the configuration
weight := config.DefaultScorerWeight
if plugin.Weight != nil {
weight = *plugin.Weight
}
referencedPlugin = framework.NewWeightedScorer(scorer, *plugin.Weight)
referencedPlugin = framework.NewWeightedScorer(scorer, weight)
}
if _, ok := referencedPlugin.(framework.Picker); ok {
pickerAdded = true
}
if err := profile.AddPlugins(referencedPlugin); err != nil {
return nil, fmt.Errorf("failed to load scheduler config - %w", err)
}
}
if !pickerAdded {
// There isn't a picker in this profile, add one
thePicker := picker.NewMaxScorePicker(picker.DefaultMaxNumOfEndpoints)
if err := profile.AddPlugins(thePicker); err != nil {
return nil, fmt.Errorf("failed to load scheduler config - %w", err)
}
}
profiles[namedProfile.Name] = profile
}

Expand All @@ -89,7 +105,10 @@ func LoadSchedulerConfig(configProfiles []configapi.SchedulingProfile, handle pl
}
}
if profileHandler == nil {
return nil, errors.New("no profile handler was specified")
if len(profiles) != 1 {
return nil, errors.New("no profile handler was specified")
}
profileHandler = profile.NewSingleProfileHandler()
}

return scheduling.NewSchedulerConfig(profileHandler, profiles), nil
Expand Down Expand Up @@ -125,10 +144,6 @@ func instantiatePlugins(configuredPlugins []configapi.PluginSpec, handle plugins
}

func validateSchedulingProfiles(config *configapi.EndpointPickerConfig) error {
if len(config.SchedulingProfiles) == 0 {
return errors.New("there must be at least one scheduling profile in the configuration")
}

profileNames := sets.New[string]()
for _, profile := range config.SchedulingProfiles {
if profile.Name == "" {
Expand Down
Loading