Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct pod names for inline templates. Fixes #12895 #12928

Closed
wants to merge 1 commit into from

Conversation

AlbeeSo
Copy link
Contributor

@AlbeeSo AlbeeSo commented Apr 11, 2024

Fixes #12895

Motivation

if !strings.Contains(nodeName, ".inline") {

inline-cant-show-errors-9ptb9--3016472844   0/2     Error       0               25h
inline-cant-show-errors-9ptb9--3942528235   0/2     Completed   0               25h

Incorrect pod names of inline templates will lead to empty UI logs problem.

Modifications

Verification

Test with the workflow mentioned in issue, got result like image post below:
image

@shuangkun shuangkun closed this Apr 12, 2024
@shuangkun shuangkun reopened this Apr 12, 2024
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Can you add a test?

@shuangkun shuangkun self-assigned this Apr 15, 2024
@shuangkun shuangkun added the area/controller Controller issues, panics label Apr 15, 2024
@agilgur5 agilgur5 linked an issue Apr 18, 2024 that may be closed by this pull request
4 tasks
@agilgur5 agilgur5 changed the title fix: incorrect pod names of inline templates leading to empty UI logs. Fixes #12895 fix: correct pod names for inline templates. Fixes #12895 Apr 22, 2024
@agilgur5
Copy link

agilgur5 commented Apr 22, 2024

I see the UI logic for pod names has diverged a bit -- #7605 added a templateName !== '' check to the UI but not the back-end (and the conditional was subtly changed in #11016 too). This looks vaguely correct based on that, but the UI seems to have no check for inline templates? Conversely, #10921 added the inline check to the back-end but not the UI?

This logic could perhaps be simplified to just templateName != "" and the inline check removed?

@agilgur5 agilgur5 added the problem/more information needed Not enough information has been provide to diagnose this issue. label May 4, 2024

This comment was marked as resolved.

@github-actions github-actions bot added the problem/stale This has not had a response in some time label May 19, 2024

This comment was marked as resolved.

@github-actions github-actions bot closed this Jun 2, 2024
@shuangkun shuangkun reopened this Jun 18, 2024
@github-actions github-actions bot removed problem/stale This has not had a response in some time problem/more information needed Not enough information has been provide to diagnose this issue. labels Jun 19, 2024
@agilgur5
Copy link

@shuangkun do you or @AlbeeSo plan to update this PR with the requested changes?
No changes have been made in 4 days since you re-opened the PR, so I'm not sure why it was re-opened

@agilgur5 agilgur5 added the problem/more information needed Not enough information has been provide to diagnose this issue. label Jun 23, 2024
@shuangkun shuangkun closed this Jun 28, 2024
@agilgur5
Copy link

agilgur5 commented Jul 1, 2024

Superseded by #13261

@agilgur5 agilgur5 added solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) and removed problem/more information needed Not enough information has been provide to diagnose this issue. labels Jul 1, 2024
@agilgur5 agilgur5 added this to the v3.5.x patches milestone Jul 1, 2024
@argoproj argoproj locked as resolved and limited conversation to collaborators Jul 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/controller Controller issues, panics area/server solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline templates still do not create the pod name correctly Inline Workflows logs in UI always empty
4 participants