Skip to content

Commit fe34b90

Browse files
nirrozenbaumnirrozenbaum
authored
minor refactor to config loading (#1114)
Signed-off-by: Nir Rozenbaum <[email protected]>
1 parent 0e1e964 commit fe34b90

File tree

3 files changed

+116
-157
lines changed

3 files changed

+116
-157
lines changed

‎cmd/epp/runner/runner.go

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"flag"
2222
"fmt"
2323
"net/http/pprof"
24+
"os"
2425

2526
"github.com/go-logr/logr"
2627
"github.com/prometheus/client_golang/prometheus"
@@ -314,27 +315,35 @@ func (r *Runner) initializeScheduler() (*scheduling.Scheduler, error) {
314315
}
315316

316317
func (r *Runner) parseConfiguration(ctx context.Context) error {
317-
if len(*configText) != 0 || len(*configFile) != 0 {
318-
theConfig, err := loader.LoadConfig([]byte(*configText), *configFile)
319-
if err != nil {
320-
return fmt.Errorf("failed to load the configuration - %w", err)
321-
}
322-
323-
epp := newEppHandle(ctx)
318+
if *configText == "" && *configFile == "" {
319+
return nil // configuring through code, not through file
320+
}
324321

325-
err = loader.LoadPluginReferences(theConfig.Plugins, epp)
322+
var configBytes []byte
323+
if *configText != "" {
324+
configBytes = []byte(*configText)
325+
} else if *configFile != "" { // if config was specified through a file
326+
var err error
327+
configBytes, err = os.ReadFile(*configFile)
326328
if err != nil {
327-
return fmt.Errorf("failed to instantiate the plugins - %w", err)
329+
return fmt.Errorf("failed to load config from a file '%s' - %w", *configFile, err)
328330
}
331+
}
329332

330-
r.schedulerConfig, err = loader.LoadSchedulerConfig(theConfig.SchedulingProfiles, epp)
331-
if err != nil {
332-
return fmt.Errorf("failed to create Scheduler configuration - %w", err)
333-
}
333+
handle := newEppHandle(ctx)
334+
config, err := loader.LoadConfig(configBytes, handle)
335+
if err != nil {
336+
return fmt.Errorf("failed to load the configuration - %w", err)
337+
}
334338

335-
// Add requestControl plugins
336-
r.requestControlConfig.AddPlugins(epp.Plugins().GetAllPlugins()...)
339+
r.schedulerConfig, err = loader.LoadSchedulerConfig(config.SchedulingProfiles, handle)
340+
if err != nil {
341+
return fmt.Errorf("failed to create Scheduler configuration - %w", err)
337342
}
343+
344+
// Add requestControl plugins
345+
r.requestControlConfig.AddPlugins(handle.Plugins().GetAllPlugins()...)
346+
338347
return nil
339348
}
340349

@@ -395,7 +404,7 @@ func validateFlags() error {
395404
if *poolName == "" {
396405
return fmt.Errorf("required %q flag not set", "poolName")
397406
}
398-
if len(*configText) != 0 && len(*configFile) != 0 {
407+
if *configText != "" && *configFile != "" {
399408
return fmt.Errorf("both the %s and %s flags can not be set at the same time", "configText", "configFile")
400409
}
401410

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

Lines changed: 56 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ package loader
1919
import (
2020
"errors"
2121
"fmt"
22-
"os"
2322

2423
"k8s.io/apimachinery/pkg/runtime"
2524
"k8s.io/apimachinery/pkg/runtime/serializer"
2625
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
26+
"k8s.io/apimachinery/pkg/util/sets"
2727

2828
"sigs.k8s.io/gateway-api-inference-extension/api/config/v1alpha1"
2929
configapi "sigs.k8s.io/gateway-api-inference-extension/api/config/v1alpha1"
@@ -39,77 +39,54 @@ func init() {
3939
utilruntime.Must(configapi.Install(scheme))
4040
}
4141

42-
// Load config either from supplied text or from a file
43-
func LoadConfig(configText []byte, fileName string) (*configapi.EndpointPickerConfig, error) {
44-
var err error
45-
if len(configText) == 0 {
46-
configText, err = os.ReadFile(fileName)
47-
if err != nil {
48-
return nil, fmt.Errorf("failed to load config file. Error: %s", err)
49-
}
50-
}
51-
52-
theConfig := &configapi.EndpointPickerConfig{}
42+
// Load config from supplied text that was converted to []byte
43+
func LoadConfig(configBytes []byte, handle plugins.Handle) (*configapi.EndpointPickerConfig, error) {
44+
config := &configapi.EndpointPickerConfig{}
5345

5446
codecs := serializer.NewCodecFactory(scheme, serializer.EnableStrict)
55-
err = runtime.DecodeInto(codecs.UniversalDecoder(), configText, theConfig)
47+
err := runtime.DecodeInto(codecs.UniversalDecoder(), configBytes, config)
5648
if err != nil {
57-
return nil, fmt.Errorf("the configuration is invalid. Error: %s", err)
49+
return nil, fmt.Errorf("the configuration is invalid - %w", err)
5850
}
5951

60-
// Validate loaded configuration
61-
err = validateConfiguration(theConfig)
62-
if err != nil {
63-
return nil, fmt.Errorf("the configuration is invalid. error: %s", err)
52+
// instantiate loaded plugins
53+
if err = instantiatePlugins(config.Plugins, handle); err != nil {
54+
return nil, fmt.Errorf("failed to instantiate plugins - %w", err)
6455
}
65-
return theConfig, nil
66-
}
6756

68-
func LoadPluginReferences(thePlugins []configapi.PluginSpec, handle plugins.Handle) error {
69-
for _, pluginConfig := range thePlugins {
70-
thePlugin, err := instantiatePlugin(pluginConfig, handle)
71-
if err != nil {
72-
return err
73-
}
74-
handle.Plugins().AddPlugin(pluginConfig.Name, thePlugin)
57+
if err = validateSchedulingProfiles(config); err != nil {
58+
return nil, fmt.Errorf("failed to validate scheduling profiles - %w", err)
7559
}
76-
return nil
60+
61+
return config, nil
7762
}
7863

7964
func LoadSchedulerConfig(configProfiles []v1alpha1.SchedulingProfile, handle plugins.Handle) (*scheduling.SchedulerConfig, error) {
80-
81-
var profiles = map[string]*framework.SchedulerProfile{}
82-
83-
for _, configProfile := range configProfiles {
84-
profile := framework.SchedulerProfile{}
85-
86-
for _, plugin := range configProfile.Plugins {
87-
var err error
88-
thePlugin := handle.Plugins().Plugin(plugin.PluginRef)
89-
if theScorer, ok := thePlugin.(framework.Scorer); ok {
65+
profiles := map[string]*framework.SchedulerProfile{}
66+
for _, namedProfile := range configProfiles {
67+
profile := framework.NewSchedulerProfile()
68+
for _, plugin := range namedProfile.Plugins {
69+
referencedPlugin := handle.Plugins().Plugin(plugin.PluginRef)
70+
if scorer, ok := referencedPlugin.(framework.Scorer); ok {
9071
if plugin.Weight == nil {
9172
return nil, fmt.Errorf("scorer '%s' is missing a weight", plugin.PluginRef)
9273
}
93-
thePlugin = framework.NewWeightedScorer(theScorer, *plugin.Weight)
74+
referencedPlugin = framework.NewWeightedScorer(scorer, *plugin.Weight)
9475
}
95-
err = profile.AddPlugins(thePlugin)
96-
if err != nil {
97-
return nil, err
76+
if err := profile.AddPlugins(referencedPlugin); err != nil {
77+
return nil, fmt.Errorf("failed to load scheduler config - %w", err)
9878
}
9979
}
100-
profiles[configProfile.Name] = &profile
80+
profiles[namedProfile.Name] = profile
10181
}
10282

10383
var profileHandler framework.ProfileHandler
104-
var profileHandlerName string
105-
106-
for pluginName, thePlugin := range handle.Plugins().GetAllPluginsWithNames() {
107-
if theProfileHandler, ok := thePlugin.(framework.ProfileHandler); ok {
84+
for pluginName, plugin := range handle.Plugins().GetAllPluginsWithNames() {
85+
if theProfileHandler, ok := plugin.(framework.ProfileHandler); ok {
10886
if profileHandler != nil {
109-
return nil, fmt.Errorf("only one profile handler is allowed. Both %s and %s are profile handlers", profileHandlerName, pluginName)
87+
return nil, fmt.Errorf("only one profile handler is allowed. Both %s and %s are profile handlers", profileHandler.TypedName().Name, pluginName)
11088
}
11189
profileHandler = theProfileHandler
112-
profileHandlerName = pluginName
11390
}
11491
}
11592
if profileHandler == nil {
@@ -119,62 +96,61 @@ func LoadSchedulerConfig(configProfiles []v1alpha1.SchedulingProfile, handle plu
11996
return scheduling.NewSchedulerConfig(profileHandler, profiles), nil
12097
}
12198

122-
func instantiatePlugin(pluginSpec configapi.PluginSpec, handle plugins.Handle) (plugins.Plugin, error) {
123-
factory, ok := plugins.Registry[pluginSpec.Type]
124-
if !ok {
125-
return nil, fmt.Errorf("failed to instantiate the plugin. plugin type %s not found", pluginSpec.Type)
126-
}
127-
thePlugin, err := factory(pluginSpec.Name, pluginSpec.Parameters, handle)
128-
if err != nil {
129-
return nil, fmt.Errorf("failed to instantiate the plugin type %s. Error: %s", pluginSpec.Type, err)
130-
}
131-
return thePlugin, err
132-
}
133-
134-
func validateConfiguration(theConfig *configapi.EndpointPickerConfig) error {
135-
names := make(map[string]struct{})
99+
func instantiatePlugins(configuredPlugins []configapi.PluginSpec, handle plugins.Handle) error {
100+
pluginNames := sets.New[string]() // set of plugin names, a name must be unique
136101

137-
for _, pluginConfig := range theConfig.Plugins {
102+
for _, pluginConfig := range configuredPlugins {
138103
if pluginConfig.Type == "" {
139-
return fmt.Errorf("plugin definition for %s is missing a type", pluginConfig.Name)
104+
return fmt.Errorf("plugin definition for '%s' is missing a type", pluginConfig.Name)
140105
}
141106

142-
if _, ok := names[pluginConfig.Name]; ok {
143-
return fmt.Errorf("plugin name %s used more than once", pluginConfig.Name)
107+
if pluginNames.Has(pluginConfig.Name) {
108+
return fmt.Errorf("plugin name '%s' used more than once", pluginConfig.Name)
144109
}
145-
names[pluginConfig.Name] = struct{}{}
110+
pluginNames.Insert(pluginConfig.Name)
146111

147-
_, ok := plugins.Registry[pluginConfig.Type]
112+
factory, ok := plugins.Registry[pluginConfig.Type]
148113
if !ok {
149-
return fmt.Errorf("plugin type %s is not found", pluginConfig.Type)
114+
return fmt.Errorf("plugin type '%s' is not found in registry", pluginConfig.Type)
150115
}
116+
117+
plugin, err := factory(pluginConfig.Name, pluginConfig.Parameters, handle)
118+
if err != nil {
119+
return fmt.Errorf("failed to instantiate the plugin type '%s' - %w", pluginConfig.Type, err)
120+
}
121+
122+
handle.Plugins().AddPlugin(pluginConfig.Name, plugin)
151123
}
152124

153-
if len(theConfig.SchedulingProfiles) == 0 {
125+
return nil
126+
}
127+
128+
func validateSchedulingProfiles(config *configapi.EndpointPickerConfig) error {
129+
if len(config.SchedulingProfiles) == 0 {
154130
return errors.New("there must be at least one scheduling profile in the configuration")
155131
}
156132

157-
names = map[string]struct{}{}
158-
for _, profile := range theConfig.SchedulingProfiles {
133+
profileNames := sets.New[string]()
134+
for _, profile := range config.SchedulingProfiles {
159135
if profile.Name == "" {
160-
return errors.New("SchedulingProfiles need a name")
136+
return errors.New("SchedulingProfile must have a name")
161137
}
162138

163-
if _, ok := names[profile.Name]; ok {
164-
return fmt.Errorf("the name %s has been specified for more than one SchedulingProfile", profile.Name)
139+
if profileNames.Has(profile.Name) {
140+
return fmt.Errorf("the name '%s' has been specified for more than one SchedulingProfile", profile.Name)
165141
}
166-
names[profile.Name] = struct{}{}
142+
profileNames.Insert(profile.Name)
167143

168144
if len(profile.Plugins) == 0 {
169-
return errors.New("SchedulingProfiles need at least one plugin")
145+
return fmt.Errorf("SchedulingProfile '%s' must have at least one plugin", profile.Name)
170146
}
171147
for _, plugin := range profile.Plugins {
172148
if len(plugin.PluginRef) == 0 {
173-
return errors.New("SchedulingProfile's plugins need a plugin reference")
149+
return fmt.Errorf("SchedulingProfile '%s' plugins must have a plugin reference", profile.Name)
174150
}
175151

176152
notFound := true
177-
for _, pluginConfig := range theConfig.Plugins {
153+
for _, pluginConfig := range config.Plugins {
178154
if plugin.PluginRef == pluginConfig.Name {
179155
notFound = false
180156
break

0 commit comments

Comments
 (0)