-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
added logging of output for flaky build tests #6240
added logging of output for flaky build tests #6240
Conversation
for i := 0; i <= 3; i++ { | ||
out, err = oc.Run("start-build").Args(buildName, "--follow", "--wait").Output() | ||
if err != nil { | ||
continue |
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.
it would be good to log the error here so we know it is retrying it
I don't think I'm a fan of this... do we know the root cause? Are we expecting customers to add retry logic around builds to get reliable results as well? |
we added a way to log the output of the build whenever it fails so that we can identify what the problem is rather than add a retry logic to it.
Basically ginkgo still flushes to stdout on failure, but with this change we can make sure that we catch the problem easier next time and after that we can remove it. |
lgtm. [merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4276/) (Image: devenv-rhel7_2910) |
Evaluated for origin merge up to 638175a |
@rhcarvalho @bparees @mfojtik see my above comment as to why the bundle install failed |
@PI-Victor that failure was, i think, because we were running scl enable ror40 on the ruby22 image which doesn't have ror40. if we're still doing that anywhere, we should track it down and fix it. eg i fixed one instance here: |
possibly that is the same issue you hit, depending when the build was from. |
well the link to the actual build from jenkins you mentioned in the issue, is from 5th of Dec. |
@PI-Victor I don't think the change hurts anything, we're still going to want that info if we hit failures. so i'd leave it. |
related to Issue: #6215
also an older conversation about this #5912 (comment)
@bparees @mfojtik PTAL