From cd4fe67985acfe93e93ddb2116b60852dad6e0a6 Mon Sep 17 00:00:00 2001 From: Nick Miyake Date: Mon, 30 May 2016 14:50:46 -0700 Subject: [PATCH 1/3] Add option to remove containers with conflicting names on startup Fixes #64 --- circle.yml | 3 +- .../docker/compose/DockerComposeRule.java | 53 ++++++++++++- .../compose/DockerCompositionBuilder.java | 6 ++ .../compose/execution/AbstractCommand.java | 56 +++++++++++++ .../AbstractSynchronousExecutable.java | 75 ++++++++++++++++++ .../execution/DefaultDockerCompose.java | 49 +++--------- .../docker/compose/execution/Docker.java | 35 +++++++++ ...tions.java => DockerCommandLocations.java} | 6 +- .../execution/DockerComposeExecutable.java | 5 +- .../compose/execution/DockerExecutable.java | 63 +++++++++++++++ ...ion.java => DockerExecutionException.java} | 6 +- .../docker/compose/execution/Executable.java | 24 ++++++ .../docker/compose/execution/Retryer.java | 8 +- .../SynchronousDockerComposeExecutable.java | 52 +------------ .../SynchronousDockerExecutable.java | 26 +++++++ ...eRuleRestartContainersIntegrationTest.java | 78 +++++++++++++++++++ .../compose/DockerComposeRuleShould.java | 49 ++++++++++++ ...java => DockerCommandLocationsShould.java} | 16 ++-- .../execution/DockerComposeShould.java | 4 +- .../docker/compose/execution/DockerTest.java | 49 ++++++++++++ .../compose/execution/RetryerShould.java | 10 +-- .../RetryingDockerComposeShould.java | 8 +- .../named-containers-docker-compose.yaml | 7 ++ 23 files changed, 568 insertions(+), 120 deletions(-) create mode 100644 src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java create mode 100644 src/main/java/com/palantir/docker/compose/execution/AbstractSynchronousExecutable.java create mode 100644 src/main/java/com/palantir/docker/compose/execution/Docker.java rename src/main/java/com/palantir/docker/compose/execution/{DockerComposeLocations.java => DockerCommandLocations.java} (89%) create mode 100644 src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java rename src/main/java/com/palantir/docker/compose/execution/{DockerComposeExecutionException.java => DockerExecutionException.java} (79%) create mode 100644 src/main/java/com/palantir/docker/compose/execution/Executable.java create mode 100644 src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java create mode 100644 src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java rename src/test/java/com/palantir/docker/compose/execution/{DockerComposeLocationsShould.java => DockerCommandLocationsShould.java} (77%) create mode 100644 src/test/java/com/palantir/docker/compose/execution/DockerTest.java create mode 100644 src/test/resources/named-containers-docker-compose.yaml diff --git a/circle.yml b/circle.yml index cb9242532..83f17b12e 100644 --- a/circle.yml +++ b/circle.yml @@ -11,9 +11,10 @@ machine: dependencies: pre: - sudo pip install docker-compose + - sudo cp /usr/bin/docker /usr/local/bin/docker override: - ./gradlew resolveConfigurations - + test: pre: - ./gradlew findbugsMain findbugsTest checkstyleMain checkstyleTest javadoc --info diff --git a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java index 386523f7a..ece0c065a 100644 --- a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java +++ b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java @@ -6,6 +6,7 @@ import static com.palantir.docker.compose.connection.waiting.ClusterHealthCheck.serviceHealthCheck; import static com.palantir.docker.compose.connection.waiting.ClusterHealthCheck.transformingHealthCheck; +import com.google.common.collect.ImmutableSet; import com.palantir.docker.compose.configuration.DockerComposeFiles; import com.palantir.docker.compose.configuration.ProjectName; import com.palantir.docker.compose.connection.Cluster; @@ -18,16 +19,23 @@ import com.palantir.docker.compose.connection.waiting.ClusterWait; import com.palantir.docker.compose.connection.waiting.HealthCheck; import com.palantir.docker.compose.execution.DefaultDockerCompose; +import com.palantir.docker.compose.execution.Docker; import com.palantir.docker.compose.execution.DockerCompose; import com.palantir.docker.compose.execution.DockerComposeExecArgument; import com.palantir.docker.compose.execution.DockerComposeExecOption; import com.palantir.docker.compose.execution.DockerComposeExecutable; +import com.palantir.docker.compose.execution.DockerExecutable; +import com.palantir.docker.compose.execution.DockerExecutionException; import com.palantir.docker.compose.execution.RetryingDockerCompose; import com.palantir.docker.compose.logging.DoNothingLogCollector; import com.palantir.docker.compose.logging.FileLogCollector; import com.palantir.docker.compose.logging.LogCollector; import java.io.IOException; +import java.util.Collection; import java.util.List; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.immutables.value.Value; import org.joda.time.Duration; import org.junit.rules.ExternalResource; @@ -41,6 +49,7 @@ public abstract class DockerComposeRule extends ExternalResource { public static final int DEFAULT_RETRY_ATTEMPTS = 2; private static final Logger log = LoggerFactory.getLogger(DockerComposeRule.class); + private static final Pattern NAME_CONFLICT_PATTERN = Pattern.compile("The name \"([^\"]*)\" is already in use"); public DockerPort hostNetworkedPort(int port) { return new DockerPort(machine().getIp(), port, port); @@ -69,6 +78,11 @@ public DockerComposeExecutable executable() { .build(); } + @Value.Default + public Docker docker() { + return new Docker(DockerExecutable.builder().dockerConfiguration(machine()).build()); + } + @Value.Default public DockerCompose dockerCompose() { DockerCompose dockerCompose = new DefaultDockerCompose(executable(), machine()); @@ -93,6 +107,11 @@ protected boolean skipShutdown() { return false; } + @Value.Default + protected boolean removeConflictingContainersOnStartup() { + return false; + } + @Value.Default protected LogCollector logCollector() { return new DoNothingLogCollector(); @@ -102,7 +121,20 @@ protected LogCollector logCollector() { public void before() throws IOException, InterruptedException { log.debug("Starting docker-compose cluster"); dockerCompose().build(); - dockerCompose().up(); + + try { + dockerCompose().up(); + } catch (DockerExecutionException e) { + Set conflictingContainerNames = getConflictingContainerNames(e.getMessage()); + if (removeConflictingContainersOnStartup() && !conflictingContainerNames.isEmpty()) { + // if rule is configured to remove containers with existing name on startup and conflicting containers + // were detected, remove the containers and attempt to start again + removeDockerContainers(conflictingContainerNames); + dockerCompose().up(); + } else { + throw e; + } + } log.debug("Starting log collection"); @@ -187,4 +219,23 @@ public ImmutableDockerComposeRule.Builder waitingForHostNetworkedPort(int port, return addClusterWait(new ClusterWait(clusterHealthCheck, timeout)); } } + + private void removeDockerContainers(Collection containerNames) throws IOException, InterruptedException { + Docker docker = docker(); + try { + docker.rm(containerNames.toArray(new String[containerNames.size()])); + } catch (DockerExecutionException e) { + log.debug("docker rm failed, but continuing execution", e); + } + dockerCompose().up(); + } + + private static Set getConflictingContainerNames(String input) { + ImmutableSet.Builder builder = ImmutableSet.builder(); + Matcher matcher = NAME_CONFLICT_PATTERN.matcher(input); + while (matcher.find()) { + builder.add(matcher.group(1)); + } + return builder.build(); + } } diff --git a/src/main/java/com/palantir/docker/compose/DockerCompositionBuilder.java b/src/main/java/com/palantir/docker/compose/DockerCompositionBuilder.java index 3728a034f..c1d60ecc5 100644 --- a/src/main/java/com/palantir/docker/compose/DockerCompositionBuilder.java +++ b/src/main/java/com/palantir/docker/compose/DockerCompositionBuilder.java @@ -88,6 +88,12 @@ public DockerCompositionBuilder saveLogsTo(String path) { return this; } + + public DockerCompositionBuilder removeConflictingContainersOnStartup(boolean removeConflictingContainersOnStartup) { + builder.removeConflictingContainersOnStartup(removeConflictingContainersOnStartup); + return this; + } + public DockerCompositionBuilder retryAttempts(int retryAttempts) { builder.retryAttempts(retryAttempts); return this; diff --git a/src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java b/src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java new file mode 100644 index 000000000..84e7a6379 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java @@ -0,0 +1,56 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import static java.util.stream.Collectors.joining; + +import java.io.IOException; +import java.util.Arrays; + +public abstract class AbstractCommand { + + private final String commandName; + private final AbstractSynchronousExecutable executable; + + public AbstractCommand(String commandName, AbstractSynchronousExecutable executable) { + this.commandName = commandName; + this.executable = executable; + } + + protected final String executeCommand(ErrorHandler errorHandler, String... commands) + throws IOException, InterruptedException { + ProcessResult result = executable.run(commands); + + if (result.exitCode() != 0) { + errorHandler.handle(result.exitCode(), result.output(), commands); + } + + return result.output(); + } + + protected final ErrorHandler throwingOnError() { + return (exitCode, output, commands) -> { + String message = constructNonZeroExitErrorMessage(exitCode, commands) + "\nThe output was:\n" + output; + throw new DockerExecutionException(message); + }; + } + + private String constructNonZeroExitErrorMessage(int exitCode, String... commands) { + return "'" + commandName + " " + Arrays.stream(commands).collect(joining(" ")) + "' returned exit code " + + exitCode; + } + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/AbstractSynchronousExecutable.java b/src/main/java/com/palantir/docker/compose/execution/AbstractSynchronousExecutable.java new file mode 100644 index 000000000..1103dced5 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/AbstractSynchronousExecutable.java @@ -0,0 +1,75 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import static com.google.common.base.Throwables.propagate; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.concurrent.Executors.newSingleThreadExecutor; +import static java.util.stream.Collectors.joining; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; + +public abstract class AbstractSynchronousExecutable { + public static final int HOURS_TO_WAIT_FOR_STD_OUT_TO_CLOSE = 12; + public static final int MINUTES_TO_WAIT_AFTER_STD_OUT_CLOSES = 1; + private final T executable; + private final Consumer logConsumer; + + public AbstractSynchronousExecutable(T executable, Consumer logConsumer) { + this.executable = executable; + this.logConsumer = logConsumer; + } + + public ProcessResult run(String... commands) throws IOException, InterruptedException { + Process process = executable.execute(commands); + + Future outputProcessing = newSingleThreadExecutor() + .submit(() -> processOutputFrom(process)); + + String output = waitForResultFrom(outputProcessing); + + process.waitFor(MINUTES_TO_WAIT_AFTER_STD_OUT_CLOSES, TimeUnit.MINUTES); + + return new ProcessResult(process.exitValue(), output); + } + + private String processOutputFrom(Process process) { + return asReader(process.getInputStream()).lines() + .peek(logConsumer) + .collect(joining(System.lineSeparator())); + } + + private String waitForResultFrom(Future outputProcessing) { + try { + return outputProcessing.get(HOURS_TO_WAIT_FOR_STD_OUT_TO_CLOSE, TimeUnit.HOURS); + } catch (InterruptedException | ExecutionException | TimeoutException e) { + throw propagate(e); + } + } + + private BufferedReader asReader(InputStream inputStream) { + return new BufferedReader(new InputStreamReader(inputStream, UTF_8)); + } + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java b/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java index 1f8090374..dc4082474 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java +++ b/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java @@ -16,7 +16,6 @@ package com.palantir.docker.compose.execution; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.stream.Collectors.joining; import static org.apache.commons.lang3.Validate.validState; import static org.joda.time.Duration.standardMinutes; @@ -31,19 +30,17 @@ import com.palantir.docker.compose.connection.Ports; import java.io.IOException; import java.io.OutputStream; -import java.util.Arrays; import org.apache.commons.io.IOUtils; import org.joda.time.Duration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DefaultDockerCompose implements DockerCompose { +public class DefaultDockerCompose extends AbstractCommand implements DockerCompose { public static final Version VERSION_1_7_0 = Version.valueOf("1.7.0"); private static final Duration COMMAND_TIMEOUT = standardMinutes(2); private static final Logger log = LoggerFactory.getLogger(DefaultDockerCompose.class); - private final SynchronousDockerComposeExecutable executable; private final DockerMachine dockerMachine; private final DockerComposeExecutable rawExecutable; @@ -56,34 +53,35 @@ public DefaultDockerCompose(DockerComposeFiles dockerComposeFiles, DockerMachine } public DefaultDockerCompose(DockerComposeExecutable rawExecutable, DockerMachine dockerMachine) { + super("docker-compose", new SynchronousDockerComposeExecutable(rawExecutable, log::debug)); + this.rawExecutable = rawExecutable; - this.executable = new SynchronousDockerComposeExecutable(rawExecutable, log::debug); this.dockerMachine = dockerMachine; } @Override public void build() throws IOException, InterruptedException { - executeDockerComposeCommand(throwingOnError(), "build"); + executeCommand(throwingOnError(), "build"); } @Override public void up() throws IOException, InterruptedException { - executeDockerComposeCommand(throwingOnError(), "up", "-d"); + executeCommand(throwingOnError(), "up", "-d"); } @Override public void down() throws IOException, InterruptedException { - executeDockerComposeCommand(swallowingDownCommandDoesNotExist(), "down", "--volumes"); + executeCommand(swallowingDownCommandDoesNotExist(), "down", "--volumes"); } @Override public void kill() throws IOException, InterruptedException { - executeDockerComposeCommand(throwingOnError(), "kill"); + executeCommand(throwingOnError(), "kill"); } @Override public void rm() throws IOException, InterruptedException { - executeDockerComposeCommand(throwingOnError(), "rm", "--force", "-v"); + executeCommand(throwingOnError(), "rm", "--force", "-v"); } @Override @@ -91,7 +89,7 @@ public String exec(DockerComposeExecOption dockerComposeExecOption, String conta DockerComposeExecArgument dockerComposeExecArgument) throws IOException, InterruptedException { verifyDockerComposeVersionAtLeast(VERSION_1_7_0, "You need at least docker-compose 1.7 to run docker-compose exec"); String[] fullArgs = constructFullDockerComposeExecArguments(dockerComposeExecOption, containerName, dockerComposeExecArgument); - return executeDockerComposeCommand(throwingOnError(), fullArgs); + return executeCommand(throwingOnError(), fullArgs); } private void verifyDockerComposeVersionAtLeast(Version targetVersion, String message) throws IOException, InterruptedException { @@ -99,7 +97,7 @@ private void verifyDockerComposeVersionAtLeast(Version targetVersion, String mes } private Version version() throws IOException, InterruptedException { - String versionOutput = executeDockerComposeCommand(throwingOnError(), "-v"); + String versionOutput = executeCommand(throwingOnError(), "-v"); return DockerComposeVersion.parseFromDockerComposeVersion(versionOutput); } @@ -115,7 +113,7 @@ private String[] constructFullDockerComposeExecArguments(DockerComposeExecOption @Override public ContainerNames ps() throws IOException, InterruptedException { - String psOutput = executeDockerComposeCommand(throwingOnError(), "ps"); + String psOutput = executeCommand(throwingOnError(), "ps"); return ContainerNames.parseFromDockerComposePs(psOutput); } @@ -150,34 +148,11 @@ private Process followLogs(String container) throws IOException, InterruptedExce @Override public Ports ports(String service) throws IOException, InterruptedException { - String psOutput = executeDockerComposeCommand(throwingOnError(), "ps", service); + String psOutput = executeCommand(throwingOnError(), "ps", service); validState(!Strings.isNullOrEmpty(psOutput), "No container with name '" + service + "' found"); return Ports.parseFromDockerComposePs(psOutput, dockerMachine.getIp()); } - private ErrorHandler throwingOnError() { - return (exitCode, output, commands) -> { - String message = constructNonZeroExitErrorMessage(exitCode, commands) + "\nThe output was:\n" + output; - throw new DockerComposeExecutionException(message); - }; - } - - private String executeDockerComposeCommand(ErrorHandler errorHandler, String... commands) - throws IOException, InterruptedException { - ProcessResult result = executable.run(commands); - - if (result.exitCode() != 0) { - errorHandler.handle(result.exitCode(), result.output(), commands); - } - - return result.output(); - } - - - private String constructNonZeroExitErrorMessage(int exitCode, String... commands) { - return "'docker-compose " + Arrays.stream(commands).collect(joining(" ")) + "' returned exit code " + exitCode; - } - private ErrorHandler swallowingDownCommandDoesNotExist() { return (exitCode, output, commands) -> { if (downCommandWasPresent(output)) { diff --git a/src/main/java/com/palantir/docker/compose/execution/Docker.java b/src/main/java/com/palantir/docker/compose/execution/Docker.java new file mode 100644 index 000000000..fc7d2c759 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/Docker.java @@ -0,0 +1,35 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import com.google.common.collect.ObjectArrays; +import java.io.IOException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class Docker extends AbstractCommand { + + private static final Logger log = LoggerFactory.getLogger(Docker.class); + + public Docker(DockerExecutable rawExecutable) { + super("docker", new SynchronousDockerExecutable(rawExecutable, log::debug)); + } + + public void rm(String... containerNames) throws IOException, InterruptedException { + executeCommand(throwingOnError(), ObjectArrays.concat(new String[] {"rm", "-f"}, containerNames, String.class)); + } + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/DockerComposeLocations.java b/src/main/java/com/palantir/docker/compose/execution/DockerCommandLocations.java similarity index 89% rename from src/main/java/com/palantir/docker/compose/execution/DockerComposeLocations.java rename to src/main/java/com/palantir/docker/compose/execution/DockerCommandLocations.java index 200f98e9a..a1e0c0a06 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DockerComposeLocations.java +++ b/src/main/java/com/palantir/docker/compose/execution/DockerCommandLocations.java @@ -22,13 +22,13 @@ import java.util.Optional; import java.util.function.Predicate; -public class DockerComposeLocations { +public class DockerCommandLocations { private static final Predicate IS_NOT_NULL = path -> path != null; private static final Predicate FILE_EXISTS = path -> new File(path).exists(); private final List possiblePaths; - public DockerComposeLocations(String... possiblePaths) { + public DockerCommandLocations(String... possiblePaths) { this.possiblePaths = asList(possiblePaths); } @@ -42,6 +42,6 @@ public Optional preferredLocation() { @Override public String toString() { - return "DockerComposeLocations{possiblePaths=" + possiblePaths + "}"; + return "DockerCommandLocations{possiblePaths=" + possiblePaths + "}"; } } diff --git a/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java b/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java index 3770ee86d..2d6b2cf68 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java +++ b/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java @@ -25,10 +25,10 @@ import org.slf4j.LoggerFactory; @Value.Immutable -public abstract class DockerComposeExecutable { +public abstract class DockerComposeExecutable implements Executable { private static final Logger log = LoggerFactory.getLogger(DockerComposeExecutable.class); - private static final DockerComposeLocations DOCKER_COMPOSE_LOCATIONS = new DockerComposeLocations( + private static final DockerCommandLocations DOCKER_COMPOSE_LOCATIONS = new DockerCommandLocations( System.getenv("DOCKER_COMPOSE_LOCATION"), "/usr/local/bin/docker-compose" ); @@ -51,6 +51,7 @@ protected String dockerComposePath() { return pathToUse; } + @Override public Process execute(String... commands) throws IOException { List args = ImmutableList.builder() .add(dockerComposePath()) diff --git a/src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java b/src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java new file mode 100644 index 000000000..bb20a0fb9 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java @@ -0,0 +1,63 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import com.google.common.collect.ImmutableList; +import java.io.IOException; +import java.util.List; +import org.immutables.value.Value; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Value.Immutable +public abstract class DockerExecutable implements Executable { + private static final Logger log = LoggerFactory.getLogger(DockerExecutable.class); + + private static final DockerCommandLocations DOCKER_LOCATIONS = new DockerCommandLocations( + System.getenv("DOCKER_LOCATION"), + "/usr/local/bin/docker" + ); + + @Value.Parameter protected abstract DockerConfiguration dockerConfiguration(); + + @Value.Derived + protected String dockerPath() { + String pathToUse = DOCKER_LOCATIONS.preferredLocation() + .orElseThrow(() -> new IllegalStateException( + "Could not find docker, looked in: " + DOCKER_LOCATIONS)); + + log.debug("Using docker found at " + pathToUse); + + return pathToUse; + } + + @Override + public Process execute(String... commands) throws IOException { + List args = ImmutableList.builder() + .add(dockerPath()) + .add(commands) + .build(); + + return dockerConfiguration().configuredDockerComposeProcess() + .command(args) + .redirectErrorStream(true) + .start(); + } + + public static ImmutableDockerExecutable.Builder builder() { + return ImmutableDockerExecutable.builder(); + } +} diff --git a/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutionException.java b/src/main/java/com/palantir/docker/compose/execution/DockerExecutionException.java similarity index 79% rename from src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutionException.java rename to src/main/java/com/palantir/docker/compose/execution/DockerExecutionException.java index 5bef1f98d..bab99b7c9 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutionException.java +++ b/src/main/java/com/palantir/docker/compose/execution/DockerExecutionException.java @@ -15,11 +15,11 @@ */ package com.palantir.docker.compose.execution; -public class DockerComposeExecutionException extends RuntimeException { - public DockerComposeExecutionException() { +public class DockerExecutionException extends RuntimeException { + public DockerExecutionException() { } - public DockerComposeExecutionException(String message) { + public DockerExecutionException(String message) { super(message); } } diff --git a/src/main/java/com/palantir/docker/compose/execution/Executable.java b/src/main/java/com/palantir/docker/compose/execution/Executable.java new file mode 100644 index 000000000..8b3bf6d7c --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/Executable.java @@ -0,0 +1,24 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import java.io.IOException; + +public interface Executable { + + Process execute(String... commands) throws IOException; + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/Retryer.java b/src/main/java/com/palantir/docker/compose/execution/Retryer.java index 1914d3f07..f3f2dc734 100644 --- a/src/main/java/com/palantir/docker/compose/execution/Retryer.java +++ b/src/main/java/com/palantir/docker/compose/execution/Retryer.java @@ -22,7 +22,7 @@ public class Retryer { private static final Logger log = LoggerFactory.getLogger(Retryer.class); - public interface RetryableDockerComposeOperation { + public interface RetryableDockerOperation { T call() throws IOException, InterruptedException; } @@ -32,12 +32,12 @@ public Retryer(int retryAttempts) { this.retryAttempts = retryAttempts; } - public T runWithRetries(RetryableDockerComposeOperation operation) throws IOException, InterruptedException { - DockerComposeExecutionException lastExecutionException = null; + public T runWithRetries(RetryableDockerOperation operation) throws IOException, InterruptedException { + DockerExecutionException lastExecutionException = null; for (int i = 0; i <= retryAttempts; i++) { try { return operation.call(); - } catch (DockerComposeExecutionException e) { + } catch (DockerExecutionException e) { lastExecutionException = e; log.warn("Caught exception: " + e.getMessage() + ". Retrying."); } diff --git a/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java b/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java index ebcc0f3cb..217af75cd 100644 --- a/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java +++ b/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java @@ -15,61 +15,13 @@ */ package com.palantir.docker.compose.execution; -import static com.google.common.base.Throwables.propagate; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.concurrent.Executors.newSingleThreadExecutor; -import static java.util.stream.Collectors.joining; - -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.function.Consumer; -public class SynchronousDockerComposeExecutable { - public static final int HOURS_TO_WAIT_FOR_STD_OUT_TO_CLOSE = 12; - public static final int MINUTES_TO_WAIT_AFTER_STD_OUT_CLOSES = 1; - private final DockerComposeExecutable dockerComposeExecutable; - private final Consumer logConsumer; +public class SynchronousDockerComposeExecutable extends AbstractSynchronousExecutable { public SynchronousDockerComposeExecutable(DockerComposeExecutable dockerComposeExecutable, Consumer logConsumer) { - this.dockerComposeExecutable = dockerComposeExecutable; - this.logConsumer = logConsumer; - } - - public ProcessResult run(String... commands) throws IOException, InterruptedException { - Process process = dockerComposeExecutable.execute(commands); - - Future outputProcessing = newSingleThreadExecutor() - .submit(() -> processOutputFrom(process)); - - String output = waitForResultFrom(outputProcessing); - - process.waitFor(MINUTES_TO_WAIT_AFTER_STD_OUT_CLOSES, TimeUnit.MINUTES); - - return new ProcessResult(process.exitValue(), output); - } - - private String processOutputFrom(Process process) { - return asReader(process.getInputStream()).lines() - .peek(logConsumer) - .collect(joining(System.lineSeparator())); + super(dockerComposeExecutable, logConsumer); } - private String waitForResultFrom(Future outputProcessing) { - try { - return outputProcessing.get(HOURS_TO_WAIT_FOR_STD_OUT_TO_CLOSE, TimeUnit.HOURS); - } catch (InterruptedException | ExecutionException | TimeoutException e) { - throw propagate(e); - } - } - - private BufferedReader asReader(InputStream inputStream) { - return new BufferedReader(new InputStreamReader(inputStream, UTF_8)); - } } diff --git a/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java b/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java new file mode 100644 index 000000000..5d4ff651e --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java @@ -0,0 +1,26 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import java.util.function.Consumer; + +public class SynchronousDockerExecutable extends AbstractSynchronousExecutable { + + public SynchronousDockerExecutable(DockerExecutable dockerExecutable, Consumer logConsumer) { + super(dockerExecutable, logConsumer); + } + +} diff --git a/src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java b/src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java new file mode 100644 index 000000000..2cfe7f96f --- /dev/null +++ b/src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java @@ -0,0 +1,78 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose; + +import com.palantir.docker.compose.connection.DockerMachine; +import com.palantir.docker.compose.execution.Docker; +import com.palantir.docker.compose.execution.DockerExecutable; +import com.palantir.docker.compose.execution.DockerExecutionException; +import java.io.IOException; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class DockerComposeRuleRestartContainersIntegrationTest { + + private static final Logger log = LoggerFactory.getLogger(DockerComposeRuleRestartContainersIntegrationTest.class); + private static final String DOCKER_COMPOSE_YAML_PATH = "src/test/resources/named-containers-docker-compose.yaml"; + + @Rule + public ExpectedException exception = ExpectedException.none(); + + /** + * Tests assume that container names in the YAML file are not already in use. In order to enforce this invariant, + * this cleanup method is run before each test. This method runs "docker rm" on the container names used by the + * tests. The names are hard-coded and should match the names used by the containers defined in the + * DOCKER_COMPOSE_YAML_PATH file. + */ + @Before + public void cleanup() throws IOException, InterruptedException { + Docker docker = new Docker( + DockerExecutable.builder().dockerConfiguration(DockerMachine.localMachine().build()).build()); + try { + docker.rm("/test-1.container.name", "/test-2.container.name"); + } catch (DockerExecutionException e) { + log.debug("docker rm failed in cleanup, but continuing", e); + } + } + + @Test + public void testDockerComposeRuleFailsWithExistingContainers() throws IOException, InterruptedException { + DockerComposition composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH).build(); + composition.before(); + composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH).build(); + + exception.expect(DockerExecutionException.class); + exception.expectMessage("'docker-compose up -d' returned exit code"); + composition.before(); + } + + @Test + public void testDockerComposeRuleRemovesExistingContainers() throws IOException, InterruptedException { + DockerComposition composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH).build(); + composition.before(); + + composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH) + .removeConflictingContainersOnStartup(true) + .build(); + composition.before(); + composition.after(); + } + +} diff --git a/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java b/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java index de0d1bb3a..29ef32298 100644 --- a/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java +++ b/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java @@ -24,6 +24,7 @@ import static org.joda.time.Duration.millis; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -39,7 +40,9 @@ import com.palantir.docker.compose.connection.DockerPort; import com.palantir.docker.compose.connection.waiting.HealthCheck; import com.palantir.docker.compose.connection.waiting.SuccessOrFailure; +import com.palantir.docker.compose.execution.Docker; import com.palantir.docker.compose.execution.DockerCompose; +import com.palantir.docker.compose.execution.DockerExecutionException; import com.palantir.docker.compose.logging.LogCollector; import java.io.File; import java.io.IOException; @@ -217,6 +220,52 @@ public void not_shut_down_when_skipShutdown_is_true() throws InterruptedExceptio verify(logCollector, times(1)).stopCollecting(); } + @Test + public void before_fails_when_docker_up_throws_exception() throws IOException, InterruptedException { + doThrow(new DockerExecutionException("")).when(dockerCompose).up(); + rule = defaultBuilder().removeConflictingContainersOnStartup(true).build(); + exception.expect(DockerExecutionException.class); + rule.before(); + } + + @Test + public void before_retries_if_docker_up_throws_container_conflict_exception() + throws IOException, InterruptedException { + doThrow(new DockerExecutionException("The name \"testName\" is already in use")) + .doNothing() + .when(dockerCompose).up(); + Docker mockDocker = mock(Docker.class); + + rule = defaultBuilder().docker(mockDocker).removeConflictingContainersOnStartup(true).build(); + rule.before(); + } + + @Test + public void docker_execution_exception_in_before_retry_is_ignored() + throws IOException, InterruptedException { + doThrow(new DockerExecutionException("The name \"testName\" is already in use")) + .doNothing() + .when(dockerCompose).up(); + Docker mockDocker = mock(Docker.class); + doThrow(DockerExecutionException.class).when(mockDocker).rm(any()); + + rule = defaultBuilder().docker(mockDocker).removeConflictingContainersOnStartup(true).build(); + rule.before(); + } + + @Test + public void non_docker_execution_exception_in_before_retry_is_thrown() + throws IOException, InterruptedException { + doThrow(new DockerExecutionException("The name \"testName\" is already in use")) + .doNothing() + .when(dockerCompose).up(); + Docker mockDocker = mock(Docker.class); + doThrow(RuntimeException.class).when(mockDocker).rm(any()); + rule = defaultBuilder().docker(mockDocker).removeConflictingContainersOnStartup(true).build(); + exception.expect(RuntimeException.class); + rule.before(); + } + public Container withComposeExecutableReturningContainerFor(String containerName) { final Container container = new Container(containerName, dockerCompose); when(dockerCompose.container(containerName)).thenReturn(container); diff --git a/src/test/java/com/palantir/docker/compose/execution/DockerComposeLocationsShould.java b/src/test/java/com/palantir/docker/compose/execution/DockerCommandLocationsShould.java similarity index 77% rename from src/test/java/com/palantir/docker/compose/execution/DockerComposeLocationsShould.java rename to src/test/java/com/palantir/docker/compose/execution/DockerCommandLocationsShould.java index 5dbbc0072..222e6cf53 100644 --- a/src/test/java/com/palantir/docker/compose/execution/DockerComposeLocationsShould.java +++ b/src/test/java/com/palantir/docker/compose/execution/DockerCommandLocationsShould.java @@ -25,7 +25,7 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; -public class DockerComposeLocationsShould { +public class DockerCommandLocationsShould { private static final String badLocation = "file/that/does/not/exist"; private static final String otherBadLocation = "another/file/that/does/not/exist"; @@ -39,32 +39,32 @@ public void setup() throws IOException { } @Test public void - provide_the_first_docker_compose_location_if_it_exists() throws IOException { - DockerComposeLocations dockerComposeLocations = new DockerComposeLocations( + provide_the_first_docker_command_location_if_it_exists() throws IOException { + DockerCommandLocations dockerCommandLocations = new DockerCommandLocations( badLocation, goodLocation, otherBadLocation); - assertThat(dockerComposeLocations.preferredLocation().get(), + assertThat(dockerCommandLocations.preferredLocation().get(), is(goodLocation)); } @Test public void skip_paths_from_environment_variables_that_are_unset() { - DockerComposeLocations dockerComposeLocations = new DockerComposeLocations( + DockerCommandLocations dockerCommandLocations = new DockerCommandLocations( System.getenv("AN_UNSET_DOCKER_COMPOSE_PATH"), goodLocation); - assertThat(dockerComposeLocations.preferredLocation().get(), + assertThat(dockerCommandLocations.preferredLocation().get(), is(goodLocation)); } @Test public void have_no_preferred_path_when_all_possible_paths_are_all_invalid() { - DockerComposeLocations dockerComposeLocations = new DockerComposeLocations( + DockerCommandLocations dockerCommandLocations = new DockerCommandLocations( badLocation); - assertThat(dockerComposeLocations.preferredLocation(), + assertThat(dockerCommandLocations.preferredLocation(), is(empty())); } } diff --git a/src/test/java/com/palantir/docker/compose/execution/DockerComposeShould.java b/src/test/java/com/palantir/docker/compose/execution/DockerComposeShould.java index fa6420dc4..dd9503d61 100644 --- a/src/test/java/com/palantir/docker/compose/execution/DockerComposeShould.java +++ b/src/test/java/com/palantir/docker/compose/execution/DockerComposeShould.java @@ -103,7 +103,7 @@ public void call_docker_compose_with_the_follow_flag_when_the_version_is_at_leas @Test public void throw_exception_when_kill_exits_with_a_non_zero_exit_code() throws IOException, InterruptedException { when(executedProcess.exitValue()).thenReturn(1); - exception.expect(DockerComposeExecutionException.class); + exception.expect(DockerExecutionException.class); exception.expectMessage("'docker-compose kill' returned exit code 1"); compose.kill(); } @@ -122,7 +122,7 @@ public void throw_exception_when_down_fails_for_a_reason_other_than_the_command_ when(executedProcess.exitValue()).thenReturn(1); when(executedProcess.getInputStream()).thenReturn(toInputStream("")); - exception.expect(DockerComposeExecutionException.class); + exception.expect(DockerExecutionException.class); compose.down(); } diff --git a/src/test/java/com/palantir/docker/compose/execution/DockerTest.java b/src/test/java/com/palantir/docker/compose/execution/DockerTest.java new file mode 100644 index 000000000..6ef96894d --- /dev/null +++ b/src/test/java/com/palantir/docker/compose/execution/DockerTest.java @@ -0,0 +1,49 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import static org.apache.commons.io.IOUtils.toInputStream; +import static org.mockito.Matchers.anyVararg; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import org.junit.Before; +import org.junit.Test; + +public class DockerTest { + + private final DockerExecutable executor = mock(DockerExecutable.class); + private final Docker docker = new Docker(executor); + + private final Process executedProcess = mock(Process.class); + + @Before + public void setup() throws IOException, InterruptedException { + when(executor.execute(anyVararg())).thenReturn(executedProcess); + when(executedProcess.getInputStream()).thenReturn(toInputStream("0.0.0.0:7000->7000/tcp")); + when(executedProcess.exitValue()).thenReturn(0); + } + + @Test + public void rm_calls_docker_rm_with_force_flag() throws IOException, InterruptedException { + docker.rm("testContainer"); + + verify(executor).execute("rm", "-f", "testContainer"); + } + +} diff --git a/src/test/java/com/palantir/docker/compose/execution/RetryerShould.java b/src/test/java/com/palantir/docker/compose/execution/RetryerShould.java index 608a3571c..4bc1a6c1e 100644 --- a/src/test/java/com/palantir/docker/compose/execution/RetryerShould.java +++ b/src/test/java/com/palantir/docker/compose/execution/RetryerShould.java @@ -27,7 +27,7 @@ import org.junit.Test; public class RetryerShould { - private final Retryer.RetryableDockerComposeOperation operation = mock(Retryer.RetryableDockerComposeOperation.class); + private final Retryer.RetryableDockerOperation operation = mock(Retryer.RetryableDockerOperation.class); private final Retryer retryer = new Retryer(1); @Test @@ -41,7 +41,7 @@ public void not_retry_if_the_operation_was_successful_and_return_result() throws @Test public void retry_the_operation_if_it_failed_once_and_return_the_result_of_the_next_successful_call() throws Exception { when(operation.call()).thenAnswer(MockitoMultiAnswer.of( - firstInvocation -> { throw new DockerComposeExecutionException(); }, + firstInvocation -> { throw new DockerExecutionException(); }, secondInvocation -> "hola" )); @@ -51,17 +51,17 @@ public void retry_the_operation_if_it_failed_once_and_return_the_result_of_the_n @Test public void throw_the_last_exception_when_the_operation_fails_more_times_than_the_number_of_specified_retry_attempts() throws Exception { - DockerComposeExecutionException finalException = new DockerComposeExecutionException(); + DockerExecutionException finalException = new DockerExecutionException(); when(operation.call()).thenAnswer(MockitoMultiAnswer.of( - firstInvocation -> { throw new DockerComposeExecutionException(); }, + firstInvocation -> { throw new DockerExecutionException(); }, secondInvocation -> { throw finalException; } )); try { retryer.runWithRetries(operation); fail("Should have caught exception"); - } catch (DockerComposeExecutionException actualException) { + } catch (DockerExecutionException actualException) { assertThat(actualException, is(finalException)); } diff --git a/src/test/java/com/palantir/docker/compose/execution/RetryingDockerComposeShould.java b/src/test/java/com/palantir/docker/compose/execution/RetryingDockerComposeShould.java index ca95b4e6c..3ef6f2dda 100644 --- a/src/test/java/com/palantir/docker/compose/execution/RetryingDockerComposeShould.java +++ b/src/test/java/com/palantir/docker/compose/execution/RetryingDockerComposeShould.java @@ -44,8 +44,8 @@ public void before() throws IOException, InterruptedException { } private void retryerJustCallsOperation() throws IOException, InterruptedException { - when(retryer.runWithRetries(any(Retryer.RetryableDockerComposeOperation.class))).thenAnswer(invocation -> { - Retryer.RetryableDockerComposeOperation operation = (Retryer.RetryableDockerComposeOperation) invocation.getArguments()[0]; + when(retryer.runWithRetries(any(Retryer.RetryableDockerOperation.class))).thenAnswer(invocation -> { + Retryer.RetryableDockerOperation operation = (Retryer.RetryableDockerOperation) invocation.getArguments()[0]; return operation.call(); }); } @@ -71,11 +71,11 @@ public void call_ps_on_the_underlying_docker_compose_and_returns_the_same_value( } private void verifyRetryerWasUsed() throws IOException, InterruptedException { - verify(retryer).runWithRetries(any(Retryer.RetryableDockerComposeOperation.class)); + verify(retryer).runWithRetries(any(Retryer.RetryableDockerOperation.class)); } private void verifyRetryerWasNotUsed() throws IOException, InterruptedException { - verify(retryer, times(0)).runWithRetries(any(Retryer.RetryableDockerComposeOperation.class)); + verify(retryer, times(0)).runWithRetries(any(Retryer.RetryableDockerOperation.class)); } @Test diff --git a/src/test/resources/named-containers-docker-compose.yaml b/src/test/resources/named-containers-docker-compose.yaml new file mode 100644 index 000000000..1e5b8233e --- /dev/null +++ b/src/test/resources/named-containers-docker-compose.yaml @@ -0,0 +1,7 @@ +test-1: + container_name: test-1.container.name + image: kiasaki/alpine-postgres + +test-2: + container_name: test-2.container.name + image: kiasaki/alpine-postgres From 5e20c6c97585a82add2238b3cad7f7d0ee1ab5c2 Mon Sep 17 00:00:00 2001 From: Nick Miyake Date: Thu, 2 Jun 2016 11:26:22 -0700 Subject: [PATCH 2/3] Address PR feedback --- circle.yml | 1 - .../docker/compose/DockerComposeRule.java | 57 +++----- .../compose/execution/AbstractCommand.java | 56 -------- ...ynchronousExecutable.java => Command.java} | 33 ++++- ...lictingContainerRemovingDockerCompose.java | 98 +++++++++++++ .../execution/DefaultDockerCompose.java | 28 ++-- .../execution/DelegatingDockerCompose.java | 81 +++++++++++ .../docker/compose/execution/Docker.java | 14 +- .../execution/DockerComposeExecutable.java | 8 +- .../compose/execution/DockerExecutable.java | 8 +- .../compose/execution/ErrorHandler.java | 2 +- .../docker/compose/execution/Executable.java | 2 + .../execution/RetryingDockerCompose.java | 53 +------ .../SynchronousDockerComposeExecutable.java | 27 ---- .../SynchronousDockerExecutable.java | 26 ---- ...eRuleRestartContainersIntegrationTest.java | 36 +---- .../compose/DockerComposeRuleShould.java | 42 +++--- ...ecutableShould.java => CommandShould.java} | 40 ++++-- ...gContainerRemovingDockerComposeShould.java | 131 ++++++++++++++++++ .../{DockerTest.java => DockerShould.java} | 4 +- 20 files changed, 454 insertions(+), 293 deletions(-) delete mode 100644 src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java rename src/main/java/com/palantir/docker/compose/execution/{AbstractSynchronousExecutable.java => Command.java} (68%) create mode 100644 src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java create mode 100644 src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java delete mode 100644 src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java delete mode 100644 src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java rename src/test/java/com/palantir/docker/compose/execution/{SynchronousDockerComposeExecutableShould.java => CommandShould.java} (61%) create mode 100644 src/test/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerComposeShould.java rename src/test/java/com/palantir/docker/compose/execution/{DockerTest.java => DockerShould.java} (92%) diff --git a/circle.yml b/circle.yml index 83f17b12e..8473231e0 100644 --- a/circle.yml +++ b/circle.yml @@ -11,7 +11,6 @@ machine: dependencies: pre: - sudo pip install docker-compose - - sudo cp /usr/bin/docker /usr/local/bin/docker override: - ./gradlew resolveConfigurations diff --git a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java index ece0c065a..3989878b4 100644 --- a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java +++ b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java @@ -6,7 +6,6 @@ import static com.palantir.docker.compose.connection.waiting.ClusterHealthCheck.serviceHealthCheck; import static com.palantir.docker.compose.connection.waiting.ClusterHealthCheck.transformingHealthCheck; -import com.google.common.collect.ImmutableSet; import com.palantir.docker.compose.configuration.DockerComposeFiles; import com.palantir.docker.compose.configuration.ProjectName; import com.palantir.docker.compose.connection.Cluster; @@ -18,6 +17,7 @@ import com.palantir.docker.compose.connection.waiting.ClusterHealthCheck; import com.palantir.docker.compose.connection.waiting.ClusterWait; import com.palantir.docker.compose.connection.waiting.HealthCheck; +import com.palantir.docker.compose.execution.ConflictingContainerRemovingDockerCompose; import com.palantir.docker.compose.execution.DefaultDockerCompose; import com.palantir.docker.compose.execution.Docker; import com.palantir.docker.compose.execution.DockerCompose; @@ -25,17 +25,12 @@ import com.palantir.docker.compose.execution.DockerComposeExecOption; import com.palantir.docker.compose.execution.DockerComposeExecutable; import com.palantir.docker.compose.execution.DockerExecutable; -import com.palantir.docker.compose.execution.DockerExecutionException; import com.palantir.docker.compose.execution.RetryingDockerCompose; import com.palantir.docker.compose.logging.DoNothingLogCollector; import com.palantir.docker.compose.logging.FileLogCollector; import com.palantir.docker.compose.logging.LogCollector; import java.io.IOException; -import java.util.Collection; import java.util.List; -import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.immutables.value.Value; import org.joda.time.Duration; import org.junit.rules.ExternalResource; @@ -49,7 +44,6 @@ public abstract class DockerComposeRule extends ExternalResource { public static final int DEFAULT_RETRY_ATTEMPTS = 2; private static final Logger log = LoggerFactory.getLogger(DockerComposeRule.class); - private static final Pattern NAME_CONFLICT_PATTERN = Pattern.compile("The name \"([^\"]*)\" is already in use"); public DockerPort hostNetworkedPort(int port) { return new DockerPort(machine().getIp(), port, port); @@ -70,7 +64,7 @@ public ProjectName projectName() { } @Value.Default - public DockerComposeExecutable executable() { + public DockerComposeExecutable dockerComposeExecutable() { return DockerComposeExecutable.builder() .dockerComposeFiles(files()) .dockerConfiguration(machine()) @@ -78,14 +72,21 @@ public DockerComposeExecutable executable() { .build(); } + @Value.Default + public DockerExecutable dockerExecutable() { + return DockerExecutable.builder() + .dockerConfiguration(machine()) + .build(); + } + @Value.Default public Docker docker() { - return new Docker(DockerExecutable.builder().dockerConfiguration(machine()).build()); + return new Docker(dockerExecutable()); } @Value.Default public DockerCompose dockerCompose() { - DockerCompose dockerCompose = new DefaultDockerCompose(executable(), machine()); + DockerCompose dockerCompose = new DefaultDockerCompose(dockerComposeExecutable(), machine()); return new RetryingDockerCompose(retryAttempts(), dockerCompose); } @@ -109,7 +110,7 @@ protected boolean skipShutdown() { @Value.Default protected boolean removeConflictingContainersOnStartup() { - return false; + return true; } @Value.Default @@ -122,19 +123,11 @@ public void before() throws IOException, InterruptedException { log.debug("Starting docker-compose cluster"); dockerCompose().build(); - try { - dockerCompose().up(); - } catch (DockerExecutionException e) { - Set conflictingContainerNames = getConflictingContainerNames(e.getMessage()); - if (removeConflictingContainersOnStartup() && !conflictingContainerNames.isEmpty()) { - // if rule is configured to remove containers with existing name on startup and conflicting containers - // were detected, remove the containers and attempt to start again - removeDockerContainers(conflictingContainerNames); - dockerCompose().up(); - } else { - throw e; - } + DockerCompose upDockerCompose = dockerCompose(); + if (removeConflictingContainersOnStartup()) { + upDockerCompose = new ConflictingContainerRemovingDockerCompose(upDockerCompose, docker()); } + upDockerCompose.up(); log.debug("Starting log collection"); @@ -220,22 +213,4 @@ public ImmutableDockerComposeRule.Builder waitingForHostNetworkedPort(int port, } } - private void removeDockerContainers(Collection containerNames) throws IOException, InterruptedException { - Docker docker = docker(); - try { - docker.rm(containerNames.toArray(new String[containerNames.size()])); - } catch (DockerExecutionException e) { - log.debug("docker rm failed, but continuing execution", e); - } - dockerCompose().up(); - } - - private static Set getConflictingContainerNames(String input) { - ImmutableSet.Builder builder = ImmutableSet.builder(); - Matcher matcher = NAME_CONFLICT_PATTERN.matcher(input); - while (matcher.find()) { - builder.add(matcher.group(1)); - } - return builder.build(); - } } diff --git a/src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java b/src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java deleted file mode 100644 index 84e7a6379..000000000 --- a/src/main/java/com/palantir/docker/compose/execution/AbstractCommand.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2016 Palantir Technologies, Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.palantir.docker.compose.execution; - -import static java.util.stream.Collectors.joining; - -import java.io.IOException; -import java.util.Arrays; - -public abstract class AbstractCommand { - - private final String commandName; - private final AbstractSynchronousExecutable executable; - - public AbstractCommand(String commandName, AbstractSynchronousExecutable executable) { - this.commandName = commandName; - this.executable = executable; - } - - protected final String executeCommand(ErrorHandler errorHandler, String... commands) - throws IOException, InterruptedException { - ProcessResult result = executable.run(commands); - - if (result.exitCode() != 0) { - errorHandler.handle(result.exitCode(), result.output(), commands); - } - - return result.output(); - } - - protected final ErrorHandler throwingOnError() { - return (exitCode, output, commands) -> { - String message = constructNonZeroExitErrorMessage(exitCode, commands) + "\nThe output was:\n" + output; - throw new DockerExecutionException(message); - }; - } - - private String constructNonZeroExitErrorMessage(int exitCode, String... commands) { - return "'" + commandName + " " + Arrays.stream(commands).collect(joining(" ")) + "' returned exit code " - + exitCode; - } - -} diff --git a/src/main/java/com/palantir/docker/compose/execution/AbstractSynchronousExecutable.java b/src/main/java/com/palantir/docker/compose/execution/Command.java similarity index 68% rename from src/main/java/com/palantir/docker/compose/execution/AbstractSynchronousExecutable.java rename to src/main/java/com/palantir/docker/compose/execution/Command.java index 1103dced5..becf63a95 100644 --- a/src/main/java/com/palantir/docker/compose/execution/AbstractSynchronousExecutable.java +++ b/src/main/java/com/palantir/docker/compose/execution/Command.java @@ -24,24 +24,48 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.util.Arrays; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.function.Consumer; -public abstract class AbstractSynchronousExecutable { +public class Command { public static final int HOURS_TO_WAIT_FOR_STD_OUT_TO_CLOSE = 12; public static final int MINUTES_TO_WAIT_AFTER_STD_OUT_CLOSES = 1; - private final T executable; + private final Executable executable; private final Consumer logConsumer; - public AbstractSynchronousExecutable(T executable, Consumer logConsumer) { + public Command(Executable executable, Consumer logConsumer) { this.executable = executable; this.logConsumer = logConsumer; } - public ProcessResult run(String... commands) throws IOException, InterruptedException { + public String execute(ErrorHandler errorHandler, String... commands) throws IOException, InterruptedException { + ProcessResult result = run(commands); + + if (result.exitCode() != 0) { + errorHandler.handle(result.exitCode(), result.output(), executable.commandName(), commands); + } + + return result.output(); + } + + public static ErrorHandler throwingOnError() { + return (exitCode, output, commandName, commands) -> { + String message = + constructNonZeroExitErrorMessage(exitCode, commandName, commands) + "\nThe output was:\n" + output; + throw new DockerExecutionException(message); + }; + } + + private static String constructNonZeroExitErrorMessage(int exitCode, String commandName, String... commands) { + return "'" + commandName + " " + Arrays.stream(commands).collect(joining(" ")) + "' returned exit code " + + exitCode; + } + + private ProcessResult run(String... commands) throws IOException, InterruptedException { Process process = executable.execute(commands); Future outputProcessing = newSingleThreadExecutor() @@ -71,5 +95,4 @@ private String waitForResultFrom(Future outputProcessing) { private BufferedReader asReader(InputStream inputStream) { return new BufferedReader(new InputStreamReader(inputStream, UTF_8)); } - } diff --git a/src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java b/src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java new file mode 100644 index 000000000..4aa742c83 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java @@ -0,0 +1,98 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; +import java.io.IOException; +import java.util.Collection; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ConflictingContainerRemovingDockerCompose extends DelegatingDockerCompose { + private static final Logger log = LoggerFactory.getLogger(ConflictingContainerRemovingDockerCompose.class); + private static final Pattern NAME_CONFLICT_PATTERN = Pattern.compile("The name \"([^\"]*)\" is already in use"); + + private final Docker docker; + private final int retryAttempts; + + public ConflictingContainerRemovingDockerCompose(DockerCompose dockerCompose, Docker docker) { + this(dockerCompose, docker, 1); + } + + public ConflictingContainerRemovingDockerCompose(DockerCompose dockerCompose, Docker docker, int retryAttempts) { + super(dockerCompose); + + Preconditions.checkArgument(retryAttempts >= 1, "retryAttempts must be at least 1, was " + retryAttempts); + this.docker = docker; + this.retryAttempts = retryAttempts; + } + + @Override + public void up() throws IOException, InterruptedException { + int currRetryAttempt = 0; + + while (true) { + try { + super.up(); + return; + } catch (DockerExecutionException e) { + Set conflictingContainerNames = getConflictingContainerNames(e.getMessage()); + if (conflictingContainerNames.isEmpty()) { + // failed due to reason other than conflicting containers, so re-throw + throw e; + } + + if (currRetryAttempt == retryAttempts) { + break; + } + + log.debug("docker-compose up failed due to container name conflicts (container names: {}). " + + "Removing containers and attempting docker-compose up again (attempt {}).", + conflictingContainerNames, currRetryAttempt + 1); + removeContainers(conflictingContainerNames); + + currRetryAttempt++; + } + } + + throw new DockerExecutionException("docker-compose up failed"); + } + + private void removeContainers(Collection containerNames) throws IOException, InterruptedException { + try { + docker.rm(containerNames); + } catch (DockerExecutionException e) { + // there are cases such as in CircleCI where 'docker rm' returns a non-0 exit code and "fails", + // but container is still effectively removed as far as conflict resolution is concerned. Because + // of this, be permissive and do not fail task even if 'rm' fails. + log.debug("docker rm failed, but continuing execution", e); + } + } + + private static Set getConflictingContainerNames(String output) { + ImmutableSet.Builder builder = ImmutableSet.builder(); + Matcher matcher = NAME_CONFLICT_PATTERN.matcher(output); + while (matcher.find()) { + builder.add(matcher.group(1)); + } + return builder.build(); + } + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java b/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java index dc4082474..b96ddcdb4 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java +++ b/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java @@ -35,12 +35,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DefaultDockerCompose extends AbstractCommand implements DockerCompose { +public class DefaultDockerCompose implements DockerCompose { public static final Version VERSION_1_7_0 = Version.valueOf("1.7.0"); private static final Duration COMMAND_TIMEOUT = standardMinutes(2); private static final Logger log = LoggerFactory.getLogger(DefaultDockerCompose.class); + private final Command command; private final DockerMachine dockerMachine; private final DockerComposeExecutable rawExecutable; @@ -53,35 +54,34 @@ public DefaultDockerCompose(DockerComposeFiles dockerComposeFiles, DockerMachine } public DefaultDockerCompose(DockerComposeExecutable rawExecutable, DockerMachine dockerMachine) { - super("docker-compose", new SynchronousDockerComposeExecutable(rawExecutable, log::debug)); - this.rawExecutable = rawExecutable; + this.command = new Command(rawExecutable, log::debug); this.dockerMachine = dockerMachine; } @Override public void build() throws IOException, InterruptedException { - executeCommand(throwingOnError(), "build"); + command.execute(Command.throwingOnError(), "build"); } @Override public void up() throws IOException, InterruptedException { - executeCommand(throwingOnError(), "up", "-d"); + command.execute(Command.throwingOnError(), "up", "-d"); } @Override public void down() throws IOException, InterruptedException { - executeCommand(swallowingDownCommandDoesNotExist(), "down", "--volumes"); + command.execute(swallowingDownCommandDoesNotExist(), "down", "--volumes"); } @Override public void kill() throws IOException, InterruptedException { - executeCommand(throwingOnError(), "kill"); + command.execute(Command.throwingOnError(), "kill"); } @Override public void rm() throws IOException, InterruptedException { - executeCommand(throwingOnError(), "rm", "--force", "-v"); + command.execute(Command.throwingOnError(), "rm", "--force", "-v"); } @Override @@ -89,7 +89,7 @@ public String exec(DockerComposeExecOption dockerComposeExecOption, String conta DockerComposeExecArgument dockerComposeExecArgument) throws IOException, InterruptedException { verifyDockerComposeVersionAtLeast(VERSION_1_7_0, "You need at least docker-compose 1.7 to run docker-compose exec"); String[] fullArgs = constructFullDockerComposeExecArguments(dockerComposeExecOption, containerName, dockerComposeExecArgument); - return executeCommand(throwingOnError(), fullArgs); + return command.execute(Command.throwingOnError(), fullArgs); } private void verifyDockerComposeVersionAtLeast(Version targetVersion, String message) throws IOException, InterruptedException { @@ -97,7 +97,7 @@ private void verifyDockerComposeVersionAtLeast(Version targetVersion, String mes } private Version version() throws IOException, InterruptedException { - String versionOutput = executeCommand(throwingOnError(), "-v"); + String versionOutput = command.execute(Command.throwingOnError(), "-v"); return DockerComposeVersion.parseFromDockerComposeVersion(versionOutput); } @@ -113,7 +113,7 @@ private String[] constructFullDockerComposeExecArguments(DockerComposeExecOption @Override public ContainerNames ps() throws IOException, InterruptedException { - String psOutput = executeCommand(throwingOnError(), "ps"); + String psOutput = command.execute(Command.throwingOnError(), "ps"); return ContainerNames.parseFromDockerComposePs(psOutput); } @@ -148,15 +148,15 @@ private Process followLogs(String container) throws IOException, InterruptedExce @Override public Ports ports(String service) throws IOException, InterruptedException { - String psOutput = executeCommand(throwingOnError(), "ps", service); + String psOutput = command.execute(Command.throwingOnError(), "ps", service); validState(!Strings.isNullOrEmpty(psOutput), "No container with name '" + service + "' found"); return Ports.parseFromDockerComposePs(psOutput, dockerMachine.getIp()); } private ErrorHandler swallowingDownCommandDoesNotExist() { - return (exitCode, output, commands) -> { + return (exitCode, output, commandName, commands) -> { if (downCommandWasPresent(output)) { - throwingOnError().handle(exitCode, output, commands); + Command.throwingOnError().handle(exitCode, output, commandName, commands); } log.warn("It looks like `docker-compose down` didn't work."); diff --git a/src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java b/src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java new file mode 100644 index 000000000..c09fa3855 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java @@ -0,0 +1,81 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import com.palantir.docker.compose.connection.Container; +import com.palantir.docker.compose.connection.ContainerNames; +import com.palantir.docker.compose.connection.Ports; +import java.io.IOException; +import java.io.OutputStream; + +public abstract class DelegatingDockerCompose implements DockerCompose { + private final DockerCompose dockerCompose; + + public DelegatingDockerCompose(DockerCompose dockerCompose) { + this.dockerCompose = dockerCompose; + } + + @Override + public void build() throws IOException, InterruptedException { + dockerCompose.build(); + } + + @Override + public void up() throws IOException, InterruptedException { + dockerCompose.up(); + } + + @Override + public void down() throws IOException, InterruptedException { + dockerCompose.down(); + } + + @Override + public void kill() throws IOException, InterruptedException { + dockerCompose.kill(); + } + + @Override + public void rm() throws IOException, InterruptedException { + dockerCompose.rm(); + } + + @Override + public String exec(DockerComposeExecOption dockerComposeExecOption, String containerName, + DockerComposeExecArgument dockerComposeExecArgument) throws IOException, InterruptedException { + return dockerCompose.exec(dockerComposeExecOption, containerName, dockerComposeExecArgument); + } + + @Override + public ContainerNames ps() throws IOException, InterruptedException { + return dockerCompose.ps(); + } + + @Override + public Container container(String containerName) { + return dockerCompose.container(containerName); + } + + @Override + public boolean writeLogs(String container, OutputStream output) throws IOException { + return dockerCompose.writeLogs(container, output); + } + + @Override + public Ports ports(String service) throws IOException, InterruptedException { + return dockerCompose.ports(service); + } +} diff --git a/src/main/java/com/palantir/docker/compose/execution/Docker.java b/src/main/java/com/palantir/docker/compose/execution/Docker.java index fc7d2c759..3cb6b067d 100644 --- a/src/main/java/com/palantir/docker/compose/execution/Docker.java +++ b/src/main/java/com/palantir/docker/compose/execution/Docker.java @@ -17,19 +17,27 @@ import com.google.common.collect.ObjectArrays; import java.io.IOException; +import java.util.Collection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class Docker extends AbstractCommand { +public class Docker { private static final Logger log = LoggerFactory.getLogger(Docker.class); + private final Command command; + public Docker(DockerExecutable rawExecutable) { - super("docker", new SynchronousDockerExecutable(rawExecutable, log::debug)); + this.command = new Command(rawExecutable, log::debug); + } + + public void rm(Collection containerNames) throws IOException, InterruptedException { + rm(containerNames.toArray(new String[containerNames.size()])); } public void rm(String... containerNames) throws IOException, InterruptedException { - executeCommand(throwingOnError(), ObjectArrays.concat(new String[] {"rm", "-f"}, containerNames, String.class)); + command.execute(Command.throwingOnError(), + ObjectArrays.concat(new String[] {"rm", "-f"}, containerNames, String.class)); } } diff --git a/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java b/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java index 2d6b2cf68..7a580b130 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java +++ b/src/main/java/com/palantir/docker/compose/execution/DockerComposeExecutable.java @@ -30,7 +30,8 @@ public abstract class DockerComposeExecutable implements Executable { private static final DockerCommandLocations DOCKER_COMPOSE_LOCATIONS = new DockerCommandLocations( System.getenv("DOCKER_COMPOSE_LOCATION"), - "/usr/local/bin/docker-compose" + "/usr/local/bin/docker-compose", + "/usr/bin/docker-compose" ); @Value.Parameter protected abstract DockerComposeFiles dockerComposeFiles(); @@ -40,6 +41,11 @@ public abstract class DockerComposeExecutable implements Executable { return ProjectName.random(); } + @Override + public final String commandName() { + return "docker-compose"; + } + @Value.Derived protected String dockerComposePath() { String pathToUse = DOCKER_COMPOSE_LOCATIONS.preferredLocation() diff --git a/src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java b/src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java index bb20a0fb9..1ff5cf01e 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java +++ b/src/main/java/com/palantir/docker/compose/execution/DockerExecutable.java @@ -28,11 +28,17 @@ public abstract class DockerExecutable implements Executable { private static final DockerCommandLocations DOCKER_LOCATIONS = new DockerCommandLocations( System.getenv("DOCKER_LOCATION"), - "/usr/local/bin/docker" + "/usr/local/bin/docker", + "/usr/bin/docker" ); @Value.Parameter protected abstract DockerConfiguration dockerConfiguration(); + @Override + public final String commandName() { + return "docker"; + } + @Value.Derived protected String dockerPath() { String pathToUse = DOCKER_LOCATIONS.preferredLocation() diff --git a/src/main/java/com/palantir/docker/compose/execution/ErrorHandler.java b/src/main/java/com/palantir/docker/compose/execution/ErrorHandler.java index 6a13a9834..185f09b53 100644 --- a/src/main/java/com/palantir/docker/compose/execution/ErrorHandler.java +++ b/src/main/java/com/palantir/docker/compose/execution/ErrorHandler.java @@ -17,5 +17,5 @@ @FunctionalInterface public interface ErrorHandler { - void handle(int exitCode, String output, String... commands); + void handle(int exitCode, String output, String commandName, String... commands); } diff --git a/src/main/java/com/palantir/docker/compose/execution/Executable.java b/src/main/java/com/palantir/docker/compose/execution/Executable.java index 8b3bf6d7c..880eae83b 100644 --- a/src/main/java/com/palantir/docker/compose/execution/Executable.java +++ b/src/main/java/com/palantir/docker/compose/execution/Executable.java @@ -19,6 +19,8 @@ public interface Executable { + String commandName(); + Process execute(String... commands) throws IOException; } diff --git a/src/main/java/com/palantir/docker/compose/execution/RetryingDockerCompose.java b/src/main/java/com/palantir/docker/compose/execution/RetryingDockerCompose.java index 9e373daf7..77ced9094 100644 --- a/src/main/java/com/palantir/docker/compose/execution/RetryingDockerCompose.java +++ b/src/main/java/com/palantir/docker/compose/execution/RetryingDockerCompose.java @@ -15,76 +15,31 @@ */ package com.palantir.docker.compose.execution; -import com.palantir.docker.compose.connection.Container; import com.palantir.docker.compose.connection.ContainerNames; -import com.palantir.docker.compose.connection.Ports; import java.io.IOException; -import java.io.OutputStream; -public class RetryingDockerCompose implements DockerCompose { +public class RetryingDockerCompose extends DelegatingDockerCompose { private final Retryer retryer; - private final DockerCompose dockerCompose; public RetryingDockerCompose(int retryAttempts, DockerCompose dockerCompose) { this(new Retryer(retryAttempts), dockerCompose); } public RetryingDockerCompose(Retryer retryer, DockerCompose dockerCompose) { + super(dockerCompose); this.retryer = retryer; - this.dockerCompose = dockerCompose; - } - - @Override - public void build() throws IOException, InterruptedException { - dockerCompose.build(); } @Override public void up() throws IOException, InterruptedException { retryer.runWithRetries(() -> { - dockerCompose.up(); + super.up(); return null; }); } - @Override - public void down() throws IOException, InterruptedException { - dockerCompose.down(); - } - - @Override - public void kill() throws IOException, InterruptedException { - dockerCompose.kill(); - } - - @Override - public void rm() throws IOException, InterruptedException { - dockerCompose.rm(); - } - - @Override - public String exec(DockerComposeExecOption dockerComposeExecOption, String containerName, - DockerComposeExecArgument dockerComposeExecArgument) throws IOException, InterruptedException { - return dockerCompose.exec(dockerComposeExecOption, containerName, dockerComposeExecArgument); - } - @Override public ContainerNames ps() throws IOException, InterruptedException { - return retryer.runWithRetries(dockerCompose::ps); - } - - @Override - public Container container(String containerName) { - return dockerCompose.container(containerName); - } - - @Override - public boolean writeLogs(String container, OutputStream output) throws IOException { - return dockerCompose.writeLogs(container, output); - } - - @Override - public Ports ports(String service) throws IOException, InterruptedException { - return dockerCompose.ports(service); + return retryer.runWithRetries(super::ps); } } diff --git a/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java b/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java deleted file mode 100644 index 217af75cd..000000000 --- a/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutable.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2016 Palantir Technologies, Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.palantir.docker.compose.execution; - -import java.util.function.Consumer; - -public class SynchronousDockerComposeExecutable extends AbstractSynchronousExecutable { - - public SynchronousDockerComposeExecutable(DockerComposeExecutable dockerComposeExecutable, - Consumer logConsumer) { - super(dockerComposeExecutable, logConsumer); - } - -} diff --git a/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java b/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java deleted file mode 100644 index 5d4ff651e..000000000 --- a/src/main/java/com/palantir/docker/compose/execution/SynchronousDockerExecutable.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2016 Palantir Technologies, Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.palantir.docker.compose.execution; - -import java.util.function.Consumer; - -public class SynchronousDockerExecutable extends AbstractSynchronousExecutable { - - public SynchronousDockerExecutable(DockerExecutable dockerExecutable, Consumer logConsumer) { - super(dockerExecutable, logConsumer); - } - -} diff --git a/src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java b/src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java index 2cfe7f96f..0329dc822 100644 --- a/src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java +++ b/src/test/java/com/palantir/docker/compose/DockerComposeRuleRestartContainersIntegrationTest.java @@ -15,48 +15,26 @@ */ package com.palantir.docker.compose; -import com.palantir.docker.compose.connection.DockerMachine; -import com.palantir.docker.compose.execution.Docker; -import com.palantir.docker.compose.execution.DockerExecutable; import com.palantir.docker.compose.execution.DockerExecutionException; import java.io.IOException; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class DockerComposeRuleRestartContainersIntegrationTest { - private static final Logger log = LoggerFactory.getLogger(DockerComposeRuleRestartContainersIntegrationTest.class); private static final String DOCKER_COMPOSE_YAML_PATH = "src/test/resources/named-containers-docker-compose.yaml"; @Rule public ExpectedException exception = ExpectedException.none(); - /** - * Tests assume that container names in the YAML file are not already in use. In order to enforce this invariant, - * this cleanup method is run before each test. This method runs "docker rm" on the container names used by the - * tests. The names are hard-coded and should match the names used by the containers defined in the - * DOCKER_COMPOSE_YAML_PATH file. - */ - @Before - public void cleanup() throws IOException, InterruptedException { - Docker docker = new Docker( - DockerExecutable.builder().dockerConfiguration(DockerMachine.localMachine().build()).build()); - try { - docker.rm("/test-1.container.name", "/test-2.container.name"); - } catch (DockerExecutionException e) { - log.debug("docker rm failed in cleanup, but continuing", e); - } - } - @Test - public void testDockerComposeRuleFailsWithExistingContainers() throws IOException, InterruptedException { + public void test_docker_compose_rule_fails_with_existing_containers() throws IOException, InterruptedException { DockerComposition composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH).build(); composition.before(); - composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH).build(); + composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH) + .removeConflictingContainersOnStartup(false) + .build(); exception.expect(DockerExecutionException.class); exception.expectMessage("'docker-compose up -d' returned exit code"); @@ -64,13 +42,11 @@ public void testDockerComposeRuleFailsWithExistingContainers() throws IOExceptio } @Test - public void testDockerComposeRuleRemovesExistingContainers() throws IOException, InterruptedException { + public void test_docker_compose_rule_removes_existing_containers() throws IOException, InterruptedException { DockerComposition composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH).build(); composition.before(); - composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH) - .removeConflictingContainersOnStartup(true) - .build(); + composition = DockerComposition.of(DOCKER_COMPOSE_YAML_PATH).build(); composition.before(); composition.after(); } diff --git a/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java b/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java index 29ef32298..a1cf1e1db 100644 --- a/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java +++ b/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java @@ -32,6 +32,7 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.palantir.docker.compose.configuration.DockerComposeFiles; import com.palantir.docker.compose.configuration.MockDockerEnvironment; import com.palantir.docker.compose.connection.Container; @@ -75,6 +76,7 @@ public class DockerComposeRuleShould { private HealthCheck> healthCheck; private final DockerCompose dockerCompose = mock(DockerCompose.class); + private final Docker mockDocker = mock(Docker.class); private final MockDockerEnvironment env = new MockDockerEnvironment(dockerCompose); private DockerComposeFiles mockFiles = mock(DockerComposeFiles.class); private DockerMachine machine = mock(DockerMachine.class); @@ -223,46 +225,34 @@ public void not_shut_down_when_skipShutdown_is_true() throws InterruptedExceptio @Test public void before_fails_when_docker_up_throws_exception() throws IOException, InterruptedException { doThrow(new DockerExecutionException("")).when(dockerCompose).up(); - rule = defaultBuilder().removeConflictingContainersOnStartup(true).build(); + rule = defaultBuilder().build(); exception.expect(DockerExecutionException.class); rule.before(); } @Test - public void before_retries_if_docker_up_throws_container_conflict_exception() - throws IOException, InterruptedException { - doThrow(new DockerExecutionException("The name \"testName\" is already in use")) + public void before_retries_when_docker_up_reports_conflicting_containers() throws IOException, InterruptedException { + String conflictingContainer = "conflictingContainer"; + doThrow(new DockerExecutionException("The name \"" + conflictingContainer + "\" is already in use")) .doNothing() .when(dockerCompose).up(); - Docker mockDocker = mock(Docker.class); - - rule = defaultBuilder().docker(mockDocker).removeConflictingContainersOnStartup(true).build(); + rule = defaultBuilder().docker(mockDocker).build(); rule.before(); - } - @Test - public void docker_execution_exception_in_before_retry_is_ignored() - throws IOException, InterruptedException { - doThrow(new DockerExecutionException("The name \"testName\" is already in use")) - .doNothing() - .when(dockerCompose).up(); - Docker mockDocker = mock(Docker.class); - doThrow(DockerExecutionException.class).when(mockDocker).rm(any()); - - rule = defaultBuilder().docker(mockDocker).removeConflictingContainersOnStartup(true).build(); - rule.before(); + verify(dockerCompose, times(2)).up(); + verify(mockDocker).rm(ImmutableSet.of(conflictingContainer)); } @Test - public void non_docker_execution_exception_in_before_retry_is_thrown() + public void when_remove_conflicting_containers_on_startup_is_set_to_false_before_does_not_retry_on_conflicts() throws IOException, InterruptedException { - doThrow(new DockerExecutionException("The name \"testName\" is already in use")) - .doNothing() + String conflictingContainer = "conflictingContainer"; + doThrow(new DockerExecutionException("The name \"" + conflictingContainer + "\" is already in use")) .when(dockerCompose).up(); - Docker mockDocker = mock(Docker.class); - doThrow(RuntimeException.class).when(mockDocker).rm(any()); - rule = defaultBuilder().docker(mockDocker).removeConflictingContainersOnStartup(true).build(); - exception.expect(RuntimeException.class); + rule = defaultBuilder().docker(mockDocker).removeConflictingContainersOnStartup(false).build(); + + exception.expect(DockerExecutionException.class); + exception.expectMessage("The name \"conflictingContainer\" is already in use"); rule.before(); } diff --git a/src/test/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutableShould.java b/src/test/java/com/palantir/docker/compose/execution/CommandShould.java similarity index 61% rename from src/test/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutableShould.java rename to src/test/java/com/palantir/docker/compose/execution/CommandShould.java index 8f49db14f..e06e2a71c 100644 --- a/src/test/java/com/palantir/docker/compose/execution/SynchronousDockerComposeExecutableShould.java +++ b/src/test/java/com/palantir/docker/compose/execution/CommandShould.java @@ -20,6 +20,8 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.core.Is.is; import static org.mockito.Matchers.anyVararg; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import java.io.IOException; @@ -33,45 +35,63 @@ import org.mockito.runners.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) -public class SynchronousDockerComposeExecutableShould { +public class CommandShould { @Mock private Process executedProcess; @Mock private DockerComposeExecutable dockerComposeExecutable; - private SynchronousDockerComposeExecutable dockerCompose; + @Mock private ErrorHandler errorHandler; + private Command dockerComposeCommand; private final List consumedLogLines = new ArrayList<>(); private final Consumer logConsumer = s -> consumedLogLines.add(s); @Before public void setup() throws IOException { when(dockerComposeExecutable.execute(anyVararg())).thenReturn(executedProcess); - dockerCompose = new SynchronousDockerComposeExecutable(dockerComposeExecutable, logConsumer); + dockerComposeCommand = new Command(dockerComposeExecutable, logConsumer); givenTheUnderlyingProcessHasOutput(""); givenTheUnderlyingProcessTerminatesWithAnExitCodeOf(0); } @Test public void - respond_with_the_exit_code_of_the_executed_process() throws IOException, InterruptedException { + invoke_error_handler_when_exit_code_of_the_executed_process_is_non_0() throws IOException, InterruptedException { int expectedExitCode = 1; - givenTheUnderlyingProcessTerminatesWithAnExitCodeOf(expectedExitCode); + dockerComposeCommand.execute(errorHandler, "rm", "-f"); - assertThat(dockerCompose.run("rm", "-f").exitCode(), is(expectedExitCode)); + verify(errorHandler).handle(expectedExitCode, "", "docker-compose", "rm", "-f"); } @Test public void - respond_with_the_output_of_the_executed_process() throws IOException, InterruptedException { - String expectedOutput = "some output"; + not_invoke_error_handler_when_exit_code_of_the_executed_process_is_0() throws IOException, InterruptedException { + dockerComposeCommand.execute(errorHandler, "rm", "-f"); + + verifyZeroInteractions(errorHandler); + } + @Test public void + return_output_when_exit_code_of_the_executed_process_is_non_0() throws IOException, InterruptedException { + String expectedOutput = "test output"; + givenTheUnderlyingProcessTerminatesWithAnExitCodeOf(1); + givenTheUnderlyingProcessHasOutput(expectedOutput); + String commandOutput = dockerComposeCommand.execute(errorHandler, "rm", "-f"); + + assertThat(commandOutput, is(expectedOutput)); + } + + @Test public void + return_output_when_exit_code_of_the_executed_process_is_0() throws IOException, InterruptedException { + String expectedOutput = "test output"; givenTheUnderlyingProcessHasOutput(expectedOutput); + String commandOutput = dockerComposeCommand.execute(errorHandler, "rm", "-f"); - assertThat(dockerCompose.run("rm", "-f").output(), is(expectedOutput)); + assertThat(commandOutput, is(expectedOutput)); } @Test public void give_the_output_to_the_specified_consumer_as_it_is_available() throws IOException, InterruptedException { givenTheUnderlyingProcessHasOutput("line 1\nline 2"); - dockerCompose.run("rm", "-f"); + dockerComposeCommand.execute(errorHandler, "rm", "-f"); assertThat(consumedLogLines, contains("line 1", "line 2")); } diff --git a/src/test/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerComposeShould.java b/src/test/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerComposeShould.java new file mode 100644 index 000000000..4655b8d91 --- /dev/null +++ b/src/test/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerComposeShould.java @@ -0,0 +1,131 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.docker.compose.execution; + +import static org.mockito.Matchers.anySet; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +import com.google.common.collect.ImmutableSet; +import java.io.IOException; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class ConflictingContainerRemovingDockerComposeShould { + private final DockerCompose dockerCompose = mock(DockerCompose.class); + private final Docker docker = mock(Docker.class); + + @Rule + public ExpectedException exception = ExpectedException.none(); + + @Test + public void require_retry_attempts_to_be_at_least_1() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("retryAttempts must be at least 1, was 0"); + new ConflictingContainerRemovingDockerCompose(dockerCompose, docker, 0); + } + + @Test + public void call_up_only_once_if_successful() throws IOException, InterruptedException { + ConflictingContainerRemovingDockerCompose conflictingContainerRemovingDockerCompose = + new ConflictingContainerRemovingDockerCompose(dockerCompose, docker); + conflictingContainerRemovingDockerCompose.up(); + + verify(dockerCompose, times(1)).up(); + verifyZeroInteractions(docker); + } + + @Test + public void call_rm_and_retry_up_if_conflicting_containers_exist() throws IOException, InterruptedException { + String conflictingContainer = "conflictingContainer"; + doThrow(new DockerExecutionException("The name \"" + conflictingContainer + "\" is already in use")) + .doNothing() + .when(dockerCompose).up(); + + ConflictingContainerRemovingDockerCompose conflictingContainerRemovingDockerCompose = + new ConflictingContainerRemovingDockerCompose(dockerCompose, docker); + conflictingContainerRemovingDockerCompose.up(); + + verify(dockerCompose, times(2)).up(); + verify(docker).rm(ImmutableSet.of(conflictingContainer)); + } + + @Test + public void retry_specified_number_of_times() throws IOException, InterruptedException { + String conflictingContainer = "conflictingContainer"; + DockerExecutionException dockerException = new DockerExecutionException( + "The name \"" + conflictingContainer + "\" is already in use"); + doThrow(dockerException) + .doThrow(dockerException) + .doNothing() + .when(dockerCompose).up(); + + ConflictingContainerRemovingDockerCompose conflictingContainerRemovingDockerCompose = + new ConflictingContainerRemovingDockerCompose(dockerCompose, docker, 3); + conflictingContainerRemovingDockerCompose.up(); + + verify(dockerCompose, times(3)).up(); + verify(docker, times(2)).rm(ImmutableSet.of(conflictingContainer)); + } + + @Test + public void ignore_docker_execution_exceptions_in_rm() throws IOException, InterruptedException { + String conflictingContainer = "conflictingContainer"; + doThrow(new DockerExecutionException("The name \"" + conflictingContainer + "\" is already in use")) + .doNothing() + .when(dockerCompose).up(); + doThrow(DockerExecutionException.class).when(docker).rm(anySet()); + + ConflictingContainerRemovingDockerCompose conflictingContainerRemovingDockerCompose = + new ConflictingContainerRemovingDockerCompose(dockerCompose, docker); + conflictingContainerRemovingDockerCompose.up(); + + verify(dockerCompose, times(2)).up(); + verify(docker).rm(ImmutableSet.of(conflictingContainer)); + } + + @Test + public void fail_on_non_docker_execution_exceptions_in_rm() throws IOException, InterruptedException { + String conflictingContainer = "conflictingContainer"; + doThrow(new DockerExecutionException("The name \"" + conflictingContainer + "\" is already in use")) + .doNothing() + .when(dockerCompose).up(); + doThrow(RuntimeException.class).when(docker).rm(anySet()); + + exception.expect(RuntimeException.class); + ConflictingContainerRemovingDockerCompose conflictingContainerRemovingDockerCompose = + new ConflictingContainerRemovingDockerCompose(dockerCompose, docker); + conflictingContainerRemovingDockerCompose.up(); + } + + @Test + public void throw_exception_if_retry_attempts_exceeded() throws IOException, InterruptedException { + String conflictingContainer = "conflictingContainer"; + doThrow(new DockerExecutionException("The name \"" + conflictingContainer + "\" is already in use")) + .when(dockerCompose).up(); + + exception.expect(DockerExecutionException.class); + exception.expectMessage("docker-compose up failed"); + ConflictingContainerRemovingDockerCompose conflictingContainerRemovingDockerCompose = + new ConflictingContainerRemovingDockerCompose(dockerCompose, docker); + conflictingContainerRemovingDockerCompose.up(); + } + +} diff --git a/src/test/java/com/palantir/docker/compose/execution/DockerTest.java b/src/test/java/com/palantir/docker/compose/execution/DockerShould.java similarity index 92% rename from src/test/java/com/palantir/docker/compose/execution/DockerTest.java rename to src/test/java/com/palantir/docker/compose/execution/DockerShould.java index 6ef96894d..57e77d99f 100644 --- a/src/test/java/com/palantir/docker/compose/execution/DockerTest.java +++ b/src/test/java/com/palantir/docker/compose/execution/DockerShould.java @@ -25,7 +25,7 @@ import org.junit.Before; import org.junit.Test; -public class DockerTest { +public class DockerShould { private final DockerExecutable executor = mock(DockerExecutable.class); private final Docker docker = new Docker(executor); @@ -40,7 +40,7 @@ public void setup() throws IOException, InterruptedException { } @Test - public void rm_calls_docker_rm_with_force_flag() throws IOException, InterruptedException { + public void call_docker_rm_with_force_flag_on_rm() throws IOException, InterruptedException { docker.rm("testContainer"); verify(executor).execute("rm", "-f", "testContainer"); From 0b2e287740d5536850996b392b85ad12e785c836 Mon Sep 17 00:00:00 2001 From: Nick Miyake Date: Tue, 14 Jun 2016 15:15:46 -0700 Subject: [PATCH 3/3] Address second round of PR feedback --- .../ConflictingContainerRemovingDockerCompose.java | 14 +++----------- .../compose/execution/DelegatingDockerCompose.java | 9 +++++++-- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java b/src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java index 4aa742c83..29ddf60ed 100644 --- a/src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java +++ b/src/main/java/com/palantir/docker/compose/execution/ConflictingContainerRemovingDockerCompose.java @@ -46,11 +46,9 @@ public ConflictingContainerRemovingDockerCompose(DockerCompose dockerCompose, Do @Override public void up() throws IOException, InterruptedException { - int currRetryAttempt = 0; - - while (true) { + for (int currRetryAttempt = 0; currRetryAttempt <= retryAttempts; currRetryAttempt++) { try { - super.up(); + getDockerCompose().up(); return; } catch (DockerExecutionException e) { Set conflictingContainerNames = getConflictingContainerNames(e.getMessage()); @@ -59,16 +57,10 @@ public void up() throws IOException, InterruptedException { throw e; } - if (currRetryAttempt == retryAttempts) { - break; - } - log.debug("docker-compose up failed due to container name conflicts (container names: {}). " - + "Removing containers and attempting docker-compose up again (attempt {}).", + + "Removing containers and attempting docker-compose up again (attempt {}).", conflictingContainerNames, currRetryAttempt + 1); removeContainers(conflictingContainerNames); - - currRetryAttempt++; } } diff --git a/src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java b/src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java index c09fa3855..3ea17adb4 100644 --- a/src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java +++ b/src/main/java/com/palantir/docker/compose/execution/DelegatingDockerCompose.java @@ -21,10 +21,10 @@ import java.io.IOException; import java.io.OutputStream; -public abstract class DelegatingDockerCompose implements DockerCompose { +abstract class DelegatingDockerCompose implements DockerCompose { private final DockerCompose dockerCompose; - public DelegatingDockerCompose(DockerCompose dockerCompose) { + protected DelegatingDockerCompose(DockerCompose dockerCompose) { this.dockerCompose = dockerCompose; } @@ -78,4 +78,9 @@ public boolean writeLogs(String container, OutputStream output) throws IOExcepti public Ports ports(String service) throws IOException, InterruptedException { return dockerCompose.ports(service); } + + protected final DockerCompose getDockerCompose() { + return dockerCompose; + } + }