fix(ui): Use proper podname for containersets. Fixes #13038 #13039
Conversation
agilgur5
changed the title
ui/src/app/shared/pod-name.ts
Outdated
| @@ -19,7 +19,8 @@ export function getPodName(workflow: Workflow, node: NodeStatus): string { | |||
| } | |||
|
|
|||
| const workflowName = workflow.metadata.name; | |||
| if (workflowName === node.name) { | |||
| const podNodeName = node.type == 'Container' ? node.name.replace(/\.[^/.]+$/, '') : node.name; | |||
There was a problem hiding this comment.
hmm this doesn't seem to match the back-end. these two functions should be equivalent per the comment on line 14 in this file
There was a problem hiding this comment.
Sure, reverted the change and moved it outside the function call. Maybe there are still better ways to do this
There was a problem hiding this comment.
It would be great if you could locate where in the back-end this gets set, that way we can keep them equivalent, whether in this function or elsewhere.
There was a problem hiding this comment.
Looks like the pod is created based on the pod node name.
https://github.com/argoproj/argo-workflows/blob/main/workflow/controller/container_set_template.go#L20-L24
Then the containerset nodes are created
https://github.com/argoproj/argo-workflows/blob/main/workflow/controller/container_set_template.go#L31-L46
Where the container node names are just their pod node name concatenated with the container name.
ctxNodeName := fmt.Sprintf("%s.%s", nodeName, c.Name)So removing this postfix should map from containerset node name to pod node name, which can be used to calculate the pod name using the existing function.
instauro
force-pushed
the
fix/containerset-podname
branch
2 times, most recently
from
cd6e0fd
to
495f907
Compare
instauro
force-pushed
the
fix/containerset-podname
branch
3 times, most recently
from
8447e69
to
5bd55eb
Compare
Signed-off-by: instauro <[email protected]>
instauro
force-pushed
the
fix/containerset-podname
branch
from
5bd55eb
to
262313e
Compare
Fixes #13038
Motivation
Containerset nodes currently do not retrieve their logs in the UI. This is due to the podname calculation failing, which makes the request to the backend incorrect. This fix will hopefully become obsolete when #12528 is fixed, but until then this should do the trick.
Modifications
Uses a better podname when the node type is
Container.Verification
Tried a few different containerset templates to verify the fix, and a few none-containetset templates to verify that there is no regression.