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: Use v1 pod name if no template name or ref. Fixes #7595 and #7749 #7605

Merged
merged 6 commits into from
Feb 4, 2022

Conversation

JPZ13
Copy link
Member

@JPZ13 JPZ13 commented Jan 20, 2022

Signed-off-by: J.P. Zivalich jp@pipekit.io

This PR:

@liuzqt Could you run one of your workflows locally using this PR branch to test if it is working? I want to validate that I don't have to make an additional change to the workflow controller

Signed-off-by: J.P. Zivalich <jp@pipekit.io>
Signed-off-by: J.P. Zivalich <jp@pipekit.io>
Signed-off-by: J.P. Zivalich <jp@pipekit.io>
Signed-off-by: J.P. Zivalich <jp@pipekit.io>
Signed-off-by: J.P. Zivalich <jp@pipekit.io>
Signed-off-by: J.P. Zivalich <jp@pipekit.io>
@JPZ13
Copy link
Member Author

JPZ13 commented Jan 20, 2022

@alexec - We don't yet have an example to reproduce #7595. Once we hear back from @liuzqt's team and have confirmed that this PR fixes their use case, I'll convert it from a draft to ready to review

@terrytangyuan
Copy link
Member

terrytangyuan commented Jan 21, 2022

@JPZ13 Do you have an image for them to test out? I am in touch with their team. Let me know if there’s any other information you need.

@JPZ13
Copy link
Member Author

JPZ13 commented Jan 24, 2022

@JPZ13 Do you have an image for them to test out? I am in touch with their team. Let me know if there’s any other information you need.

Thanks @terrytangyuan - I cut three images and pushed them to our organization's docker registry since I don't have push access to the argoproj one. Here are the image tags:

  • pipekit13/workflow-controller:fix-log-render-v1
  • pipekit13/argoexec:fix-log-render-v1
  • pipekit13/argocli:fix-log-render-v1

Let me know if they're able to run those or if they run into any issues

@terrytangyuan
Copy link
Member

terrytangyuan commented Jan 24, 2022

Let me know if they're able to run those or if they run into any issues

Thanks. We'll test it out.

@JPZ13
Copy link
Member Author

JPZ13 commented Feb 1, 2022

@terrytangyuan - have y'all had a chance to test these out yet?

@terrytangyuan
Copy link
Member

@terrytangyuan - have y'all had a chance to test these out yet?

Not yet. I'll check soon.

@JPZ13 JPZ13 marked this pull request as ready for review February 4, 2022 17:02
@JPZ13 JPZ13 changed the title fix: Use v1 pod name if no template name or ref. Fixes #7595 fix: Use v1 pod name if no template name or ref. Fixes #7595 and #7749 Feb 4, 2022
@JPZ13
Copy link
Member Author

JPZ13 commented Feb 4, 2022

@alexec This PR fixes #7749. I hadn't had an example to test it with before, but I was able to test it with @Hunter-Thompson's example

@alexec alexec merged commit 9324665 into argoproj:master Feb 4, 2022
@JPZ13 JPZ13 deleted the fix-log-render branch February 4, 2022 17:07
@terrytangyuan
Copy link
Member

Thanks @JPZ13!

@JPZ13
Copy link
Member Author

JPZ13 commented Feb 5, 2022

Any time, @terrytangyuan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v3.2.6 Argo UI failed to render pod log
4 participants