diff options
8 files changed, 148 insertions, 106 deletions
@@ -74,14 +74,14 @@ bind( new_local_repository( name = "com_google_protobuf", - build_file = "./third_party/protobuf/3.4.0/BUILD", - path = "./third_party/protobuf/3.4.0/", + build_file = "./third_party/protobuf/3.2.0/BUILD", + path = "./third_party/protobuf/3.2.0/", ) new_local_repository( name = "com_google_protobuf_java", - build_file = "./third_party/protobuf/3.4.0/com_google_protobuf_java.BUILD", - path = "./third_party/protobuf/3.4.0/", + build_file = "./third_party/protobuf/3.2.0/com_google_protobuf_java.BUILD", + path = "./third_party/protobuf/3.2.0/", ) new_local_repository( diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java index b3a03839d3..d2d3ee8ad6 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnResult.java @@ -94,6 +94,12 @@ public interface SpawnResult { REMOTE_EXECUTOR_OVERLOADED, /** + * The result of the remotely executed Spawn could not be retrieved due to errors in the remote + * caching layer. + */ + REMOTE_CACHE_FAILED, + + /** * The remote execution system did not allow the request due to missing authorization or * authentication. */ 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 69b751cbda..bd53704b3d 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 @@ -27,12 +27,14 @@ import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnResult; import com.google.devtools.build.lib.exec.SpawnResult.Status; import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.remote.Digests.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; +import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -209,40 +211,40 @@ class RemoteSpawnRunner implements SpawnRunner { if (options.remoteLocalFallback) { return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey); } - if (cause instanceof TimeoutException) { - return handleTimeout(policy.getFileOutErr()); - } - throw new EnvironmentalExecException(errorMessage(cause), cause, true); - } - - private SpawnResult handleTimeout(FileOutErr outErr) throws IOException { - // TODO(buchgr): Once the remote execution protocol allows it, also provide stdout/stderr - // from the action that timed out. - try (OutputStream out = outErr.getOutputStream()) { - // This is a hack to ensure that the test.log file gets created, as else the action - // will complain that one of its outputs has not been created. - String msg = "Log output for timeouts is not yet supported in remote execution."; - out.write(msg.getBytes(StandardCharsets.UTF_8)); - out.flush(); - } - return new SpawnResult.Builder().setStatus(Status.TIMEOUT).build(); + return handleError(cause, policy.getFileOutErr()); } - private String errorMessage(IOException e) { - String message = ""; - if (e instanceof RetryException - && ((RetryException) e).causedByStatusCode(Code.UNAVAILABLE)) { - message = "The remote executor/cache is unavailable"; - } else if (e instanceof CacheNotFoundException) { - message = "Failed to download from remote cache"; + private SpawnResult handleError(IOException cause, FileOutErr outErr) throws IOException, + ExecException { + final Status status; + if (cause instanceof TimeoutException) { + status = Status.TIMEOUT; + // TODO(buchgr): Once the remote execution protocol allows it, also provide stdout/stderr + // from the action that timed out. + try (OutputStream out = outErr.getOutputStream()) { + // This is a hack to ensure that the test.log file gets created, as else the action + // will complain that one of its outputs has not been created. + String msg = "Log output for timeouts is not yet supported in remote execution."; + out.write(msg.getBytes(StandardCharsets.UTF_8)); + out.flush(); + } + return new SpawnResult.Builder().setStatus(status).build(); + } else if (cause instanceof RetryException + && ((RetryException) cause).causedByStatusCode(Code.UNAVAILABLE)) { + status = Status.CONNECTION_FAILED; + } else if (cause instanceof CacheNotFoundException) { + status = Status.REMOTE_CACHE_FAILED; } else { - message = "Error in remote cache/executor"; - } - // TODO(olaola): reuse the ErrorMessage class for these errors. - if (verboseFailures) { - message += "\n" + Throwables.getStackTraceAsString(e); + status = Status.EXECUTION_FAILED; } - return message; + throw new SpawnExecException( + Throwables.getStackTraceAsString(cause), + new SpawnResult + .Builder() + .setExitCode(ExitCode.REMOTE_ERROR.getNumericExitCode()) + .setStatus(status) + .build(), + /*catastrophic=*/true); } static Action buildAction( diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD index 11d019c5fe..afbfc5bf0a 100644 --- a/src/main/protobuf/BUILD +++ b/src/main/protobuf/BUILD @@ -2,7 +2,7 @@ package(default_visibility = ["//visibility:public"]) load("//tools/build_rules:genproto.bzl", "cc_grpc_library") load("//tools/build_rules:utilities.bzl", "java_library_srcs") -load("//third_party/protobuf/3.4.0:protobuf.bzl", "cc_proto_library", "py_proto_library") +load("//third_party/protobuf/3.2.0:protobuf.bzl", "cc_proto_library", "py_proto_library") load("//third_party/grpc:build_defs.bzl", "java_grpc_library") FILES = [ diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index ba7d4e318a..a457eac130 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1082,6 +1082,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:auth_and_tls_options", "//src/main/java/com/google/devtools/build/lib:build-base", "//src/main/java/com/google/devtools/build/lib:events", + "//src/main/java/com/google/devtools/build/lib:exitcode-external", "//src/main/java/com/google/devtools/build/lib:inmemoryfs", "//src/main/java/com/google/devtools/build/lib:io", "//src/main/java/com/google/devtools/build/lib:preconditions", 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 1db31cc372..c5d5b66f62 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 @@ -31,11 +31,12 @@ import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; -import com.google.devtools.build.lib.actions.EnvironmentalExecException; +import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.authandtls.GrpcUtils; +import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnResult; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; @@ -636,13 +637,12 @@ public class GrpcRemoteExecutionClientTest { try { client.exec(simpleSpawn, simplePolicy); fail("Expected an exception"); - } catch (EnvironmentalExecException expected) { - assertThat(expected).hasMessageThat().contains("The remote executor/cache is unavailable"); + } catch (SpawnExecException expected) { + assertThat(expected.getSpawnResult().status()) + .isEqualTo(SpawnResult.Status.CONNECTION_FAILED); // Ensure we also got back the stack trace. assertThat(expected).hasMessageThat() .contains("GrpcRemoteExecutionClientTest.passUnavailableErrorWithStackTrace"); - Throwable t = expected.getCause(); - assertThat(t).isInstanceOf(RetryException.class); } } @@ -660,14 +660,11 @@ public class GrpcRemoteExecutionClientTest { try { client.exec(simpleSpawn, simplePolicy); fail("Expected an exception"); - } catch (EnvironmentalExecException expected) { - assertThat(expected).hasMessageThat().contains("Error in remote cache/executor"); + } catch (ExecException expected) { assertThat(expected).hasMessageThat().contains("whoa"); // Error details. // Ensure we also got back the stack trace. assertThat(expected).hasMessageThat() .contains("GrpcRemoteExecutionClientTest.passInternalErrorWithStackTrace"); - Throwable t = expected.getCause(); - assertThat(t).isInstanceOf(RetryException.class); } } @@ -719,14 +716,13 @@ public class GrpcRemoteExecutionClientTest { try { client.exec(simpleSpawn, simplePolicy); fail("Expected an exception"); - } catch (EnvironmentalExecException expected) { - assertThat(expected).hasMessageThat().contains("Failed to download from remote cache"); + } catch (SpawnExecException expected) { + assertThat(expected.getSpawnResult().status()) + .isEqualTo(SpawnResult.Status.REMOTE_CACHE_FAILED); + assertThat(expected).hasMessageThat().contains(stdOutDigest.getHash()); // Ensure we also got back the stack trace. assertThat(expected).hasMessageThat() .contains("GrpcRemoteExecutionClientTest.passCacheMissErrorWithStackTrace"); - Throwable t = expected.getCause(); - assertThat(t).isInstanceOf(CacheNotFoundException.class); - assertThat(((CacheNotFoundException) t).getMissingDigest()).isEqualTo(stdOutDigest); } } @@ -800,11 +796,9 @@ public class GrpcRemoteExecutionClientTest { try { client.exec(simpleSpawn, simplePolicy); fail("Expected an exception"); - } catch (EnvironmentalExecException expected) { - assertThat(expected).hasMessageThat().contains("Failed to download from remote cache"); - Throwable t = expected.getCause(); - assertThat(t).isInstanceOf(CacheNotFoundException.class); - assertThat(((CacheNotFoundException) t).getMissingDigest()).isEqualTo(stdOutDigest); + } catch (ExecException expected) { + assertThat(expected).hasMessageThat().contains("Missing digest"); + assertThat(expected).hasMessageThat().contains("476d9ec701e2de6a6c37ab5211117a7cb8333a27"); } } } 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 ea1ac727df..a6c8218f01 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 @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnResult; import com.google.devtools.build.lib.exec.SpawnResult.Status; @@ -48,6 +49,7 @@ import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -202,14 +204,7 @@ public class RemoteSpawnRunnerTest { spy(new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null, cache, null)); - Spawn spawn = new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.<ActionInput>of(), - ResourceSet.ZERO); + Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); SpawnResult res = Mockito.mock(SpawnResult.class); @@ -235,14 +230,7 @@ public class RemoteSpawnRunnerTest { ActionResult failedAction = ActionResult.newBuilder().setExitCode(1).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(failedAction); - Spawn spawn = new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.<ActionInput>of(), - ResourceSet.ZERO); + Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); RemoteSpawnRunner runner = @@ -273,15 +261,7 @@ public class RemoteSpawnRunnerTest { RemoteSpawnRunner runner = new RemoteSpawnRunner(execRoot, options, localRunner, false, reporter, cache, null); - Spawn spawn = - new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.<ActionInput>of(), - ResourceSet.ZERO); + Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); when(cache.getCachedActionResult(any(ActionKey.class))) @@ -318,15 +298,7 @@ public class RemoteSpawnRunnerTest { new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null, cache, null); - Spawn spawn = - new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.<ActionInput>of(), - ResourceSet.ZERO); + Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -364,15 +336,7 @@ public class RemoteSpawnRunnerTest { when(executor.executeRemotely(any(ExecuteRequest.class))).thenReturn(succeeded); doNothing().when(cache).download(eq(execResult), any(Path.class), any(FileOutErr.class)); - Spawn spawn = - new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.<ActionInput>of(), - ResourceSet.ZERO); + Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -398,15 +362,7 @@ public class RemoteSpawnRunnerTest { when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new TimeoutException()); - Spawn spawn = - new SimpleSpawn( - new FakeOwner("foo", "bar"), - /*arguments=*/ ImmutableList.of(), - /*environment=*/ ImmutableMap.of(), - /*executionInfo=*/ ImmutableMap.of(), - /*inputs=*/ ImmutableList.of(), - /*outputs=*/ ImmutableList.<ActionInput>of(), - ResourceSet.ZERO); + Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -417,6 +373,70 @@ public class RemoteSpawnRunnerTest { verify(cache, never()).download(eq(cachedResult), eq(execRoot), any(FileOutErr.class)); } + @Test + public void testExitCode_executorfailure() throws Exception { + // If we get a failure due to the remote cache not working, the exit code should be + // ExitCode.REMOTE_ERROR. + + RemoteOptions options = Options.getDefaults(RemoteOptions.class); + options.remoteLocalFallback = false; + + RemoteSpawnRunner runner = + new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null, + cache, executor); + + when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); + when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException()); + + Spawn spawn = newSimpleSpawn(); + SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + + try { + runner.exec(spawn, policy); + fail("Exception expected"); + } catch (SpawnExecException e) { + assertThat(e.getSpawnResult().exitCode()) + .isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode()); + } + } + + @Test + public void testExitCode_executionfailure() throws Exception { + // If we get a failure due to the remote executor not working, the exit code should be + // ExitCode.REMOTE_ERROR. + + RemoteOptions options = Options.getDefaults(RemoteOptions.class); + options.remoteLocalFallback = false; + + RemoteSpawnRunner runner = + new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null, + cache, executor); + + when(cache.getCachedActionResult(any(ActionKey.class))).thenThrow(new IOException()); + + Spawn spawn = newSimpleSpawn(); + SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + + try { + runner.exec(spawn, policy); + fail("Exception expected"); + } catch (SpawnExecException e) { + assertThat(e.getSpawnResult().exitCode()) + .isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode()); + } + } + + private static Spawn newSimpleSpawn() { + return new SimpleSpawn( + new FakeOwner("foo", "bar"), + /*arguments=*/ ImmutableList.of(), + /*environment=*/ ImmutableMap.of(), + /*executionInfo=*/ ImmutableMap.of(), + /*inputs=*/ ImmutableList.of(), + /*outputs=*/ ImmutableList.<ActionInput>of(), + ResourceSet.ZERO); + } + // TODO(buchgr): Extract a common class to be used for testing. class FakeSpawnExecutionPolicy implements SpawnExecutionPolicy { diff --git a/src/test/shell/bazel/remote_execution_test.sh b/src/test/shell/bazel/remote_execution_test.sh index 53463777b1..5dbbc90fc7 100755 --- a/src/test/shell/bazel/remote_execution_test.sh +++ b/src/test/shell/bazel/remote_execution_test.sh @@ -380,6 +380,25 @@ EOF expect_log "TIMEOUT" } +function test_exitcode() { + mkdir -p a + cat > a/BUILD <<'EOF' +genrule( + name = "foo", + srcs = [], + outs = ["foo.txt"], + cmd = "echo \"hello world\" > \"$@\"", +) +EOF + + (set +e + bazel --host_jvm_args=-Dbazel.DigestFunction=SHA1 build \ + --genrule_strategy=remote \ + --remote_executor=bazel-test-does-not-exist \ + //a:foo >& $TEST_log + [ $? -eq 34 ]) || fail "Test failed due to wrong exit code" +} + # TODO(alpha): Add a test that fails remote execution when remote worker # supports sandbox. |