argoproj / argoproj/argo-workflows
`workflows.argoproj.io/display-name` annotation location/naming/behavior changes
- Dominant language
- Go
- Stars
- 17k
- Forks
- 3.7k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 138
Description
# Description/Reasoning
While `display-name` is functional and matches it's documentation, I think its `annotations` location could be considered unexpected. I believe the unexpected naming contributed to [another bug report](https://github.com/argoproj/argo-workflows/issues/14380) and why the issue was also missed in follow up posts.
The `annotations` field exists at the `WorkflowSpec` level rather than under a `metadata` field as is typical on most kubernetes definitions (see [the example](https://github.com/MenD32/argo-workflows/blob/5951b1e217842e5431a1f0524a0c4c29c0eb3607/docs/walk-through/annotations.md#annotation-templates))
Practically this makes sense, its a node level setting not a pod level setting and suspend nodes wouldn't even have a pod and it fit within existing functionality
It doesnt stand out so much at the top template level since workloads cant be specified inline at that point, but under `steps`/`tasks` you wind up with
```
annotations:
workflows.argoproj.io/display-name: displayname
metadata:
annotations:
otherAnnotation: hasAValue
```
As it has a direct effect on behavior (and [also manifesting as bugs in other places](https://github.com/argoproj/argo-workflows/issues/14380) as a result) I think it should be a first party field of the `TemplateSpec` rather than an annotation.
Its already a primary field in the `NodeStatusSpec` and it would also avoid possible confusion caused by existing in an alternative `annotations` field
In doing this I think it should also be named `nodeName`. Typically a "human readable" display name would be much more relaxed in its value constraints and wouldnt have any functional effects on the workflow as this does
I think it would also be worth having a true "display name" since
- It could allow spaces or other characters outside of the limits for the name giving the potential for formatting and allow duplicate values
- It would detach it from functional behaviour avoiding bugs such as https://github.com/argoproj/argo-workflows/issues/14914 if the goal was simply UI improvements
That idea however recreates the original quandary.
The workflow level display name behaviours exist as annotations and not first party fields. If this was to follow suit then it would need to be an annotation which brings us back to the question of its expected positioning and if `annotations` would belong under `metadata` or not.
I imagine both aspects would need some discussion
Contributor guide
Research direction
Start by reviewing the WorkflowSpec, TemplateSpec, and NodeStatusSpec definitions, along with the annotations documentation example and linked issues 14380 and 14914. Determine whether the project wants a first-party nodeName field, a separate display name, or revised annotation placement and naming; done requires an agreed design and corresponding behavior and documentation changes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- backend, devops
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100