Fix running sauce connect with --log-file #278
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
budziam
force-pushed
the
mb/use-api-for-healthcheck
branch
3 times, most recently
from
329067b to
6d55369
Compare
waggledans
reviewed
Jan 31, 2025
| @@ -301,6 +291,18 @@ export default class SauceLabs { | |||
| args.push(`--proxy-sauce=${this.proxy}`); | |||
| } | |||
|
|
|||
| // Provide a default runner name. It's used for identifying the tunnel's initiation method. | |||
| let metadata = argv.metadata || ''; | |||
| if (!metadata.includes('runner=')) { | |||
There was a problem hiding this comment.
good catch. I noticed it before but figured it can be dealt with later
waggledans
reviewed
Jan 31, 2025
| } | ||
| args.push(`--metadata=${metadata}`); | ||
|
|
||
| const apiAddress = argv.apiAddress || ':8032'; |
There was a problem hiding this comment.
This one is problematic in the sense that it would prevent running two SC instances on the same host without specifying different apiAddress values. It's a normal constraint, just need to make sure it's gracefully handled with a clear error message. I remember it wasn't obvious what's wrong but maybe it's already fixed. Did you try running two instances on the same host without specifying apiAddress? Is it handled in a way that it's obvious why SC doesn't start?
There was a problem hiding this comment.
It fails in the following way
$ ./bin/sl sc --tunnel-name example2
2025/02/03 15:16:59.199262 [control] [INFO] Sauce Connect Proxy version=5.2.2
2025/02/03 15:16:59.207453 [control] [INFO] configuration
access-key=xxxxx
api-address=:8032
metadata=runner=node-saucelabs
region=us-west-1
tunnel-name=example2
username=[redacted]
2025/02/03 15:16:59.207560 [proxy] [INFO] no upstream proxy specified
2025/02/03 15:16:59.207563 [proxy] [INFO] localhost proxying mode=deny
2025/02/03 15:16:59.207643 [control] [INFO] please wait for 'you may start your tests' to start your tests
2025/02/03 15:16:59.207646 [control] [INFO] provisioning Sauce Connect region=us-west name=example2
2025/02/03 15:17:13.161754 [control] [INFO] Sauce Connect running id=[redacted]
2025/02/03 15:17:13.161891 [tunnel] [INFO] waiting for Sauce Connect server to be reachable
2025/02/03 15:17:21.251507 [ERROR] fatal error exiting: api: failed to open listener on address :8032: listen tcp :8032: bind: address already in use
I believe this is readable enough. WDYT?
budziam
force-pushed
the
mb/use-api-for-healthcheck
branch
from
6d55369 to
74be6d0
Compare
budziam
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One-line summary
Description
Use the API exposed by Sauce Connect for health checks instead of parsing log lines from stdout. SC5 doesn't print anything (almost) to stdout and stderr when
--log-fileis passed.Types of Changes
What types of changes does your code introduce? Keep the ones that apply: