From ba2cde2f8c5f042d9434842d9816c70b2ac6999c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Ka=C5=A1=C3=ADk?= Date: Wed, 22 Jan 2020 12:45:30 +0100 Subject: [PATCH 1/3] Removing throws Exception. Fixes #194 --- .../eap/qe/ts/common/docker/Container.java | 37 +++ .../common/docker/ContainerKillException.java | 23 ++ .../ContainerReadyConditionException.java | 7 +- .../docker/ContainerRemoveException.java | 23 ++ .../docker/ContainerStartException.java | 23 ++ .../common/docker/ContainerStopException.java | 23 ++ .../jboss/eap/qe/ts/common/docker/Docker.java | 310 ++++++++++++------ .../common/docker/DockerCommandException.java | 23 ++ .../qe/ts/common/docker/DockerException.java | 11 - .../common/docker/DockerTimeoutException.java | 11 - .../docker/MalformedDockerConfigTest.java | 14 +- 11 files changed, 376 insertions(+), 129 deletions(-) create mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java create mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerKillException.java create mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerRemoveException.java create mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStartException.java create mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStopException.java create mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerCommandException.java delete mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerException.java delete mode 100644 tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerTimeoutException.java diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java new file mode 100644 index 00000000..95edeefa --- /dev/null +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java @@ -0,0 +1,37 @@ +package org.jboss.eap.qe.ts.common.docker; + +/** + * Describes public interface available for a container (not an Arquillian one but the operating system level + * virtualization unit). + */ +public interface Container { + + /** + * Start this container + * + * @throws ContainerStartException thrown when start of container fails + */ + void start() throws ContainerStartException; + + /** + * @return Returns true if docker container is running. It does NOT check whether container is ready. + */ + boolean isRunning(); + + /** + * Stop this docker container using docker command. + * + * @throws ContainerStopException thrown when the stop command fails. This generally means that the command wasn't + * successful and container might be still running. + */ + void stop() throws ContainerStopException; + + /** + * Kill this docker container using docker command. Be aware that there might occur a situation when the docker + * command might fail. This might be caused for example by reaching file descriptors limit in system. + * + * @throws ContainerKillException thrown when the kill command fails. + */ + void kill() throws ContainerKillException; + +} diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerKillException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerKillException.java new file mode 100644 index 00000000..f84d2266 --- /dev/null +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerKillException.java @@ -0,0 +1,23 @@ +package org.jboss.eap.qe.ts.common.docker; + +/** + * An exception thrown when killing a container fails + */ +public class ContainerKillException extends Exception { + + public ContainerKillException() { + super(); + } + + public ContainerKillException(String message) { + super(message); + } + + public ContainerKillException(String message, Throwable cause) { + super(message, cause); + } + + public ContainerKillException(Throwable cause) { + super(cause); + } +} diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerReadyConditionException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerReadyConditionException.java index 62a0ac66..35e18803 100644 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerReadyConditionException.java +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerReadyConditionException.java @@ -1,7 +1,12 @@ package org.jboss.eap.qe.ts.common.docker; -public class ContainerReadyConditionException extends RuntimeException { +/** + * An exception thrown when checking for container readiness fails + */ +public class ContainerReadyConditionException extends Exception { + public ContainerReadyConditionException(String message, Throwable cause) { super(message, cause); } + } diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerRemoveException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerRemoveException.java new file mode 100644 index 00000000..f8b8ccd9 --- /dev/null +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerRemoveException.java @@ -0,0 +1,23 @@ +package org.jboss.eap.qe.ts.common.docker; + +/** + * An exception thrown when container removal fails + */ +public class ContainerRemoveException extends Exception { + + public ContainerRemoveException() { + super(); + } + + public ContainerRemoveException(String message) { + super(message); + } + + public ContainerRemoveException(String message, Throwable cause) { + super(message, cause); + } + + public ContainerRemoveException(Throwable cause) { + super(cause); + } +} diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStartException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStartException.java new file mode 100644 index 00000000..95494a3c --- /dev/null +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStartException.java @@ -0,0 +1,23 @@ +package org.jboss.eap.qe.ts.common.docker; + +/** + * An exception thrown when container start fails + */ +public class ContainerStartException extends Exception { + + public ContainerStartException() { + super(); + } + + public ContainerStartException(String message) { + super(message); + } + + public ContainerStartException(String message, Throwable cause) { + super(message, cause); + } + + public ContainerStartException(Throwable cause) { + super(cause); + } +} diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStopException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStopException.java new file mode 100644 index 00000000..d4c7bf2f --- /dev/null +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/ContainerStopException.java @@ -0,0 +1,23 @@ +package org.jboss.eap.qe.ts.common.docker; + +/** + * An exception thrown when container stop fails + */ +public class ContainerStopException extends Exception { + + public ContainerStopException() { + super(); + } + + public ContainerStopException(String message) { + super(message); + } + + public ContainerStopException(String message, Throwable cause) { + super(message, cause); + } + + public ContainerStopException(Throwable cause) { + super(cause); + } +} diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java index a4759da4..d1978bc3 100644 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java @@ -3,13 +3,16 @@ import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; +import java.io.PrintStream; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -23,27 +26,85 @@ *

