feat: Template executor plugins. Fixes #5201 #7256

alexec · 2021-11-20T01:18:52Z

Signed-off-by: Alex Collins alex_collins@intuit.com
Fixes #5201

Don't bother creating a PR until you've done this:

Run make pre-commit -B to fix codegen, lint, and commit message problems.

Create your PR as a draft.

Your PR needs to pass the required checks before it can be approved. If the check is not required (e.g. E2E tests) it
does not need to pass.
Once required tests have passed, you can make it "Ready for review".
Say how how you tested your changes. If you changed the UI, attach screenshots.

Tips:

If changes were requested, and you've made them, then dismiss the review to get it looked at again.
Add you organization to USERS.md if you like.
You can ask for help!

docs/plugins.md

alexec · 2021-12-01T18:02:56Z

I have added many reviewers. This is a fundamental change to how we do templates and I want it to get thorough scrutiny.

docs/plugins.md

util/errors/errors.go

workflow/controller/controller.go

workflow/util/plugins/plugin.go

terrytangyuan · 2021-12-01T21:01:26Z

Also added some small comments in https://docs.google.com/document/d/1ul_5iGPGOSo7hkRMyh5YGh2X-WcA5ms3REjmK-TSsqY/edit#. Thought it might be helpful to add the link here for others to chime in as well.

cmd/argo/commands/get.go

cmd/argo/commands/plugin/io.go

cmd/workflow-controller/main.go

docs/executor_plugins.md

docs/plugins.md

pkg/apis/workflow/v1alpha1/workflow_types.go

pkg/plugins/executor/template_executor_plugin.go

crenshaw-dev · 2021-12-01T21:35:05Z

workflow/util/plugins/configmap.go

+			Kind: cm.Labels[common.LabelKeyConfigMapType],
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        strings.TrimSuffix(cm.Name, "-executor-plugin"),


Do we need to apply any validation to the name?

cmd/argo/commands/plugin/io.go

docs/plugins.md

examples/plugins/executor/hello/hello-executor-plugin-configmap.yaml

cmd/argo/commands/plugin/io.go

simster7

Quick first-pass. This is incredibly exciting!

docs/executor_plugins.md

plugins/executor/hello/README.md

workflow/executor/agent.go

crenshaw-dev · 2021-12-06T19:01:50Z

@alexec I see some features will be deferred to a v2. Is there anything that should be done in this PR to support versioning? A couple things that come to mind:

adding a version field to the plugin.yaml spec
adding warnings in documentation that this is an early feature and that it might change significantly

I also just realized that minimum-version notes should be added once we know what release this will be in.

alexec · 2021-12-07T00:46:57Z

Ready for review again.

alexec · 2021-12-07T00:51:38Z

adding a version field to the plugin.yaml spec

This would be apiVersion: argoproj.io/v1alpha1.

adding warnings in documentation that this is an early feature and that it might change significantly

I don't expect we'd change the spec. But maybe refine it.

crenshaw-dev · 2021-12-07T15:08:18Z

This would be apiVersion: argoproj.io/v1alpha1.

lol of course, I should read files. 😆

crenshaw-dev

Just a very small number of grammar-type comments.

cmd/argo/commands/plugin/build.go

cmd/argo/commands/plugin/io.go

docs/executor_plugins.md

docs/plugins.md

docs/cli/argo_plugin.md

alexec · 2021-12-07T19:23:41Z

@crenshaw-dev you got me thinking. I'm not sure we should ExecutorPlugin and ControllerPlugin, or if we should have just Plugin, with something in the spec that determines what to run.

crenshaw-dev · 2021-12-07T19:26:47Z

@alexec faaaaiirr.... I guess it depends on how much we think the specs might diverge. I guess we could take the strategy of nesting them. e.g.

kind: ArgoWorkflowPlugin # or whatever
spec:
  executor: {}
  controller: {}

Or something like that.

I don't have a strong feeling, besides I do believe that different plugin types should be independently enabled for better security.

