Uploading failed action outputs to the remote cache, because even if the tests fails, we still want to be able to download the logs and other outputs from CAS.

This fixes a bug introduced by https://github.com/bazelbuild/bazel/commit/562fcf9f5dfd14daea718f77da95b43b1400689b. To reproduce: run a failing test vs a BES service, the test log would not be uploaded.

TESTED=unit tests
PiperOrigin-RevId: 169143428

5 files changed, 31 insertions, 16 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
index ee7ef666e3..b49a103b5c 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
@@ -281,10 +281,18 @@ public class GrpcRemoteCache implements RemoteActionCache {
   }
 
   @Override
-  public void upload(ActionKey actionKey, Path execRoot, Collection<Path> files, FileOutErr outErr)
+  public void upload(
+      ActionKey actionKey,
+      Path execRoot,
+      Collection<Path> files,
+      FileOutErr outErr,
+      boolean uploadAction)
       throws IOException, InterruptedException {
     ActionResult.Builder result = ActionResult.newBuilder();
     upload(execRoot, files, outErr, result);
+    if (!uploadAction) {
+      return;
+    }
     try {
       retrier.execute(
           () ->
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java
index 4c7cf7408a..d480a93b38 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java
@@ -75,9 +75,15 @@ interface RemoteActionCache {
 
   /**
    * Upload the result of a locally executed action to the cache by uploading any necessary files,
-   * stdin / stdout, as well as adding an entry for the given action key to the cache.
+   * stdin / stdout, as well as adding an entry for the given action key to the cache if
+   * uploadAction is true.
    */
-  void upload(ActionKey actionKey, Path execRoot, Collection<Path> files, FileOutErr outErr)
+  void upload(
+      ActionKey actionKey,
+      Path execRoot,
+      Collection<Path> files,
+      FileOutErr outErr,
+      boolean uploadAction)
       throws IOException, InterruptedException;
 
   /** Release resources associated with the cache. The cache may not be used after calling this. */
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index 2b74e88b3f..774a5802a7 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -128,11 +128,9 @@ final class RemoteSpawnCache implements SpawnCache {
         @Override
         public void store(SpawnResult result, Collection<Path> files)
             throws InterruptedException, IOException {
-          if (result.status() != Status.SUCCESS || result.exitCode() != 0) {
-            return;
-          }
           try {
-            remoteCache.upload(actionKey, execRoot, files, policy.getFileOutErr());
+            boolean uploadAction = Status.SUCCESS.equals(result.status()) && result.exitCode() == 0;
+            remoteCache.upload(actionKey, execRoot, files, policy.getFileOutErr(), uploadAction);
           } catch (IOException e) {
             if (verboseFailures) {
               report(Event.debug("Upload to remote cache failed: " + e.getMessage()));
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 a5990ff683..b4d28cd995 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
@@ -329,10 +329,6 @@ class RemoteSpawnRunner implements SpawnRunner {
       ActionKey actionKey) throws ExecException, IOException, InterruptedException {
     Map<Path, Long> ctimesBefore = getInputCtimes(inputMap);
     SpawnResult result = fallbackRunner.exec(spawn, policy);
-    if (!Status.SUCCESS.equals(result.status()) || result.exitCode() != 0) {
-      // Don't upload failed actions.
-      return result;
-    }
     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.
@@ -342,7 +338,8 @@ class RemoteSpawnRunner implements SpawnRunner {
     }
     List<Path> outputFiles = listExistingOutputFiles(execRoot, spawn);
     try {
-      remoteCache.upload(actionKey, execRoot, outputFiles, policy.getFileOutErr());
+      boolean uploadAction = Status.SUCCESS.equals(result.status()) && result.exitCode() == 0;
+      remoteCache.upload(actionKey, execRoot, outputFiles, policy.getFileOutErr(), uploadAction);
     } catch (IOException e) {
       if (verboseFailures) {
         report(Event.debug("Upload to remote cache failed: " + e.getMessage()));
diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
index 6fef48f178..0a435fabed 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
@@ -170,8 +170,12 @@ public final class SimpleBlobStoreActionCache implements RemoteActionCache {
 
   @Override
   public void upload(
-      ActionKey actionKey, Path execRoot, Collection<Path> files, FileOutErr outErr)
-          throws IOException, InterruptedException {
+      ActionKey actionKey,
+      Path execRoot,
+      Collection<Path> files,
+      FileOutErr outErr,
+      boolean uploadAction)
+      throws IOException, InterruptedException {
     ActionResult.Builder result = ActionResult.newBuilder();
     upload(result, execRoot, files);
     if (outErr.getErrorPath().exists()) {
@@ -182,8 +186,10 @@ public final class SimpleBlobStoreActionCache implements RemoteActionCache {
       Digest stdout = uploadFileContents(outErr.getOutputPath());
       result.setStdoutDigest(stdout);
     }
-    blobStore.putActionResult(
-        actionKey.getDigest().getHash(), new ByteArrayInputStream(result.build().toByteArray()));
+    if (uploadAction) {
+      blobStore.putActionResult(
+          actionKey.getDigest().getHash(), new ByteArrayInputStream(result.build().toByteArray()));
+    }
   }
 
   public void upload(ActionResult.Builder result, Path execRoot, Collection<Path> files)