feat(controller): Add podMetadata field to workflow spec. Resolves #4985 #5031

dylanhellems · 2021-02-04T22:24:11Z

Closes #4985

Checklist:

My organization is added to USERS.md.

CLAassistant · 2021-02-04T22:24:16Z

All committers have signed the CLA.

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

alexec · 2021-02-05T01:07:35Z

workflow/controller/workflowpod.go

@@ -615,6 +615,14 @@ func isResourcesSpecified(ctr *apiv1.Container) bool {

 // addMetadata applies metadata specified in the template
 func (woc *wfOperationCtx) addMetadata(pod *apiv1.Pod, tmpl *wfv1.Template, opts *createWorkflowPodOpts) {
+	// inherit workflow annotations and labels
+	for k, v := range woc.wf.ObjectMeta.Annotations {


I'm not sure we just want to copy the workflows annotations and labels to the pods. Instead, perhaps a new spec.podMetadata field might be better - @jessesuen ?

terrytangyuan · 2021-02-05T01:11:39Z

workflow/controller/workflowpod.go

@@ -615,6 +615,14 @@ func isResourcesSpecified(ctr *apiv1.Container) bool {

 // addMetadata applies metadata specified in the template
 func (woc *wfOperationCtx) addMetadata(pod *apiv1.Pod, tmpl *wfv1.Template, opts *createWorkflowPodOpts) {
+	// inherit workflow annotations and labels
+	for k, v := range woc.wf.ObjectMeta.Annotations {
+		pod.ObjectMeta.Annotations[k] = v


I agree that we probably don't want to inherit directly as we don't know for sure whether to apply the same set of annotations/labels. Perhaps we could add something similar to podSpecPatch, e.g. podMetadataPatch.

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

dylanhellems · 2021-02-05T17:29:02Z

@alexec @terrytangyuan Makes sense; I've added a podMetadata field to the workflow spec. I have a couple of questions though:

~~I had to update the maxWorkflowSize in the packer and hydrator tests. Is that an arbitrary value for testing or are there implications to that change that I'm unaware of?~~ Was due to not using a pointer
~~I'm having trouble getting codegen working locally. Any thoughts on what may be going wrong? Here is the output I'm seeing:~~ see: feat(controller): Add podMetadata field to workflow spec. Resolves #4985 #5031 (comment)

dylanhellems@Dylans-MBP argo % make codegen                                                        
# protoc pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto
[ -e vendor ] || go mod vendor
protoc -I /usr/local/include -I /Users/dylanhellems/Documents/repos/argo -I /Users/dylanhellems/Documents/repos/argo/vendor -I /Users/dylanhellems/go/src -I /Users/dylanhellems/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/gogoproto -I /Users/dylanhellems/go/pkg/mod/github.com/grpc-ecosystem/grpc-gateway@v1.16.0/third_party/googleapis --gogofast_out=plugins=grpc:/Users/dylanhellems/go/src --grpc-gateway_out=logtostderr=true:/Users/dylanhellems/go/src --swagger_out=logtostderr=true,fqn_for_swagger_name=true:. pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto
github.com/argoproj/argo/pkg/apis/workflow/v1alpha1/generated.proto: File not found.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:6:1: Import "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1/generated.proto" was not found or had errors.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:15:5: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplate" is not defined.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:31:5: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplate" is not defined.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:41:5: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplate" is not defined.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:46:87: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplate" is not defined.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:53:81: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplate" is not defined.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:57:84: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplateList" is not defined.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:61:87: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplate" is not defined.
pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.proto:72:83: "github.com.argoproj.argo.v3.pkg.apis.workflow.v1alpha1.ClusterWorkflowTemplate" is not defined.
make: *** [pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.swagger.json] Error 1

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

alexec · 2021-02-05T17:49:56Z

pkg/apis/workflow/v1alpha1/workflow_types.go

@@ -351,6 +351,9 @@ type WorkflowSpec struct {

 	// RetryStrategy for all templates in the workflow.
 	RetryStrategy *RetryStrategy `json:"retryStrategy,omitempty" protobuf:"bytes,37,opt,name=retryStrategy"`
+
+	// PodMetadata defines additional metadata that should be applied to workflow pods
+	PodMetadata Metadata `json:"podMetadata,omitempty" protobuf:"bytes,38,opt,name=podMetadata"`


should be pointer type

dylanhellems · 2021-02-05T17:55:04Z

Codegen issue above was because I wasn't working within the gopath. Working now.

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

dylanhellems · 2021-02-08T15:01:04Z

@alexec This should be ready for another look

alexec · 2021-02-10T19:02:52Z

Please sync with master.

alexec

Please resolve conflicts.

…-annotations

dylanhellems · 2021-02-10T21:53:36Z

@alexec Conflicts resolved

alexec · 2021-02-10T22:22:11Z

LGTM

feat: Pods inherit workflow annotations and labels

19e7d6a

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

dylanhellems marked this pull request as ready for review February 4, 2021 22:41

dylanhellems mentioned this pull request Feb 4, 2021

Default annotations for workflow pods #4985

Closed

alexec changed the title ~~feat: Pods inherit workflow annotations and labels~~ feat: Pods inherit workflow annotations and labels. Resolves #4985 Feb 5, 2021

alexec reviewed Feb 5, 2021

View reviewed changes

terrytangyuan reviewed Feb 5, 2021

View reviewed changes

feat: add podMetadata field to wf spec

7eefbc9

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

dylanhellems changed the title ~~feat: Pods inherit workflow annotations and labels. Resolves #4985~~ feat: Add podMetadata field to workflow spec. Resolves #4985 Feb 5, 2021

Merge branch 'master' into pods-inherit-workflow-annotations

cde9b37

dylanhellems requested review from alexec and terrytangyuan February 5, 2021 17:32

feat: add example

c73dfdd

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

alexec reviewed Feb 5, 2021

View reviewed changes

fix: make podMetadata field a pointer + codegen

bc3c432

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

dylanhellems requested a review from jessesuen as a code owner February 5, 2021 18:02

dylanhellems added 3 commits February 5, 2021 13:23

fix: nil reference in tests

6e95ce9

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

fix: more codegen

8b90998

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

fix: rest of codegen

5f3e154

Signed-off-by: Dylan Hellems <dylanhellems@gmail.com>

dylanhellems requested a review from alexec February 5, 2021 19:45

Merge branch 'master' into pods-inherit-workflow-annotations

841ad84

dylanhellems added 2 commits February 8, 2021 13:28

Merge branch 'master' into pods-inherit-workflow-annotations

c2b9375

Merge branch 'master' into pods-inherit-workflow-annotations

d51b1c8

alexec assigned jessesuen Feb 8, 2021

jessesuen approved these changes Feb 10, 2021

View reviewed changes

jessesuen removed their assignment Feb 10, 2021

alexec changed the title ~~feat: Add podMetadata field to workflow spec. Resolves #4985~~ feat(controller): Add podMetadata field to workflow spec. Resolves #4985 Feb 10, 2021

alexec requested changes Feb 10, 2021

View reviewed changes

Merge remote-tracking branch 'argo/master' into pods-inherit-workflow…

cd7486e

…-annotations

dylanhellems requested a review from alexec February 10, 2021 21:52

Merge branch 'master' into pods-inherit-workflow-annotations

236f5d6

Merge branch 'master' into pods-inherit-workflow-annotations

18eb99b

alexec approved these changes Feb 10, 2021

View reviewed changes

alexec merged commit f461b04 into argoproj:master Feb 10, 2021

simster7 mentioned this pull request Feb 16, 2021

v2.12.9 cherry-pick #5119

Closed

33 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(controller): Add podMetadata field to workflow spec. Resolves #4985 #5031

feat(controller): Add podMetadata field to workflow spec. Resolves #4985 #5031

dylanhellems commented Feb 4, 2021 •

edited

Loading

CLAassistant commented Feb 4, 2021 •

edited

Loading

alexec Feb 5, 2021

terrytangyuan Feb 5, 2021 •

edited

Loading

dylanhellems commented Feb 5, 2021 •

edited

Loading

alexec Feb 5, 2021

dylanhellems Feb 5, 2021

dylanhellems commented Feb 5, 2021

dylanhellems commented Feb 8, 2021

alexec commented Feb 10, 2021

alexec left a comment

dylanhellems commented Feb 10, 2021

alexec commented Feb 10, 2021

feat(controller): Add podMetadata field to workflow spec. Resolves #4985 #5031

feat(controller): Add podMetadata field to workflow spec. Resolves #4985 #5031

Conversation

dylanhellems commented Feb 4, 2021 • edited Loading

CLAassistant commented Feb 4, 2021 • edited Loading

alexec Feb 5, 2021

Choose a reason for hiding this comment

terrytangyuan Feb 5, 2021 • edited Loading

Choose a reason for hiding this comment

dylanhellems commented Feb 5, 2021 • edited Loading

alexec Feb 5, 2021

Choose a reason for hiding this comment

dylanhellems Feb 5, 2021

Choose a reason for hiding this comment

dylanhellems commented Feb 5, 2021

dylanhellems commented Feb 8, 2021

alexec commented Feb 10, 2021

alexec left a comment

Choose a reason for hiding this comment

dylanhellems commented Feb 10, 2021

alexec commented Feb 10, 2021

dylanhellems commented Feb 4, 2021 •

edited

Loading

CLAassistant commented Feb 4, 2021 •

edited

Loading

terrytangyuan Feb 5, 2021 •

edited

Loading

dylanhellems commented Feb 5, 2021 •

edited

Loading