alexec · 2021-12-07T19:29:03Z

Some more thoughts:

Users cannot install controller plugins. Architecture does not allow it.
Some use cases might require both executor and controller parts (thought nothing comes to mind now).

simster7

Second pass

cmd/argo/commands/executorplugin/root.go

cmd/argoexec/commands/agent.go

docs/executor_plugins.md

pkg/apis/workflow/v1alpha1/object_types.go

simster7 · 2021-12-07T22:39:44Z

docs/executor_plugins.md

+  templates:
+    - name: main
+      plugin:
+        hello: { }


I think it would be more powerful to show a plugin with a non-empty schema as an example. Maybe argosay instead of hello-world where you return the input?

plugins/executor/slack/plugin.yaml

workflow/executor/agent.go

cmd/argo/commands/executorplugin/io.go

alexec · 2021-12-14T02:12:30Z

@simster7 @crenshaw-dev good enough?

kjagiello · 2021-12-21T12:24:54Z

Update: Nevermind me. I gave it another try and realized that the ouput parameter name provided by the plugin mismatched the parameter name in the workflow. Once it got fixed, I was able to pass the data between plugin invocations.

I'm a bit curious if it is possible to access the output parameters provided by a plugin? Consider a scenario where I'd like to pass some data between invocations of a plugin. In my case I'd like to, using a plugin, create a Slack message at the beginning of a workflow and be able to update the message from subsequent steps (in order to avoid clutter in the Slack channel). I could probably store the Slack message ID in the memory of my plugin instance, but I thought I'd be neat to keep the plugin stateless and be able to control which message(s) one wants to update. See the example workflow below.

Workflow

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: notifications-test-
spec:
  entrypoint: main
  templates:
    - name: main
      steps:
        - - name: pre-notification
            template: notification
            arguments:
              parameters:
                - name: message
                  value: "{{workflow.name}} started"

        # ...

        - - name: post-notification
            template: notification
            arguments:
              parameters:
                - name: thread-id
                  value: "{{workflow.outputs.parameters.slack-thread-id}}"
                - name: message
                  value: "{{workflow.name}} succeeded"

    - name: notification
      inputs:
        parameters:
          - name: thread-id
            value: ""
          - name: message
      plugin:
        slack:
          message: "{{inputs.parameters.message}}"
          thread_id: "{{inputs.parameters.thread-id}}"
      outputs:
        parameters:
          # A placeholder output parameter to keep the workflow validation happy
          - name: thread-id
            globalName: slack-thread-id
            value: ""

@crenshaw-dev gave me an idea that I could populate the output parameters through the response from my plugin, so I tried the following:

Plugin response:

{
    "node": {
        "phase": "Succeeded",
        "message": "Slack message sent",
        "outputs": {
            "parameters": [
                {
                    "name": "thread-id",
                    "globalName": "slack-thread-id",
                    "value": "7331"
                }
            ]
        }
    }
}

The result I'm seeing is however that {{workflow.outputs.parameters.slack-thread-id}}} never gets rendered.

notifications-test-2xwx9 succeeded (thread id: {{workflow.outputs.parameters.slack-thread-id}})

I've tried {{steps.pre-notification.outputs.parameters.thread-id}} as well, but it did not render either.

