RemoteModule only check required capabilities for a given endpoint
According to changes in bazelbuild/remote-apis#76 (and discussion in bazelbuild/remote-apis#61):
> Each endpoint should implement the Capabilities service, and populate the fields relevant to the services available at that endpoint. Clients may choose to ignore irrelevant capabilities if the client does not plan to use a given service on the specified endpoint.
This PR:
1. Refactor `RemoteModule` to use a `ChannelFactory` when creating channels so that tests can mock into channels connected to a fake server.
2. Add tests for verifying server capabilities.
3. Only check required capabilities for a given endpoint.
- If `--remote_executor` and `--remote_cache point` to the same endpoint, we require that endpoint has both execution and cache capabilities.
- If they point to different endpoints, we check the endpoint with execution or cache capabilities respectively.
Fixes #11901.
Closes #12008.
PiperOrigin-RevId: 329875749
diff --git a/src/main/java/com/google/devtools/build/lib/remote/ChannelFactory.java b/src/main/java/com/google/devtools/build/lib/remote/ChannelFactory.java
new file mode 100644
index 0000000..216f6cd
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/remote/ChannelFactory.java
@@ -0,0 +1,27 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.remote;
+
+import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
+import io.grpc.ClientInterceptor;
+import io.grpc.ManagedChannel;
+import java.io.IOException;
+import java.util.List;
+
+/** A factory interface for creating a {@link ManagedChannel}. */
+public interface ChannelFactory {
+ ManagedChannel newChannel(
+ String target, String proxy, AuthAndTLSOptions options, List<ClientInterceptor> interceptors)
+ throws IOException;
+}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCacheClientFactory.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCacheClientFactory.java
index 9989016..aef605b 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCacheClientFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCacheClientFactory.java
@@ -19,7 +19,6 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
-import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
import com.google.devtools.build.lib.remote.disk.DiskAndRemoteCacheClient;
import com.google.devtools.build.lib.remote.disk.DiskCacheClient;
@@ -58,17 +57,8 @@
return new DiskAndRemoteCacheClient(diskCacheClient, remoteCacheClient, options);
}
- public static ReferenceCountedChannel createGrpcChannel(
- String target,
- String proxyUri,
- AuthAndTLSOptions authOptions,
- @Nullable List<ClientInterceptor> interceptors)
- throws IOException {
- return new ReferenceCountedChannel(
- GoogleAuthUtils.newChannel(target, proxyUri, authOptions, interceptors));
- }
-
public static ReferenceCountedChannel createGrpcChannelPool(
+ ChannelFactory channelFactory,
int poolSize,
String target,
String proxyUri,
@@ -77,7 +67,7 @@
throws IOException {
List<ManagedChannel> channels = new ArrayList<>();
for (int i = 0; i < poolSize; i++) {
- channels.add(GoogleAuthUtils.newChannel(target, proxyUri, authOptions, interceptors));
+ channels.add(channelFactory.newChannel(target, proxyUri, authOptions, interceptors));
}
return new ReferenceCountedChannelPool(ImmutableList.copyOf(channels));
}
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 aa56a3e..e64e9bf 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
@@ -18,6 +18,7 @@
import build.bazel.remote.execution.v2.RequestMetadata;
import build.bazel.remote.execution.v2.ServerCapabilities;
import com.google.auth.Credentials;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.base.Throwables;
@@ -50,6 +51,7 @@
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.remote.RemoteServerCapabilities.ServerCapabilitiesRequirement;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
import com.google.devtools.build.lib.remote.downloader.GrpcRemoteDownloader;
import com.google.devtools.build.lib.remote.logging.LoggingInterceptor;
@@ -83,6 +85,7 @@
import io.grpc.CallCredentials;
import io.grpc.ClientInterceptor;
import io.grpc.Context;
+import io.grpc.ManagedChannel;
import java.io.IOException;
import java.util.List;
import java.util.Optional;
@@ -103,6 +106,20 @@
private RemoteOutputsMode remoteOutputsMode;
private RemoteOutputService remoteOutputService;
+ private ChannelFactory channelFactory =
+ new ChannelFactory() {
+ @Override
+ public ManagedChannel newChannel(
+ String target,
+ String proxy,
+ AuthAndTLSOptions options,
+ List<ClientInterceptor> interceptors)
+ throws IOException {
+ return GoogleAuthUtils.newChannel(
+ target, proxy, options, interceptors.isEmpty() ? null : interceptors);
+ }
+ };
+
private final BuildEventArtifactUploaderFactoryDelegate
buildEventArtifactUploaderFactoryDelegate = new BuildEventArtifactUploaderFactoryDelegate();
@@ -135,7 +152,8 @@
CallCredentials credentials,
RemoteRetrier retrier,
CommandEnvironment env,
- DigestUtil digestUtil)
+ DigestUtil digestUtil,
+ ServerCapabilitiesRequirement requirement)
throws AbruptExitException {
RemoteServerCapabilities rsc =
new RemoteServerCapabilities(
@@ -158,7 +176,11 @@
return;
}
checkClientServerCompatibility(
- capabilities, remoteOptions, digestUtil.getDigestFunction(), env.getReporter());
+ capabilities,
+ remoteOptions,
+ digestUtil.getDigestFunction(),
+ env.getReporter(),
+ requirement);
}
@Override
@@ -286,6 +308,7 @@
try {
execChannel =
RemoteCacheClientFactory.createGrpcChannelPool(
+ channelFactory,
poolSize,
remoteOptions.remoteExecutor,
remoteOptions.remoteProxy,
@@ -312,6 +335,7 @@
try {
cacheChannel =
RemoteCacheClientFactory.createGrpcChannelPool(
+ channelFactory,
poolSize,
remoteOptions.remoteCache,
remoteOptions.remoteProxy,
@@ -336,6 +360,7 @@
try {
downloaderChannel =
RemoteCacheClientFactory.createGrpcChannelPool(
+ channelFactory,
poolSize,
remoteOptions.remoteDownloader,
remoteOptions.remoteProxy,
@@ -362,14 +387,50 @@
retryScheduler,
Retrier.ALLOW_ALL_CALLS);
- // We always query the execution server for capabilities, if it is defined. A remote
- // execution/cache system should have all its servers to return the capabilities pertaining
- // to the system as a whole.
+ // We only check required capabilities for a given endpoint.
+ //
+ // If --remote_executor and --remote_cache point to the same endpoint, we require that
+ // endpoint has both execution and cache capabilities.
+ //
+ // If they point to different endpoints, we check the endpoint with execution or cache
+ // capabilities respectively.
if (execChannel != null) {
- verifyServerCapabilities(remoteOptions, execChannel, credentials, retrier, env, digestUtil);
- }
- if (cacheChannel != execChannel) {
- verifyServerCapabilities(remoteOptions, cacheChannel, credentials, retrier, env, digestUtil);
+ if (cacheChannel != execChannel) {
+ verifyServerCapabilities(
+ remoteOptions,
+ execChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.EXECUTION);
+ verifyServerCapabilities(
+ remoteOptions,
+ cacheChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.CACHE);
+ } else {
+ verifyServerCapabilities(
+ remoteOptions,
+ execChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
+ }
+ } else {
+ verifyServerCapabilities(
+ remoteOptions,
+ cacheChannel,
+ credentials,
+ retrier,
+ env,
+ digestUtil,
+ ServerCapabilitiesRequirement.CACHE);
}
ByteStreamUploader uploader =
@@ -560,11 +621,12 @@
ServerCapabilities capabilities,
RemoteOptions remoteOptions,
DigestFunction.Value digestFunction,
- Reporter reporter)
+ Reporter reporter,
+ ServerCapabilitiesRequirement requirement)
throws AbruptExitException {
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- capabilities, remoteOptions, digestFunction);
+ capabilities, remoteOptions, digestFunction, requirement);
for (String warning : st.getWarnings()) {
reporter.handle(Event.warn(warning));
}
@@ -786,4 +848,9 @@
return delegate.create();
}
}
+
+ @VisibleForTesting
+ void setChannelFactory(ChannelFactory channelFactory) {
+ this.channelFactory = channelFactory;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java
index 84d3b42..bd528c3 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java
@@ -152,14 +152,27 @@
}
}
+ public enum ServerCapabilitiesRequirement {
+ NONE,
+ CACHE,
+ EXECUTION,
+ EXECUTION_AND_CACHE,
+ }
+
/** Compare the remote server capabilities with those requested by current execution. */
public static ClientServerCompatibilityStatus checkClientServerCompatibility(
ServerCapabilities capabilities,
RemoteOptions remoteOptions,
- DigestFunction.Value digestFunction) {
+ DigestFunction.Value digestFunction,
+ ServerCapabilitiesRequirement requirement) {
ClientServerCompatibilityStatus.Builder result = new ClientServerCompatibilityStatus.Builder();
- boolean remoteExecution = !Strings.isNullOrEmpty(remoteOptions.remoteExecutor);
- if (!remoteExecution && Strings.isNullOrEmpty(remoteOptions.remoteCache)) {
+ boolean shouldCheckExecutionCapabilities =
+ (requirement == ServerCapabilitiesRequirement.EXECUTION
+ || requirement == ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
+ boolean shouldCheckCacheCapabilities =
+ (requirement == ServerCapabilitiesRequirement.CACHE
+ || requirement == ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
+ if (!(shouldCheckCacheCapabilities || shouldCheckExecutionCapabilities)) {
return result.build();
}
@@ -173,17 +186,7 @@
result.addWarning(st.getMessage());
}
- // Check cache digest function.
- CacheCapabilities cacheCap = capabilities.getCacheCapabilities();
- if (!cacheCap.getDigestFunctionList().contains(digestFunction)) {
- result.addError(
- String.format(
- "Cannot use hash function %s with remote cache. "
- + "Server supported functions are: %s",
- digestFunction, cacheCap.getDigestFunctionList()));
- }
-
- if (remoteExecution) {
+ if (shouldCheckExecutionCapabilities) {
// Check remote execution is enabled.
ExecutionCapabilities execCap = capabilities.getExecutionCapabilities();
if (!execCap.getExecEnabled()) {
@@ -206,37 +209,54 @@
digestFunction, execCap.getDigestFunction()));
}
- // Check updating remote cache is allowed, if we ever need to do that.
- if (remoteOptions.remoteLocalFallback
- && remoteOptions.remoteUploadLocalResults
- && !cacheCap.getActionCacheUpdateCapabilities().getUpdateEnabled()) {
- result.addError(
- "--remote_local_fallback and --remote_upload_local_results are set, "
- + "but the current account is not authorized to write local results "
- + "to the remote cache.");
- }
// Check execution priority is in the supported range.
checkPriorityInRange(
remoteOptions.remoteExecutionPriority,
"remote_execution_priority",
execCap.getExecutionPriorityCapabilities(),
result);
- } else {
- // Local execution: check updating remote cache is allowed.
- if (remoteOptions.remoteUploadLocalResults
- && !cacheCap.getActionCacheUpdateCapabilities().getUpdateEnabled()) {
- result.addError(
- "--remote_upload_local_results is set, but the current account is not authorized "
- + "to write local results to the remote cache.");
- }
}
- // Check result cache priority is in the supported range.
- checkPriorityInRange(
- remoteOptions.remoteResultCachePriority,
- "remote_result_cache_priority",
- cacheCap.getCachePriorityCapabilities(),
- result);
+ if (shouldCheckCacheCapabilities) {
+ // Check cache digest function.
+ CacheCapabilities cacheCap = capabilities.getCacheCapabilities();
+ if (!cacheCap.getDigestFunctionList().contains(digestFunction)) {
+ result.addError(
+ String.format(
+ "Cannot use hash function %s with remote cache. "
+ + "Server supported functions are: %s",
+ digestFunction, cacheCap.getDigestFunctionList()));
+ }
+
+ // Check updating remote cache is allowed, if we ever need to do that.
+ boolean remoteExecution = !Strings.isNullOrEmpty(remoteOptions.remoteExecutor);
+ if (remoteExecution) {
+ if (remoteOptions.remoteLocalFallback
+ && remoteOptions.remoteUploadLocalResults
+ && !cacheCap.getActionCacheUpdateCapabilities().getUpdateEnabled()) {
+ result.addError(
+ "--remote_local_fallback and --remote_upload_local_results are set, "
+ + "but the current account is not authorized to write local results "
+ + "to the remote cache.");
+ }
+ } else {
+ // Local execution: check updating remote cache is allowed.
+ if (remoteOptions.remoteUploadLocalResults
+ && !cacheCap.getActionCacheUpdateCapabilities().getUpdateEnabled()) {
+ result.addError(
+ "--remote_upload_local_results is set, but the current account is not authorized "
+ + "to write local results to the remote cache.");
+ }
+ }
+
+ // Check result cache priority is in the supported range.
+ checkPriorityInRange(
+ remoteOptions.remoteResultCachePriority,
+ "remote_result_cache_priority",
+ cacheCap.getCachePriorityCapabilities(),
+ result);
+ }
+
return result.build();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD
index b6d9387..6e854c1 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD
@@ -46,7 +46,11 @@
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
+ "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
+ "//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
"//src/main/java/com/google/devtools/build/lib/analysis/platform:platform_utils",
"//src/main/java/com/google/devtools/build/lib/authandtls",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
@@ -54,11 +58,13 @@
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy",
+ "//src/main/java/com/google/devtools/build/lib/exec:bin_tools",
"//src/main/java/com/google/devtools/build/lib/exec:execution_options",
"//src/main/java/com/google/devtools/build/lib/exec:remote_local_fallback_registry",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_cache",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
+ "//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/remote",
"//src/main/java/com/google/devtools/build/lib/remote/common",
"//src/main/java/com/google/devtools/build/lib/remote/disk",
@@ -66,6 +72,7 @@
"//src/main/java/com/google/devtools/build/lib/remote/merkletree",
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/remote/util",
+ "//src/main/java/com/google/devtools/build/lib/runtime/commands",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java
new file mode 100644
index 0000000..47165f0
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java
@@ -0,0 +1,333 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.remote;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import build.bazel.remote.execution.v2.ActionCacheUpdateCapabilities;
+import build.bazel.remote.execution.v2.CacheCapabilities;
+import build.bazel.remote.execution.v2.CapabilitiesGrpc.CapabilitiesImplBase;
+import build.bazel.remote.execution.v2.DigestFunction.Value;
+import build.bazel.remote.execution.v2.ExecutionCapabilities;
+import build.bazel.remote.execution.v2.GetCapabilitiesRequest;
+import build.bazel.remote.execution.v2.ServerCapabilities;
+import build.bazel.semver.SemVer;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.analysis.ServerDirectories;
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.CoreOptions;
+import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
+import com.google.devtools.build.lib.exec.BinTools;
+import com.google.devtools.build.lib.pkgcache.PackageOptions;
+import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.runtime.BlazeModule;
+import com.google.devtools.build.lib.runtime.BlazeRuntime;
+import com.google.devtools.build.lib.runtime.BlazeServerStartupOptions;
+import com.google.devtools.build.lib.runtime.BlazeWorkspace;
+import com.google.devtools.build.lib.runtime.ClientOptions;
+import com.google.devtools.build.lib.runtime.Command;
+import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.runtime.CommonCommandOptions;
+import com.google.devtools.build.lib.runtime.commands.BuildCommand;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.common.options.Options;
+import com.google.devtools.common.options.OptionsParser;
+import com.google.devtools.common.options.OptionsParsingResult;
+import io.grpc.BindableService;
+import io.grpc.Server;
+import io.grpc.ServerInterceptors;
+import io.grpc.inprocess.InProcessChannelBuilder;
+import io.grpc.inprocess.InProcessServerBuilder;
+import io.grpc.stub.StreamObserver;
+import io.grpc.util.MutableHandlerRegistry;
+import java.io.IOException;
+import java.util.ArrayList;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link RemoteModule}. */
+@RunWith(JUnit4.class)
+public class RemoteModuleTest {
+
+ private static CommandEnvironment createTestCommandEnvironment(RemoteOptions remoteOptions)
+ throws IOException, AbruptExitException {
+ CoreOptions coreOptions = Options.getDefaults(CoreOptions.class);
+ CommonCommandOptions commonCommandOptions = Options.getDefaults(CommonCommandOptions.class);
+ PackageOptions packageOptions = Options.getDefaults(PackageOptions.class);
+ ClientOptions clientOptions = Options.getDefaults(ClientOptions.class);
+
+ AuthAndTLSOptions authAndTLSOptions = Options.getDefaults(AuthAndTLSOptions.class);
+
+ OptionsParsingResult options = mock(OptionsParsingResult.class);
+ when(options.getOptions(CoreOptions.class)).thenReturn(coreOptions);
+ when(options.getOptions(CommonCommandOptions.class)).thenReturn(commonCommandOptions);
+ when(options.getOptions(PackageOptions.class)).thenReturn(packageOptions);
+ when(options.getOptions(ClientOptions.class)).thenReturn(clientOptions);
+ when(options.getOptions(RemoteOptions.class)).thenReturn(remoteOptions);
+ when(options.getOptions(AuthAndTLSOptions.class)).thenReturn(authAndTLSOptions);
+
+ String productName = "bazel";
+ Scratch scratch = new Scratch();
+ ServerDirectories serverDirectories =
+ new ServerDirectories(
+ scratch.dir("install"), scratch.dir("output"), scratch.dir("user_root"));
+ BlazeRuntime runtime =
+ new BlazeRuntime.Builder()
+ .setProductName(productName)
+ .setFileSystem(scratch.getFileSystem())
+ .setServerDirectories(serverDirectories)
+ .setStartupOptionsProvider(
+ OptionsParser.builder().optionsClasses(BlazeServerStartupOptions.class).build())
+ .addBlazeModule(
+ new BlazeModule() {
+ @Override
+ public BuildOptions getDefaultBuildOptions(BlazeRuntime runtime) {
+ return BuildOptions.getDefaultBuildOptionsForFragments(
+ runtime.getRuleClassProvider().getConfigurationOptions());
+ }
+ })
+ .build();
+
+ BlazeDirectories directories =
+ new BlazeDirectories(
+ serverDirectories,
+ scratch.dir("/workspace"),
+ scratch.dir("/system_javabase"),
+ productName);
+ BlazeWorkspace workspace = runtime.initWorkspace(directories, BinTools.empty(directories));
+ Command command = BuildCommand.class.getAnnotation(Command.class);
+ return workspace.initCommand(command, options, new ArrayList<>(), 0, 0);
+ }
+
+ static class CapabilitiesImpl extends CapabilitiesImplBase {
+
+ private int requestCount;
+ private final ServerCapabilities caps;
+
+ CapabilitiesImpl(ServerCapabilities caps) {
+ this.caps = caps;
+ }
+
+ @Override
+ public void getCapabilities(
+ GetCapabilitiesRequest request, StreamObserver<ServerCapabilities> responseObserver) {
+ ++requestCount;
+ responseObserver.onNext(caps);
+ responseObserver.onCompleted();
+ }
+
+ public int getRequestCount() {
+ return requestCount;
+ }
+ }
+
+ private static Server createFakeServer(String serverName, BindableService... services) {
+ MutableHandlerRegistry executionServerRegistry = new MutableHandlerRegistry();
+ for (BindableService service : services) {
+ executionServerRegistry.addService(ServerInterceptors.intercept(service));
+ }
+ return InProcessServerBuilder.forName(serverName)
+ .fallbackHandlerRegistry(executionServerRegistry)
+ .directExecutor()
+ .build();
+ }
+
+ @Test
+ public void testVerifyCapabilities_executionAndCacheForSingleEndpoint() throws Exception {
+ ServerCapabilities caps =
+ ServerCapabilities.newBuilder()
+ .setLowApiVersion(ApiVersion.current.toSemVer())
+ .setHighApiVersion(ApiVersion.current.toSemVer())
+ .setExecutionCapabilities(
+ ExecutionCapabilities.newBuilder()
+ .setExecEnabled(true)
+ .setDigestFunction(Value.SHA256)
+ .build())
+ .setCacheCapabilities(
+ CacheCapabilities.newBuilder().addDigestFunction(Value.SHA256).build())
+ .build();
+ CapabilitiesImpl executionServerCapabilitiesImpl = new CapabilitiesImpl(caps);
+ String executionServerName = "execution-server";
+ Server executionServer = createFakeServer(executionServerName, executionServerCapabilitiesImpl);
+ executionServer.start();
+
+ try {
+ RemoteModule remoteModule = new RemoteModule();
+ remoteModule.setChannelFactory(
+ (target, proxy, options, interceptors) ->
+ InProcessChannelBuilder.forName(target).directExecutor().build());
+
+ RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+ remoteOptions.remoteExecutor = executionServerName;
+
+ CommandEnvironment env = createTestCommandEnvironment(remoteOptions);
+
+ remoteModule.beforeCommand(env);
+
+ assertThat(Thread.interrupted()).isFalse();
+ assertThat(executionServerCapabilitiesImpl.getRequestCount()).isEqualTo(1);
+ } finally {
+ executionServer.shutdownNow();
+ executionServer.awaitTermination();
+ }
+ }
+
+ @Test
+ public void testVerifyCapabilities_cacheOnlyEndpoint() throws Exception {
+ ServerCapabilities cacheOnlyCaps =
+ ServerCapabilities.newBuilder()
+ .setLowApiVersion(ApiVersion.current.toSemVer())
+ .setHighApiVersion(ApiVersion.current.toSemVer())
+ .setCacheCapabilities(
+ CacheCapabilities.newBuilder()
+ .addDigestFunction(Value.SHA256)
+ .setActionCacheUpdateCapabilities(
+ ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
+ .build())
+ .build();
+ CapabilitiesImpl cacheServerCapabilitiesImpl = new CapabilitiesImpl(cacheOnlyCaps);
+ String cacheServerName = "cache-server";
+ Server cacheServer = createFakeServer(cacheServerName, cacheServerCapabilitiesImpl);
+ cacheServer.start();
+
+ try {
+ RemoteModule remoteModule = new RemoteModule();
+ remoteModule.setChannelFactory(
+ (target, proxy, options, interceptors) ->
+ InProcessChannelBuilder.forName(target).directExecutor().build());
+
+ RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+ remoteOptions.remoteCache = cacheServerName;
+
+ CommandEnvironment env = createTestCommandEnvironment(remoteOptions);
+
+ remoteModule.beforeCommand(env);
+
+ assertThat(Thread.interrupted()).isFalse();
+ assertThat(cacheServerCapabilitiesImpl.getRequestCount()).isEqualTo(1);
+ } finally {
+ cacheServer.shutdownNow();
+ cacheServer.awaitTermination();
+ }
+ }
+
+ @Test
+ public void testVerifyCapabilities_executionAndCacheForDifferentEndpoints() throws Exception {
+ ServerCapabilities caps =
+ ServerCapabilities.newBuilder()
+ .setLowApiVersion(ApiVersion.current.toSemVer())
+ .setHighApiVersion(ApiVersion.current.toSemVer())
+ .setExecutionCapabilities(
+ ExecutionCapabilities.newBuilder()
+ .setExecEnabled(true)
+ .setDigestFunction(Value.SHA256)
+ .build())
+ .setCacheCapabilities(
+ CacheCapabilities.newBuilder().addDigestFunction(Value.SHA256).build())
+ .build();
+ CapabilitiesImpl executionServerCapabilitiesImpl = new CapabilitiesImpl(caps);
+ String executionServerName = "execution-server";
+ Server executionServer = createFakeServer(executionServerName, executionServerCapabilitiesImpl);
+ executionServer.start();
+
+ CapabilitiesImpl cacheServerCapabilitiesImpl = new CapabilitiesImpl(caps);
+ String cacheServerName = "cache-server";
+ Server cacheServer = createFakeServer(cacheServerName, cacheServerCapabilitiesImpl);
+ cacheServer.start();
+
+ try {
+ RemoteModule remoteModule = new RemoteModule();
+ remoteModule.setChannelFactory(
+ (target, proxy, options, interceptors) ->
+ InProcessChannelBuilder.forName(target).directExecutor().build());
+
+ RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+ remoteOptions.remoteExecutor = executionServerName;
+ remoteOptions.remoteCache = cacheServerName;
+
+ CommandEnvironment env = createTestCommandEnvironment(remoteOptions);
+
+ remoteModule.beforeCommand(env);
+
+ assertThat(Thread.interrupted()).isFalse();
+ assertThat(executionServerCapabilitiesImpl.getRequestCount()).isEqualTo(1);
+ assertThat(cacheServerCapabilitiesImpl.getRequestCount()).isEqualTo(1);
+ } finally {
+ executionServer.shutdownNow();
+ cacheServer.shutdownNow();
+
+ executionServer.awaitTermination();
+ cacheServer.awaitTermination();
+ }
+ }
+
+ @Test
+ public void testVerifyCapabilities_executionOnlyAndCacheOnlyEndpoints() throws Exception {
+ ServerCapabilities executionOnlyCaps =
+ ServerCapabilities.newBuilder()
+ .setLowApiVersion(ApiVersion.current.toSemVer())
+ .setHighApiVersion(ApiVersion.current.toSemVer())
+ .setExecutionCapabilities(
+ ExecutionCapabilities.newBuilder()
+ .setExecEnabled(true)
+ .setDigestFunction(Value.SHA256)
+ .build())
+ .build();
+ CapabilitiesImpl executionServerCapabilitiesImpl = new CapabilitiesImpl(executionOnlyCaps);
+ String executionServerName = "execution-server";
+ Server executionServer = createFakeServer(executionServerName, executionServerCapabilitiesImpl);
+ executionServer.start();
+
+ ServerCapabilities cacheOnlyCaps =
+ ServerCapabilities.newBuilder()
+ .setLowApiVersion(SemVer.newBuilder().setMajor(2).build())
+ .setHighApiVersion(SemVer.newBuilder().setMajor(2).build())
+ .setCacheCapabilities(
+ CacheCapabilities.newBuilder().addDigestFunction(Value.SHA256).build())
+ .build();
+ CapabilitiesImpl cacheServerCapabilitiesImpl = new CapabilitiesImpl(cacheOnlyCaps);
+ String cacheServerName = "cache-server";
+ Server cacheServer = createFakeServer(cacheServerName, cacheServerCapabilitiesImpl);
+ cacheServer.start();
+
+ try {
+ RemoteModule remoteModule = new RemoteModule();
+ remoteModule.setChannelFactory(
+ (target, proxy, options, interceptors) ->
+ InProcessChannelBuilder.forName(target).directExecutor().build());
+
+ RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+ remoteOptions.remoteExecutor = executionServerName;
+ remoteOptions.remoteCache = cacheServerName;
+
+ CommandEnvironment env = createTestCommandEnvironment(remoteOptions);
+
+ remoteModule.beforeCommand(env);
+
+ assertThat(Thread.interrupted()).isFalse();
+ assertThat(executionServerCapabilitiesImpl.getRequestCount()).isEqualTo(1);
+ assertThat(cacheServerCapabilitiesImpl.getRequestCount()).isEqualTo(1);
+ } finally {
+ executionServer.shutdownNow();
+ cacheServer.shutdownNow();
+
+ executionServer.awaitTermination();
+ cacheServer.awaitTermination();
+ }
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java
index 62d567e5..e490854 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java
@@ -33,6 +33,7 @@
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.remote.RemoteRetrier.ExponentialBackoff;
+import com.google.devtools.build.lib.remote.RemoteServerCapabilities.ServerCapabilitiesRequirement;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.TestUtils;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
@@ -209,7 +210,8 @@
RemoteServerCapabilities.checkClientServerCompatibility(
ServerCapabilities.getDefaultInstance(),
Options.getDefaults(RemoteOptions.class),
- DigestFunction.Value.SHA256);
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.NONE);
assertThat(st.isOk()).isTrue();
}
@@ -231,7 +233,7 @@
remoteOptions.remoteCache = "server:port";
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.getErrors()).isEmpty();
assertThat(st.getWarnings()).hasSize(1);
assertThat(st.getWarnings().get(0)).containsMatch("API.*deprecated.*100.0");
@@ -254,7 +256,7 @@
remoteOptions.remoteCache = "server:port";
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0)).containsMatch("API.*not supported.*100.0");
}
@@ -277,7 +279,7 @@
remoteOptions.remoteCache = "server:port";
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0)).containsMatch("Cannot use hash function");
}
@@ -298,7 +300,7 @@
remoteOptions.remoteCache = "server:port";
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0))
.containsMatch("not authorized to write local results to the remote cache");
@@ -307,7 +309,7 @@
remoteOptions.remoteUploadLocalResults = false;
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.isOk()).isTrue();
}
@@ -332,7 +334,10 @@
remoteOptions.remoteExecutor = "server:port";
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0)).containsMatch("Remote execution is not supported");
assertThat(st.getErrors().get(0)).containsMatch("not authorized to use remote execution");
@@ -361,7 +366,10 @@
remoteOptions.remoteExecutor = "server:port";
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0)).containsMatch("Cannot use hash function");
}
@@ -388,7 +396,10 @@
remoteOptions.remoteLocalFallback = true;
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0))
.containsMatch("not authorized to write local results to the remote cache");
@@ -397,7 +408,10 @@
remoteOptions.remoteLocalFallback = false;
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.isOk()).isTrue();
// Ignored when no uploading local results.
@@ -405,7 +419,10 @@
remoteOptions.remoteUploadLocalResults = false;
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.isOk()).isTrue();
}
@@ -432,7 +449,7 @@
remoteOptions.remoteResultCachePriority = 11;
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0)).containsMatch("remote_result_cache_priority");
@@ -440,14 +457,14 @@
remoteOptions.remoteResultCachePriority = 10;
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.isOk()).isTrue();
// Check not performed if the value is 0.
remoteOptions.remoteResultCachePriority = 0;
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.isOk()).isTrue();
}
@@ -479,7 +496,10 @@
remoteOptions.remoteExecutionPriority = 11;
RemoteServerCapabilities.ClientServerCompatibilityStatus st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.getErrors()).hasSize(1);
assertThat(st.getErrors().get(0)).containsMatch("remote_execution_priority");
@@ -487,14 +507,20 @@
remoteOptions.remoteExecutionPriority = 10;
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.isOk()).isTrue();
// Check not performed if the value is 0.
remoteOptions.remoteExecutionPriority = 0;
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION_AND_CACHE);
assertThat(st.isOk()).isTrue();
// Ignored when no remote execution requested.
@@ -503,7 +529,51 @@
remoteOptions.remoteCache = "server:port";
st =
RemoteServerCapabilities.checkClientServerCompatibility(
- caps, remoteOptions, DigestFunction.Value.SHA256);
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
+ assertThat(st.isOk()).isTrue();
+ }
+
+ @Test
+ public void testCheckClientServerCompatibility_executionCapsOnly() throws Exception {
+ ServerCapabilities caps =
+ ServerCapabilities.newBuilder()
+ .setLowApiVersion(ApiVersion.current.toSemVer())
+ .setHighApiVersion(ApiVersion.current.toSemVer())
+ .setExecutionCapabilities(
+ ExecutionCapabilities.newBuilder()
+ .setDigestFunction(DigestFunction.Value.SHA256)
+ .setExecEnabled(true)
+ .build())
+ .build();
+ RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+ remoteOptions.remoteExecutor = "server:port";
+ RemoteServerCapabilities.ClientServerCompatibilityStatus st =
+ RemoteServerCapabilities.checkClientServerCompatibility(
+ caps,
+ remoteOptions,
+ DigestFunction.Value.SHA256,
+ ServerCapabilitiesRequirement.EXECUTION);
+ assertThat(st.isOk()).isTrue();
+ }
+
+ @Test
+ public void testCheckClientServerCompatibility_cacheCapsOnly() throws Exception {
+ ServerCapabilities caps =
+ ServerCapabilities.newBuilder()
+ .setLowApiVersion(ApiVersion.current.toSemVer())
+ .setHighApiVersion(ApiVersion.current.toSemVer())
+ .setCacheCapabilities(
+ CacheCapabilities.newBuilder()
+ .addDigestFunction(DigestFunction.Value.SHA256)
+ .setActionCacheUpdateCapabilities(
+ ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
+ .build())
+ .build();
+ RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+ remoteOptions.remoteCache = "server:port";
+ RemoteServerCapabilities.ClientServerCompatibilityStatus st =
+ RemoteServerCapabilities.checkClientServerCompatibility(
+ caps, remoteOptions, DigestFunction.Value.SHA256, ServerCapabilitiesRequirement.CACHE);
assertThat(st.isOk()).isTrue();
}
}
