Make the fallback strategy for Bazel's remote execution configurable.
RELNOTES: When using Bazel's remote execution feature and Bazel has to
fallback to local execution for an action, Bazel used non-sandboxed
local execution until now. From this release on, you can use the new
flag --remote_local_fallback_strategy=<strategy> to tell Bazel which
strategy to use in that case.
PiperOrigin-RevId: 206566380
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
index cb40215..4be5e97 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
@@ -58,6 +58,17 @@
this.spawnRunner = spawnRunner;
}
+ /**
+ * Get's the {@link SpawnRunner} that this {@link AbstractSpawnStrategy} uses to actually run
+ * spawns.
+ *
+ * <p>This is considered a stop-gap until we refactor the entire SpawnStrategy / SpawnRunner
+ * mechanism to no longer need Spawn strategies.
+ */
+ public SpawnRunner getSpawnRunner() {
+ return spawnRunner;
+ }
+
@Override
public List<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index 88bc907..2b44f48 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -15,22 +15,24 @@
import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.ActionContext;
-import com.google.devtools.build.lib.actions.ResourceManager;
+import com.google.devtools.build.lib.actions.ExecutionStrategy;
+import com.google.devtools.build.lib.actions.ExecutorInitException;
+import com.google.devtools.build.lib.exec.AbstractSpawnStrategy;
import com.google.devtools.build.lib.exec.ActionContextProvider;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.SpawnRunner;
-import com.google.devtools.build.lib.exec.apple.XcodeLocalEnvProvider;
-import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
-import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
-import com.google.devtools.build.lib.exec.local.LocalSpawnRunner;
-import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider;
-import com.google.devtools.build.lib.exec.local.WindowsLocalEnvProvider;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
-import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.vfs.Path;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
/**
@@ -43,6 +45,7 @@
private final RemoteRetrier retrier;
private final DigestUtil digestUtil;
private final Path logDir;
+ private final AtomicReference<SpawnRunner> fallbackRunner = new AtomicReference<>();
RemoteActionContextProvider(
CommandEnvironment env,
@@ -64,7 +67,7 @@
ExecutionOptions executionOptions =
checkNotNull(env.getOptions().getOptions(ExecutionOptions.class));
RemoteOptions remoteOptions = checkNotNull(env.getOptions().getOptions(RemoteOptions.class));
- String buildRequestId = env.getBuildRequestId().toString();
+ String buildRequestId = env.getBuildRequestId();
String commandId = env.getCommandId().toString();
if (executor == null && cache != null) {
@@ -84,7 +87,7 @@
env.getExecRoot(),
remoteOptions,
env.getOptions().getOptions(ExecutionOptions.class),
- createFallbackRunner(env),
+ fallbackRunner,
executionOptions.verboseFailures,
env.getReporter(),
buildRequestId,
@@ -98,21 +101,37 @@
}
}
- private static SpawnRunner createFallbackRunner(CommandEnvironment env) {
- LocalExecutionOptions localExecutionOptions =
- env.getOptions().getOptions(LocalExecutionOptions.class);
- LocalEnvProvider localEnvProvider =
- OS.getCurrent() == OS.DARWIN
- ? new XcodeLocalEnvProvider(env.getClientEnv())
- : (OS.getCurrent() == OS.WINDOWS
- ? new WindowsLocalEnvProvider(env.getClientEnv())
- : new PosixLocalEnvProvider(env.getClientEnv()));
- return
- new LocalSpawnRunner(
- env.getExecRoot(),
- localExecutionOptions,
- ResourceManager.instance(),
- localEnvProvider);
+ @Override
+ public void executorCreated(Iterable<ActionContext> usedContexts) throws ExecutorInitException {
+ SortedSet<String> validStrategies = new TreeSet<>();
+ fallbackRunner.set(null);
+
+ RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
+ String strategyName = remoteOptions.remoteLocalFallbackStrategy;
+
+ for (ActionContext context : usedContexts) {
+ if (context instanceof AbstractSpawnStrategy) {
+ ExecutionStrategy annotation = context.getClass().getAnnotation(ExecutionStrategy.class);
+ if (annotation != null) {
+ Collections.addAll(validStrategies, annotation.name());
+ if (!strategyName.equals("remote")
+ && Arrays.asList(annotation.name()).contains(strategyName)) {
+ AbstractSpawnStrategy spawnStrategy = (AbstractSpawnStrategy) context;
+ SpawnRunner spawnRunner = Preconditions.checkNotNull(spawnStrategy.getSpawnRunner());
+ fallbackRunner.set(spawnRunner);
+ }
+ }
+ }
+ }
+
+ if (fallbackRunner.get() == null) {
+ validStrategies.remove("remote");
+ throw new ExecutorInitException(
+ String.format(
+ "'%s' is an invalid value for --remote_local_fallback_strategy. Valid values are: %s",
+ strategyName, validStrategies),
+ ExitCode.COMMAND_LINE_ERROR);
+ }
}
@Override
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 7141d35..14f373e 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
@@ -280,14 +280,8 @@
@Override
public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
return "build".equals(command.name())
- ? ImmutableList.<Class<? extends OptionsBase>>of(
- RemoteOptions.class, AuthAndTLSOptions.class)
- : ImmutableList.<Class<? extends OptionsBase>>of();
- }
-
- public static boolean remoteEnabled(RemoteOptions options) {
- return SimpleBlobStoreFactory.isRemoteCacheOptions(options)
- || GrpcRemoteCache.isRemoteCacheOptions(options);
+ ? ImmutableList.of(RemoteOptions.class, AuthAndTLSOptions.class)
+ : ImmutableList.of();
}
static RemoteRetrier createExecuteRetrier(
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
index 90f4207..b73784f 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
@@ -103,6 +103,14 @@
public boolean remoteLocalFallback;
@Option(
+ name = "remote_local_fallback_strategy",
+ defaultValue = "local",
+ documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+ effectTags = {OptionEffectTag.UNKNOWN},
+ help = "The strategy to use when remote execution has to fallback to local execution.")
+ public String remoteLocalFallbackStrategy;
+
+ @Option(
name = "remote_upload_local_results",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 95511d0..e99cc6e 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -72,6 +72,7 @@
import java.util.SortedMap;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
/** A client for the remote execution service. */
@@ -82,7 +83,7 @@
private final Path execRoot;
private final RemoteOptions remoteOptions;
private final ExecutionOptions executionOptions;
- private final SpawnRunner fallbackRunner;
+ private final AtomicReference<SpawnRunner> fallbackRunner;
private final boolean verboseFailures;
@Nullable private final Reporter cmdlineReporter;
@@ -101,7 +102,7 @@
Path execRoot,
RemoteOptions remoteOptions,
ExecutionOptions executionOptions,
- SpawnRunner fallbackRunner,
+ AtomicReference<SpawnRunner> fallbackRunner,
boolean verboseFailures,
@Nullable Reporter cmdlineReporter,
String buildRequestId,
@@ -135,7 +136,7 @@
public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
throws ExecException, InterruptedException, IOException {
if (!Spawns.mayBeExecutedRemotely(spawn) || remoteCache == null) {
- return fallbackRunner.exec(spawn, context);
+ return fallbackRunner.get().exec(spawn, context);
}
context.report(ProgressStatus.EXECUTING, getName());
@@ -445,7 +446,7 @@
if (uploadToCache && remoteCache != null && actionKey != null) {
return execLocallyAndUpload(spawn, context, inputMap, remoteCache, actionKey);
}
- return fallbackRunner.exec(spawn, context);
+ return fallbackRunner.get().exec(spawn, context);
}
@VisibleForTesting
@@ -457,7 +458,7 @@
ActionKey actionKey)
throws ExecException, IOException, InterruptedException {
Map<Path, Long> ctimesBefore = getInputCtimes(inputMap);
- SpawnResult result = fallbackRunner.exec(spawn, context);
+ SpawnResult result = fallbackRunner.get().exec(spawn, context);
Map<Path, Long> ctimesAfter = getInputCtimes(inputMap);
for (Map.Entry<Path, Long> e : ctimesBefore.entrySet()) {
// Skip uploading to remote cache, because an input was modified during execution.
@@ -494,7 +495,10 @@
}
}
- /** Resolve a collection of {@link com.google.build.lib.actions.ActionInput}s to {@link Path}s. */
+ /**
+ * Resolve a collection of {@link com.google.devtools.build.lib.actions.ActionInput}s to {@link
+ * Path}s.
+ */
static Collection<Path> resolveActionInputs(
Path execRoot, Collection<? extends ActionInput> actionInputs) {
return actionInputs
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index 6b024b5..12d77f2 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -82,6 +82,7 @@
import java.util.Collection;
import java.util.SortedMap;
import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicReference;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
@@ -168,7 +169,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -228,7 +229,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -282,7 +283,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -333,7 +334,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -369,7 +370,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
false,
reporter,
"build-req-id",
@@ -427,7 +428,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -468,7 +469,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -506,7 +507,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -543,7 +544,7 @@
execRoot,
Options.getDefaults(RemoteOptions.class),
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -585,7 +586,7 @@
execRoot,
Options.getDefaults(RemoteOptions.class),
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -630,7 +631,7 @@
execRoot,
Options.getDefaults(RemoteOptions.class),
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -670,7 +671,7 @@
execRoot,
Options.getDefaults(RemoteOptions.class),
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -712,7 +713,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -755,7 +756,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -804,7 +805,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -851,7 +852,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -893,7 +894,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -931,7 +932,7 @@
execRoot,
options,
Options.getDefaults(ExecutionOptions.class),
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
@@ -966,7 +967,7 @@
execRoot,
Options.getDefaults(RemoteOptions.class),
executionOptions,
- localRunner,
+ new AtomicReference<>(localRunner),
true,
/*cmdlineReporter=*/ null,
"build-req-id",
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index d5e0b9f..9b2ac86 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -171,6 +171,54 @@
# TODO(ulfjack): Check that the test failure gets reported correctly.
}
+function test_local_fallback_works_with_local_strategy() {
+ mkdir -p gen1
+ cat > gen1/BUILD <<'EOF'
+genrule(
+name = "gen1",
+srcs = [],
+outs = ["out1"],
+cmd = "touch \"$@\"",
+tags = ["no-remote"],
+)
+EOF
+
+ bazel build \
+ --spawn_strategy=remote \
+ --remote_executor=localhost:${worker_port} \
+ --remote_local_fallback_strategy=local \
+ --build_event_text_file=gen1.log \
+ //gen1 >& $TEST_log \
+ || fail "Expected success"
+
+ mv gen1.log $TEST_log
+ expect_log "1 process: 1 local"
+}
+
+function test_local_fallback_works_with_sandboxed_strategy() {
+ mkdir -p gen2
+ cat > gen2/BUILD <<'EOF'
+genrule(
+name = "gen2",
+srcs = [],
+outs = ["out2"],
+cmd = "touch \"$@\"",
+tags = ["no-remote"],
+)
+EOF
+
+ bazel build \
+ --spawn_strategy=remote \
+ --remote_executor=localhost:${worker_port} \
+ --remote_local_fallback_strategy=sandboxed \
+ --build_event_text_file=gen2.log \
+ //gen2 >& $TEST_log \
+ || fail "Expected success"
+
+ mv gen2.log $TEST_log
+ expect_log "1 process: 1 .*-sandbox"
+}
+
function is_file_uploaded() {
h=$(shasum -a256 < $1)
if [ -e "$cas_path/${h:0:64}" ]; then return 0; else return 1; fi
