-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat: Surface artifacts through sidecar container logs. #7883
feat: Surface artifacts through sidecar container logs. #7883
Conversation
Skipping CI for Draft Pull Request. |
The following is the coverage report on the affected files.
|
/kind feat |
@ericzzzzzzz: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/kind feature |
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
d39a81f
to
6019951
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
c2c1c22
to
8efcd61
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
Thanks for the changes. I would love to see a test where we extract both results and artifacts from sidecar logs so that we can validate that they both work at the same time. |
Otherwise lgtm |
bc59e03
to
1b04d6f
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
1b04d6f
to
b86b023
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ericzzzzzzz ! This generally looks good. Just some nit picking and do we have any failure test cases for the coverage?
b86b023
to
e684c6d
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
dc6c3f8
to
6c9e9b5
Compare
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
Thanks for the review! I added some tests, the some failure cases are related to file operations and we don't have established pattern to mock those, others are related to json marshal operation, it shouldn't happen as we are passing a struct with all tagged public fields |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chitrangpatel, JeromeJu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Understood - then maybe wrapping the errors with the appropriate context might be the way to go. |
I'm merging this PR since it is basically taking the exact same approach as for larger results to extract artifacts. This has been open for a while and blocking other work. The same functionality has already been implemented for surfacing the information via TerminationMessages. There is nothing novel here. /lgtm |
Changes
result-from:sidecar-logs
is used to guard this feature, it is also the same as surfacing step results through sidecar logs.artifacts path
to determine if we should create an artifact sidecar container should be created for a task.Fixes: #7869
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes