Skip to content
This repository was archived by the owner on Jun 14, 2019. It is now read-only.

Conversation

@droslean
Copy link
Member

@droslean droslean commented Oct 17, 2018

Adds template's pod annotation ci-operator.openshift.io/always-show-output, to output the logs for each container, no matter what the exit code was.

closes #118

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 17, 2018
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 17, 2018
@droslean
Copy link
Member Author

/assign @petr-muller @bbguimaraes @stevekuznetsov

@petr-muller
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: droslean, petr-muller

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [droslean,petr-muller]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@bbguimaraes
Copy link
Contributor

As stated multiple times in the issues and in private chat, this needs to be a per-container option. One of the architectural principles of this tool is to be very selective about the logs that are stored (i.e. only those that are strictly needed to determine the cause of failure). This implementation needs to be changed.

@droslean
Copy link
Member Author

@bbguimaraes follow up issue opened. #185

@smarterclayton
Copy link
Contributor

I don't like this change. It made debugging failures significantly worse, because I always have to look at output that is irrelevant (e2e, teardown, or setup). The reason why we didn't show success output is because that makes debugging failures much more efficient (if I have 200k lines of output, I want the last chunk of output to be failures).

I would like to revert this behavior and replace it with something more focused - probably something like "always output logs to the artifacts directory". A user having to jump into "artifacts" to get at successful container logs is fine for me. Being unable to quickly triage PR flakes and failures by clicking "logs, build logs, end of logs" is not fine.

@smarterclayton
Copy link
Contributor

I'm also ok with having the more specific option (store only specific container logs), but I also think it should follow the underlying principle of "show failures and minimal success info in the build log, store anything else in artifacts"

@smarterclayton
Copy link
Contributor

An example of where it made success case worse - we had a significant regression in e2e-gcp times - I went to https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/21292/pull-ci-openshift-origin-master-e2e-gcp/891/?log#log to try and see where that is. Before, I would have ~60 lines and could have easily checked the time of a section. Now, I might as well give up.

@droslean
Copy link
Member Author

@smarterclayton #192

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

artifacts: annotation for logs when something else fails

7 participants