Skip to content
This repository was archived by the owner on Sep 12, 2024. It is now read-only.

Commit 90a2b2d

Browse files
committed
Honor env vars when using builder
Previously, we would honor HELIOS_DOMAIN and HELIOS_ENDPOINTS only if the caller used TemporaryJobs.create(). We now honor those variables even if the caller uses the builder directly. They can then override those values with hardcoded ones using builder methods. -RN TemporaryJobs builder object now honors HELIOS_DOMAIN and HELIOS_ENPOINTS.Previously they were honored only if the caller used TemporaryJobs.create().
1 parent fcc4e36 commit 90a2b2d

File tree

1 file changed

+57
-54
lines changed

1 file changed

+57
-54
lines changed

helios-testing/src/main/java/com/spotify/helios/testing/TemporaryJobs.java

+57-54
Original file line numberDiff line numberDiff line change
@@ -240,61 +240,8 @@ private static List<String> getListByKey(final String key, final Config config)
240240
* @see <a href="https://github.com/spotify/helios/blob/master/docs/testing_framework.md#
241241
* configuration-by-file">Helios Testing Framework - Configuration By File</a>
242242
*/
243-
244243
public static TemporaryJobs create() {
245-
// Builder will be initialized with values from a testing profile if one is present. If domain
246-
// or endpoints is specified in the profile, they will be used to create a helios client.
247-
final Builder builder = builder();
248-
249-
// If set, use HELIOS_HOST_FILTER to override value from test profile
250-
final String heliosHostFilter = System.getenv("HELIOS_HOST_FILTER");
251-
if (heliosHostFilter != null) {
252-
builder.hostFilter(heliosHostFilter);
253-
}
254-
255-
// If set, use HELIOS_DOMAIN to override client which may have been created from test profile
256-
final String domain = System.getenv("HELIOS_DOMAIN");
257-
if (!isNullOrEmpty(domain)) {
258-
return builder.domain(domain).build();
259-
}
260-
261-
// If set, use HELIOS_ENDPOINTS to override client which may have been created from test profile
262-
final String endpoints = System.getenv("HELIOS_ENDPOINTS");
263-
if (!isNullOrEmpty(endpoints)) {
264-
return builder.endpointStrings(Splitter.on(',').splitToList(endpoints)).build();
265-
}
266-
267-
// If we get here and client is set in the builder, it was created using a testing profile,
268-
// and the values were not overridden by environment variables. Let's use it.
269-
if (builder.client != null) {
270-
return builder.build();
271-
}
272-
273-
// If we get here, we were not able to create a client based on environment variables or a
274-
// testing profile, so check if DOCKER_HOST is set. If so, try to connect to that host on port
275-
// 5801, assuming it has a helios master running. If not, attempt to connect to
276-
// http://localhost:5801 as a last attempt.
277-
final String dockerHost = System.getenv("DOCKER_HOST");
278-
if (dockerHost == null) {
279-
builder.endpoints("http://localhost:5801");
280-
} else {
281-
try {
282-
final URI uri = new URI(dockerHost);
283-
builder.endpoints("http://" + uri.getHost() + ":5801");
284-
} catch (URISyntaxException e) {
285-
throw Throwables.propagate(e);
286-
}
287-
}
288-
289-
// We usually require the caller to specify a host filter, so jobs aren't accidentally deployed
290-
// to arbitrary hosts. But at this point the master is either running on localhost or the docker
291-
// host. Either way, this is probably a test machine with one master and one agent both running
292-
// on the same box, so it is safe to provide a default filter that will deploy anywhere.
293-
if (heliosHostFilter == null) {
294-
builder.hostFilter(DEFAULT_LOCAL_HOST_FILTER);
295-
}
296-
297-
return builder.build();
244+
return builder().build();
298245
}
299246

300247
public static TemporaryJobs create(final HeliosClient client) {
@@ -521,6 +468,62 @@ private Builder(final String profile, final Config preConfig) {
521468
if (this.config.hasPath("hostPickingStrategy")) {
522469
processHostPickingStrategy();
523470
}
471+
472+
// Configuration from profile may be overridden by environment variables
473+
configureWithEnv();
474+
}
475+
476+
private void configureWithEnv() {
477+
// Use HELIOS_HOST_FILTER if set
478+
final String heliosHostFilter = System.getenv("HELIOS_HOST_FILTER");
479+
if (heliosHostFilter != null) {
480+
hostFilter(heliosHostFilter);
481+
}
482+
483+
// Use HELIOS_DOMAIN if set
484+
final String domain = System.getenv("HELIOS_DOMAIN");
485+
if (!isNullOrEmpty(domain)) {
486+
domain(domain);
487+
return;
488+
}
489+
490+
// Use HELIOS_ENDPOINTS if set
491+
final String endpoints = System.getenv("HELIOS_ENDPOINTS");
492+
if (!isNullOrEmpty(endpoints)) {
493+
endpointStrings(Splitter.on(',').splitToList(endpoints));
494+
return;
495+
}
496+
497+
// If we get here and client is set, we know which master we'll be talking to, so just return
498+
// as rest of this method handles the case where the helios master wasn't specified.
499+
if (client != null) {
500+
return;
501+
}
502+
503+
// If we get here, we did not create a client based on environment variables or a testing
504+
// profile, so check if DOCKER_HOST is set. If so, try to connect to that host on port 5801,
505+
// assuming it has a helios master running. If not, attempt to connect to
506+
// http://localhost:5801 as a last attempt.
507+
final String dockerHost = System.getenv("DOCKER_HOST");
508+
if (dockerHost == null) {
509+
endpoints("http://localhost:5801");
510+
} else {
511+
try {
512+
final URI uri = new URI(dockerHost);
513+
endpoints("http://" + uri.getHost() + ":5801");
514+
} catch (URISyntaxException e) {
515+
throw Throwables.propagate(e);
516+
}
517+
}
518+
519+
// We usually require the caller to specify a host filter, so jobs aren't accidentally
520+
// deployed to arbitrary hosts. But at this point the master is either running on localhost
521+
// or the docker host. Either way, this is probably a test machine with one master and one
522+
// agent both running on the same box, so it is safe to provide a default filter that will
523+
// deploy anywhere.
524+
if (heliosHostFilter == null) {
525+
hostFilter(DEFAULT_LOCAL_HOST_FILTER);
526+
}
524527
}
525528

526529
private void processHostPickingStrategy() {

0 commit comments

Comments
 (0)