From 412b01e98a813beba6975526c1dbfe5755565c7d Mon Sep 17 00:00:00 2001
From: Julio Merino <julio.merino+oss@snowflake.com>
Date: Wed, 19 Jul 2023 01:47:16 -0700
Subject: [PATCH] Add the remote_require_cached flag

When set to true, this flag causes Bazel to abort the build whenever it encounters an action that is not cached.

This is very useful when trying to troubleshoot action caching issues across machines because it allows running a build on one and having it fail on another as soon as there is a problem without tainting what already exists in the cache.

My workflow is to essentially do:

1. Machine 1: bazel clean
2. Machine 1: bazel build ...
3. Machine 2: bazel clean
4. Machine 2: bazel build --remote_require_cached ...

which makes step 4 fail on the first action that wasn't cached as expected. Then I can address that problem and re-run step 4 to encounter the next issue.

Closes #18942.

PiperOrigin-RevId: 549242966
Change-Id: Ib46a2eb8cce6f4444968882e99c21284fc6bc4f8
---
 .../build/lib/remote/RemoteSpawnRunner.java   | 16 ++++++++++
 .../lib/remote/options/RemoteOptions.java     | 12 ++++++++
 .../bazel/remote/remote_execution_test.sh     | 30 +++++++++++++++++++
 3 files changed, 58 insertions(+)

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 4be0774599b770..b23b585ab24e51 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
@@ -237,6 +237,22 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
       return execLocallyAndUploadOrFail(action, spawn, context, uploadLocalResults, e);
     }
 
+    if (remoteOptions.remoteRequireCached) {
+      return new SpawnResult.Builder()
+          .setStatus(SpawnResult.Status.EXECUTION_DENIED)
+          .setExitCode(1)
+          .setFailureMessage(
+              "Action must be cached due to --experimental_remote_require_cached but it is not")
+          .setFailureDetail(
+              FailureDetail.newBuilder()
+                  .setSpawn(
+                      FailureDetails.Spawn.newBuilder()
+                          .setCode(FailureDetails.Spawn.Code.EXECUTION_DENIED))
+                  .build())
+          .setRunnerName("remote")
+          .build();
+    }
+
     AtomicBoolean useCachedResult = new AtomicBoolean(acceptCachedResult);
     AtomicBoolean forceUploadInput = new AtomicBoolean(false);
     try {
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 793eccdfa51756..092d5cafdd31ff 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
@@ -250,6 +250,18 @@ public String getTypeDescription() {
       help = "Whether to accept remotely cached action results.")
   public boolean remoteAcceptCached;
 
+  @Option(
+      name = "experimental_remote_require_cached",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.REMOTE,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help =
+          "If set to true, enforce that all actions that can run remotely are cached, or else "
+              + "fail the build. This is useful to troubleshoot non-determinism issues as it "
+              + "allows checking whether actions that should be cached are actually cached "
+              + "without spuriously injecting new results into the cache.")
+  public boolean remoteRequireCached;
+
   @Option(
       name = "remote_local_fallback",
       defaultValue = "false",
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index ff06d5d7c5cfb2..ffe8f428457a63 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -1253,6 +1253,36 @@ EOF
   expect_not_log "1 local"
 }
 
+function test_require_cached() {
+  mkdir -p a
+  cat > a/BUILD <<'EOF'
+genrule(
+  name = "foo",
+  srcs = ["foo.in"],
+  outs = ["foo.out"],
+  cmd = "cp \"$<\" \"$@\"",
+)
+EOF
+
+  echo "input 1" >a/foo.in
+  bazel build \
+    --remote_executor=grpc://localhost:${worker_port} \
+    //a:foo >& $TEST_log || fail "Failed to build //a:foo"
+
+  expect_log "1 remote"
+
+  echo "input 2" >a/foo.in
+  if bazel build \
+    --remote_executor=grpc://localhost:${worker_port} \
+    --experimental_remote_require_cached \
+    //a:foo >& $TEST_log; then
+    fail "Build of //a:foo succeeded but it should have failed"
+  fi
+
+  expect_log "Action must be cached due to --experimental_remote_require_cached but it is not"
+  expect_not_log "remote cache hit"
+}
+
 function test_nobuild_runfile_links() {
   mkdir data && echo "hello" > data/hello && echo "world" > data/world
     cat > WORKSPACE <<EOF