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

Abort builds if Sauce Connect blocks for > 2 min. #724

Closed
wants to merge 2 commits into from

Conversation

DylanLacey
Copy link

Builds opening a Sauce Connect tunnel currently block until Sauce
Connect has produced a readyfile. In some cases there is no readyfile
producted; This blocks the build until the global no-output timer (10
minutes as of this commit) terminates it.

Readyfiles are produced once a Sauce Connect tunnel is fully up. This
can fail to happen for a variety of reasons, including the user having
too many tunnels open at once.

This commit introduces a 2 minute wait time (as advised by @ldx, Sauce
Labs employee and total Sauce Connect smart guy) for the readyfile,
after which the entire build is terminated.

The travis_jigger function is used to post a notification to the
console and then ultimately kill the process via travis_terminate 2
(similarly to travis_assert). An earlier revision used travis_wait
but this produced confusing output.

This commit closes travis-ci/travis-ci#3091.

Builds opening a Sauce Connect tunnel currently block until Sauce
Connect has produced a readyfile.  In some cases there is no readyfile
producted; This blocks the build until the global no-output timer (10
minutes as of this commit) terminates it.

Readyfiles are produced once a Sauce Connect tunnel is fully up.  This
can fail to happen for a variety of reasons, including the user having
too many tunnels open at once.

This commit introduces a 2 minute wait time (as advised by @ldx, Sauce
Labs employee and total Sauce Connect smart guy) for the readyfile,
after which the entire build is terminated.

The `travis_jigger` function is used to post a notification to the
console and then ultimately kill the process via `travis_terminate 2`
(similarly to `travis_assert`).  An earlier revision used `travis_wait`
but this produced confusing output.

This commit closes travis-ci/travis-ci#3091.
BanzaiMan added a commit that referenced this pull request Jun 2, 2016
Abort builds if Sauce Connect blocks for > 2 min.
@BanzaiMan
Copy link
Contributor

This simple test shows that it works (the timeout of 2 minutes is respected, since the build has incorrect credentials). Ideally, there is a better way to detect that the credentials are invalid and the tunnel is never going to come up.

@bootstraponline
Copy link

Ideally, there is a better way to detect that the credentials are invalid

Make a REST API call to Sauce?

@DanielHeath
Copy link

Almost none of the variable interpolations in this file are quoted, so if there are space characters anywhere you'll be in trouble.

Compare:
while [ ! -f ${readyfile} ]; do
while [ ! -f "${readyfile}" ]; do

If $readyfile is set to missingfile || echo 'hacked you', the former will print 'hacked you' to the console while the latter will not.

@DylanLacey
Copy link
Author

Making a REST API call is a good approach for credentials, but the problem I was trying to solve was the generalised problem of Sauce Connect start up problems. This can happen for a variety of reasons, some detectable in advance (assuming nothing changes between checks and actual start up, which takes several seconds) and some not.

Currently, an issue starting Sauce Connect will cause a build to hang until Travis's hung build timeout is reached, at which point the entire build is terminated. This change should account for most problems, known or otherwise, and end builds in 2 minutes, 5 times faster then the default.

@DanielHeath good call; I'll have a whack at correcting that and send an updated PR.

@DylanLacey
Copy link
Author

Globbing updated; I left it out for arguments that can't have spaces.

@BanzaiMan
Copy link
Contributor

OK. Let's test this again tomorrow (July 15).

@tristanbes
Copy link

what's the status of this PR please ?

@meatballhat
Copy link
Contributor

I'm sorry for the long delay! This PR is no longer applicable. Please holler if you feel it should be reopened, assuming the problems haven't been addressed elsewhere.

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