* Intended to be used as a JUnit @ClassRule. Example of usage - see org.jboss.eap.qe.ts.common.docker.DockerTest test */ -public class Docker extends ExternalResource { +public class Docker extends ExternalResource implements Container { + + private final String uuid; + private final String name; + private final String image; + private final List ports; + private final Map environmentVariables; + private final List options; + private final List commandArguments; + private final ContainerReadyCondition containerReadyCondition; + private final long containerReadyTimeout; + private final PrintStream out; + + private ExecutorService outputPrintingThread; + + private Docker(Builder builder) { + this.uuid = builder.uuid; + this.name = builder.name; + this.image = builder.image; + this.ports = builder.ports; + this.options = builder.options; + this.environmentVariables = builder.environmentVariables; + this.commandArguments = builder.commandArguments; + this.containerReadyCondition = builder.containerReadyCondition; + this.containerReadyTimeout = builder.containerReadyTimeoutInMillis; + this.out = builder.out; + } + + /** + * Start this container + * + * @throws ContainerStartException thrown when start of container fails + */ + public void start() throws ContainerStartException { + if (!isDockerPresent()) { + throw new ContainerStartException("'docker' command is not present on this machine!"); + } - private String uuid; - private String name; - private String image; - private List ports = new ArrayList<>(); - private Map environmentVariables = new HashMap<>(); - private List options = new ArrayList<>(); - private List commandArguments = new ArrayList<>(); - private ContainerReadyCondition containerReadyCondition; - private long containerReadyTimeout; - private ExecutorService outputPrinter; - private Process dockerRunProcess; + this.out.println(Ansi.ansi().reset().a("Starting container ").fgCyan().a(name).reset() + .a(" with ID ").fgYellow().a(uuid).reset()); - private Docker() { - } // avoid instantiation, use Builder + final List dockerStartCommand = composeStartCommand(); + Process dockerRunProcess; + try { + dockerRunProcess = new ProcessBuilder() + .redirectErrorStream(true) + .command(dockerStartCommand) + .start(); + } catch (IOException e) { + throw new ContainerStartException("Failed to start the '" + String.join(" ", dockerStartCommand) + "' command"); + } - public void start() throws Exception { + this.outputPrintingThread = startPrinterThread(dockerRunProcess, this.out, this.name); - checkDockerPresent(); + long startTime = System.currentTimeMillis(); + try { + while (!isContainerReady(this.uuid, this.containerReadyCondition)) { + if (System.currentTimeMillis() - startTime > containerReadyTimeout) { + stop(); + removeDockerContainer(this.uuid); + throw new ContainerStartException(uuid + " - Container was not ready in " + containerReadyTimeout + " ms"); + } + // fail fast mechanism in case of malformed docker command, for example bad arguments, invalid format of port mapping, image version,... + if (!dockerRunProcess.isAlive() && dockerRunProcess.exitValue() != 0) { + throw new ContainerStartException( + uuid + " - Starting of docker container using command: \"" + String.join(" ", dockerStartCommand) + + "\" failed. Check that provided command is correct."); + } + } + } catch (ContainerStopException e) { + throw new ContainerStartException("Unable to stop container after failed start!", e); + } catch (ContainerRemoveException e) { + throw new ContainerStartException("Unable to remove container after failed start!", e); + } catch (ContainerReadyConditionException e) { + throw new ContainerStartException("There was a problem when checking container readiness!", e); + } + } + private List composeStartCommand() { List cmd = new ArrayList<>(); cmd.add("docker"); @@ -67,85 +128,83 @@ public void start() throws Exception { cmd.addAll(commandArguments); - System.out.println(Ansi.ansi().reset().a("Starting container ").fgCyan().a(name).reset() - .a(" with ID ").fgYellow().a(uuid).reset()); + return cmd; + } - dockerRunProcess = new ProcessBuilder() - .redirectErrorStream(true) - .command(cmd) - .start(); - outputPrinter = Executors.newSingleThreadExecutor(); + private ExecutorService startPrinterThread(final Process dockerRunProcess, final PrintStream out, + final String containerName) { + final ExecutorService outputPrinter = Executors.newSingleThreadExecutor(); outputPrinter.execute(() -> { try (BufferedReader reader = new BufferedReader( new InputStreamReader(dockerRunProcess.getInputStream(), StandardCharsets.UTF_8))) { String line; while ((line = reader.readLine()) != null) { - System.out.println(Ansi.ansi().fgCyan().a(name).reset().a("> ").a(line)); + out.println(Ansi.ansi().fgCyan().a(containerName).reset().a("> ").a(line)); } } catch (IOException ignored) { // ignore as any stop of docker container breaks the reader stream // note that shutdown of docker would be already logged } }); - - long startTime = System.currentTimeMillis(); - while (!isContainerReady()) { - if (System.currentTimeMillis() - startTime > containerReadyTimeout) { - stop(); - throw new DockerTimeoutException(uuid + " - Container was not ready in " + containerReadyTimeout + " ms"); - } - // fail fast mechanism in case of malformed docker command, for example bad arguments, invalid format of port mapping, image version,... - if (!dockerRunProcess.isAlive() && dockerRunProcess.exitValue() != 0) { - throw new DockerException(uuid + " - Starting of docker container using command: \"" + String.join(" ", cmd) - + "\" failed. Check that provided command is correct."); - } - } + return outputPrinter; } - private boolean isContainerReady() throws Exception { - CompletableFuture condition = new CompletableFuture().supplyAsync(() -> containerReadyCondition.isReady()); + private boolean isContainerReady(final String uuid, final ContainerReadyCondition condition) + throws ContainerReadyConditionException { + final CompletableFuture conditionFuture = CompletableFuture.supplyAsync(condition::isReady); try { - return condition.get(containerReadyTimeout, TimeUnit.MILLISECONDS); + return conditionFuture.get(containerReadyTimeout, TimeUnit.MILLISECONDS); } catch (TimeoutException ex) { - stop(); - // in case condition hangs interrupt it so there are no zombie threads - condition.cancel(true); - throw new ContainerReadyConditionException(uuid + " - Provided ContainerReadyCondition.isReady() method took " + "longer than containerReadyTimeout: " + containerReadyTimeout + " ms. Check it does not hang and does " + "not take longer then containerReadyTimeout. It's expected that ContainerReadyCondition.isReady() method " + "is short lived (takes less than 1 second).", ex); + } catch (ExecutionException e) { + throw new ContainerReadyConditionException("There was an exception in thread which was checking the " + + "container readiness.", e); + } catch (InterruptedException e) { + throw new ContainerReadyConditionException("The thread waiting for container readiness was interrupted!", e); + } finally { + // in case condition hangs interrupt it so there are no zombie threads + conditionFuture.cancel(true); } } - private void checkDockerPresent() throws Exception { - Process dockerInfoProcess = new ProcessBuilder() - .redirectErrorStream(true) - .command(new String[] { "docker", "info" }) - .start(); - dockerInfoProcess.waitFor(); - if (dockerInfoProcess.exitValue() != 0) { - throw new DockerException("Docker is either not present or not installed on this machine. It must be installed " + - "and started up for executing tests with docker container."); + private boolean isDockerPresent() { + try { + Process dockerInfoProcess = new ProcessBuilder() + .redirectErrorStream(true) + .command(new String[] { "docker", "info" }) + .start(); + dockerInfoProcess.waitFor(); + return dockerInfoProcess.exitValue() == 0; + } catch (IOException | InterruptedException e) { + throw new IllegalStateException("There was an exception when checking for docker command presence!"); } } /** * @return Returns true if docker container is running. It does NOT check whether container is ready. */ - public boolean isRunning() throws Exception { - Process dockerRunProcess = new ProcessBuilder() - .redirectErrorStream(true) - .command(new String[] { "docker", "ps" }) - .start(); - - dockerRunProcess.waitFor(); + public boolean isRunning() { + Process dockerRunProcess; + try { + dockerRunProcess = new ProcessBuilder() + .redirectErrorStream(true) + .command(new String[] { "docker", "ps" }) + .start(); + + dockerRunProcess.waitFor(); + } catch (IOException | InterruptedException ignored) { + //when we cannot start the process for making the check container is not running + return false; + } try (BufferedReader reader = new BufferedReader( new InputStreamReader(dockerRunProcess.getInputStream(), StandardCharsets.UTF_8))) { String line; while ((line = reader.readLine()) != null) { - if (line.contains(uuid)) { + if (line.contains(this.uuid)) { return true; } } @@ -156,44 +215,89 @@ public boolean isRunning() throws Exception { return false; } - public void stop() throws Exception { - System.out.println(Ansi.ansi().reset().a("Stopping container ").fgCyan().a(name).reset() - .a(" with ID ").fgYellow().a(uuid).reset()); + /** + * Stop this docker container using docker command. + * + * @throws ContainerStopException thrown when the stop command fails. This generally means that the command wasn't + * successful and container might be still running. + */ + public void stop() throws ContainerStopException { + this.out.println(Ansi.ansi().reset().a("Stopping container ").fgCyan().a(this.name).reset() + .a(" with ID ").fgYellow().a(this.uuid).reset()); + + try { + runDockerCommand("stop", this.uuid); + } catch (DockerCommandException e) { + throw new ContainerStopException("Failed to stop container '" + this.uuid + "'!", e); + } - new ProcessBuilder() - .command("docker", "stop", uuid) - .start() - .waitFor(10, TimeUnit.SECONDS); - terminateThreadPools(); - removeDockerContainer(); + try { + terminatePrintingThread(); + } catch (InterruptedException e) { + throw new ContainerStopException("A thread was interrupted while waiting for its termination!", e); + } } - public void kill() throws Exception { - System.out.println(Ansi.ansi().reset().a("Killing container ").fgCyan().a(name).reset() + /** + * Kill this docker container using docker command. Be aware that there might occur a situation when the docker + * command might fail. This might be caused for example by reaching file descriptors limit in system. + * + * @throws ContainerKillException thrown when the kill command fails. + */ + public void kill() throws ContainerKillException { + this.out.println(Ansi.ansi().reset().a("Killing container ").fgCyan().a(this.name).reset() .a(" with ID ").fgYellow().a(uuid).reset()); - new ProcessBuilder() - .command("docker", "kill", uuid) - .start() - .waitFor(10, TimeUnit.SECONDS); - terminateThreadPools(); - removeDockerContainer(); + try { + runDockerCommand("kill", this.uuid); + } catch (DockerCommandException e) { + throw new ContainerKillException("Failed to kill the container '" + this.uuid + "'!", e); + } + + try { + terminatePrintingThread(); + } catch (InterruptedException e) { + throw new ContainerKillException("Interrupted when waiting for printer thread termination!", e); + } } - private void terminateThreadPools() throws Exception { - outputPrinter.shutdown(); - outputPrinter.awaitTermination(10, TimeUnit.SECONDS); + private void terminatePrintingThread() throws InterruptedException { + outputPrintingThread.shutdown(); + outputPrintingThread.awaitTermination(10, TimeUnit.SECONDS); } - private void removeDockerContainer() throws Exception { - new ProcessBuilder() - .command("docker", "rm", uuid) - .start() - .waitFor(10, TimeUnit.SECONDS); + private void removeDockerContainer(final String uuid) throws ContainerRemoveException { + try { + runDockerCommand("rm", uuid); + } catch (DockerCommandException e) { + throw new ContainerRemoveException("Failed to remove the container '" + uuid + "'!", e); + } + } + + private int runDockerCommand(final String... commandArguments) throws DockerCommandException { + final String dockerCommand = "docker"; + final List cmd = new ArrayList<>(); + cmd.add(dockerCommand); + Collections.addAll(cmd, commandArguments); + try { + final Process process; + + process = new ProcessBuilder() + .command(cmd) + .start(); + + process.waitFor(10, TimeUnit.SECONDS); + + return process.exitValue(); + } catch (InterruptedException e) { + throw new DockerCommandException("Interrupted while waiting for '" + String.join(" ", cmd) + "' to return!", e); + } catch (IOException e) { + throw new DockerCommandException("Failed to start command '" + String.join(" ", cmd) + "'!", e); + } } @Override - protected void before() throws Throwable { + protected void before() throws ContainerStartException { start(); } @@ -201,9 +305,11 @@ protected void before() throws Throwable { protected void after() { try { stop(); - } catch (Exception e) { - System.out.println(Ansi.ansi().reset().a("Failed stopping container ").fgCyan().a(name).reset() - .a(" with ID ").fgYellow().a(uuid).reset()); + removeDockerContainer(this.uuid); + } catch (ContainerStopException e) { + throw new IllegalStateException("Failed to stop container '" + this.uuid + "'! ", e); + } catch (ContainerRemoveException e) { + throw new IllegalStateException("Failed to remove container '" + this.uuid + "'! ", e); } } @@ -216,6 +322,7 @@ public static class Builder { private List options = new ArrayList<>(); private List commandArguments = new ArrayList<>(); private long containerReadyTimeoutInMillis = 120_000; // 2 minutes + private PrintStream out = System.out; // by default - do not make any check private ContainerReadyCondition containerReadyCondition = () -> true; @@ -291,23 +398,24 @@ public Builder setContainerReadyCondition(ContainerReadyCondition containerReady return this; } + /** + * Set default output stream to which the container stdout will be written. Default is System.out. + * + * @param out an output stream + * @return instance of this builder + */ + public Builder standardOutputStream(final PrintStream out) { + this.out = out; + return this; + } + /** * Builds instance of Docker class. * * @return build Docker instance */ public Docker build() { - Docker docker = new Docker(); - docker.uuid = this.uuid; - docker.name = this.name; - docker.image = this.image; - docker.ports = this.ports; - docker.options = this.options; - docker.environmentVariables = this.environmentVariables; - docker.commandArguments = this.commandArguments; - docker.containerReadyCondition = containerReadyCondition; - docker.containerReadyTimeout = containerReadyTimeoutInMillis; - return docker; + return new Docker(this); } } } diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerCommandException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerCommandException.java new file mode 100644 index 00000000..ae27fb8f --- /dev/null +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerCommandException.java @@ -0,0 +1,23 @@ +package org.jboss.eap.qe.ts.common.docker; + +/** + * An exception thrown when docker command fails to return/execute + */ +public class DockerCommandException extends Exception { + + public DockerCommandException() { + super(); + } + + public DockerCommandException(String message) { + super(message); + } + + public DockerCommandException(String message, Throwable cause) { + super(message, cause); + } + + public DockerCommandException(Throwable cause) { + super(cause); + } +} diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerException.java deleted file mode 100644 index 65f32681..00000000 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerException.java +++ /dev/null @@ -1,11 +0,0 @@ -package org.jboss.eap.qe.ts.common.docker; - -public class DockerException extends RuntimeException { - public DockerException(String message) { - super(message); - } - - public DockerException(String message, Throwable cause) { - super(message, cause); - } -} diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerTimeoutException.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerTimeoutException.java deleted file mode 100644 index d7ec98d4..00000000 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerTimeoutException.java +++ /dev/null @@ -1,11 +0,0 @@ -package org.jboss.eap.qe.ts.common.docker; - -public class DockerTimeoutException extends RuntimeException { - public DockerTimeoutException(String message) { - super(message); - } - - public DockerTimeoutException(String message, Throwable cause) { - super(message, cause); - } -} diff --git a/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java b/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java index 7bb3ec43..6eca8cd9 100644 --- a/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java +++ b/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java @@ -1,8 +1,11 @@ package org.jboss.eap.qe.ts.common.docker; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.hasProperty; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.not; import java.util.concurrent.TimeUnit; @@ -25,7 +28,7 @@ public void testFailFastWithMalformedDockerCommand() throws Exception { .withPortMapping("bad:mapping") .build(); - thrown.expect(DockerException.class); + thrown.expect(ContainerStartException.class); thrown.expectMessage(containsString("Starting of docker container using command: \"docker run --name")); thrown.expectMessage(endsWith("failed. Check that provided command is correct.")); @@ -47,9 +50,10 @@ public void testContainerWithHangingReadyCondition() throws Exception { }) // it's expected that server never starts and fails fast thus return false .build(); - thrown.expect(ContainerReadyConditionException.class); - thrown.expectMessage( - containsString("Provided ContainerReadyCondition.isReady() method took longer than containerReadyTimeout")); + thrown.expect(ContainerStartException.class); + thrown.expectCause(allOf(instanceOf(ContainerReadyConditionException.class), + hasProperty("message", containsString( + "Provided ContainerReadyCondition.isReady() method took longer than containerReadyTimeout")))); // throws expected Exception try { containerWithHangingReadyCondition.start(); @@ -68,7 +72,7 @@ public void testContainerReadyTimeout() throws Exception { .setContainerReadyCondition(() -> false) // never ready .build(); - thrown.expect(DockerTimeoutException.class); + thrown.expect(ContainerStartException.class); thrown.expectMessage(containsString("Container was not ready in")); try { From 8b29e854402ee501d3113a9f52761ba94ac029a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Ka=C5=A1=C3=ADk?= Date: Mon, 27 Jan 2020 18:10:08 +0100 Subject: [PATCH 2/3] Added option to only start container without re-creating it --- .../fault/tolerance/DatabaseCrashTest.java | 3 +- .../eap/qe/ts/common/docker/Container.java | 16 ++- .../jboss/eap/qe/ts/common/docker/Docker.java | 109 +++++++++++++----- .../docker/MalformedDockerConfigTest.java | 6 +- 4 files changed, 97 insertions(+), 37 deletions(-) diff --git a/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java b/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java index b11e09ce..79927452 100644 --- a/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java +++ b/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java @@ -113,7 +113,7 @@ public static void startDatabase() throws Exception { .withCmdOption("-u") .withCmdOption(String.valueOf(new UnixSystem().getUid())) .build(); - postgresDB.start(); + postgresDB.run(); } @BeforeClass @@ -205,6 +205,7 @@ public void dropDatabaseSchema() { public static void tearDown() throws Exception { // stop DB if still running and delete data directory postgresDB.stop(); + postgresDB.remove(); postgresDataDir.delete(); MicroProfileFaultToleranceServerConfiguration.disableFaultTolerance(); } diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java index 95edeefa..af11cd1e 100644 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java @@ -7,11 +7,16 @@ public interface Container { /** - * Start this container + * Start a previously stopped or killed container + */ + void start() throws ContainerStartException; + + /** + * Create and start new container * * @throws ContainerStartException thrown when start of container fails */ - void start() throws ContainerStartException; + void run() throws ContainerStartException; /** * @return Returns true if docker container is running. It does NOT check whether container is ready. @@ -34,4 +39,11 @@ public interface Container { */ void kill() throws ContainerKillException; + /** + * Remove a container from system + * + * @throws ContainerRemoveException thrown when removing fails + */ + void remove() throws ContainerRemoveException; + } diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java index d1978bc3..f7264f3e 100644 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java @@ -54,12 +54,38 @@ private Docker(Builder builder) { this.out = builder.out; } - /** - * Start this container - * - * @throws ContainerStartException thrown when start of container fails - */ + @Override public void start() throws ContainerStartException { + if (isRunning()) { + throw new ContainerStartException("Container '" + this.uuid + "' is already running!"); + } + if (!containerExists()) { + throw new ContainerStartException("Container '" + this.uuid + "' doesn't exist!"); + } + try { + final Process startProcess = runDockerCommand("start", this.uuid); + this.outputPrintingThread = startPrinterThread(startProcess, this.out, this.name); + final long startTime = System.currentTimeMillis(); + while (!isContainerReady(this.uuid, this.containerReadyCondition)) { + if (System.currentTimeMillis() - startTime > containerReadyTimeout) { + stop(); + remove(); + throw new ContainerStartException( + "Container '" + this.uuid + "' was not ready in " + this.containerReadyTimeout + " ms"); + } + // fail fast mechanism in case of malformed docker command, for example bad arguments, invalid format of port mapping, image version,... + if (!startProcess.isAlive() && startProcess.exitValue() != 0) { + throw new ContainerStartException("Failed to start '" + this.uuid + "' container!"); + } + } + } catch (DockerCommandException | ContainerReadyConditionException | ContainerStopException + | ContainerRemoveException e) { + throw new ContainerStartException("Failed to start container '" + this.uuid + "'!", e); + } + } + + @Override + public void run() throws ContainerStartException { if (!isDockerPresent()) { throw new ContainerStartException("'docker' command is not present on this machine!"); } @@ -67,7 +93,7 @@ public void start() throws ContainerStartException { this.out.println(Ansi.ansi().reset().a("Starting container ").fgCyan().a(name).reset() .a(" with ID ").fgYellow().a(uuid).reset()); - final List dockerStartCommand = composeStartCommand(); + final List dockerStartCommand = composeRunCommand(); Process dockerRunProcess; try { dockerRunProcess = new ProcessBuilder() @@ -85,7 +111,7 @@ public void start() throws ContainerStartException { while (!isContainerReady(this.uuid, this.containerReadyCondition)) { if (System.currentTimeMillis() - startTime > containerReadyTimeout) { stop(); - removeDockerContainer(this.uuid); + remove(); throw new ContainerStartException(uuid + " - Container was not ready in " + containerReadyTimeout + " ms"); } // fail fast mechanism in case of malformed docker command, for example bad arguments, invalid format of port mapping, image version,... @@ -104,7 +130,7 @@ public void start() throws ContainerStartException { } } - private List composeStartCommand() { + private List composeRunCommand() { List cmd = new ArrayList<>(); cmd.add("docker"); @@ -172,14 +198,10 @@ private boolean isContainerReady(final String uuid, final ContainerReadyConditio private boolean isDockerPresent() { try { - Process dockerInfoProcess = new ProcessBuilder() - .redirectErrorStream(true) - .command(new String[] { "docker", "info" }) - .start(); - dockerInfoProcess.waitFor(); - return dockerInfoProcess.exitValue() == 0; - } catch (IOException | InterruptedException e) { - throw new IllegalStateException("There was an exception when checking for docker command presence!"); + final int processExitValue = runDockerCommand("info").exitValue(); + return processExitValue == 0; + } catch (DockerCommandException e) { + throw new IllegalStateException("There was an exception when checking for docker command presence!", e); } } @@ -189,13 +211,35 @@ private boolean isDockerPresent() { public boolean isRunning() { Process dockerRunProcess; try { - dockerRunProcess = new ProcessBuilder() - .redirectErrorStream(true) - .command(new String[] { "docker", "ps" }) - .start(); + dockerRunProcess = runDockerCommand("ps"); + } catch (DockerCommandException ignored) { + //when we cannot start the process for making the check container is not running + return false; + } + + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(dockerRunProcess.getInputStream(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + if (line.contains(this.uuid)) { + return true; + } + } + } catch (IOException ignored) { + // ignore as any stop of docker container breaks the reader stream + // note that shutdown of docker would be already logged + } + return false; + } - dockerRunProcess.waitFor(); - } catch (IOException | InterruptedException ignored) { + /** + * @return Returns true if container exists. + */ + private boolean containerExists() { + Process dockerRunProcess; + try { + dockerRunProcess = runDockerCommand("ps", "-a"); + } catch (DockerCommandException ignored) { //when we cannot start the process for making the check container is not running return false; } @@ -262,19 +306,22 @@ public void kill() throws ContainerKillException { } private void terminatePrintingThread() throws InterruptedException { - outputPrintingThread.shutdown(); - outputPrintingThread.awaitTermination(10, TimeUnit.SECONDS); + if (outputPrintingThread != null) { + outputPrintingThread.shutdown(); + outputPrintingThread.awaitTermination(10, TimeUnit.SECONDS); + } } - private void removeDockerContainer(final String uuid) throws ContainerRemoveException { + @Override + public void remove() throws ContainerRemoveException { try { - runDockerCommand("rm", uuid); + runDockerCommand("rm", this.uuid); } catch (DockerCommandException e) { - throw new ContainerRemoveException("Failed to remove the container '" + uuid + "'!", e); + throw new ContainerRemoveException("Failed to remove the container '" + this.uuid + "'!", e); } } - private int runDockerCommand(final String... commandArguments) throws DockerCommandException { + private Process runDockerCommand(final String... commandArguments) throws DockerCommandException { final String dockerCommand = "docker"; final List cmd = new ArrayList<>(); cmd.add(dockerCommand); @@ -288,7 +335,7 @@ private int runDockerCommand(final String... commandArguments) throws DockerComm process.waitFor(10, TimeUnit.SECONDS); - return process.exitValue(); + return process; } catch (InterruptedException e) { throw new DockerCommandException("Interrupted while waiting for '" + String.join(" ", cmd) + "' to return!", e); } catch (IOException e) { @@ -298,14 +345,14 @@ private int runDockerCommand(final String... commandArguments) throws DockerComm @Override protected void before() throws ContainerStartException { - start(); + run(); } @Override protected void after() { try { stop(); - removeDockerContainer(this.uuid); + remove(); } catch (ContainerStopException e) { throw new IllegalStateException("Failed to stop container '" + this.uuid + "'! ", e); } catch (ContainerRemoveException e) { diff --git a/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java b/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java index 6eca8cd9..2cffa87b 100644 --- a/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java +++ b/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java @@ -32,7 +32,7 @@ public void testFailFastWithMalformedDockerCommand() throws Exception { thrown.expectMessage(containsString("Starting of docker container using command: \"docker run --name")); thrown.expectMessage(endsWith("failed. Check that provided command is correct.")); - containerWithInvalidVersion.start(); + containerWithInvalidVersion.run(); } @Test @@ -56,7 +56,7 @@ public void testContainerWithHangingReadyCondition() throws Exception { "Provided ContainerReadyCondition.isReady() method took longer than containerReadyTimeout")))); // throws expected Exception try { - containerWithHangingReadyCondition.start(); + containerWithHangingReadyCondition.run(); } finally { assertThat("ContainerReadyConditionException was thrown and starting container is expected to be stopped/killed. " + "However this did not happen and there is still container running which is bug.", @@ -76,7 +76,7 @@ public void testContainerReadyTimeout() throws Exception { thrown.expectMessage(containsString("Container was not ready in")); try { - containerWithShortTimeout.start(); + containerWithShortTimeout.run(); } finally { assertThat("DockerTimeoutException was thrown and starting container is expected to be stopped/killed. " + "However this did not happen and there is still container running which is bug.", From 23ddc84a2bea02e2fe069fcf19105bac5d0b537a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Ka=C5=A1=C3=ADk?= Date: Wed, 12 Feb 2020 10:51:46 +0100 Subject: [PATCH 3/3] Using --rm in docker run --- .../fault/tolerance/DatabaseCrashTest.java | 2 +- .../eap/qe/ts/common/docker/Container.java | 27 +++----- .../jboss/eap/qe/ts/common/docker/Docker.java | 62 +------------------ .../docker/MalformedDockerConfigTest.java | 6 +- 4 files changed, 14 insertions(+), 83 deletions(-) diff --git a/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java b/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java index 79927452..e0d70210 100644 --- a/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java +++ b/microprofile-fault-tolerance/src/test/java/org/jboss/eap/qe/microprofile/fault/tolerance/DatabaseCrashTest.java @@ -113,7 +113,7 @@ public static void startDatabase() throws Exception { .withCmdOption("-u") .withCmdOption(String.valueOf(new UnixSystem().getUid())) .build(); - postgresDB.run(); + postgresDB.start(); } @BeforeClass diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java index af11cd1e..02521613 100644 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Container.java @@ -12,38 +12,25 @@ public interface Container { void start() throws ContainerStartException; /** - * Create and start new container + * Check if container is running. Container in running state doesn't mean that the initialization is done and + * container is ready for work. * - * @throws ContainerStartException thrown when start of container fails - */ - void run() throws ContainerStartException; - - /** - * @return Returns true if docker container is running. It does NOT check whether container is ready. + * @return Returns true if docker container is running. */ boolean isRunning(); /** - * Stop this docker container using docker command. + * Stop this docker container. * - * @throws ContainerStopException thrown when the stop command fails. This generally means that the command wasn't - * successful and container might be still running. + * @throws ContainerStopException thrown when the stop fails. This might mean that the container is still running. */ void stop() throws ContainerStopException; /** - * Kill this docker container using docker command. Be aware that there might occur a situation when the docker - * command might fail. This might be caused for example by reaching file descriptors limit in system. + * Kill this container. * - * @throws ContainerKillException thrown when the kill command fails. + * @throws ContainerKillException thrown when the kill fails. This might mean that the container is still running. */ void kill() throws ContainerKillException; - /** - * Remove a container from system - * - * @throws ContainerRemoveException thrown when removing fails - */ - void remove() throws ContainerRemoveException; - } diff --git a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java index f7264f3e..f5f7ec78 100644 --- a/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java +++ b/tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/Docker.java @@ -56,36 +56,6 @@ private Docker(Builder builder) { @Override public void start() throws ContainerStartException { - if (isRunning()) { - throw new ContainerStartException("Container '" + this.uuid + "' is already running!"); - } - if (!containerExists()) { - throw new ContainerStartException("Container '" + this.uuid + "' doesn't exist!"); - } - try { - final Process startProcess = runDockerCommand("start", this.uuid); - this.outputPrintingThread = startPrinterThread(startProcess, this.out, this.name); - final long startTime = System.currentTimeMillis(); - while (!isContainerReady(this.uuid, this.containerReadyCondition)) { - if (System.currentTimeMillis() - startTime > containerReadyTimeout) { - stop(); - remove(); - throw new ContainerStartException( - "Container '" + this.uuid + "' was not ready in " + this.containerReadyTimeout + " ms"); - } - // fail fast mechanism in case of malformed docker command, for example bad arguments, invalid format of port mapping, image version,... - if (!startProcess.isAlive() && startProcess.exitValue() != 0) { - throw new ContainerStartException("Failed to start '" + this.uuid + "' container!"); - } - } - } catch (DockerCommandException | ContainerReadyConditionException | ContainerStopException - | ContainerRemoveException e) { - throw new ContainerStartException("Failed to start container '" + this.uuid + "'!", e); - } - } - - @Override - public void run() throws ContainerStartException { if (!isDockerPresent()) { throw new ContainerStartException("'docker' command is not present on this machine!"); } @@ -150,6 +120,8 @@ private List composeRunCommand() { cmd.addAll(options); + cmd.add("--rm"); //causes Docker to automatically remove the container when it exits + cmd.add(image); cmd.addAll(commandArguments); @@ -232,33 +204,6 @@ public boolean isRunning() { return false; } - /** - * @return Returns true if container exists. - */ - private boolean containerExists() { - Process dockerRunProcess; - try { - dockerRunProcess = runDockerCommand("ps", "-a"); - } catch (DockerCommandException ignored) { - //when we cannot start the process for making the check container is not running - return false; - } - - try (BufferedReader reader = new BufferedReader( - new InputStreamReader(dockerRunProcess.getInputStream(), StandardCharsets.UTF_8))) { - String line; - while ((line = reader.readLine()) != null) { - if (line.contains(this.uuid)) { - return true; - } - } - } catch (IOException ignored) { - // ignore as any stop of docker container breaks the reader stream - // note that shutdown of docker would be already logged - } - return false; - } - /** * Stop this docker container using docker command. * @@ -312,7 +257,6 @@ private void terminatePrintingThread() throws InterruptedException { } } - @Override public void remove() throws ContainerRemoveException { try { runDockerCommand("rm", this.uuid); @@ -345,7 +289,7 @@ private Process runDockerCommand(final String... commandArguments) throws Docker @Override protected void before() throws ContainerStartException { - run(); + start(); } @Override diff --git a/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java b/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java index 2cffa87b..6eca8cd9 100644 --- a/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java +++ b/tooling-docker/src/test/java/org/jboss/eap/qe/ts/common/docker/MalformedDockerConfigTest.java @@ -32,7 +32,7 @@ public void testFailFastWithMalformedDockerCommand() throws Exception { thrown.expectMessage(containsString("Starting of docker container using command: \"docker run --name")); thrown.expectMessage(endsWith("failed. Check that provided command is correct.")); - containerWithInvalidVersion.run(); + containerWithInvalidVersion.start(); } @Test @@ -56,7 +56,7 @@ public void testContainerWithHangingReadyCondition() throws Exception { "Provided ContainerReadyCondition.isReady() method took longer than containerReadyTimeout")))); // throws expected Exception try { - containerWithHangingReadyCondition.run(); + containerWithHangingReadyCondition.start(); } finally { assertThat("ContainerReadyConditionException was thrown and starting container is expected to be stopped/killed. " + "However this did not happen and there is still container running which is bug.", @@ -76,7 +76,7 @@ public void testContainerReadyTimeout() throws Exception { thrown.expectMessage(containsString("Container was not ready in")); try { - containerWithShortTimeout.run(); + containerWithShortTimeout.start(); } finally { assertThat("DockerTimeoutException was thrown and starting container is expected to be stopped/killed. " + "However this did not happen and there is still container running which is bug.",