feat(controller): Use different container runtime executors for each workflow. Close #4254 #4998

alexec · 2021-02-02T17:54:28Z

Adds containerRuntimeExecutors to workflow-controller-configmap so you can select the executor you want to use.

This is controlled by the operation - users cannot change this setting.

The operator can allow 1 or more executors.

Typically I'd expect this to be used for:

Trying out k8sapi or emissary executors.
Trying out any executor prior to switching the default.
Running some workflows more or less securely.

  containerRuntimeExecutor: docker
  containerRuntimeExecutors: |
    - name: k8sapi
      selector:
        matchLabels:
          workflows.argoproj.io/container-runtime-executor: k8sapi

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

terrytangyuan

This is great to have. We were thinking about something similar along the line as well but distributing based on percentage of the total number of workflows.

terrytangyuan · 2021-02-02T19:36:11Z

config/config.go

+func (c Config) GetContainerRuntimeExecutor(labels labels.Labels) (string, error) {
+	name, err := c.ContainerRuntimeExecutors.Select(labels)
+	if name != "" {
+		return name, err


Looks like err is always nil in this case so return name, nil would be more readable?

I know it is odd looking, but I think it is correct.

this is because it can return "", nil - i.e. it did not find a CRE, but there was not an error

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

alexec · 2021-02-11T18:22:43Z

@sarabala1979 can I request your review as I think @jessesuen is busy

alexec · 2021-02-17T00:42:43Z

@sarabala1979 I accidentally closed this. Can you please review/approve?

sarabala1979 · 2021-02-18T23:49:34Z

config/config.go

@@ -99,6 +101,14 @@ type Config struct {
 	InitialDelay metav1.Duration `json:"initialDelay,omitempty"`
 }

+func (c Config) GetContainerRuntimeExecutor(labels labels.Labels) (string, error) {
+	name, err := c.ContainerRuntimeExecutors.Select(labels)
+	if name != "" {


can we log the error if it is not nil?

I'm going to re-write this confusing code to make it clearer and less "clever"

sarabala1979 · 2021-02-18T23:58:01Z

config/container_runtime_executors.go

+}
+
+type ContainerRuntimeExecutor struct {
+	Name     string               `json:"name"`


the Name is kind of confusing. It should be executor or executorName or executorType

I think that "type" and "name" are synonymous. I.e. there is only one executor of type "pns", it is the "pns" executor.

I'm not sure about the correct term here, but I'd like to leave this called "name" unless you feel strongly?

sarabala1979 · 2021-02-19T16:50:03Z

I think in the future we can have defaultExecuter under ContainerRuntimeExecutors and deprecate ContainerRuntimeExecutor

alexec · 2021-02-19T16:54:40Z

I think in the future we can have defaultExecuter under ContainerRuntimeExecutors and deprecate ContainerRuntimeExecutor

I don't think I should make this change. I'd have to write additional code and deal with what happens when they're both defined, but this does not add any new capabilities but does force migration on our users at some point.

sarabala1979

Above comments minor

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

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

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

alexec · 2021-02-19T17:19:24Z

examples/selected-executor-workflow.yaml

@@ -0,0 +1,35 @@
+apiVersion: argoproj.io/v1alpha1


@sarabala1979 @simster7 this is a fully-filled out example with a test, description, and version.

@jessesuen @terrytangyuan FYI

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

feat(controller): Use different container runtime executors for each …

510cbc0

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

alexec requested review from dtaniwaki, terrytangyuan and Ark-kun February 2, 2021 17:54

terrytangyuan reviewed Feb 2, 2021

View reviewed changes

alexec marked this pull request as ready for review February 2, 2021 20:26

alexec requested review from simster7, sarabala1979 and jessesuen February 2, 2021 22:03

Merge branch 'master' into choose-executor

d60a1e8

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

simster7 assigned jessesuen Feb 3, 2021

Merge branch 'master' into choose-executor

72cbf95

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

terrytangyuan mentioned this pull request Feb 3, 2021

feat(executor): Add user agent to workflow executor #5014

Merged

1 task

alexec linked an issue Feb 3, 2021 that may be closed by this pull request

Per workflow container runtime executor setting #4254

Closed

alexec changed the title ~~feat(controller): Use different container runtime executors for each workflow~~ feat(controller): Use different container runtime executors for each workflow. Close #4254 Feb 3, 2021

alexec closed this Feb 12, 2021

alexec deleted the choose-executor branch February 12, 2021 22:56

alexec restored the choose-executor branch February 17, 2021 00:42

alexec reopened this Feb 17, 2021

alexec unassigned jessesuen Feb 18, 2021

sarabala1979 reviewed Feb 18, 2021

View reviewed changes

alexec assigned sarabala1979 Feb 19, 2021

sarabala1979 approved these changes Feb 19, 2021

View reviewed changes

Merge branch 'master' into choose-executor

dd82cd5

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

alexec added 2 commits February 19, 2021 09:18

feat(controller): Use different container runtime executors for each …

8b0f787

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

Merge branch 'master' into choose-executor

750a37a

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

alexec commented Feb 19, 2021

View reviewed changes

alexec added 2 commits February 19, 2021 09:50

feat(controller): Use different container runtime executors for each …

9d56520

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

feat(controller): Use different container runtime executors for each …

0dba822

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

alexec merged commit 687479f into argoproj:master Feb 19, 2021

alexec deleted the choose-executor branch February 19, 2021 18:48

simster7 mentioned this pull request Feb 23, 2021

Cherry-pick for v2.12.10 #5179

Closed

34 tasks

simster7 mentioned this pull request Mar 8, 2021

v2.12.10 cherry-pick #5326

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(controller): Use different container runtime executors for each workflow. Close #4254 #4998

feat(controller): Use different container runtime executors for each workflow. Close #4254 #4998

alexec commented Feb 2, 2021 •

edited

terrytangyuan left a comment •

edited

terrytangyuan Feb 2, 2021

alexec Feb 5, 2021

alexec Feb 5, 2021

alexec commented Feb 11, 2021

alexec commented Feb 17, 2021

sarabala1979 Feb 18, 2021

alexec Feb 19, 2021

sarabala1979 Feb 18, 2021

alexec Feb 19, 2021

sarabala1979 commented Feb 19, 2021

alexec commented Feb 19, 2021

sarabala1979 left a comment

alexec Feb 19, 2021

alexec Feb 19, 2021

feat(controller): Use different container runtime executors for each workflow. Close #4254 #4998

feat(controller): Use different container runtime executors for each workflow. Close #4254 #4998

Conversation

alexec commented Feb 2, 2021 • edited

terrytangyuan left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexec commented Feb 11, 2021

alexec commented Feb 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarabala1979 commented Feb 19, 2021

alexec commented Feb 19, 2021

sarabala1979 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexec commented Feb 2, 2021 •

edited

terrytangyuan left a comment •

edited