diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index 92177fcf62b26b..c52d19989a28af 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -16,7 +16,6 @@ import build.bazel.remote.execution.v2.Digest; import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.base.Preconditions; -import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -45,7 +44,6 @@ import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; -import javax.annotation.Nullable; /** A {@link BuildEventArtifactUploader} backed by {@link ByteStreamUploader}. */ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted @@ -63,16 +61,11 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted ByteStreamBuildEventArtifactUploader( ByteStreamUploader uploader, MissingDigestsFinder missingDigestsFinder, - String remoteServerName, + String remoteServerInstanceName, String buildRequestId, String commandId, - @Nullable String remoteInstanceName, int maxUploadThreads) { this.uploader = Preconditions.checkNotNull(uploader); - String remoteServerInstanceName = Preconditions.checkNotNull(remoteServerName); - if (!Strings.isNullOrEmpty(remoteInstanceName)) { - remoteServerInstanceName += "/" + remoteInstanceName; - } this.buildRequestId = buildRequestId; this.commandId = commandId; this.remoteServerInstanceName = remoteServerInstanceName; diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java index 1c27fbe703d0c7..e2a1e27769f3dc 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java @@ -18,7 +18,6 @@ import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import javax.annotation.Nullable; /** * A factory for {@link ByteStreamBuildEventArtifactUploader}. @@ -27,25 +26,22 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactUploaderFactory { private final ByteStreamUploader uploader; - private final String remoteServerName; + private final String remoteServerInstanceName; private final String buildRequestId; private final String commandId; private final MissingDigestsFinder missingDigestsFinder; - @Nullable private final String remoteInstanceName; ByteStreamBuildEventArtifactUploaderFactory( ByteStreamUploader uploader, MissingDigestsFinder missingDigestsFinder, - String remoteServerName, + String remoteServerInstanceName, String buildRequestId, - String commandId, - @Nullable String remoteInstanceName) { + String commandId) { this.uploader = uploader; this.missingDigestsFinder = missingDigestsFinder; - this.remoteServerName = remoteServerName; + this.remoteServerInstanceName = remoteServerInstanceName; this.buildRequestId = buildRequestId; this.commandId = commandId; - this.remoteInstanceName = remoteInstanceName; } @Override @@ -53,10 +49,9 @@ public BuildEventArtifactUploader create(CommandEnvironment env) { return new ByteStreamBuildEventArtifactUploader( uploader.retain(), missingDigestsFinder, - remoteServerName, + remoteServerInstanceName, buildRequestId, commandId, - remoteInstanceName, env.getOptions().getOptions(RemoteOptions.class).buildEventUploadMaxThreads); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index 669c646e42bfb3..d5f9a1485a21d1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -469,6 +469,14 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { } } + String remoteBytestreamUriPrefix = remoteOptions.remoteBytestreamUriPrefix; + if (Strings.isNullOrEmpty(remoteBytestreamUriPrefix)) { + remoteBytestreamUriPrefix = cacheChannel.authority(); + if (!Strings.isNullOrEmpty(remoteOptions.remoteInstanceName)) { + remoteBytestreamUriPrefix += "/" + remoteOptions.remoteInstanceName; + } + } + ByteStreamUploader uploader = new ByteStreamUploader( remoteOptions.remoteInstanceName, @@ -489,12 +497,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { uploader.release(); buildEventArtifactUploaderFactoryDelegate.init( new ByteStreamBuildEventArtifactUploaderFactory( - uploader, - cacheClient, - cacheChannel.authority(), - buildRequestId, - invocationId, - remoteOptions.remoteInstanceName)); + uploader, cacheClient, remoteBytestreamUriPrefix, buildRequestId, invocationId)); if (enableRemoteExecution) { RemoteExecutionClient remoteExecutor; diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 9df464991f97cb..7f7725a565cbc7 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -181,6 +181,19 @@ public final class RemoteOptions extends OptionsBase { + " the unit is omitted, the value is interpreted as seconds.") public Duration remoteTimeout; + @Option( + name = "remote_bytestream_uri_prefix", + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "The hostname and instance name to be used in bytestream:// URIs that are written into " + + "build event streams. This option can be set when builds are performed using a " + + "proxy, which causes the values of --remote_executor and --remote_instance_name " + + "to no longer correspond to the canonical name of the remote execution service. " + + "When not set, it will default to \"${hostname}/${instance_name}\".") + public String remoteBytestreamUriPrefix; + /** Returns the specified duration. Assumes seconds if unitless. */ public static class RemoteTimeoutConverter implements Converter { private static final Pattern UNITLESS_REGEX = Pattern.compile("^[0-9]+$"); diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index f41b16a16dd640..01b785281010aa 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java @@ -374,10 +374,9 @@ private ByteStreamBuildEventArtifactUploader newArtifactUploader( return new ByteStreamBuildEventArtifactUploader( uploader, missingDigestsFinder, - "localhost", + "localhost/instance", "none", "none", - "instance", /* maxUploadThreads= */ 100); } diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 1e4ab53f8e4b12..2724c6110861ca 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -1440,6 +1440,41 @@ EOF expect_log "uri:.*bytestream://localhost" } +function test_bytestream_uri_prefix() { + # Test that when --remote_bytestream_uri_prefix is set, bytestream:// + # URIs do not contain the hostname that's part of --remote_executor. + # They should use a fixed value instead. + mkdir -p a + cat > a/success.sh <<'EOF' +#!/bin/sh +exit 0 +EOF + chmod 755 a/success.sh + cat > a/BUILD <<'EOF' +sh_test( + name = "success_test", + srcs = ["success.sh"], +) + +genrule( + name = "foo", + srcs = [], + outs = ["foo.txt"], + cmd = "echo \"foo\" > \"$@\"", +) +EOF + + bazel test \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + --remote_bytestream_uri_prefix=example.com/my-instance-name \ + --build_event_text_file=$TEST_log \ + //a:foo //a:success_test || fail "Failed to test //a:foo //a:success_test" + + expect_not_log 'uri:.*file://' + expect_log "uri:.*bytestream://example.com/my-instance-name/blobs" +} + # This test is derivative of test_bep_output_groups in # build_event_stream_test.sh, which verifies that successful output groups' # artifacts appear in BEP when a top-level target fails to build.