Repository: argoproj/argo
Pull request #3477 (2020-07-14 21:21:15)

fix: Pass resolved arguments to onExit handler
Fixes: https://github.com/argoproj/argo/issues/3471


Patch:
diff --git a/cmd/argo/commands/list.go b/cmd/argo/commands/list.go
index 3fdf9bd61..4f43ba676 100644
--- a/cmd/argo/commands/list.go
+++ b/cmd/argo/commands/list.go
@@ -26,6 +26,7 @@ type listFlags struct {
 	status        []string
 	completed     bool
 	running       bool
+	resubmitted   bool
 	prefix        string
 	output        string
 	createdSince  string
@@ -66,6 +67,7 @@ func NewListCommand() *cobra.Command {
 	command.Flags().StringSliceVar(&listArgs.status, "status", []string{}, "Filter by status (comma separated)")
 	command.Flags().BoolVar(&listArgs.completed, "completed", false, "Show only completed workflows")
 	command.Flags().BoolVar(&listArgs.running, "running", false, "Show only running workflows")
+	command.Flags().BoolVar(&listArgs.resubmitted, "resubmitted", false, "Show only resubmitted workflows")
 	command.Flags().StringVarP(&listArgs.output, "output", "o", "", "Output format. One of: wide|name")
 	command.Flags().StringVar(&listArgs.createdSince, "since", "", "Show only workflows created after than a relative duration")
 	command.Flags().Int64VarP(&listArgs.chunkSize, "chunk-size", "", 0, "Return large lists in chunks rather than all at once. Pass 0 to disable.")
@@ -95,6 +97,10 @@ func listWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic
 		req, _ := labels.NewRequirement(common.LabelKeyCompleted, selection.NotEquals, []string{"true"})
 		labelSelector = labelSelector.Add(*req)
 	}
+	if flags.resubmitted {
+		req, _ := labels.NewRequirement(common.LabelKeyPreviousWorkflowName, selection.Exists, []string{})
+		labelSelector = labelSelector.Add(*req)
+	}
 	listOpts.LabelSelector = labelSelector.String()
 	listOpts.FieldSelector = flags.fields
 	var workflows wfv1.Workflows
diff --git a/docs/metrics.md b/docs/metrics.md
index db7a15475..ddaf192c7 100644
--- a/docs/metrics.md
+++ b/docs/metrics.md
@@ -6,6 +6,9 @@
 
 ## Introduction
 
+Argo emits a certain number of controller metrics that inform on the state of the controller at any given time. Furthermore,
+users can also define their own custom metrics to inform on the state of their Workflows.
+
 Custom prometheus metrics can be defined to be emitted on a `Workflow`- and `Template`-level basis. These can be useful
 for many cases; some examples:
 
@@ -18,7 +21,12 @@ best way to define metrics in Argo to avoid problems such as [cardinality explos
 
 ## Metrics and metrics in Argo
 
-Emitting metrics is the responsibility of the emitter owner. Since the user defines Workflows in Argo, the user is responsible
+There are two kinds of metrics emitted by Argo: **controller metrics** and **custom metrics**.
+
+* Controller metrics are metrics that inform on the state of the controller; i.e., they answer the question "What is the state of the controller right now?".
+* Custom metrics are metrics that inform on the state of a Workflow, or a series of Workflows. These custom metrics are defined by the user in the Workflow spec.
+
+Emitting custom metrics is the responsibility of the emitter owner. Since the user defines Workflows in Argo, the user is responsible
 for emitting metrics correctly.
 
 ### What is and isn't a Prometheus metric
@@ -46,6 +54,17 @@ at the current time, they should not be used to report historical data such as:
 Metrics are also ephemeral, meaning there is no guarantee that they will be persisted for any amount of time. If you need
 a way to view and analyze historical data, consider the [workflow archive](workflow-archive.md) or reporting to logs.
 
+### Default Controller Metrics
+
+There are several controller-level metrics. These include:
+
+* `workflows_processed_count`: a count of all Workflow updates processed by the controller
+* `count`: a count of all workflows currently accessible by the controller by status
+* `operation_duration_seconds`: a histogram of durations of operations
+* `error_count`: a count of certain errors incurred by the controller
+* `queue_depth_count`: the depth of the queue of workflows or cron workflows to be processed by the controller
+* `queue_adds_count`: the number of adds to the queue of workflows or cron workflows
+* `queue_latency`: the time workflows or cron workflows spend in the queue waiting to be processed
 
 ### Metric types
 
diff --git a/workflow/common/util.go b/workflow/common/util.go
index 6998dc3ca..44089adb0 100644
--- a/workflow/common/util.go
+++ b/workflow/common/util.go
@@ -368,6 +368,16 @@ func Replace(fstTmpl *fasttemplate.Template, replaceMap map[string]string, allow
 	replacedTmpl := fstTmpl.ExecuteFuncString(func(w io.Writer, tag string) (int, error) {
 		replacement, ok := replaceMap[tag]
 		if !ok {
+			// Attempt to resolve nested tags, if possible
+			if index := strings.LastIndex(tag, "{{"); index > 0 {
+				nestedTagPrefix := tag[:index]
+				nestedTag := tag[index+2:]
+				if replacement, ok := replaceMap[nestedTag]; ok {
+					replacement = strconv.Quote(replacement)
+					replacement = replacement[1 : len(replacement)-1]
+					return w.Write([]byte("{{" + nestedTagPrefix + replacement))
+				}
+			}
 			if allowUnresolved {
 				// just write the same string back
 				return w.Write([]byte(fmt.Sprintf("{{%s}}", tag)))
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index 2226b4363..5888c0760 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -343,7 +343,7 @@ func (woc *wfOperationCtx) operate() {
 
 		woc.log.Infof("Running OnExit handler: %s", woc.wfSpec.OnExit)
 		onExitNodeName := common.GenerateOnExitNodeName(woc.wf.ObjectMeta.Name)
-		onExitNode, err = woc.executeTemplate(onExitNodeName, &wfv1.WorkflowStep{Template: woc.wfSpec.OnExit}, tmplCtx, woc.wfSpec.Arguments, &executeTemplateOpts{onExitTemplate: true})
+		onExitNode, err = woc.executeTemplate(onExitNodeName, &wfv1.WorkflowStep{Template: woc.wfSpec.OnExit}, tmplCtx, execArgs, &executeTemplateOpts{onExitTemplate: true})
 		if err != nil {
 			// the error are handled in the callee so just log it.
 			woc.log.WithError(err).Error("error in exit template execution")


Test patch:
diff --git a/cmd/argo/commands/list_test.go b/cmd/argo/commands/list_test.go
index dfb87bb17..936279606 100644
--- a/cmd/argo/commands/list_test.go
+++ b/cmd/argo/commands/list_test.go
@@ -5,6 +5,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/argoproj/argo/workflow/common"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -39,6 +41,12 @@ func Test_listWorkflows(t *testing.T) {
 			assert.NotNil(t, workflows)
 		}
 	})
+	t.Run("Resubmitted", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{LabelSelector: common.LabelKeyPreviousWorkflowName}, listFlags{resubmitted: true})
+		if assert.NoError(t, err) {
+			assert.NotNil(t, workflows)
+		}
+	})
 	t.Run("Labels", func(t *testing.T) {
 		workflows, err := list(&metav1.ListOptions{LabelSelector: "foo"}, listFlags{labels: "foo"})
 		if assert.NoError(t, err) {
@@ -70,6 +78,11 @@ func list(listOptions *metav1.ListOptions, flags listFlags) (wfv1.Workflows, err
 	c.On("ListWorkflows", mock.Anything, &workflow.WorkflowListRequest{ListOptions: listOptions}).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{
 		{ObjectMeta: metav1.ObjectMeta{Name: "foo-", CreationTimestamp: metav1.Time{Time: time.Now().Add(-2 * time.Hour)}}, Status: wfv1.WorkflowStatus{FinishedAt: metav1.Time{Time: time.Now().Add(-2 * time.Hour)}}},
 		{ObjectMeta: metav1.ObjectMeta{Name: "bar-", CreationTimestamp: metav1.Time{Time: time.Now()}}},
+		{ObjectMeta: metav1.ObjectMeta{
+			Name:              "baz-",
+			CreationTimestamp: metav1.Time{Time: time.Now().Add(-2 * time.Hour)},
+			Labels:            map[string]string{common.LabelKeyPreviousWorkflowName: "foo-"},
+		}},
 	}}, nil)
 	workflows, err := listWorkflows(context.Background(), c, flags)
 	return workflows, err
diff --git a/workflow/common/util_test.go b/workflow/common/util_test.go
index f5b46fb36..0b66a00c7 100644
--- a/workflow/common/util_test.go
+++ b/workflow/common/util_test.go
@@ -3,6 +3,8 @@ package common
 import (
 	"testing"
 
+	"github.com/valyala/fasttemplate"
+
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -84,3 +86,74 @@ func TestDeletePod(t *testing.T) {
 		assert.NoError(t, err)
 	})
 }
+
+func TestNestedReplaceString(t *testing.T) {
+
+	replaceMap := map[string]string{"inputs.parameters.message": "hello world"}
+
+	test := `{{- with secret "{{inputs.parameters.message}}" -}}
+    {{ .Data.data.gitcreds }}
+  {{- end }}`
+	fstTmpl, err := fasttemplate.NewTemplate(test, "{{", "}}")
+	if assert.NoError(t, err) {
+		replacement, err := Replace(fstTmpl, replaceMap, true)
+		if assert.NoError(t, err) {
+			assert.Equal(t, "{{- with secret \"hello world\" -}}\n    {{ .Data.data.gitcreds }}\n  {{- end }}", replacement)
+		}
+	}
+
+	test = `{{- with {{ secret "{{inputs.parameters.message}}" -}}
+    {{ .Data.data.gitcreds }}
+  {{- end }}`
+	fstTmpl, err = fasttemplate.NewTemplate(test, "{{", "}}")
+	if assert.NoError(t, err) {
+		replacement, err := Replace(fstTmpl, replaceMap, true)
+		if assert.NoError(t, err) {
+			assert.Equal(t, "{{- with {{ secret \"hello world\" -}}\n    {{ .Data.data.gitcreds }}\n  {{- end }}", replacement)
+		}
+	}
+
+	test = `{{- with {{ secret "{{inputs.parameters.message}}" -}} }}
+    {{ .Data.data.gitcreds }}
+  {{- end }}`
+	fstTmpl, err = fasttemplate.NewTemplate(test, "{{", "}}")
+	if assert.NoError(t, err) {
+		replacement, err := Replace(fstTmpl, replaceMap, true)
+		if assert.NoError(t, err) {
+			assert.Equal(t, "{{- with {{ secret \"hello world\" -}} }}\n    {{ .Data.data.gitcreds }}\n  {{- end }}", replacement)
+		}
+	}
+
+	test = `{{- with secret "{{inputs.parameters.message}}" -}} }}
+    {{ .Data.data.gitcreds }}
+  {{- end }}`
+	fstTmpl, err = fasttemplate.NewTemplate(test, "{{", "}}")
+	if assert.NoError(t, err) {
+		replacement, err := Replace(fstTmpl, replaceMap, true)
+		if assert.NoError(t, err) {
+			assert.Equal(t, "{{- with secret \"hello world\" -}} }}\n    {{ .Data.data.gitcreds }}\n  {{- end }}", replacement)
+		}
+	}
+
+	test = `{{- with {{ {{ }} secret "{{inputs.parameters.message}}" -}} }}
+    {{ .Data.data.gitcreds }}
+  {{- end }}`
+	fstTmpl, err = fasttemplate.NewTemplate(test, "{{", "}}")
+	if assert.NoError(t, err) {
+		replacement, err := Replace(fstTmpl, replaceMap, true)
+		if assert.NoError(t, err) {
+			assert.Equal(t, "{{- with {{ {{ }} secret \"hello world\" -}} }}\n    {{ .Data.data.gitcreds }}\n  {{- end }}", replacement)
+		}
+	}
+
+	test = `{{- with {{ {{ }} secret "{{does-not-exist}}" -}} }}
+    {{ .Data.data.gitcreds }}
+  {{- end }}`
+	fstTmpl, err = fasttemplate.NewTemplate(test, "{{", "}}")
+	if assert.NoError(t, err) {
+		replacement, err := Replace(fstTmpl, replaceMap, true)
+		if assert.NoError(t, err) {
+			assert.Equal(t, test, replacement)
+		}
+	}
+}
diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
index 6a95b2d0b..133a72f71 100644
--- a/workflow/controller/operator_test.go
+++ b/workflow/controller/operator_test.go
@@ -4020,3 +4020,164 @@ status:
 		}
 	}
 }
+
+var globalVarsOnExit = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-world-6gphm-8n22g
+  namespace: default
+spec:
+  arguments:
+    parameters:
+    - name: message
+      value: nononono
+  workflowTemplateRef:
+    name: hello-world-6gphm
+status:
+  nodes:
+    hello-world-6gphm-8n22g:
+      displayName: hello-world-6gphm-8n22g
+      finishedAt: "2020-07-14T20:45:28Z"
+      hostNodeName: minikube
+      id: hello-world-6gphm-8n22g
+      inputs:
+        parameters:
+        - name: message
+          value: nononono
+      name: hello-world-6gphm-8n22g
+      outputs:
+        artifacts:
+        - archiveLogs: true
+          name: main-logs
+          s3:
+            accessKeySecret:
+              key: accesskey
+              name: my-minio-cred
+            bucket: my-bucket
+            endpoint: minio:9000
+            insecure: true
+            key: hello-world-6gphm-8n22g/hello-world-6gphm-8n22g/main.log
+            secretKeySecret:
+              key: secretkey
+              name: my-minio-cred
+        exitCode: "0"
+      phase: Succeeded
+      resourcesDuration:
+        cpu: 2
+        memory: 1
+      startedAt: "2020-07-14T20:45:25Z"
+      templateRef:
+        name: hello-world-6gphm
+        template: whalesay
+      templateScope: local/hello-world-6gphm-8n22g
+      type: Pod
+  phase: Running
+  resourcesDuration:
+    cpu: 5
+    memory: 2
+  startedAt: "2020-07-14T20:45:25Z"
+  storedTemplates:
+    namespaced/hello-world-6gphm/whalesay:
+      arguments: {}
+      container:
+        args:
+        - hello {{inputs.parameters.message}}
+        command:
+        - cowsay
+        image: docker/whalesay:latest
+        name: ""
+        resources: {}
+      inputs:
+        parameters:
+        - name: message
+      metadata: {}
+      name: whalesay
+      outputs: {}
+  storedWorkflowTemplateSpec:
+    arguments:
+      parameters:
+      - name: message
+        value: default
+    entrypoint: whalesay
+    onExit: exitContainer
+    templates:
+    - arguments: {}
+      container:
+        args:
+        - hello {{inputs.parameters.message}}
+        command:
+        - cowsay
+        image: docker/whalesay:latest
+        name: ""
+        resources: {}
+      inputs:
+        parameters:
+        - name: message
+      metadata: {}
+      name: whalesay
+      outputs: {}
+    - arguments: {}
+      container:
+        args:
+        - goodbye {{inputs.parameters.message}}
+        command:
+        - cowsay
+        image: docker/whalesay
+        name: ""
+        resources: {}
+      inputs:
+        parameters:
+        - name: message
+      metadata: {}
+      name: exitContainer
+      outputs: {}
+
+`
+
+var wftmplGlobalVarsOnExit = `
+apiVersion: argoproj.io/v1alpha1
+kind: WorkflowTemplate
+metadata:
+  name: hello-world-6gphm
+  namespace: default
+spec:
+  entrypoint: whalesay
+  onExit: exitContainer
+  arguments:
+    parameters:
+    - name: message
+      value: "default"
+  templates:
+  - name: whalesay
+    inputs:
+      parameters:
+      - name: message
+    container:
+      image: docker/whalesay:latest
+      command: [cowsay]
+      args: ["hello {{inputs.parameters.message}}"]
+  - name: exitContainer
+    inputs:
+      parameters:
+      - name: message
+    container:
+      image: docker/whalesay
+      command: [cowsay]
+      args: ["goodbye {{inputs.parameters.message}}"]
+`
+
+func TestGlobalVarsOnExit(t *testing.T) {
+	wf := unmarshalWF(globalVarsOnExit)
+	wftmpl := unmarshalWFTmpl(wftmplGlobalVarsOnExit)
+	cancel, controller := newController(wf, wftmpl)
+	defer cancel()
+	woc := newWorkflowOperationCtx(wf, controller)
+
+	woc.operate()
+
+	node := woc.wf.Status.Nodes["hello-world-6gphm-8n22g-3224262006"]
+	if assert.NotNil(t, node) && assert.NotNil(t, node.Inputs) && assert.NotEmpty(t, node.Inputs.Parameters) {
+		assert.Equal(t, "nononono", node.Inputs.Parameters[0].Value.StrVal)
+	}
+}