From a5ab54cf59166e8adf7755c8ecb600f5e79bc205 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Sat, 1 Oct 2016 00:05:44 +0100 Subject: [PATCH 1/5] New ShutdownStrategy interface will allow more complicated behaviour --- .../docker/compose/DockerComposeRule.java | 40 ++++++++++--------- .../configuration/ShutdownStrategy.java | 18 +++++++++ .../execution/GracefulShutdownStrategy.java | 24 +++++++++++ .../execution/SkipShutdownStrategy.java | 27 +++++++++++++ .../compose/DockerComposeRuleShould.java | 2 +- 5 files changed, 91 insertions(+), 20 deletions(-) create mode 100644 src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java create mode 100644 src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java create mode 100644 src/main/java/com/palantir/docker/compose/execution/SkipShutdownStrategy.java diff --git a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java index f8ed9bf26..9e188be11 100644 --- a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java +++ b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java @@ -8,6 +8,7 @@ import com.palantir.docker.compose.configuration.DockerComposeFiles; import com.palantir.docker.compose.configuration.ProjectName; +import com.palantir.docker.compose.configuration.ShutdownStrategy; import com.palantir.docker.compose.connection.Cluster; import com.palantir.docker.compose.connection.Container; import com.palantir.docker.compose.connection.ContainerCache; @@ -27,7 +28,9 @@ import com.palantir.docker.compose.execution.DockerComposeRunArgument; import com.palantir.docker.compose.execution.DockerComposeRunOption; import com.palantir.docker.compose.execution.DockerExecutable; +import com.palantir.docker.compose.execution.GracefulShutdownStrategy; import com.palantir.docker.compose.execution.RetryingDockerCompose; +import com.palantir.docker.compose.execution.SkipShutdownStrategy; import com.palantir.docker.compose.logging.DoNothingLogCollector; import com.palantir.docker.compose.logging.FileLogCollector; import com.palantir.docker.compose.logging.LogCollector; @@ -87,6 +90,11 @@ public Docker docker() { return new Docker(dockerExecutable()); } + @Value.Default + public ShutdownStrategy shutdownStrategy() { + return new GracefulShutdownStrategy(); + } + @Value.Default public DockerCompose dockerCompose() { DockerCompose dockerCompose = new DefaultDockerCompose(dockerComposeExecutable(), machine()); @@ -106,11 +114,6 @@ protected int retryAttempts() { return DEFAULT_RETRY_ATTEMPTS; } - @Value.Default - protected boolean skipShutdown() { - return false; - } - @Value.Default protected boolean removeConflictingContainersOnStartup() { return true; @@ -143,20 +146,7 @@ public void before() throws IOException, InterruptedException { @Override public void after() { try { - if (skipShutdown()) { - log.error("******************************************************************************************\n" - + "* docker-compose-rule has been configured to skip docker-compose shutdown: *\n" - + "* this means the containers will be left running after tests finish executing. *\n" - + "* If you see this message when running on CI it means you are potentially abandoning *\n" - + "* long running processes and leaking resources. *\n" - + "*******************************************************************************************"); - } else { - log.debug("Killing docker-compose cluster"); - dockerCompose().down(); - dockerCompose().kill(); - dockerCompose().rm(); - } - + shutdownStrategy().shutdown(dockerCompose()); logCollector().stopCollecting(); } catch (IOException | InterruptedException e) { throw new RuntimeException("Error cleaning up docker compose cluster", e); @@ -187,6 +177,18 @@ public Builder saveLogsTo(String path) { return logCollector(FileLogCollector.fromPath(path)); } + /** + * @deprecated Please use shutdownStrategy(new SkipShutdownStrategy()) instead. + */ + @Deprecated + public Builder skipShutdown(boolean skipShutdown) { + if (skipShutdown) { + return shutdownStrategy(new SkipShutdownStrategy()); + } + + return this; + } + public Builder waitingForService(String serviceName, HealthCheck healthCheck) { return waitingForService(serviceName, healthCheck, DEFAULT_TIMEOUT); } diff --git a/src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java b/src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java new file mode 100644 index 000000000..a5e301324 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java @@ -0,0 +1,18 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + */ + +package com.palantir.docker.compose.configuration; + +import com.palantir.docker.compose.execution.DockerCompose; +import java.io.IOException; + +/** + * How should a cluster of containers be shut down by the `after` method of + * DockerComposeRule. + */ +public interface ShutdownStrategy { + + void shutdown(DockerCompose dockerCompose) throws IOException, InterruptedException; + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java b/src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java new file mode 100644 index 000000000..ec4d71fcd --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java @@ -0,0 +1,24 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + */ + +package com.palantir.docker.compose.execution; + +import com.palantir.docker.compose.configuration.ShutdownStrategy; +import java.io.IOException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class GracefulShutdownStrategy implements ShutdownStrategy { + + private static final Logger log = LoggerFactory.getLogger(GracefulShutdownStrategy.class); + + @Override + public void shutdown(DockerCompose dockerCompose) throws IOException, InterruptedException { + log.debug("Killing docker-compose cluster"); + dockerCompose.down(); + dockerCompose.kill(); + dockerCompose.rm(); + } + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/SkipShutdownStrategy.java b/src/main/java/com/palantir/docker/compose/execution/SkipShutdownStrategy.java new file mode 100644 index 000000000..590b33056 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/SkipShutdownStrategy.java @@ -0,0 +1,27 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + */ + +package com.palantir.docker.compose.execution; + +import com.palantir.docker.compose.configuration.ShutdownStrategy; +import java.io.IOException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SkipShutdownStrategy implements ShutdownStrategy { + + private static final Logger log = LoggerFactory.getLogger(SkipShutdownStrategy.class); + + @Override + public void shutdown(DockerCompose dockerCompose) throws IOException, InterruptedException { + log.warn("******************************************************************************************\n" + + "* docker-compose-rule has been configured to skip docker-compose shutdown: *\n" + + "* this means the containers will be left running after tests finish executing. *\n" + + "* If you see this message when running on CI it means you are potentially abandoning *\n" + + "* long running processes and leaking resources. *\n" + + "******************************************************************************************"); + } + + +} diff --git a/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java b/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java index df5c24c69..9ffd22bb5 100644 --- a/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java +++ b/src/test/java/com/palantir/docker/compose/DockerComposeRuleShould.java @@ -263,7 +263,7 @@ public Container withComposeExecutableReturningContainerFor(String containerName return container; } - private ImmutableDockerComposeRule.Builder defaultBuilder() { + private DockerComposeRule.Builder defaultBuilder() { return DockerComposeRule.builder().dockerCompose(dockerCompose) .files(mockFiles) .machine(machine) From ef0acc6cddf73514ca94dd576b9210c166c1e7fa Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Sat, 1 Oct 2016 00:15:48 +0100 Subject: [PATCH 2/5] new AggressiveShutdownStrategy doesn't wait for docker stop --- .../execution/AggressiveShutdownStrategy.java | 21 +++++++++++++++++++ .../execution/GracefulShutdownStrategy.java | 4 ++++ 2 files changed, 25 insertions(+) create mode 100644 src/main/java/com/palantir/docker/compose/execution/AggressiveShutdownStrategy.java diff --git a/src/main/java/com/palantir/docker/compose/execution/AggressiveShutdownStrategy.java b/src/main/java/com/palantir/docker/compose/execution/AggressiveShutdownStrategy.java new file mode 100644 index 000000000..5e08761b0 --- /dev/null +++ b/src/main/java/com/palantir/docker/compose/execution/AggressiveShutdownStrategy.java @@ -0,0 +1,21 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + */ + +package com.palantir.docker.compose.execution; + +import com.palantir.docker.compose.configuration.ShutdownStrategy; +import java.io.IOException; + +/** + * Shuts down containers as fast as possible, without giving them time to finish + * IO or clean up any resources. + */ +public class AggressiveShutdownStrategy implements ShutdownStrategy { + + @Override + public void shutdown(DockerCompose dockerCompose) throws IOException, InterruptedException { + dockerCompose.rm(); + } + +} diff --git a/src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java b/src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java index ec4d71fcd..aea00dafe 100644 --- a/src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java +++ b/src/main/java/com/palantir/docker/compose/execution/GracefulShutdownStrategy.java @@ -9,6 +9,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * Send SIGTERM to containers first, allowing them up to 10 seconds to + * terminate before killing and rm-ing them. + */ public class GracefulShutdownStrategy implements ShutdownStrategy { private static final Logger log = LoggerFactory.getLogger(GracefulShutdownStrategy.class); From f9203261dfe212381ebcfd28e3e954b2b9c46e78 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Sat, 1 Oct 2016 00:30:24 +0100 Subject: [PATCH 3/5] ShutdownStrategy.AGGRESSIVE for discoverability --- .../com/palantir/docker/compose/DockerComposeRule.java | 8 +++----- .../docker/compose/configuration/ShutdownStrategy.java | 7 +++++++ .../DockerComposeRuleUpContainerIntegrationTest.java | 7 ++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java index 9e188be11..4694911e3 100644 --- a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java +++ b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java @@ -28,9 +28,7 @@ import com.palantir.docker.compose.execution.DockerComposeRunArgument; import com.palantir.docker.compose.execution.DockerComposeRunOption; import com.palantir.docker.compose.execution.DockerExecutable; -import com.palantir.docker.compose.execution.GracefulShutdownStrategy; import com.palantir.docker.compose.execution.RetryingDockerCompose; -import com.palantir.docker.compose.execution.SkipShutdownStrategy; import com.palantir.docker.compose.logging.DoNothingLogCollector; import com.palantir.docker.compose.logging.FileLogCollector; import com.palantir.docker.compose.logging.LogCollector; @@ -92,7 +90,7 @@ public Docker docker() { @Value.Default public ShutdownStrategy shutdownStrategy() { - return new GracefulShutdownStrategy(); + return ShutdownStrategy.GRACEFUL; } @Value.Default @@ -178,12 +176,12 @@ public Builder saveLogsTo(String path) { } /** - * @deprecated Please use shutdownStrategy(new SkipShutdownStrategy()) instead. + * @deprecated Please use shutdownStrategy(ShutdownStrategy.SKIP) instead. */ @Deprecated public Builder skipShutdown(boolean skipShutdown) { if (skipShutdown) { - return shutdownStrategy(new SkipShutdownStrategy()); + return shutdownStrategy(ShutdownStrategy.SKIP); } return this; diff --git a/src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java b/src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java index a5e301324..911fc5145 100644 --- a/src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java +++ b/src/main/java/com/palantir/docker/compose/configuration/ShutdownStrategy.java @@ -4,7 +4,10 @@ package com.palantir.docker.compose.configuration; +import com.palantir.docker.compose.execution.AggressiveShutdownStrategy; import com.palantir.docker.compose.execution.DockerCompose; +import com.palantir.docker.compose.execution.GracefulShutdownStrategy; +import com.palantir.docker.compose.execution.SkipShutdownStrategy; import java.io.IOException; /** @@ -13,6 +16,10 @@ */ public interface ShutdownStrategy { + ShutdownStrategy AGGRESSIVE = new AggressiveShutdownStrategy(); + ShutdownStrategy GRACEFUL = new GracefulShutdownStrategy(); + ShutdownStrategy SKIP = new SkipShutdownStrategy(); + void shutdown(DockerCompose dockerCompose) throws IOException, InterruptedException; } diff --git a/src/test/java/com/palantir/docker/compose/DockerComposeRuleUpContainerIntegrationTest.java b/src/test/java/com/palantir/docker/compose/DockerComposeRuleUpContainerIntegrationTest.java index ae38cb594..54367eb66 100644 --- a/src/test/java/com/palantir/docker/compose/DockerComposeRuleUpContainerIntegrationTest.java +++ b/src/test/java/com/palantir/docker/compose/DockerComposeRuleUpContainerIntegrationTest.java @@ -15,6 +15,7 @@ */ package com.palantir.docker.compose; +import static com.palantir.docker.compose.configuration.ShutdownStrategy.AGGRESSIVE; import static com.palantir.docker.compose.connection.waiting.ClusterHealthCheck.serviceHealthCheck; import static com.palantir.docker.compose.connection.waiting.HealthChecks.toHaveAllPortsOpen; import static org.hamcrest.MatcherAssert.assertThat; @@ -37,7 +38,11 @@ public class DockerComposeRuleUpContainerIntegrationTest { @Before public void setUp() throws Exception { - dockerComposeRule = DockerComposeRule.builder().file(DOCKER_COMPOSE_YAML_PATH).build(); + dockerComposeRule = DockerComposeRule + .builder() + .shutdownStrategy(AGGRESSIVE) + .file(DOCKER_COMPOSE_YAML_PATH) + .build(); } @After From 9d332476dee741c18eff9746c0abc0354cbc1dc3 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Sat, 1 Oct 2016 00:50:03 +0100 Subject: [PATCH 4/5] Better deprecation javadoc for skipShutdown(true) --- .../java/com/palantir/docker/compose/DockerComposeRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java index 4694911e3..42b575fa0 100644 --- a/src/main/java/com/palantir/docker/compose/DockerComposeRule.java +++ b/src/main/java/com/palantir/docker/compose/DockerComposeRule.java @@ -176,7 +176,7 @@ public Builder saveLogsTo(String path) { } /** - * @deprecated Please use shutdownStrategy(ShutdownStrategy.SKIP) instead. + * @deprecated Please use {@link DockerComposeRule#shutdownStrategy()} with {@link ShutdownStrategy#SKIP} instead. */ @Deprecated public Builder skipShutdown(boolean skipShutdown) { From 313ffa00e483893290acd0ef83948120b95d70b8 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Sat, 1 Oct 2016 01:39:34 +0100 Subject: [PATCH 5/5] Don't bind to explicit port! --- src/test/resources/docker-compose.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/resources/docker-compose.yaml b/src/test/resources/docker-compose.yaml index 61dcf43a9..24f03206e 100644 --- a/src/test/resources/docker-compose.yaml +++ b/src/test/resources/docker-compose.yaml @@ -5,9 +5,9 @@ db: - "POSTGRES_USER=palantir" - "POSTGRES_PASSWORD=palantir" ports: - - "5442:5432" - -db2: + - "5432" + +db2: image: kiasaki/alpine-postgres environment: - "POSTGRES_DB=source"