Skip to content

WIP: Add Task names to Pod names#1320

Closed
ian-howell wants to merge 1 commit intoargoproj:masterfrom
ian-howell:feat/task-names-in-pod-names
Closed

WIP: Add Task names to Pod names#1320
ian-howell wants to merge 1 commit intoargoproj:masterfrom
ian-howell:feat/task-names-in-pod-names

Conversation

@ian-howell
Copy link
Contributor

  • This proposal adds the name of each task to the name of the pod it is
    associated with. This makes it easier to keep track of which pod is
    performing which task.

Still a work in progress - unit tests are failing, and there's probably a better way to deal with parentheses.

This would close #1319 if it is accepted.

Let me know what you think :)

@sruon
Copy link

sruon commented Apr 13, 2019

Probably need to check the size of the resulting name to ensure it is below 253 characters as well.

@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch from 149c797 to a43fb44 Compare April 16, 2019 20:36
@ian-howell
Copy link
Contributor Author

Good call. Name length is fixed in the last push.

@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch 2 times, most recently from 18eacca to de9d989 Compare April 16, 2019 21:15
@ian-howell
Copy link
Contributor Author

Here's a before/after example of this feature:

Before

$ kubectl apply -f steps.yaml
NAME               READY     STATUS      RESTARTS   AGE
steps-1781811180   0/2       Completed   0          7s
steps-1832144037   0/2       Completed   0          7s
steps-215716891    0/2       Completed   0          11s

After

$ kubectl apply -f steps.yaml
NAME              READY     STATUS      RESTARTS   AGE
steps-0.hello1    0/2       Completed   0          12s
steps-1.hello2a   0/2       Completed   0          9s
steps-1.hello2b   0/2       Completed   0          9s

One notable side effect of this change is that the pods are always listed in step-order.

@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch from de9d989 to 3690cc9 Compare April 17, 2019 13:31
@AudriusButkevicius
Copy link

What is this blocked on? I think this is a great addition.
Furthermore, it would be nice to expose parameters in the workflow name, if you have parameterized workflows.

@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch from 3690cc9 to aacd99c Compare July 25, 2019 14:51
@ian-howell
Copy link
Contributor Author

Hey @AudriusButkevicius, I honestly can't recall why I stopped working on this - it's definitely a more involved change than I at first thought it would be. I've rebased my branch, and I'll take another swing at it as soon as I find some time to do so.

@AudriusButkevicius
Copy link

I am happy to help out if its clear what the remaining work is, but it seems PRs on this project aren't getting that much attention.

@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch 2 times, most recently from 8652897 to 330573b Compare July 26, 2019 20:50
@ian-howell
Copy link
Contributor Author

@jessesuen @sarabala1979 I believe this is ready to review

@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch from 330573b to 673aa0d Compare July 30, 2019 17:30
@sarabala1979
Copy link
Member

Can you keep the same naming convention? can you change the . to - in step name
steps-5l6mg-0.hello1-1579193971 to steps-5l6mg-0-hello1-1579193971

@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch from 673aa0d to 05e6562 Compare July 31, 2019 13:35
@ian-howell
Copy link
Contributor Author

@sarabala1979 done.

@ian-howell
Copy link
Contributor Author

@sarabala1979 could you take another look at this when you get a moment?

@sarabala1979
Copy link
Member

LGTM. Resolve the conflicts

This changes the way that Argo names pods. The new behavior will now
create a display the parent-child relationships between nodes in the
Workflow. For example, if Workflow dag-nested has a DAG named "a", which
in turn has a sub-DAG named "b", the resultant Pod will be named
"dag-nested-a-b-XXXXXXXXXX" where the last section is a unique hash.

The argo CLI tool's "get" and "logs" commands have also been updated to
reflect the new behavior.
@ian-howell ian-howell force-pushed the feat/task-names-in-pod-names branch from 05e6562 to 36116a8 Compare September 5, 2019 13:49
@ian-howell
Copy link
Contributor Author

@sarabala1979 Conflicts are resolved.

Here's the message from the gate failure:

golangci-lint run --config golangci.yml                                                                                                                                                    
level=error msg="[runner] 0/12 linters finished: deadline exceeded"                                                                                                                        
level=error msg="Deadline exceeded: try increase it by passing --deadline option"                                                                                                          
Makefile:133: recipe for target 'lint' failed                                                                                                                                              
make: *** [lint] Error 4                                                                                                                                                                   

I'm not sure how to fix this one without increasing the deadline on the linter...

@alexec
Copy link
Contributor

alexec commented Mar 18, 2020

@ian-howell is this still wanted please?

@alexec
Copy link
Contributor

alexec commented Apr 5, 2020

Added wontfix label as this has no activity for 6 months.

@simster7
Copy link
Member

simster7 commented Apr 6, 2020

Added wontfix label as this has no activity for 6 months.

Seems like this PR is done and even approved, it just was never merged. @ian-howell do you think you could resolve the conflicts?

@alexec
Copy link
Contributor

alexec commented Apr 6, 2020

@simster7 +1

@alexec alexec closed this Apr 30, 2020
@astanway
Copy link

Chiming in - this is a highly desired feature, and it has huge advantages for downstream metrics and monitoring, given podnames are a first class citizen in Grafana, GKE logs, Prometheus, Stackdriver, etc. I strongly support adding it and I hope to see it in a release

@BrookRoberts
Copy link

+1, if (as seems) this feature exists in an MR and is ready to go it would be a very useful and addition!

@fidel-perez
Copy link

+1. We really need this in our project. Is there any other way in Argo to change the name of the pods being started by templates?

@simster7
Copy link
Member

simster7 commented Jun 5, 2020

I’ll take a look into this early next week

@vitalyrychkov
Copy link
Contributor

Hi Simon, did you have a chance to have a look ever since? I was also looking for this feature.

@davidghiurco
Copy link

I need to add a +1 on this, since fine-grained pod monitoring with prometheus is impossible when all the pods in a workflow are named random jargon that does not differentiate what task they are performing.

@pc-kdholakia
Copy link

+1 on this, it will ease the monitoring process.

@alexec
Copy link
Contributor

alexec commented Sep 3, 2020

Better to thumbs-up the issue, not this closed PR.

@ghiander
Copy link

ghiander commented Aug 9, 2023

Hello, what happened with this? It's a very useful feature to have, I am struggling to configure Grafana to observe some specific steps without having their names in the pod names

@aaron-arellano
Copy link

aaron-arellano commented Aug 21, 2023

this feature has causes a lot of upstream issues outside of Argo, particularly syncing of the kube downward API and etcd. I mentioned this in various open threads but pinging here so more engineers that worked on this feature are aware. This should be reverted until a real solution is created, not continuous patches.

#10107 #10267

@agilgur5
Copy link

agilgur5 commented Oct 2, 2023

Hello, what happened with this?

This was replaced by #6712.

This should be reverted until a real solution is created, not continuous patches.

This PR was never merged, see #6712, which can be turned off by setting POD_NAMES=v1. The eventual plan seems to be to replace with #10267.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add taskname to the workflow pod's name