fix!: only run specified plugin. Fixes #10304

[jswxstw]

Fixes #10304

Motivation

Multiple plugins in AgentPod will attempt to execute the same task, but the required plugin has already been specified in the template.

Modifications

1. Keep the sidecar container name consistent with the plugin name
2. Maintain plugins as map[string]executorplugins.TemplateExecutor and try to select the plugin specified in the template to execute the task, and fallback to the original logic if it was not found.

Verification

UT and local test.

[dgolja]

I encountered a similar issue while working on #13026 (comment) and this PR nicely solve the issue, however I am worried about the back compatibility.

If I understand this correctly the name in the sidecar.container defines the identifier of the plugin which is use to load the right plugin. Problem is a lot of current plugins will break. For example all plugins from the plugin-directory may not work properly anymore, because for name they use <PLUGIN_NAME>-executor-plugin.

Also there is no strict name convention for the name value, so some plugins took the liberty to name it a bit differently (see 1). Also all private plugins may break if they are not using the correct name.

[jswxstw]

For example all plugins from the plugin-directory may not work properly anymore, because for name they use <PLUGIN_NAME>-executor-plugin.

That's a good question which I didn't think aboud it before. One way to fix it is that replacing the sidecar name with plugin name when loading.

[dgolja]

I was thinking if we really want to provide back compatibility, but also a way forward we could extend ExecutorPlugin spec by adding sidecar.provides []string. Optionally you would list all the RPC the plugin provide and use that values as a filter. In case no value is provided load the plugin regardless if its used or not.

Another problem is that as it is now you could theoretically write an plugin which can provide more than one RPC, that's why provides should be a list.

[jswxstw]

Optionally you would list all the RPC the plugin provide and use that values as a filter. In case no value is provided load the plugin regardless if its used or not.

In fact, compatibility can be ensured without extending the spec. For example, prioritize running the specified plugin, and if it is not found, then fallback to the existing polling logic.
However, if we want to load only the plugins required for the current workflow for the agent pod, it seems impossible without extending the spec, as #10304 (comment) talked about.

Another problem is that as it is now you could theoretically write an plugin which can provide more than one RPC, that's why provides should be a list.

It seems like you want the same plugin to perform multiple functions, but this can actually be achieved within a single RPC without extending the entire executor plugins runtime framework.

[dgolja]

However, if we want to load only the plugins required for the current workflow for the agent pod, it seems impossible without extending the spec, as #10304 (comment) talked about.

That would be the real end goal for me. As it is now I find it really impractical and resource wasteful to load all the plugins and eventually unusable of you have a lot of plugins.

It seems like you want the same plugin to perform multiple functions, but this can actually be achieved within a single RPC without extending the entire executor plugins runtime framework.

Personally I would have one plugin for one RPC/task, but what I tried to say is that some users may already have plugins which provide multiple functionality.

For example no one is stopping you that in your plugin /api/v1/template.execute can work for multiple plugin(s). Having this in mind we should be careful to not break the back compatibility.

[jswxstw]

That would be the real end goal for me. As it is now I find it really impractical and resource wasteful to load all the plugins and eventually unusable of you have a lot of plugins.

I couldn't agree more, but I currently can't find a elegant way to achieve this goal.

For example no one is stopping you that in your plugin /api/v1/template.execute can work for multiple plugin(s). Having this in mind we should be careful to not break the back compatibility.

So, the key point is how to declare which plugins the current workflow requires and which plugin should be specified to execute the current template.
Based on the existing logic, it seems that the design is aimed at achieving the above goal based on plugin names. However, there is no strong restriction, which makes it easy to introduce compatibility issues once modifications are made.

[jswxstw]

/retest