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

add best practice try/catch, timeout, specifically with extended test… #14354

Conversation

gabemontero
Copy link
Contributor

…s in mind

the diff came out a little wonky - try viewing with ignore white space option

but in general existing contents were put inside

try {
   timeout(...) {
   } 
} catch ... {
}

@openshift/devex

@gabemontero
Copy link
Contributor Author

[testextended][extended:core(openshift pipeline build)]

@gabemontero
Copy link
Contributor Author

fyi ... adding ?w=1 to the file diff url is the ignore whitespace trigger ... the file diff renders as I described when employed

}
}
try {
timeout(time: 2, unit: 'HOURS') {
Copy link
Contributor

Choose a reason for hiding this comment

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

2 hours? i'm worried about making our test runs take a really long time to fail. is there something in the extended test itself that's going to cap this? regardless i'd hope we could bring it down to 15-20 minutes tops.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah 2 hours was NOT based on any examination of deployment times or some such ... just a "make it really big so it is not a factor" rationale

I'll switch to 20 minutes.

@bparees bparees self-assigned this May 25, 2017
@gabemontero
Copy link
Contributor Author

ah ... still waiting on the new client plugin version with the exists since we still can't update the centos image ... will address in separate PR

the tests based on this PR's changes passed, but will clean up above, plus reduce timeout

@gabemontero gabemontero force-pushed the bump-jenkins-pipeline-test-timeouts branch from 9a5cd58 to 71b034b Compare May 25, 2017 15:51
@gabemontero gabemontero force-pushed the bump-jenkins-pipeline-test-timeouts branch from 71b034b to a919167 Compare May 25, 2017 15:53
@gabemontero
Copy link
Contributor Author

ok updates pushed

@bparees
Copy link
Contributor

bparees commented May 25, 2017

lgtm if it passes tests

@gabemontero
Copy link
Contributor Author

and they just passed :-)

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to a919167

@bparees
Copy link
Contributor

bparees commented May 25, 2017

looks like the passing run was your previous commit. this run failed with

+ oc start-build openshift-jee-sample-docker --from-file=target/ROOT.war --follow -n extended-test-jenkins-pipeline-37mv8-n85lx
Uploading file "target/ROOT.war" as binary input for the build ...
Unable to connect to the server: net/http: TLS handshake timeout
[Pipeline] }
[Pipeline] // stage
[Pipeline] }
[Pipeline] // node
[Pipeline] }
[Pipeline] // timeout
[Pipeline] echo
in catch block
[Pipeline] echo
Caught: hudson.AbortException: script returned exit code 1

does this imply the 20 minute timeout might not be long enough?

@gabemontero
Copy link
Contributor Author

the extended failure this time was the very familiar flake #13984

Further confirmation that the --follow results in the TLS handshake timeout, while in debug PR #14013, where that has been replaced with --wait, we get the more generic exit status -1

And I really am beginning to wonder if there is any correlation with #14148 and some of the console's issues with follow since the rebase.

@openshift-bot
Copy link
Contributor

Evaluated for origin testextended up to a919167

@gabemontero
Copy link
Contributor Author

gabemontero commented May 25, 2017 via email

@bparees
Copy link
Contributor

bparees commented May 25, 2017

[merge][severity:bug]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1772/) (Base Commit: 4b3473a)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/501/) (Base Commit: 4b3473a) (Extended Tests: core(openshift pipeline build))

@bparees
Copy link
Contributor

bparees commented May 27, 2017

flake #13980
flake #10773
flake #14385
[merge][severity:bug]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to a919167

@openshift-bot
Copy link
Contributor

openshift-bot commented May 27, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/817/) (Base Commit: 21c14af) (Extended Tests: bug) (Image: devenv-rhel7_6280)

@openshift-bot openshift-bot merged commit c4adfeb into openshift:master May 27, 2017
@gabemontero gabemontero deleted the bump-jenkins-pipeline-test-timeouts branch May 28, 2017 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants