[WIP] general pluggability proposal/discussion #940
Conversation
Signed-off-by: Nir Rozenbaum <nirro@il.ibm.com>
k8s-ci-robot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nirrozenbaum The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/M
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
started an initial doc to discuss general pluggability management and structuring. will add more information as we make progress with the proposal. |
| @@ -0,0 +1,46 @@ | |||
| # Pluggability | |||
There was a problem hiding this comment.
Reminder: rename the dir to 0940- now that you have a PR number assigned
|
|
||
| ## Proposal | ||
|
|
||
| This proposal aims to define how to structure and setup plugins for different layers of the system. For the sake of the discussion, the proposal focuses on extension points from the scheduling layer, which is the most stable one in terms of pluggability, and from the RequestControl layer. |
There was a problem hiding this comment.
Am I correct in understanding that there are two topics here?
- how to structure plugins' code within the GIE repository
- how to define what plugins are available in the system
If so, can you dedicate a proposal section to each?
|
|
||
| ### Plugins maintainace | ||
| Since plugins may implement multiple extension points from different layers of EPP, it makes sense to group plugin implementations in under one directory. | ||
| For example, instead of keeping plugins under `/pkg/epp/scheduling/framework/plugins`, this proposal suggests to define a shared package to be used for all plugins of the system - `pkg/epp/plugins`. |
There was a problem hiding this comment.
I agree that moving all system plugins under a single epp/plugins directory would be helpful.
Can you please also add a concrete suggestion on how to structure plugin code underneath?
For example, should they still be grouped by layer and function whenever possible (e.g., scheduling/filter, scheduling/scorer, ...) or not?
Making a concrete suggestion would make it easier to discuss and converge.
There was a problem hiding this comment.
+1
The language here is pretty "soft" - it just suggests an example. Can you just rephrase this to a concrete proposal instead of just a suggestion? I think we are mostly on the same page and we can agree on something and move forward with that
|
|
||
| ### Plugins by code | ||
|
|
||
| All extensible structures should be initialized with the plugins in main (under cmd package) and should support setting up extensions by code. Example can be found [here](https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/cmd/epp/main.go#L208-L227). |
There was a problem hiding this comment.
I think you're saying that we need functionality to
- declare which plugins are available to the system
- configure a specific (subset) of available plugins into the current schedule
While the second item (configuration) can be done by either code or configuration, the first is more easily done solely in code (unless we resort to Go Plugins and DSO which come with their own complexity).
|
|
||
| All extensible structures should be initialized with the plugins in main (under cmd package) and should support setting up extensions by code. Example can be found [here](https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/cmd/epp/main.go#L208-L227). | ||
|
|
||
| Additionally, most of the code in the current main file should move to a separate file (may be under separate package) to have a very minimal main.go file which only handles the initialization of plugins and calling a `runner` to run the EPP with the given extensions configuration. This will allow to configure plugins with no code duplication and with minimal code. |
There was a problem hiding this comment.
... initialization of plugins
This refers to the set of available plugins - correct?
| All extensible structures should be initialized with the plugins in main (under cmd package) and should support setting up extensions by code. Example can be found [here](https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/cmd/epp/main.go#L208-L227). | ||
|
|
||
| Additionally, most of the code in the current main file should move to a separate file (may be under separate package) to have a very minimal main.go file which only handles the initialization of plugins and calling a `runner` to run the EPP with the given extensions configuration. This will allow to configure plugins with no code duplication and with minimal code. | ||
| By doing the above, we end up with a main file that looks very similar to [kube-scheduler main](https://github.com/kubernetes/kubernetes/blob/master/cmd/kube-scheduler/scheduler.go#L29-L33) |
There was a problem hiding this comment.
The registration of available plugins should still be done in an EPP instance (e.g., GIE, llm-d) specific file, such as cmd/main to allow variability between implementations, and can not be done in a shared package
|
|
||
| ## Design Principles | ||
| - Plugins may implement more than one extension point. | ||
| More specifically, plugins may implement multiple extension points that are defined in different layers, e.g., a plugin may implement extension point from scheduling layer and another one from requestcontrol layer. |
There was a problem hiding this comment.
nit: link the docs for scheduling framework and requestcontrol framework
|
|
||
| ## Proposal | ||
|
|
||
| This proposal aims to define how to structure and setup plugins for different layers of the system. For the sake of the discussion, the proposal focuses on extension points from the scheduling layer, which is the most stable one in terms of pluggability, and from the RequestControl layer. |
There was a problem hiding this comment.
| This proposal aims to define how to structure and setup plugins for different layers of the system. For the sake of the discussion, the proposal focuses on extension points from the scheduling layer, which is the most stable one in terms of pluggability, and from the RequestControl layer. | |
| This proposal aims to define how to structure and setup plugins for different layers of the system. For the sake of the discussion, the proposal focuses on extension points from the scheduling layer, which is the most stable one in terms of pluggability. |
| This proposal aims to define how to structure and setup plugins for different layers of the system. For the sake of the discussion, the proposal focuses on extension points from the scheduling layer, which is the most stable one in terms of pluggability, and from the RequestControl layer. | ||
| RequestControl might end up having multiple extension points, two of those are already agreed on all parties and will be used as a basis for the discussion: | ||
|
|
||
| - `PreRequest` a plugin that takes the scheduling result and use it to execute logic before the request is sent to the selected endpoint. |
There was a problem hiding this comment.
| - `PreRequest` a plugin that takes the scheduling result and use it to execute logic before the request is sent to the selected endpoint. | |
| - `PreRequest` a plugin that takes the scheduling result and use it to execute logic before the request is sent to the selected endpoint. |
|
|
||
| Please note that there is no intention to cover all extension points of the system, but only to specify extension points from at least two different layers in order to discuss how to maintain and structure plugins in the repo. | ||
|
|
||
| ### Plugins maintainace |
There was a problem hiding this comment.
| ### Plugins maintainace | |
| ### Plugins maintenance |
|
|
||
| ### Plugins maintainace | ||
| Since plugins may implement multiple extension points from different layers of EPP, it makes sense to group plugin implementations in under one directory. | ||
| For example, instead of keeping plugins under `/pkg/epp/scheduling/framework/plugins`, this proposal suggests to define a shared package to be used for all plugins of the system - `pkg/epp/plugins`. |
There was a problem hiding this comment.
+1
The language here is pretty "soft" - it just suggests an example. Can you just rephrase this to a concrete proposal instead of just a suggestion? I think we are mostly on the same page and we can agree on something and move forward with that
| Since plugins may implement multiple extension points from different layers of EPP, it makes sense to group plugin implementations in under one directory. | ||
| For example, instead of keeping plugins under `/pkg/epp/scheduling/framework/plugins`, this proposal suggests to define a shared package to be used for all plugins of the system - `pkg/epp/plugins`. | ||
|
|
||
| Additionally, from the same reason, this proposal suggests to have a single Plugin interface across the codebase and all specific plugins should embed this plugin inteface in the extension point. This will allow to register a general `Plugin` instance to EPP and auto-register the plugin in the right extension points (might be very useful when configuration via extrenal configuration file). |
There was a problem hiding this comment.
Can you be specific and perhaps add the code snippet here? I think I get what you are seeing about the Plugin interface which perhaps only has a Name() method. But for someone new to this it can be quite confusing.
|
|
||
| All extensible structures should be initialized with the plugins in main (under cmd package) and should support setting up extensions by code. Example can be found [here](https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/cmd/epp/main.go#L208-L227). | ||
|
|
||
| Additionally, most of the code in the current main file should move to a separate file (may be under separate package) to have a very minimal main.go file which only handles the initialization of plugins and calling a `runner` to run the EPP with the given extensions configuration. This will allow to configure plugins with no code duplication and with minimal code. |
There was a problem hiding this comment.
I think this is a nice to have a not necessarily a requirement. I think what you want to see is that we should have some utilities to facilitate the plugin registration, and by doing so you end up with minimal code in main.go. But simplifying main.go isn't the goal of this proposal
So what I suggest is just describing that requirement for that utility package , and don't mention about how to simplify main.go (it's just a result of defining that utility package)/
|
@nirrozenbaum: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
No description provided.