feat: Add template node to pod name. Fixes #1319 by JPZ13 · Pull Request #6712 · argoproj/argo-workflows

JPZ13 · 2021-09-12T19:26:22Z

This PR is a continuation of Alex's PoC #6647 that fixes #1319. It:

Sets pod names to a concatenation of the wf name, template name, and hash
Allows for an override of the pod name to the node id if the env USE_LEGACY_POD_NAMES is set to true
Stores the node id as an annotation on the pod

Sample terminal output:

$ kubectl get pods -n argo
NAME                                   READY   STATUS      RESTARTS   AGE
minio-786d5cbc44-c9ppg                 1/1     Running     7          19h
retry-test-p7jzr-whalesay-2308805457   0/2     Completed   0          13h
retry-test-p7jzr-whalesay-2378053249   0/2     Completed   0          13h

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

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

codecov · 2021-09-12T19:33:30Z

Codecov Report

Merging #6712 (8119b0b) into master (6d46fd9) will increase coverage by 0.10%.
The diff coverage is 42.99%.

❗ Current head 8119b0b differs from pull request most recent head a78d98b. Consider uploading reports for the commit a78d98b to get more accurate results

@@            Coverage Diff             @@
##           master    #6712      +/-   ##
==========================================
+ Coverage   48.57%   48.67%   +0.10%     
==========================================
  Files         262      264       +2     
  Lines       18991    19080      +89     
==========================================
+ Hits         9224     9287      +63     
- Misses       8739     8750      +11     
- Partials     1028     1043      +15

Impacted Files	Coverage Δ
cmd/argo/commands/client/conn.go	`15.09% <0.00%> (ø)`
cmd/argo/commands/clustertemplate/create.go	`13.72% <0.00%> (ø)`
cmd/argo/commands/clustertemplate/delete.go	`0.00% <0.00%> (ø)`
cmd/argo/commands/clustertemplate/get.go	`0.00% <0.00%> (ø)`
cmd/argo/commands/clustertemplate/lint.go	`0.00% <0.00%> (ø)`
cmd/argo/commands/clustertemplate/list.go	`0.00% <0.00%> (ø)`
cmd/argo/commands/cron/create.go	`0.00% <0.00%> (ø)`
cmd/argo/commands/cron/delete.go	`0.00% <0.00%> (ø)`
cmd/argo/commands/cron/get.go	`23.18% <0.00%> (ø)`
cmd/argo/commands/cron/lint.go	`0.00% <0.00%> (ø)`
... and 58 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d46fd9...a78d98b. Read the comment docs.

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

alexec · 2021-09-13T21:04:26Z

docs/environment-variables.md

 | `RETRY_BACKOFF_STEPS` | `int` | `5` | The retry backoff steps when retrying API calls. |
 | `RETRY_HOST_NAME_LABEL_KEY` | `string` | `kubernetes.io/hostname` | The label key for host name used when retrying templates. |
 | `TRANSIENT_ERROR_PATTERN` | `string` | `""` | The regular expression that represents additional patterns for transient errors. |
+| `USE_LEGACY_POD_NAMES` | `bool` | `false` | Whether to have pod names contain the template name (false) or be the node id (true). |


how about POD_NAMES=v1 default is v2

alexec · 2021-09-13T21:05:30Z

workflow/controller/node_counters_test.go

+	testNodePodExists(t, woc)
+}
+
+func testNodePodExists(t *testing.T, woc *wfOperationCtx) {


should be Test uppercase T

alexec · 2021-09-13T21:06:33Z

workflow/controller/workflowpod.go

 			},
 			Annotations: map[string]string{
 				common.AnnotationKeyNodeName: nodeName,
+				common.AnnotationKeyNodeID:   nodeID, // TODO - test annotation is applied


alexec · 2021-09-13T21:07:02Z

workflow/util/util.go

 }
+
+// PodName return a deterministic pod name
+func PodName(workflowName, nodeName, templateName, nodeID string) string {


please put this func into own file

alexec · 2021-09-13T21:07:17Z

workflow/util/util_test.go

 	}
 }
+
+func TestPodName(t *testing.T) {


alexec

some small changes

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

alexec · 2021-09-13T21:35:37Z

LGTM. Passing to @sarabala1979 for him to review.

alexec · 2021-09-15T18:03:02Z

@sarabala1979 I'm going to assume you're lack of reply means you are not able to review this. I'll merge.

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com> Signed-off-by: kriti-sc <kathuriakriti1@gmail.com>

Signed-off-by: kriti-sc <kathuriakriti1@gmail.com>

vivekthangathurai · 2023-02-26T14:45:07Z

I am using v3.2.8 and not seeing this behaviour.

alexec and others added 7 commits September 1, 2021 11:06

feat: Add template node to pod name. Fixes argoproj#1319

39ac8ee

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

Test pod name prefix length

fb3254d

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Add pod name tests

0d42040

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Remove stale todo

03ae33e

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Refactor pod exists

a662294

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Add nodepodexists test

e6528a0

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Add pod exists test

ea22899

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

JPZ13 added 5 commits September 12, 2021 12:46

Refactor

41d9f09

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Refactor execution control error handling

2ef8817

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Export pod name

59a40ba

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Fix pod deletion for retry

87c2d23

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Add env handling for legacy names

778926d

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

JPZ13 force-pushed the podname-poc branch from 333293c to 778926d Compare September 13, 2021 05:27

JPZ13 added 2 commits September 12, 2021 22:40

Add missing env doc

5ab15a4

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

Add meta id test

f5344fa

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

JPZ13 marked this pull request as ready for review September 13, 2021 15:23

alexec reviewed Sep 13, 2021

View reviewed changes

workflow/util/util_test.go Outdated

}

}

