feat: Customize workfow metadata from event data #4783

michaeljohnalbers · 2020-12-19T18:57:23Z

Signed-off-by: Michael Albers michael.john.albers@gmail.com

Checklist:

My organization is added to USERS.md.

Fixes #4640

CLAassistant · 2020-12-19T18:57:28Z

All committers have signed the CLA.

alexec · 2020-12-22T20:07:13Z

pkg/apis/workflow/v1alpha1/event_types.go

@@ -40,6 +40,17 @@ type Submit struct {
 	// WorkflowTemplateRef the workflow template to submit
 	WorkflowTemplateRef WorkflowTemplateRef `json:"workflowTemplateRef" protobuf:"bytes,1,opt,name=workflowTemplateRef"`

+	// WorkflowName optional means to customize the name of the submitted workflow
+	WorkflowName *WorkflowName `json:"workflowName,omitempty" protobuf:"bytes,2,opt,name=workflowName"`


this is great.

We could allow for future changes, e.g. templating labels and annotations.

We could make the names fully based on the expression environment - no suffix.

metadata: name: payload.message + timestamp

Useful as follows:

metadata: name: payload.github.repo + "-" + payload.github.sha1

Maybe in the future:

metadata: annotations: sha1: payload.github.sha1

alexec · 2020-12-22T20:07:48Z

pkg/apis/workflow/v1alpha1/event_types.go

+	Value *AnyString `json:"value,omitempty" protobuf:"bytes,1,opt,name=value"`
+
+	// ValueFrom is the source for the workflow name
+	ValueFrom *ValueFrom `json:"valueFrom,omitempty" protobuf:"bytes,2,opt,name=valueFrom"`


I don't think you need value or valueFrom at all

What should I be using instead? I used those to stay somewhat similar to how parameters are defined.

my comment is irrelevant - I want you to replace workflowName with metadata

alexec · 2020-12-22T20:08:17Z

server/event/dispatch/operation.go

+
+		// Prevent expressions that evaluate to types which will generate invalid names.
+		switch v := result.(type) {
+		case string:


can just return v, nil here

michaeljohnalbers · 2020-12-23T18:15:03Z

@alexec The codegen check is failing. The comment in the PR creation form says to run "make codegen" to fix it. When I run that nothing changes in the git repo. Any ideas on what I can try?

alexec · 2020-12-23T18:56:07Z

Option 1

You can force files to be updated by deleting the change files and running again.
What I do: apply the diffs printed in the job - copy the diff from the build job and run pbpaste|cut -c 30-|git apply

michaeljohnalbers · 2020-12-25T21:43:36Z

@alexec I reworked the change to fix your comments. However, codegen is still failing. It's complaining about the changes I made to the operation_test.go file. Why is that? I assume you'd want unit tests added for this change.

alexec · 2020-12-29T18:03:48Z

You need to apply this patch to fix the codegen issue:

diff --git a/server/event/dispatch/operation_test.go b/server/event/dispatch/operation_test.go
index 1c9703ab..f75bb41d 100644
--- a/server/event/dispatch/operation_test.go
+++ b/server/event/dispatch/operation_test.go
@@ -199,8 +199,8 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "\"my-wfeb-2\"",
-						Labels: map[string]string{"aLabel": "\"someValue\""},
+						Name:        "\"my-wfeb-2\"",
+						Labels:      map[string]string{"aLabel": "\"someValue\""},
 						Annotations: map[string]string{"anAnnotation": "\"otherValue\""},
 					},
 				},
@@ -214,8 +214,8 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "\"my-wfeb-\" + payload.foo.bar",
-						Labels: map[string]string{"aLabel": "payload.list[0]"},
+						Name:        "\"my-wfeb-\" + payload.foo.bar",
+						Labels:      map[string]string{"aLabel": "payload.list[0]"},
 						Annotations: map[string]string{"anAnnotation": "payload.list[1]"},
 					},
 				},
@@ -228,7 +228,7 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Event: wfv1.Event{Selector: "true"},
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
-					ObjectMeta: metav1.ObjectMeta{Name: "payload.......foo[.numeric]"},
+					ObjectMeta:          metav1.ObjectMeta{Name: "payload.......foo[.numeric]"},
 				},
 			},
 		},
@@ -263,7 +263,7 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Event: wfv1.Event{Selector: "true"},
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
-					ObjectMeta: metav1.ObjectMeta{Name: "payload.foo.numeric"},
+					ObjectMeta:          metav1.ObjectMeta{Name: "payload.foo.numeric"},
 				},
 			},
 		},
@@ -274,7 +274,7 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Event: wfv1.Event{Selector: "true"},
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
-					ObjectMeta: metav1.ObjectMeta{Name: "payload.foo.bool"},
+					ObjectMeta:          metav1.ObjectMeta{Name: "payload.foo.bool"},
 				},
 			},
 		},
@@ -285,7 +285,7 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Event: wfv1.Event{Selector: "true"},
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
-					ObjectMeta: metav1.ObjectMeta{Name: "payload.foo"},
+					ObjectMeta:          metav1.ObjectMeta{Name: "payload.foo"},
 				},
 			},
 		},
@@ -296,7 +296,7 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Event: wfv1.Event{Selector: "true"},
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
-					ObjectMeta: metav1.ObjectMeta{Name: "payload.list"},
+					ObjectMeta:          metav1.ObjectMeta{Name: "payload.list"},
 				},
 			},
 		},
@@ -307,12 +307,12 @@ func Test_populateWorkflowMetadata(t *testing.T) {
 				Event: wfv1.Event{Selector: "true"},
 				Submit: &wfv1.Submit{
 					WorkflowTemplateRef: wfv1.WorkflowTemplateRef{Name: "my-wft"},
-					ObjectMeta: metav1.ObjectMeta{Name: "payload.nothing"},
+					ObjectMeta:          metav1.ObjectMeta{Name: "payload.nothing"},
 				},
 			},
 		},
 	}, "my-ns", "my-discriminator",
-	&wfv1.Item{Value: json.RawMessage(`{"foo": {"bar": "baz", "numeric": 8675309, "bool": true}, "list": ["one", "two"]}`)})
+		&wfv1.Item{Value: json.RawMessage(`{"foo": {"bar": "baz", "numeric": 8675309, "bool": true}, "list": ["one", "two"]}`)})
 
 	assert.NoError(t, err)
 	operation.Dispatch()

alexec

Looks great - thank you for going above-and-beyond with also implementing labels and annotations - I wasn't expecting this!

alexec · 2020-12-29T18:04:41Z

pkg/apis/workflow/v1alpha1/event_types.go

 	// Arguments extracted from the event and then set as arguments to the workflow created.
-	Arguments *Arguments `json:"arguments,omitempty" protobuf:"bytes,2,opt,name=arguments"`
+	Arguments *Arguments `json:"arguments,omitempty" protobuf:"bytes,3,opt,name=arguments"`


it is important not to change these numbers as it breaks clients - can you leave this as 3 and make metadata 2?

I knew I shouldn't have done that. Fixed.

alexec · 2020-12-29T18:05:02Z

server/event/dispatch/operation.go

+			return nil, err
+		}
+
+		if len(wf.Name) == 0 {


minor - I'd prefer wf.Name == ""

alexec · 2020-12-29T18:05:24Z

server/event/dispatch/operation.go

+		if err != nil {
+			return err
+		}
+		wf.Labels[labelKey] = evalLabel


I think you might need to check if wf.Labels == nil here

Fixed. Did the same for annotations.

alexec · 2020-12-29T18:07:04Z

server/event/dispatch/operation.go

+		return "", fmt.Errorf("failed to evaluate workflow %s expression: %w", errorInfo, err)
+	}
+
+	switch v := result.(type) {


minor - v, ok := result.(string) is a more conventional way of doing this

Fixed. Thanks for showing me this.

Signed-off-by: Michael Albers <michael.john.albers@gmail.com>

alexec

LGTM

paveq · 2021-02-23T09:58:38Z

I wonder if this supports getting name from metadata (HTTP headers), I tried following but got errors in argo-server:

  submit:
    metadata:
      name: 'metadata["x-event-id"] + "-test"'

michaeljohnalbers mentioned this pull request Dec 19, 2020

Allow custom name in workflow started from event #4640

Closed

alexec reviewed Dec 22, 2020

View reviewed changes

michaeljohnalbers requested a review from jessesuen as a code owner December 25, 2020 20:31

michaeljohnalbers changed the title ~~Set workflow name from event binding~~ feat(controller) Customize workfow metadata from event data Dec 25, 2020

michaeljohnalbers changed the title ~~feat(controller) Customize workfow metadata from event data~~ feat: Customize workfow metadata from event data Dec 25, 2020

alexec requested changes Dec 29, 2020

View reviewed changes

michaeljohnalbers added 3 commits December 30, 2020 18:24

feat: Customize workflow metadata from event data

55fd8c8

Signed-off-by: Michael Albers <michael.john.albers@gmail.com>

(feat) fixes for PR comments

45822d9

Signed-off-by: Michael Albers <michael.john.albers@gmail.com>

(feat) fix style issue

6bb3023

Signed-off-by: Michael Albers <michael.john.albers@gmail.com>

alexec approved these changes Dec 31, 2020

View reviewed changes

Merge branch 'master' into CustomNamesFromEvents

c7b4cee

alexec merged commit b73bd2b into argoproj:master Jan 2, 2021

alexec added this to the v3.0 milestone Jan 2, 2021

simster7 mentioned this pull request Jan 4, 2021

Cherry pick for v2.12.3 #4826

Closed

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Customize workfow metadata from event data #4783

feat: Customize workfow metadata from event data #4783

michaeljohnalbers commented Dec 19, 2020

CLAassistant commented Dec 19, 2020 •

edited

alexec Dec 22, 2020

alexec Dec 22, 2020

michaeljohnalbers Dec 23, 2020

alexec Dec 23, 2020

alexec Dec 22, 2020

michaeljohnalbers commented Dec 23, 2020

alexec commented Dec 23, 2020 •

edited

michaeljohnalbers commented Dec 25, 2020 •

edited

alexec commented Dec 29, 2020

alexec left a comment

alexec Dec 29, 2020

michaeljohnalbers Dec 31, 2020

alexec Dec 29, 2020

michaeljohnalbers Dec 31, 2020

alexec Dec 29, 2020

michaeljohnalbers Dec 31, 2020

alexec Dec 29, 2020

michaeljohnalbers Dec 31, 2020

alexec left a comment

paveq commented Feb 23, 2021

feat: Customize workfow metadata from event data #4783

feat: Customize workfow metadata from event data #4783

Conversation

michaeljohnalbers commented Dec 19, 2020

CLAassistant commented Dec 19, 2020 • 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

Choose a reason for hiding this comment

michaeljohnalbers commented Dec 23, 2020

alexec commented Dec 23, 2020 • edited

michaeljohnalbers commented Dec 25, 2020 • edited

alexec commented Dec 29, 2020

alexec left a comment

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

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

Choose a reason for hiding this comment

alexec left a comment

Choose a reason for hiding this comment

paveq commented Feb 23, 2021

CLAassistant commented Dec 19, 2020 •

edited

alexec commented Dec 23, 2020 •

edited

michaeljohnalbers commented Dec 25, 2020 •

edited