Add APIs for the instantiated plugins to the EPP Handle #1039

shmuelk · 2025-06-22T08:25:31Z

This PR adds a set of APIs for managing the instantiated plugins in the EPP. These APIs include:

Plugin - returns the named plugin instance
AddPlugin - Adds a plugin to the set of known plugin instances
GetAllPlugins - Returns all of the known plugins
GetAllPluginsWithNames - Returns all of the known plugins with their names

This PR is the first of two to solve issue #1011 which requires referencing other instantiated plugins.

netlify · 2025-06-22T08:25:35Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`846a7cf`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/685a5315b5b6cb0008197ca5
😎 Deploy Preview	https://deploy-preview-1039--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-06-22T08:25:40Z

Hi @shmuelk. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

nirrozenbaum · 2025-06-22T18:26:22Z

pkg/epp/scheduling/scheduler_config.go

@@ -39,7 +39,7 @@ type SchedulerConfig struct {
 	profiles       map[string]*framework.SchedulerProfile
 }

-func LoadSchedulerConfig(configProfiles []v1alpha1.SchedulingProfile, references map[string]plugins.Plugin) (*SchedulerConfig, error) {
+func LoadSchedulerConfig(configProfiles []v1alpha1.SchedulingProfile, handle plugins.Handle) (*SchedulerConfig, error) {


ideally, I think it would be better if this function (and same one in requestcontrol config) gets out of this package and into a dedicated package for file configuration parsing.
this function is not part of the struct and is a free function. I think it would be better to define a configurationParser struct with receiver functions like LoadSchedulerConfig.

skipped this comment in previous PR cause it was too big and we had too many comments on it..

All of the functions in this file are static functions (without a receiver). Why is this so different from NewSchedulerConfig? This just an additional way to create a SchedulerConfig.

there are no functions other than NewSchedulerConfig :).
the main point I was trying to make is that as we make progress and add more extension points (e.g., in flow control) the code to iterate over the plugins and call some "AddPlugins" or similar function will be duplicated across different places. instead, I think we should have a single package and struct where we handle the file parsing and creation of the relevant configs from it.
this is a matter of decoupling between scheduler package and the configuration parsing.
IMO the scheduler or any other layer shouldn't care about the configuration file and it should get from outside its configuration. Additionally, scheduling package shouldn't have unit-tests to test configuration file parsing and validity.

All of the configuration loading code has been moved to pkg/epp/common/config/loader

nirrozenbaum · 2025-06-22T18:27:55Z

pkg/epp/scheduling/scheduler_config_test.go

@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/scheduling/framework/plugins/multi/prefix"
 	"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"
+	"sigs.k8s.io/gateway-api-inference-extension/test/utils"


ditto about this file.
nothing in this file is testing SchedulerConfig. this is testing the configuration file parsing and the logic around it.

This file has been merged into config_loader_test.go in the new loader package.

pkg/epp/plugins/plugins.go

nirrozenbaum · 2025-06-23T18:07:38Z

/ok-to-test

ahg-g · 2025-06-23T18:48:40Z

/lgtm

nirrozenbaum · 2025-06-23T18:52:23Z

cmd/epp/runner/runner.go

-		theConfig, err := config.LoadConfig([]byte(*configText), *configFile)
+		theConfig, err := loader.LoadConfig([]byte(*configText), *configFile)
 		if err != nil {
 			setupLog.Error(err, "Failed to load the configuration")
 			return err
 		}

-		epp := eppHandle{}
-		instantiatedPlugins, err := config.LoadPluginReferences(theConfig.Plugins, epp)
+		epp := newEppHandle()
+
+		err = loader.LoadPluginReferences(theConfig.Plugins, epp)
 		if err != nil {
 			setupLog.Error(err, "Failed to instantiate the plugins")
 			return err
 		}

-		r.schedulerConfig, err = scheduling.LoadSchedulerConfig(theConfig.SchedulingProfiles, instantiatedPlugins)
+		r.schedulerConfig, err = loader.LoadSchedulerConfig(theConfig.SchedulingProfiles, epp)
 		if err != nil {
 			setupLog.Error(err, "Failed to create Scheduler configuration")
 			return err
 		}

-		// Add requestcontrol plugins
-		if instantiatedPlugins != nil {
-			r.requestControlConfig = requestcontrol.LoadRequestControlConfig(instantiatedPlugins)
-		}
+		// Add requestControl plugins
+		r.requestControlConfig.AddPlugins(epp.Plugins().GetAllPlugins()...)


in the follow up PR you plan to do, can we extract this to a helper function?
something like parseConfigFile or loadConfigFromFile or any similar name?

nirrozenbaum · 2025-06-23T18:59:37Z

/lgtm
just need to rebase to get this merged.

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

nirrozenbaum · 2025-06-24T07:55:56Z

/lgtm
/approve

Thanks!

k8s-ci-robot · 2025-06-24T07:56:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nirrozenbaum, shmuelk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [nirrozenbaum]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from ahg-g and robscott June 22, 2025 08:25

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 22, 2025

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 22, 2025

nirrozenbaum reviewed Jun 22, 2025

View reviewed changes

pkg/epp/plugins/plugins.go Show resolved Hide resolved

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 23, 2025

shmuelk closed this Jun 23, 2025

shmuelk force-pushed the epp-handle-1 branch from b91fa6d to 5d4d3ec Compare June 23, 2025 15:05

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 23, 2025

shmuelk reopened this Jun 23, 2025

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 23, 2025

k8s-ci-robot assigned ahg-g Jun 23, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 23, 2025

nirrozenbaum reviewed Jun 23, 2025

View reviewed changes

k8s-ci-robot assigned nirrozenbaum Jun 23, 2025

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 24, 2025

shmuelk added 5 commits June 24, 2025 10:22

Added plugin instance APIs to plugins.Handle

26eab51

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

An implementation of the new plugins.Handle APIs

df96c24

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

Moved all configuration loading code to new package

ce1dea2

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

Updates due to new and moved APIs

0da7c51

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

Cleanup of old configuration loading code

846a7cf

Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

shmuelk force-pushed the epp-handle-1 branch from 287dcc4 to 846a7cf Compare June 24, 2025 07:26

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 24, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 24, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2025

k8s-ci-robot merged commit 4485f63 into kubernetes-sigs:main Jun 24, 2025
9 checks passed

shmuelk mentioned this pull request Jun 24, 2025

feat: Added a factory function for the DecisionTree filter #1053

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add APIs for the instantiated plugins to the EPP Handle #1039

Add APIs for the instantiated plugins to the EPP Handle #1039

Uh oh!

shmuelk commented Jun 22, 2025

Uh oh!

netlify bot commented Jun 22, 2025 •

edited

Loading

Uh oh!

k8s-ci-robot commented Jun 22, 2025

Uh oh!

nirrozenbaum Jun 22, 2025 •

edited

Loading

Uh oh!

shmuelk Jun 23, 2025

Uh oh!

nirrozenbaum Jun 23, 2025

Uh oh!

shmuelk Jun 23, 2025

Uh oh!

nirrozenbaum Jun 22, 2025

Uh oh!

shmuelk Jun 23, 2025

Uh oh!

Uh oh!

nirrozenbaum commented Jun 23, 2025

Uh oh!

ahg-g commented Jun 23, 2025

Uh oh!

nirrozenbaum Jun 23, 2025 •

edited

Loading

Uh oh!

nirrozenbaum commented Jun 23, 2025

Uh oh!

nirrozenbaum commented Jun 24, 2025

Uh oh!

k8s-ci-robot commented Jun 24, 2025

Uh oh!

Uh oh!

Uh oh!

Add APIs for the instantiated plugins to the EPP Handle #1039

Add APIs for the instantiated plugins to the EPP Handle #1039

Uh oh!

Conversation

shmuelk commented Jun 22, 2025

Uh oh!

netlify bot commented Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for gateway-api-inference-extension ready!

Uh oh!

k8s-ci-robot commented Jun 22, 2025

Uh oh!

nirrozenbaum Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shmuelk Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

nirrozenbaum Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

shmuelk Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

nirrozenbaum Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

shmuelk Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nirrozenbaum commented Jun 23, 2025

Uh oh!

ahg-g commented Jun 23, 2025

Uh oh!

nirrozenbaum Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nirrozenbaum commented Jun 23, 2025

Uh oh!

nirrozenbaum commented Jun 24, 2025

Uh oh!

k8s-ci-robot commented Jun 24, 2025

Uh oh!

Uh oh!

Uh oh!

netlify bot commented Jun 22, 2025 •

edited

Loading

nirrozenbaum Jun 22, 2025 •

edited

Loading

nirrozenbaum Jun 23, 2025 •

edited

Loading