Reading through the changeset I see that this is not something that seems to be officially supported (i.e. don't see any tests for it), so I wonder if this is a use-case that you might want to support in the future?

alexec · 2022-01-18T20:00:06Z

@kjagiello you should be able to use output parameters. I wonder if your JSON should be this:

{
    "node": {
        "phase": "Succeeded",
        "message": "Slack message sent",
        "outputs": {
            "parameters": [
                {
                    "name": "slack-thread-id",
                    "value": "7331"
                }
            ]
        }
    }
}

Signed-off-by: Alex Collins <alex_collins@intuit.com>

sarabala1979 · 2022-01-21T19:35:24Z

cmd/argo/commands/executorplugin/build.go

+	plugin "github.com/argoproj/argo-workflows/v3/workflow/util/plugins"
+)
+
+func NewBuildCommand() *cobra.Command {


will this command work for all languages like java, go, etc?

alexec · 2022-01-21T20:16:20Z

will this command work for all languages like java, go, etc?

Yes. Thought for complied languages, you'll need to build the image with your complied source code.

sarabala1979 · 2022-01-21T23:45:18Z

@alexec If have multiple plugins and all are used in the same workflow, how the Agent pod resource will be configured
How the below scenario is handled:

if one of the plugin containers is not ready but the agent container start processing (I believe step fail with 404)
if both plugins configured same port, do we have validation

alexec · 2022-01-22T00:13:44Z

if one of the plugin containers is not ready but the agent container start processing (I believe step fail with 404)

No. It be retried. This is a common case.

if both plugins configured same port, do we have validation

No validation. I'm not sure the pod will schedule. I will test.

sarabala1979

Most code review was done by others. We need to detailed doc or video to understand plug-in development. you can add it later. Please test all corner cases.

alexec closed this Nov 22, 2021

alexec reopened this Nov 24, 2021

alexec commented Dec 1, 2021

View reviewed changes

docs/plugins.md Outdated Show resolved Hide resolved

alexec marked this pull request as ready for review December 1, 2021 18:01

alexec requested review from terrytangyuan, simster7, Ark-kun, crenshaw-dev, jannfis, jessesuen, NikeNano and tczhao December 1, 2021 18:01

terrytangyuan reviewed Dec 1, 2021

View reviewed changes

crenshaw-dev requested changes Dec 1, 2021

View reviewed changes

NikeNano reviewed Dec 1, 2021

View reviewed changes

cmd/argo/commands/plugin/io.go Outdated Show resolved Hide resolved

NikeNano reviewed Dec 1, 2021

View reviewed changes

docs/plugins.md Outdated Show resolved Hide resolved

NikeNano reviewed Dec 1, 2021

View reviewed changes

examples/plugins/executor/hello/hello-executor-plugin-configmap.yaml Outdated Show resolved Hide resolved

crenshaw-dev requested changes Dec 2, 2021

View reviewed changes

cmd/argo/commands/plugin/io.go Outdated Show resolved Hide resolved

cmd/argo/commands/plugin/io.go Outdated Show resolved Hide resolved

simster7 reviewed Dec 3, 2021

View reviewed changes

docs/executor_plugins.md Show resolved Hide resolved

docs/executor_plugins.md Show resolved Hide resolved

docs/executor_plugins.md Show resolved Hide resolved

plugins/executor/hello/README.md Show resolved Hide resolved

workflow/executor/agent.go Show resolved Hide resolved

crenshaw-dev requested changes Dec 7, 2021

View reviewed changes

simster7 reviewed Dec 7, 2021

View reviewed changes

alexec force-pushed the eplug branch from 17f5037 to f7adcb7 Compare December 8, 2021 23:36

NikeNano reviewed Dec 10, 2021

View reviewed changes

cmd/argo/commands/executorplugin/io.go Outdated Show resolved Hide resolved

NikeNano reviewed Dec 10, 2021

View reviewed changes

cmd/argo/commands/executorplugin/io.go Outdated Show resolved Hide resolved

sarabala1979 self-assigned this Dec 14, 2021

alexec enabled auto-merge (squash) December 14, 2021 21:08

alexec changed the title ~~feat: Executor plugins.~~ feat: Template executor plugins. Dec 14, 2021

feat: Add template executor plugins. Fixes argoproj#5201

437d24d

Signed-off-by: Alex Collins <alex_collins@intuit.com>

alexec force-pushed the eplug branch from 376a38d to 437d24d Compare January 19, 2022 17:01

alexec changed the title ~~feat: Template executor plugins.~~ feat: Template executor plugins. Fixes #5201 Jan 19, 2022

alexec mentioned this pull request Jan 19, 2022

v3.3 #7589

Closed

28 tasks

alexec added 3 commits January 21, 2022 10:11

Merge branch 'master' into eplug

3310f78

Signed-off-by: Alex Collins <alex_collins@intuit.com>

feat: add ui

0a9b10c

Signed-off-by: Alex Collins <alex_collins@intuit.com>

docs: update

96736c2

Signed-off-by: Alex Collins <alex_collins@intuit.com>

sarabala1979 reviewed Jan 21, 2022

View reviewed changes

sarabala1979 approved these changes Jan 22, 2022

View reviewed changes

alexec merged commit 774bf47 into argoproj:master Jan 22, 2022

terrytangyuan mentioned this pull request Mar 10, 2022

Switch to use the new configmaps informer for retrieving configmaps #7304

Open

alexec deleted the eplug branch May 3, 2022 20:31

agilgur5 added the area/plugins label Oct 8, 2023

agilgur5 added the area/executor label Jan 27, 2024

tooptoop4 mentioned this pull request Feb 19, 2024

fix: only cleanup agent pod if exists #12679

Closed

agilgur5 added the area/agent Argo Agent that runs for HTTP and Plugin templates label Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Template executor plugins. Fixes #5201 #7256

feat: Template executor plugins. Fixes #5201 #7256

alexec commented Nov 20, 2021 •

edited

Loading

alexec commented Dec 1, 2021

terrytangyuan commented Dec 1, 2021 •

edited

Loading

crenshaw-dev Dec 1, 2021

simster7 left a comment

crenshaw-dev commented Dec 6, 2021

alexec commented Dec 7, 2021

alexec commented Dec 7, 2021 •

edited

Loading

crenshaw-dev commented Dec 7, 2021

crenshaw-dev left a comment

alexec commented Dec 7, 2021

crenshaw-dev commented Dec 7, 2021

alexec commented Dec 7, 2021

simster7 left a comment

simster7 Dec 7, 2021

alexec commented Dec 14, 2021

kjagiello commented Dec 21, 2021 •

edited

Loading

alexec commented Jan 18, 2022

sarabala1979 Jan 21, 2022

alexec commented Jan 21, 2022

sarabala1979 commented Jan 21, 2022 •

edited

Loading

alexec commented Jan 22, 2022

sarabala1979 left a comment

feat: Template executor plugins. Fixes #5201 #7256

feat: Template executor plugins. Fixes #5201 #7256

Conversation

alexec commented Nov 20, 2021 • edited Loading

alexec commented Dec 1, 2021

terrytangyuan commented Dec 1, 2021 • edited Loading

crenshaw-dev Dec 1, 2021

Choose a reason for hiding this comment

simster7 left a comment

Choose a reason for hiding this comment

crenshaw-dev commented Dec 6, 2021

alexec commented Dec 7, 2021

alexec commented Dec 7, 2021 • edited Loading

crenshaw-dev commented Dec 7, 2021

crenshaw-dev left a comment

Choose a reason for hiding this comment

alexec commented Dec 7, 2021

crenshaw-dev commented Dec 7, 2021

alexec commented Dec 7, 2021

simster7 left a comment

Choose a reason for hiding this comment

simster7 Dec 7, 2021

Choose a reason for hiding this comment

alexec commented Dec 14, 2021

kjagiello commented Dec 21, 2021 • edited Loading

alexec commented Jan 18, 2022

sarabala1979 Jan 21, 2022

Choose a reason for hiding this comment

alexec commented Jan 21, 2022

sarabala1979 commented Jan 21, 2022 • edited Loading

alexec commented Jan 22, 2022

sarabala1979 left a comment

Choose a reason for hiding this comment

alexec commented Nov 20, 2021 •

edited

Loading

terrytangyuan commented Dec 1, 2021 •

edited

Loading

alexec commented Dec 7, 2021 •

edited

Loading

kjagiello commented Dec 21, 2021 •

edited

Loading

sarabala1979 commented Jan 21, 2022 •

edited

Loading