func TestPodName(t *testing.T) {

Copy link

Contributor

alexec Sep 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

own file

alexec requested changes Sep 13, 2021

View reviewed changes

Refactor pod name to separate file

a78d98b

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com>

alexec requested a review from sarabala1979 September 13, 2021 21:35

alexec approved these changes Sep 13, 2021

View reviewed changes

alexec merged commit e5b131a into argoproj:master Sep 15, 2021

tczhao mentioned this pull request Sep 18, 2021

fix(controller): TestPodExists unit test #6763

Merged

sarabala1979 mentioned this pull request Sep 28, 2021

v3.1.13 cherry pick #6820

Closed

36 tasks

micahbeeman-wf mentioned this pull request Oct 14, 2021

fix(controller): Fix getPodByNode, TestGetPodByNode. Fixes #6458 #6897

Merged

tczhao mentioned this pull request Oct 15, 2021

created pod name in argo3.2.0 doesn't align with supplied pod name #6936

Closed

sarabala1979 mentioned this pull request Oct 15, 2021

Workflow executes steps twice with different labels #6874

Closed

tczhao mentioned this pull request Oct 19, 2021

Log viewing problem in UI due pod name changes #6962

Closed

alexec added a commit that referenced this pull request Oct 20, 2021

docs: Mention #6712 in upgrading guide

cc92614

alexec added a commit that referenced this pull request Oct 20, 2021

docs: Mention #6712 in upgrading guide (#6992)

0d43e2e

sarabala1979 mentioned this pull request Oct 21, 2021

v3.2.2 cherry pick #7002

Closed

24 tasks

kriti-sc pushed a commit to kriti-sc/argo-workflows that referenced this pull request Oct 24, 2021

feat: Add template node to pod name. Fixes argoproj#1319 (argoproj#6712)

a493cb3

Signed-off-by: J.P. Zivalich <j.p.zivalich@gmail.com> Signed-off-by: kriti-sc <kathuriakriti1@gmail.com>

kriti-sc pushed a commit to kriti-sc/argo-workflows that referenced this pull request Oct 24, 2021

docs: Mention argoproj#6712 in upgrading guide (argoproj#6992)

458e1cd

Signed-off-by: kriti-sc <kathuriakriti1@gmail.com>

guillaumBrisard mentioned this pull request Nov 25, 2021

POD_NAMES v2 as Default #7279

Closed

alexec mentioned this pull request Dec 9, 2021

workflows stuck because controller is not terminating istio sidecar #7214

Closed

JPZ13 mentioned this pull request Feb 17, 2022

REQUEST: New membership for JPZ13 argoproj/argoproj#113

Closed

5 tasks

mweibel mentioned this pull request Nov 4, 2022

Wrong Pod name in argo get command result from CLI #9906

Closed

3 tasks

crenshaw-dev mentioned this pull request Nov 2, 2023

REQUEST: Promotion to reviewer for JPZ13 argoproj/argoproj#186

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add template node to pod name. Fixes #1319#6712

feat: Add template node to pod name. Fixes #1319#6712
alexec merged 15 commits intoargoproj:masterfrom
JPZ13:podname-poc

JPZ13 commented Sep 12, 2021 •

edited

Loading

Uh oh!

codecov bot commented Sep 12, 2021 •

edited

Loading

Uh oh!

alexec Sep 13, 2021

Uh oh!

alexec Sep 13, 2021

Uh oh!

alexec Sep 13, 2021

Uh oh!

alexec Sep 13, 2021

Uh oh!

alexec Sep 13, 2021

Uh oh!

alexec left a comment

Uh oh!

alexec commented Sep 13, 2021

Uh oh!

alexec commented Sep 15, 2021

Uh oh!

vivekthangathurai commented Feb 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

JPZ13 commented Sep 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alexec Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

alexec Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

alexec Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

alexec Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

alexec Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

alexec left a comment

Choose a reason for hiding this comment

Uh oh!

alexec commented Sep 13, 2021

Uh oh!

alexec commented Sep 15, 2021

Uh oh!

vivekthangathurai commented Feb 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JPZ13 commented Sep 12, 2021 •

edited

Loading

codecov bot commented Sep 12, 2021 •

edited

Loading