Propagating and printing server logs on demand.

WANT_LGTM=all
TESTED=RBE, unit tests
RELNOTES: None
PiperOrigin-RevId: 189938345

2 files changed, 198 insertions, 15 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
index 4f3252b71b..3bc5b37b1e 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
@@ -120,6 +120,7 @@ public class GrpcRemoteExecutionClientTest {
   private final MutableHandlerRegistry serviceRegistry = new MutableHandlerRegistry();
   private FileSystem fs;
   private Path execRoot;
+  private Path logDir;
   private SimpleSpawn simpleSpawn;
   private FakeActionInputFileCache fakeFileCache;
   private Digest inputDigest;
@@ -195,6 +196,7 @@ public class GrpcRemoteExecutionClientTest {
     Chunker.setDefaultChunkSizeForTesting(1000); // Enough for everything to be one chunk.
     fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256);
     execRoot = fs.getPath("/exec/root");
+    logDir = fs.getPath("/server-logs");
     FileSystemUtils.createDirectoryAndParents(execRoot);
     fakeFileCache = new FakeActionInputFileCache(execRoot);
     simpleSpawn =
@@ -255,7 +257,8 @@ public class GrpcRemoteExecutionClientTest {
             "command-id",
             remoteCache,
             executor,
-            DIGEST_UTIL);
+            DIGEST_UTIL,
+            logDir);
     inputDigest = fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz");
   }
 
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 98b56706d7..29470f4438 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
@@ -61,8 +61,11 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import com.google.devtools.common.options.Options;
 import com.google.devtools.remoteexecution.v1test.ActionResult;
+import com.google.devtools.remoteexecution.v1test.Digest;
 import com.google.devtools.remoteexecution.v1test.ExecuteRequest;
 import com.google.devtools.remoteexecution.v1test.ExecuteResponse;
+import com.google.devtools.remoteexecution.v1test.LogFile;
+import com.google.protobuf.ByteString;
 import com.google.rpc.Code;
 import java.io.IOException;
 import java.time.Duration;
@@ -85,6 +88,7 @@ public class RemoteSpawnRunnerTest {
       ImmutableMap.of(ExecutionRequirements.NO_CACHE, "");
 
   private Path execRoot;
+  private Path logDir;
   private DigestUtil digestUtil;
   private FakeActionInputFileCache fakeFileCache;
   private FileOutErr outErr;
@@ -97,12 +101,17 @@ public class RemoteSpawnRunnerTest {
   @Mock
   private SpawnRunner localRunner;
 
+  // The action key of the Spawn returned by newSimpleSpawn().
+  private final String simpleActionId =
+      "eb45b20cc979d504f96b9efc9a08c48103c6f017afa09c0df5c70a5f92a98ea8";
+
   @Before
   public final void setUp() throws Exception {
     MockitoAnnotations.initMocks(this);
     digestUtil = new DigestUtil(HashFunction.SHA256);
     FileSystem fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256);
     execRoot = fs.getPath("/exec/root");
+    logDir = fs.getPath("/server-logs");
     FileSystemUtils.createDirectoryAndParents(execRoot);
     fakeFileCache = new FakeActionInputFileCache(execRoot);
 
@@ -136,7 +145,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     ExecuteResponse succeeded = ExecuteResponse.newBuilder().setResult(
         ActionResult.newBuilder().setExitCode(0).build()).build();
@@ -194,7 +204,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             null,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     // Throw an IOException to trigger the local fallback.
     when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(IOException.class);
@@ -246,7 +257,8 @@ public class RemoteSpawnRunnerTest {
                 "command-id",
                 cache,
                 null,
-                digestUtil));
+                digestUtil,
+                logDir));
 
     Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
@@ -294,7 +306,8 @@ public class RemoteSpawnRunnerTest {
                 "command-id",
                 cache,
                 null,
-                digestUtil));
+                digestUtil,
+                logDir));
 
     try {
       runner.exec(spawn, policy);
@@ -328,7 +341,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             null,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
@@ -379,7 +393,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             null,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
@@ -418,7 +433,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             null,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     Spawn spawn = newSimpleSpawn();
     SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
@@ -454,7 +470,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
     when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException());
@@ -476,6 +493,163 @@ public class RemoteSpawnRunnerTest {
   }
 
   @Test
