diff options
author | toby <smorgrav@yahoo-inc.com> | 2017-04-25 16:16:39 +0200 |
---|---|---|
committer | toby <smorgrav@yahoo-inc.com> | 2017-05-09 17:02:07 +0200 |
commit | 8967f476ddbb8c52fb56b3afcc761860db4ea9ed (patch) | |
tree | 41ea8b7521a14984e71875627c504f4c46551d1e /docker-api | |
parent | c7efb4ecdde276e192e813bf52956cf6a0ca9c9c (diff) |
Re-introduce exec in docker container with timeout method
Diffstat (limited to 'docker-api')
4 files changed, 102 insertions, 13 deletions
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java index b628bd77d91..d2cffda329f 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java @@ -72,13 +72,36 @@ public interface Docker { void deleteUnusedDockerImages(); /** - * Execute a command in docker container as "yahoo" user - * TODO: Make this function interruptible + * Execute a command in docker container as "yahoo". Will block until the command is finished. * - * @param args Program arguments. args[0] must be the program filename. - * @throws RuntimeException (or some subclass thereof) on failure, including docker failure, command failure + * @param containerName The name of the container + * @param command The command with arguments to run. + * + * @return exitcodes, stdout and stderr in the ProcessResult + */ + ProcessResult executeInContainer(ContainerName containerName, String... command); + + /** + * Execute a command in docker container as "root". Will block until the command is finished. + * + * @param containerName The name of the container + * @param command The command with arguments to run. + * + * @return exitcodes, stdout and stderr in the ProcessResult */ - ProcessResult executeInContainer(ContainerName containerName, String... args); + ProcessResult executeInContainerAsRoot(ContainerName containerName, String... command); + + /** + * Execute a command in docker container as "root" + * The timeout will not kill the process spawned. + * + * @param containerName The name of the container + * @param timeoutSeconds Timeout for the process to finish in seconds or without timeout if empty + * @param command The command with arguments to run. + * + * @return exitcodes, stdout and stderr in the ProcessResult + */ + ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... command); + - ProcessResult executeInContainerAsRoot(ContainerName containerName, String... args); } diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerExecTimeoutException.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerExecTimeoutException.java new file mode 100644 index 00000000000..153fc0257a3 --- /dev/null +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerExecTimeoutException.java @@ -0,0 +1,16 @@ +package com.yahoo.vespa.hosted.dockerapi; + +/** + * Runtime exception to be thrown when the exec commands did not finish in time. + * + * The underlying process has not been killed. If you need the process to be + * killed you need to wrap it into a commands that times out. + * + * @author smorgrav + */ +@SuppressWarnings("serial") +public class DockerExecTimeoutException extends RuntimeException { + public DockerExecTimeoutException(String msg) { + super(msg); + } +} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java index 6b1d63dd12f..66cd50ddb20 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java @@ -225,22 +225,26 @@ public class DockerImpl implements Docker { @Override public ProcessResult executeInContainer(ContainerName containerName, String... args) { - return executeInContainerAsUser(containerName, "yahoo", args); + return executeInContainerAsUser(containerName, "yahoo", Optional.empty(), args); } @Override public ProcessResult executeInContainerAsRoot(ContainerName containerName, String... args) { - return executeInContainerAsUser(containerName, "root", args); + return executeInContainerAsUser(containerName, "root", Optional.empty(), args); + } + + @Override + public ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... args) { + return executeInContainerAsUser(containerName, "root", Optional.of(timeoutSeconds), args); } /** * Execute command in container as user, "user" can be "username", "username:group", "uid" or "uid:gid" */ - private ProcessResult executeInContainerAsUser(ContainerName containerName, String user, String... args) { - assert args.length >= 1; + private ProcessResult executeInContainerAsUser(ContainerName containerName, String user, Optional<Long> timeoutSeconds, String... command) { try { final ExecCreateCmdResponse response = dockerClient.execCreateCmd(containerName.asString()) - .withCmd(args) + .withCmd(command) .withAttachStdout(true) .withAttachStderr(true) .withUser(user) @@ -249,7 +253,16 @@ public class DockerImpl implements Docker { ByteArrayOutputStream output = new ByteArrayOutputStream(); ByteArrayOutputStream errors = new ByteArrayOutputStream(); ExecStartCmd execStartCmd = dockerClient.execStartCmd(response.getId()); - execStartCmd.exec(new ExecStartResultCallback(output, errors)).awaitCompletion(); + ExecStartResultCallback callback = execStartCmd.exec(new ExecStartResultCallback(output, errors)); + + if (timeoutSeconds.isPresent()) { + if (!callback.awaitCompletion(timeoutSeconds.get(), TimeUnit.SECONDS)) { + throw new DockerExecTimeoutException(String.format("Command '%s' did not finish within %s seconds.", command[0], timeoutSeconds)); + } + } else { + // Wait for completion no timeout + callback.awaitCompletion(); + } final InspectExecResponse state = dockerClient.inspectExecCmd(execStartCmd.getExecId()).exec(); assert !state.isRunning(); @@ -260,7 +273,7 @@ public class DockerImpl implements Docker { } catch (DockerException | InterruptedException e) { numberOfDockerDaemonFails.add(); throw new RuntimeException("Container '" + containerName.asString() - + "' failed to execute " + Arrays.toString(args), e); + + "' failed to execute " + Arrays.toString(command), e); } } diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java index 2c28eac5e4f..f91fccb2cc7 100644 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java @@ -115,6 +115,43 @@ public class DockerTest { assertThat(docker.getAllContainersManagedBy(MANAGER_NAME).isEmpty(), is(true)); } + /** + * Test the expected behavior for exec when it times out - it should throw an exception when it times out, + * and before the process completes. + * + * The test timeout value is set quite high to avoid noise if screwdriver is slow but lower than the process time. + */ + @Test(expected = DockerExecTimeoutException.class, timeout = 2000) + public void testContainerExecHounorsTimeout() throws IOException, InterruptedException, ExecutionException { + final ContainerName containerName = new ContainerName("docker-test-foo"); + final String containerHostname = "hostName1"; + + docker.createContainerCommand(dockerImage, containerName, containerHostname).withManagedBy(MANAGER_NAME).create(); + docker.startContainer(containerName); + docker.executeInContainerAsRoot(containerName, 1L, "sh", "-c", "sleep 5"); + } + + /** + * Test the expected behavior for exec that completes before specified timeout - it should return when the process finishes and not + * wait for the timeout. Some previous tests indicated that this was not behaving correctly. + * + * No timeout implies infinite timeout. + * + * The test timeout value is set quite high to avoid noise if screwdriver is slow + */ + @Test(timeout = 4000) + public void testContainerExecDoesNotBlockUntilTimeoutWhenCommandFinishesBeforeTimeout() throws IOException, InterruptedException, ExecutionException { + final ContainerName containerName = new ContainerName("docker-test-foo"); + final String containerHostname = "hostName1"; + + docker.createContainerCommand(dockerImage, containerName, containerHostname).withManagedBy(MANAGER_NAME).create(); + docker.startContainer(containerName); + docker.executeInContainerAsRoot(containerName, 2L, "sh", "-c", "echo hei"); + + // Also test that this is the behavoir when not specifying timeout + docker.executeInContainerAsRoot(containerName,"sh", "-c", "echo hei"); + } + @Test public void testDockerNetworking() throws InterruptedException, ExecutionException, IOException { String hostName1 = "docker10.test.yahoo.com"; |