workers: WorkerSpawnStrategy will now only execute SpawnActions if their ExecutionInfo contains the tag "support-workers" set to "1" and fallback to non-worker execution if it is not present.

This will eventually allow us to safely automatically decide whether to use workers to execute an action or not.

RELNOTES[INC]: If you maintain a rule that uses persistent workers, you'll have to specify execution_requirements={"supports-workers": 1} in the ctx.action that intends to run a tool with workers. The WorkerSpawnStrategy will alert you with a warning message if you forget to make this change and fallback to non-worker based execution.

--
MOS_MIGRATED_REVID=127822788

2 files changed, 45 insertions, 16 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
index dd07b80013..b84e96991e 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
@@ -45,7 +45,6 @@ import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
 import com.google.devtools.common.options.OptionsClassProvider;
 import com.google.protobuf.ByteString;
-
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
@@ -61,14 +60,16 @@ import java.util.List;
   contextType = SpawnActionContext.class
 )
 public final class WorkerSpawnStrategy implements SpawnActionContext {
+  public static final String ERROR_MESSAGE_PREFIX =
+      "Worker strategy cannot execute this %s action, ";
   public static final String REASON_NO_FLAGFILE =
-      "Not using worker strategy, because last argument does not contain a @flagfile";
-  public static final String REASON_NO_TOOLS =
-      "Not using worker strategy, because the action has no tools";
+      "because the last argument does not contain a @flagfile";
+  public static final String REASON_NO_TOOLS = "because the action has no tools";
+  public static final String REASON_NO_EXECUTION_INFO =
+      "because the action's execution info does not contain 'supports-workers=1'";
 
   private final Path execRoot;
   private final WorkerPool workers;
-  private final WorkerOptions options;
   private final boolean verboseFailures;
   private final int maxRetries;
 
@@ -79,7 +80,6 @@ public final class WorkerSpawnStrategy implements SpawnActionContext {
       boolean verboseFailures,
       int maxRetries) {
     Preconditions.checkNotNull(optionsProvider);
-    this.options = optionsProvider.getOptions(WorkerOptions.class);
     this.workers = Preconditions.checkNotNull(workers);
     this.execRoot = blazeDirs.getExecRoot();
     this.verboseFailures = verboseFailures;
@@ -103,24 +103,27 @@ public final class WorkerSpawnStrategy implements SpawnActionContext {
           spawn.asShellCommand(executor.getExecRoot()));
     }
 
+    if (!spawn.getExecutionInfo().containsKey("supports-workers")
+        || !spawn.getExecutionInfo().get("supports-workers").equals("1")) {
+      eventHandler.handle(
+          Event.warn(
+              String.format(ERROR_MESSAGE_PREFIX + REASON_NO_EXECUTION_INFO, spawn.getMnemonic())));
+      standaloneStrategy.exec(spawn, actionExecutionContext);
+      return;
+    }
+
     // We assume that the spawn to be executed always gets a @flagfile argument, which contains the
     // flags related to the work itself (as opposed to start-up options for the executed tool).
     // Thus, we can extract the last element from its args (which will be the @flagfile), expand it
     // and put that into the WorkRequest instead.
     if (!Iterables.getLast(spawn.getArguments()).startsWith("@")) {
-      if (options.workerVerbose) {
-        eventHandler.handle(Event.info(REASON_NO_FLAGFILE));
-      }
-      standaloneStrategy.exec(spawn, actionExecutionContext);
-      return;
+      throw new UserExecException(
+          String.format(ERROR_MESSAGE_PREFIX + REASON_NO_FLAGFILE, spawn.getMnemonic()));
     }
 
     if (Iterables.isEmpty(spawn.getToolFiles())) {
-      if (options.workerVerbose) {
-        eventHandler.handle(Event.info(REASON_NO_TOOLS));
-      }
-      standaloneStrategy.exec(spawn, actionExecutionContext);
-      return;
+      throw new UserExecException(
+          String.format(ERROR_MESSAGE_PREFIX + REASON_NO_TOOLS, spawn.getMnemonic()));
     }
 
     executor
diff --git a/src/test/shell/bazel/bazel_worker_test.sh b/src/test/shell/bazel/bazel_worker_test.sh
index 274e67320d..91e2749230 100755
--- a/src/test/shell/bazel/bazel_worker_test.sh
+++ b/src/test/shell/bazel/bazel_worker_test.sh
@@ -99,6 +99,7 @@ def _impl(ctx):
       executable=worker,
       progress_message="Working on %s" % ctx.label.name,
       mnemonic="Work",
+      execution_requirements={"supports-workers": "1"},
       arguments=ctx.attr.worker_args + ["@" + argfile.path],
   )
 
@@ -408,4 +409,29 @@ EOF
     || fail "Worker log was not deleted"
 }
 
+function test_missing_execution_requirements_gives_warning() {
+  prepare_example_worker
+  cat >>BUILD <<'EOF'
+work(
+  name = "hello_world",
+  worker = ":worker",
+  args = ["--write_uuid", "--write_counter"],
+)
+EOF
+
+  sed -i.bak '/execution_requirements/d' work.bzl
+  rm -f work.bzl.bak
+
+  bazel build --worker_verbose --strategy=Work=worker --worker_max_instances=1 --worker_quit_after_build :hello_world &> $TEST_log \
+    || fail "build failed"
+
+  expect_log "Worker strategy cannot execute this Work action, because the action's execution info does not contain 'supports-workers=1'"
+  expect_not_log "Created new Work worker (id [0-9]\+)"
+  expect_not_log "Destroying Work worker (id [0-9]\+)"
+
+  # WorkerSpawnStrategy falls back to standalone strategy, so we still expect the output to be generated.
+  [ -e "bazel-bin/hello_world.out" ] \
+    || fail "Worker did not produce output"
+}
+
 run_suite "Worker integration tests"