Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.model.Container;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.Maps;
import com.google.common.util.concurrent.Uninterruptibles;
import lombok.NonNull;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.SystemUtils;
import org.junit.runner.Description;
Expand All @@ -19,25 +18,24 @@
import org.testcontainers.containers.output.OutputFrame;
import org.testcontainers.containers.output.Slf4jLogConsumer;
import org.testcontainers.containers.startupcheck.IndefiniteWaitOneShotStartupCheckStrategy;
import org.testcontainers.containers.wait.strategy.*;
import org.testcontainers.containers.wait.strategy.Wait;
import org.testcontainers.containers.wait.strategy.WaitAllStrategy;
import org.testcontainers.containers.wait.strategy.WaitStrategy;
import org.testcontainers.lifecycle.Startable;
import org.testcontainers.utility.*;
import org.yaml.snakeyaml.Yaml;
import org.zeroturnaround.exec.InvalidExitValueException;
import org.zeroturnaround.exec.ProcessExecutor;
import org.zeroturnaround.exec.stream.slf4j.Slf4jStream;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.*;
import java.time.Duration;
import java.util.AbstractMap.SimpleEntry;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static com.google.common.base.Preconditions.checkArgument;
Expand All @@ -58,8 +56,7 @@ public class DockerComposeContainer<SELF extends DockerComposeContainer<SELF>> e
*/
private final String identifier;
private final List<File> composeFiles;
private final Set<String> spawnedContainerIds = new HashSet<>();
private final Set<String> spawnedNetworkIds = new HashSet<>();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two sets were never being written to, so they and all code that read them was redundant. Removed.

private Set<ParsedDockerComposeFile> parsedComposeFiles;
private final Map<String, Integer> scalingPreferences = new HashMap<>();
private DockerClient dockerClient;
private boolean localCompose;
Expand Down Expand Up @@ -107,10 +104,11 @@ public DockerComposeContainer(String identifier, File... composeFiles) {
public DockerComposeContainer(String identifier, List<File> composeFiles) {

this.composeFiles = composeFiles;
this.parsedComposeFiles = composeFiles.stream().map(ParsedDockerComposeFile::new).collect(Collectors.toSet());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perform light parsing of the docker compose files at instantiation. We could delay this if it's felt doing this is too heavy at instantiation-time, but if we do that we just need to make sure we do it at some point.


// Use a unique identifier so that containers created for this compose environment can be identified
this.identifier = identifier;
project = randomProjectId();
this.project = randomProjectId();

this.dockerClient = DockerClientFactory.instance().client();
}
Expand Down Expand Up @@ -154,15 +152,25 @@ public void start() {
log.warn("Exception while pulling images, using local images if available", e);
}
}
applyScaling(); // scale before up, so that all scaled instances are available first for linking
createServices();
startAmbassadorContainers();
waitUntilServiceStarted();
}
}

private void pullImages() {
runWithCompose("pull");
// Pull images using our docker client rather than compose itself,
// (a) as a workaround for https://github.com/docker/compose/issues/5854, which prevents authenticated image pulls being possible when credential helpers are in use
// (b) so that credential helper-based auth still works when compose is running from within a container
parsedComposeFiles.stream()
.flatMap(it -> it.getServiceImageNames().stream())
.forEach(imageName -> {
try {
DockerClientFactory.instance().checkAndPullImage(dockerClient, imageName);
} catch (Exception e) {
log.warn("Failed to pull image '{}'. Exception message was {}", imageName, e.getMessage());
}
});
}

