From 3944eac253ca17c60aa5179fe7a149c329a5671d Mon Sep 17 00:00:00 2001 From: Richard North Date: Sat, 8 Jul 2017 18:42:24 +0100 Subject: [PATCH] Improve cleanup for docker-compose. Reduce unnecessary cleanup attempts which cause errors to be logged. docker-compose down is now trusted to have cleanup up properly if it exits with a 0 status code. --- .../containers/DockerComposeContainer.java | 60 ++++++++++++++----- .../utility/ResourceReaper.java | 20 ++++--- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java b/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java index 00d772b3126..085b24fb8ef 100644 --- a/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java +++ b/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java @@ -8,6 +8,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.Maps; import com.google.common.util.concurrent.Uninterruptibles; +import org.apache.commons.lang.StringUtils; import org.junit.runner.Description; import org.rnorth.ducttape.ratelimits.RateLimiter; import org.rnorth.ducttape.ratelimits.RateLimiterBuilder; @@ -46,8 +47,9 @@ public class DockerComposeContainer> e private final String identifier; private final Map ambassadorContainers = new HashMap<>(); private final List composeFiles; - private Set spawnedContainerIds = Collections.emptySet(); - private Map scalingPreferences = new HashMap<>(); + private final Set spawnedContainerIds = new HashSet<>(); + private final Set spawnedNetworkIds = new HashSet<>(); + private final Map scalingPreferences = new HashMap<>(); private DockerClient dockerClient; private boolean localCompose; private boolean pull = true; @@ -117,14 +119,14 @@ public void starting(Description description) { private void pullImages() { getDockerCompose("pull") - .start(); + .invoke(); } private void createServices() { // Start the docker-compose container, which starts up the services getDockerCompose("up -d") - .start(); + .invoke(); } private void tailChildContainerLogs() { @@ -158,7 +160,7 @@ private void applyScaling() { } getDockerCompose(sb.toString()) - .start(); + .invoke(); } } @@ -173,17 +175,20 @@ private void registerContainersForShutdown() { // Ensure that the default network for this compose environment, if any, is also cleaned up ResourceReaper.instance().registerNetworkForCleanup(identifier + "_default"); + spawnedNetworkIds.add(identifier + "_default"); + // Compose can define their own networks as well; ensure these are cleaned up dockerClient.listNetworksCmd().exec().forEach(network -> { if (network.getName().contains(identifier)) { + spawnedNetworkIds.add(network.getId()); ResourceReaper.instance().registerNetworkForCleanup(network.getId()); } }); // remember the IDs to allow containers to be killed as soon as we reach stop() - spawnedContainerIds = containers.stream() + spawnedContainerIds.addAll(containers.stream() .map(Container::getId) - .collect(Collectors.toSet()); + .collect(Collectors.toSet())); } catch (DockerException e) { logger().debug("Failed to stop a service container with exception", e); @@ -240,15 +245,25 @@ public void finished(Description description) { ambassadorContainers.forEach((String address, AmbassadorContainer container) -> container.stop()); // Kill the services using docker-compose - getDockerCompose("down -v") - .start(); + try { + getDockerCompose("down -v").invoke(); - // remove the networks before removing the containers - ResourceReaper.instance().removeNetworks(identifier); + // If we reach here then docker-compose down has cleared networks and containers; + // we can unregister from ResourceReaper + spawnedNetworkIds.forEach(id -> ResourceReaper.instance().unregisterNetwork(identifier)); + spawnedContainerIds.forEach(id -> ResourceReaper.instance().unregisterContainer(id)); + } catch (ContainerLaunchException e) { + // docker-compose down failed; use ResourceReaper to ensure cleanup + + // remove the networks before removing the containers + spawnedNetworkIds.forEach(id -> ResourceReaper.instance().removeNetworks(identifier)); + + // kill the spawned service containers + spawnedContainerIds.forEach(id -> ResourceReaper.instance().stopAndRemoveContainer(id)); + } - // kill the spawned service containers - spawnedContainerIds.forEach(id -> ResourceReaper.instance().stopAndRemoveContainer(id)); spawnedContainerIds.clear(); + spawnedNetworkIds.clear(); } } @@ -372,7 +387,7 @@ interface DockerCompose { DockerCompose withEnv(Map env); - void start(); + void invoke(); default void validateFileList(List composeFiles) { checkNotNull(composeFiles); @@ -417,7 +432,7 @@ public ContainerisedDockerCompose(List composeFiles, String identifier) { } @Override - public void start() { + public void invoke() { super.start(); this.followOutput(new Slf4jLogConsumer(logger())); @@ -431,6 +446,19 @@ public void start() { logger().info("Docker Compose has finished running"); AuditLogger.doComposeLog(this.getCommandParts(), this.getEnv()); + + final Integer exitCode = this.dockerClient.inspectContainerCmd(containerId) + .exec() + .getState() + .getExitCode(); + + if (exitCode == null || exitCode != 0) { + throw new ContainerLaunchException( + "Local Docker Compose exited abnormally with code " + + exitCode + + " whilst running command: " + + StringUtils.join(this.getCommandParts(), ' ')); + } } } @@ -468,7 +496,7 @@ public DockerCompose withEnv(Map env) { } @Override - public void start() { + public void invoke() { // bail out early if (!CommandLine.executableExists(COMPOSE_EXECUTABLE)) { throw new ContainerLaunchException("Local Docker Compose not found. Is " + COMPOSE_EXECUTABLE + " on the PATH?"); diff --git a/core/src/main/java/org/testcontainers/utility/ResourceReaper.java b/core/src/main/java/org/testcontainers/utility/ResourceReaper.java index a9ed8f1ba45..57e3eddd119 100644 --- a/core/src/main/java/org/testcontainers/utility/ResourceReaper.java +++ b/core/src/main/java/org/testcontainers/utility/ResourceReaper.java @@ -10,7 +10,10 @@ import org.slf4j.LoggerFactory; import org.testcontainers.DockerClientFactory; -import java.util.*; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; /** @@ -143,13 +146,6 @@ public void removeNetworks(String identifier) { private void removeNetwork(String networkName) { try { - try { - // First try to remove by name - dockerClient.removeNetworkCmd(networkName).exec(); - } catch (Exception e) { - LOGGER.trace("Error encountered removing network by name ({}) - it may not have been removed", networkName); - } - List networks; try { // Then try to list all networks with the same name @@ -172,4 +168,12 @@ private void removeNetwork(String networkName) { registeredNetworks.remove(networkName); } } + + public void unregisterNetwork(String identifier) { + registeredNetworks.remove(identifier); + } + + public void unregisterContainer(String identifier) { + registeredContainers.remove(identifier); + } }