+  public void testHumanReadableServerLogsSavedForFailingAction() throws Exception {
+    RemoteSpawnRunner runner =
+        new RemoteSpawnRunner(
+            execRoot,
+            Options.getDefaults(RemoteOptions.class),
+            localRunner,
+            true,
+            /*cmdlineReporter=*/ null,
+            "build-req-id",
+            "command-id",
+            cache,
+            executor,
+            digestUtil,
+            logDir);
+
+    Digest logDigest = digestUtil.computeAsUtf8("bla");
+    when(executor.executeRemotely(any(ExecuteRequest.class)))
+        .thenReturn(
+            ExecuteResponse.newBuilder()
+                .putServerLogs(
+                    "logname",
+                    LogFile.newBuilder().setHumanReadable(true).setDigest(logDigest).build())
+                .setResult(ActionResult.newBuilder().setExitCode(31).build())
+                .build());
+
+    Spawn spawn = newSimpleSpawn();
+    SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
+
+    SpawnResult res = runner.exec(spawn, policy);
+    assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT);
+
+    verify(executor).executeRemotely(any(ExecuteRequest.class));
+    Path logPath = logDir.getRelative(simpleActionId).getRelative("logname");
+    verify(cache).downloadFile(eq(logPath), eq(logDigest), eq(false), eq(null));
+  }
+
+  @Test
+  public void testHumanReadableServerLogsSavedForFailingActionWithStatus() throws Exception {
+    RemoteSpawnRunner runner =
+        new RemoteSpawnRunner(
+            execRoot,
+            Options.getDefaults(RemoteOptions.class),
+            localRunner,
+            true,
+            /*cmdlineReporter=*/ null,
+            "build-req-id",
+            "command-id",
+            cache,
+            executor,
+            digestUtil,
+            logDir);
+
+    Digest logDigest = digestUtil.computeAsUtf8("bla");
+    com.google.rpc.Status timeoutStatus =
+        com.google.rpc.Status.newBuilder().setCode(Code.DEADLINE_EXCEEDED.getNumber()).build();
+    ExecuteResponse resp =
+        ExecuteResponse.newBuilder()
+            .putServerLogs(
+                "logname", LogFile.newBuilder().setHumanReadable(true).setDigest(logDigest).build())
+            .setStatus(timeoutStatus)
+            .build();
+    when(executor.executeRemotely(any(ExecuteRequest.class)))
+        .thenThrow(new Retrier.RetryException(
+                "", 1, new ExecutionStatusException(resp.getStatus(), resp)));
+
+    Spawn spawn = newSimpleSpawn();
+    SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
+
+    SpawnResult res = runner.exec(spawn, policy);
+    assertThat(res.status()).isEqualTo(Status.TIMEOUT);
+
+    verify(executor).executeRemotely(any(ExecuteRequest.class));
+    Path logPath = logDir.getRelative(simpleActionId).getRelative("logname");
+    verify(cache).downloadFile(eq(logPath), eq(logDigest), eq(false), eq(null));
+  }
+
+  @Test
+  public void testNonHumanReadableServerLogsNotSaved() throws Exception {
+    RemoteSpawnRunner runner =
+        new RemoteSpawnRunner(
+            execRoot,
+            Options.getDefaults(RemoteOptions.class),
+            localRunner,
+            true,
+            /*cmdlineReporter=*/ null,
+            "build-req-id",
+            "command-id",
+            cache,
+            executor,
+            digestUtil,
+            logDir);
+
+    Digest logDigest = digestUtil.computeAsUtf8("bla");
+    ActionResult result = ActionResult.newBuilder().setExitCode(31).build();
+    when(executor.executeRemotely(any(ExecuteRequest.class)))
+        .thenReturn(
+            ExecuteResponse.newBuilder()
+                .putServerLogs(
+                    "logname",
+                    LogFile.newBuilder().setDigest(logDigest).build())
+                .setResult(result)
+                .build());
+
+    Spawn spawn = newSimpleSpawn();
+    SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
+
+    SpawnResult res = runner.exec(spawn, policy);
+    assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT);
+
+    verify(executor).executeRemotely(any(ExecuteRequest.class));
+    verify(cache).download(eq(result), eq(execRoot), any(FileOutErr.class));
+    verify(cache, never())
+        .downloadFile(
+            any(Path.class), any(Digest.class), any(Boolean.class), any(ByteString.class));
+  }
+
+  @Test
+  public void testServerLogsNotSavedForSuccessfulAction() throws Exception {
+    RemoteSpawnRunner runner =
+        new RemoteSpawnRunner(
+            execRoot,
+            Options.getDefaults(RemoteOptions.class),
+            localRunner,
+            true,
+            /*cmdlineReporter=*/ null,
+            "build-req-id",
+            "command-id",
+            cache,
+            executor,
+            digestUtil,
+            logDir);
+
+    Digest logDigest = digestUtil.computeAsUtf8("bla");
+    ActionResult result = ActionResult.newBuilder().setExitCode(0).build();
+    when(executor.executeRemotely(any(ExecuteRequest.class)))
+        .thenReturn(
+            ExecuteResponse.newBuilder()
+                .putServerLogs(
+                    "logname",
+                    LogFile.newBuilder().setHumanReadable(true).setDigest(logDigest).build())
+                .setResult(result)
+                .build());
+
+    Spawn spawn = newSimpleSpawn();
+    SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn);
+
+    SpawnResult res = runner.exec(spawn, policy);
+    assertThat(res.status()).isEqualTo(Status.SUCCESS);
+
+    verify(executor).executeRemotely(any(ExecuteRequest.class));
+    verify(cache).download(eq(result), eq(execRoot), any(FileOutErr.class));
+    verify(cache, never())
+        .downloadFile(
+            any(Path.class), any(Digest.class), any(Boolean.class), any(ByteString.class));
+  }
+
+  @Test
   public void cacheDownloadFailureTriggersRemoteExecution() throws Exception {
     // If downloading a cached action fails, remote execution should be tried.
 
@@ -492,7 +666,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(cachedResult);
@@ -533,7 +708,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
@@ -580,7 +756,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
@@ -625,7 +802,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
@@ -665,7 +843,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
     when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException());
@@ -701,7 +880,8 @@ public class RemoteSpawnRunnerTest {
             "command-id",
             cache,
             executor,
-            digestUtil);
+            digestUtil,
+            logDir);
 
     when(cache.getCachedActionResult(any(ActionKey.class))).thenThrow(new IOException());