Skip to content

Commit

Permalink
Improve cleanup for docker-compose.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rnorth committed Jul 8, 2017
1 parent 3cce55a commit a7dbb82
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,8 +47,9 @@ public class DockerComposeContainer<SELF extends DockerComposeContainer<SELF>> e
private final String identifier;
private final Map<String, AmbassadorContainer> ambassadorContainers = new HashMap<>();
private final List<File> composeFiles;
private Set<String> spawnedContainerIds = Collections.emptySet();
private Map<String, Integer> scalingPreferences = new HashMap<>();
private final Set<String> spawnedContainerIds = new HashSet<>();
private final Set<String> spawnedNetworkIds = new HashSet<>();
private final Map<String, Integer> scalingPreferences = new HashMap<>();
private DockerClient dockerClient;
private boolean localCompose;
private boolean pull = true;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -158,7 +160,7 @@ private void applyScaling() {
}

getDockerCompose(sb.toString())
.start();
.invoke();
}
}

Expand All @@ -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);
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -372,7 +387,7 @@ interface DockerCompose {

DockerCompose withEnv(Map<String, String> env);

void start();
void invoke();

default void validateFileList(List<File> composeFiles) {
checkNotNull(composeFiles);
Expand Down Expand Up @@ -417,7 +432,7 @@ public ContainerisedDockerCompose(List<File> composeFiles, String identifier) {
}

@Override
public void start() {
public void invoke() {
super.start();

this.followOutput(new Slf4jLogConsumer(logger()));
Expand All @@ -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(
"Containerised Docker Compose exited abnormally with code " +
exitCode +
" whilst running command: " +
StringUtils.join(this.getCommandParts(), ' '));
}
}
}

Expand Down Expand Up @@ -468,7 +496,7 @@ public DockerCompose withEnv(Map<String, String> 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?");
Expand Down
20 changes: 12 additions & 8 deletions core/src/main/java/org/testcontainers/utility/ResourceReaper.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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<Network> networks;
try {
// Then try to list all networks with the same name
Expand All @@ -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);
}
}

0 comments on commit a7dbb82

Please sign in to comment.