public SELF withServices(@NonNull String... services) {
Expand All @@ -171,15 +179,22 @@ public SELF withServices(@NonNull String... services) {
}

private void createServices() {
// Run the docker-compose container, which starts up the services
String command = "up -d";
// Apply scaling
final String servicesWithScalingSettings = Stream.concat(services.stream(), scalingPreferences.keySet().stream())
Copy link
Member Author

@rnorth rnorth Sep 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scaling behaviour changed in newer docker-compose; rather than old behaviour of:

docker-compose scale someservice=2
docker-compose up -d

we now have to do:

docker-compose up -d --scale someservice=2

.map(service -> "--scale " + service + "=" + scalingPreferences.getOrDefault(service, 1))
.collect(joining(" "));

String flags = "-d";

if (build) {
command += " --build";
flags += " --build";
}

if (!services.isEmpty()) {
command += " " + String.join(" ", services);
// Run the docker-compose container, which starts up the services
if(Strings.isNullOrEmpty(servicesWithScalingSettings)) {
runWithCompose("up " + flags);
} else {
runWithCompose("up " + flags + " " + servicesWithScalingSettings);
}

runWithCompose(command);
Expand Down Expand Up @@ -221,10 +236,6 @@ private void runWithCompose(String cmd) {
checkNotNull(composeFiles);
checkArgument(!composeFiles.isEmpty(), "No docker compose file have been provided");

for (File composeFile : composeFiles) {
validate(composeFile);
}

final DockerCompose dockerCompose;
if (localCompose) {
dockerCompose = new LocalDockerCompose(composeFiles, project);
Expand All @@ -238,72 +249,6 @@ private void runWithCompose(String cmd) {
.invoke();
}

@SuppressWarnings("unchecked")
private static void validate(File composeFile) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted to ParsedDockerComposeFile

Yaml yaml = new Yaml();
try (FileInputStream fileInputStream = FileUtils.openInputStream(composeFile)) {
Object template = yaml.load(fileInputStream);
validate(template, composeFile.getAbsolutePath());
} catch (IOException e) {
log.warn("Failed to read YAML from {}", composeFile.getAbsolutePath(), e);
}
}

@VisibleForTesting
static void validate(Object template, String identifier) {
if (!(template instanceof Map)) {
return;
}

Map<String, ?> map = (Map<String, ?>) template;

final Map<String, ?> servicesMap;
if (map.containsKey("version")) {
if (!map.containsKey("services")) {
log.debug("Compose file {} has an unknown format: 'version' is set but 'services' is not defined", identifier);
return;
}
Object services = map.get("services");
if (!(services instanceof Map)) {
log.debug("Compose file {} has an unknown format: 'services' is not Map", identifier);
return;
}

servicesMap = (Map<String, ?>) services;
} else {
servicesMap = map;
}

for (Map.Entry<String, ?> entry : servicesMap.entrySet()) {
String serviceName = entry.getKey();
Object serviceDefinition = entry.getValue();
if (!(serviceDefinition instanceof Map)) {
log.debug("Compose file {} has an unknown format: service '{}' is not Map", identifier, serviceName);
break;
}

if (((Map) serviceDefinition).containsKey("container_name")) {
throw new IllegalStateException(String.format(
"Compose file %s has 'container_name' property set for service '%s' but this property is not supported by Testcontainers, consider removing it",
identifier,
serviceName
));
}
}
}

private void applyScaling() {
// Apply scaling
if (!scalingPreferences.isEmpty()) {
StringBuilder sb = new StringBuilder("scale");
for (Map.Entry<String, Integer> scale : scalingPreferences.entrySet()) {
sb.append(" ").append(scale.getKey()).append("=").append(scale.getValue());
}

runWithCompose(sb.toString());
}
}

private void registerContainersForShutdown() {
ResourceReaper.instance().registerFilterForCleanup(Arrays.asList(
new SimpleEntry<>("label", "com.docker.compose.project=" + project)
Expand Down Expand Up @@ -333,29 +278,12 @@ public void stop() {
ambassadorContainer.stop();

// Kill the services using docker-compose
try {
String cmd = "down -v";
if (removeImages != null) {
cmd += " --rmi " + removeImages.dockerRemoveImagesType();
}
runWithCompose(cmd);

// If we reach here then docker-compose down has cleared networks and containers;
// we can unregister from ResourceReaper
spawnedContainerIds.forEach(ResourceReaper.instance()::unregisterContainer);
spawnedNetworkIds.forEach(ResourceReaper.instance()::unregisterNetwork);
} catch (Exception e) {
// docker-compose down failed; use ResourceReaper to ensure cleanup

// kill the spawned service containers
spawnedContainerIds.forEach(ResourceReaper.instance()::stopAndRemoveContainer);

// remove the networks after removing the containers
spawnedNetworkIds.forEach(ResourceReaper.instance()::removeNetworkById);
String cmd = "down -v";
if (removeImages != null) {
cmd += " --rmi " + removeImages.dockerRemoveImagesType();
}
runWithCompose(cmd);

spawnedContainerIds.clear();
spawnedNetworkIds.clear();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed essentially dead code in this section

} finally {
project = randomProjectId();
}
Expand Down Expand Up @@ -597,9 +525,6 @@ interface DockerCompose {
class ContainerisedDockerCompose extends GenericContainer<ContainerisedDockerCompose> implements DockerCompose {

private static final String DOCKER_SOCKET_PATH = "/var/run/docker.sock";
private static final String DOCKER_CONFIG_FILE = "/root/.docker/config.json";
private static final String DOCKER_CONFIG_ENV = "DOCKER_CONFIG_FILE";
private static final String DOCKER_CONFIG_PROPERTY = "dockerConfigFile";
public static final char UNIX_PATH_SEPERATOR = ':';

public ContainerisedDockerCompose(List<File> composeFiles, String identifier) {
Expand Down Expand Up @@ -630,27 +555,6 @@ public ContainerisedDockerCompose(List<File> composeFiles, String identifier) {
addEnv("DOCKER_HOST", "unix:///docker.sock");
setStartupCheckStrategy(new IndefiniteWaitOneShotStartupCheckStrategy());
setWorkingDirectory(containerPwd);

String dockerConfigPath = determineDockerConfigPath();
if (dockerConfigPath != null && !dockerConfigPath.isEmpty()) {
addFileSystemBind(dockerConfigPath, DOCKER_CONFIG_FILE, READ_ONLY);
}
}

private String determineDockerConfigPath() {
String dockerConfigEnv = System.getenv(DOCKER_CONFIG_ENV);
String dockerConfigProperty = System.getProperty(DOCKER_CONFIG_PROPERTY);
Path dockerConfig = Paths.get(System.getProperty("user.home"), ".docker", "config.json");

if (dockerConfigEnv != null && !dockerConfigEnv.trim().isEmpty() && Files.exists(Paths.get(dockerConfigEnv))) {
return dockerConfigEnv;
} else if (dockerConfigProperty != null && !dockerConfigProperty.trim().isEmpty() && Files.exists(Paths.get(dockerConfigProperty))) {
return dockerConfigProperty;
} else if (Files.exists(dockerConfig)) {
return dockerConfig.toString();
} else {
return null;
}
Copy link
Member Author

@rnorth rnorth Sep 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposing docker config.json to the containerised compose now does more harm than good, as now understands, but can't use, the credential helpers.

Instead, I removed this config passthrough altogether.

}

private String getDockerSocketHostPath() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package org.testcontainers.containers;

import com.google.common.annotations.VisibleForTesting;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.io.FileUtils;
import org.yaml.snakeyaml.Yaml;

import java.io.File;
import java.io.FileInputStream;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

/**
* Representation of a docker-compose file, with partial parsing for validation and extraction of a minimal set of
* data.
*/
@Slf4j
@EqualsAndHashCode
class ParsedDockerComposeFile {

private final Map<String, Object> composeFileContent;
private final String composeFileName;

@Getter
private Set<String> serviceImageNames = new HashSet<>();

ParsedDockerComposeFile(File composeFile) {
Yaml yaml = new Yaml();
try (FileInputStream fileInputStream = FileUtils.openInputStream(composeFile)) {
composeFileContent = yaml.load(fileInputStream);
} catch (Exception e) {
throw new IllegalArgumentException("Unable to parse YAML file from " + composeFile.getAbsolutePath(), e);
}
this.composeFileName = composeFile.getAbsolutePath();

parseAndValidate();
}

@VisibleForTesting
ParsedDockerComposeFile(Map<String, Object> testContent) {
this.composeFileContent = testContent;
this.composeFileName = "";

parseAndValidate();
}

private void parseAndValidate() {
final Map<String, ?> servicesMap;
if (composeFileContent.containsKey("version")) {
if ("2.0".equals(composeFileContent.get("version"))) {
log.warn("Testcontainers may not be able to clean up networks spawned using Docker Compose v2.0 files. " +
"Please see https://github.com/testcontainers/moby-ryuk/issues/2, and specify 'version: \"2.1\"' or " +
"higher in {}", composeFileName);
}

final Object servicesElement = composeFileContent.get("services");
if (servicesElement == null) {
log.debug("Compose file {} has an unknown format: 'version' is set but 'services' is not defined", composeFileName);
return;
}
if (!(servicesElement instanceof Map)) {
log.debug("Compose file {} has an unknown format: 'services' is not Map", composeFileName);
return;
}

servicesMap = (Map<String, ?>) servicesElement;
} else {
servicesMap = composeFileContent;
}

for (Map.Entry<String, ?> entry : servicesMap.entrySet()) {
String serviceName = entry.getKey();
Object serviceDefinition = entry.getValue();
if (!(serviceDefinition instanceof Map)) {
log.debug("Compose file {} has an unknown format: service '{}' is not Map", composeFileName, serviceName);
break;
}

final Map serviceDefinitionMap = (Map) serviceDefinition;
if (serviceDefinitionMap.containsKey("container_name")) {
throw new IllegalStateException(String.format(
"Compose file %s has 'container_name' property set for service '%s' but this property is not supported by Testcontainers, consider removing it",
composeFileName,
serviceName
));
}
if (serviceDefinitionMap.containsKey("image") && serviceDefinitionMap.get("image") instanceof String) {
serviceImageNames.add((String) serviceDefinitionMap.get("image"));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public String getVncRecordedContainerImage() {
}

public String getDockerComposeContainerImage() {
return (String) properties.getOrDefault("compose.container.image", "docker/compose:1.8.0");
return (String) properties.getOrDefault("compose.container.image", "docker/compose:1.24.1");
}

public String getTinyImage() {
Expand Down
Loading