From 6211310b4feb1fb2ec3978536ee29e49b101ce10 Mon Sep 17 00:00:00 2001 From: reggiemcdonald Date: Thu, 14 May 2020 16:44:05 -0600 Subject: [PATCH 1/2] Added ability to include container_name in compose file This commit updates the ParsedDockerComposeFile to pull out the container name for a service if included in the compose file. DockerComposeContainer was then updated with logic to use this specified name if present. --- .../containers/DockerComposeContainer.java | 34 +++++++++++++++---- .../containers/ParsedDockerComposeFile.java | 13 ++++--- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java b/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java index c3fe9ab17f9..4e89e075f8c 100644 --- a/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java +++ b/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java @@ -75,6 +75,7 @@ public class DockerComposeContainer> e private final List composeFiles; private Set parsedComposeFiles; private final Map scalingPreferences = new HashMap<>(); + private final Map customContainerNames = new HashMap<>(); private DockerClient dockerClient; private boolean localCompose; private boolean pull = true; @@ -122,6 +123,8 @@ public DockerComposeContainer(String identifier, List composeFiles) { this.composeFiles = composeFiles; this.parsedComposeFiles = composeFiles.stream().map(ParsedDockerComposeFile::new).collect(Collectors.toSet()); + this.parsedComposeFiles + .forEach(composeFile -> this.customContainerNames.putAll(composeFile.getContainerNames())); // Use a unique identifier so that containers created for this compose environment can be identified this.identifier = identifier; @@ -300,12 +303,19 @@ private void registerContainersForShutdown() { @VisibleForTesting List listChildContainers() { + Set names = customContainerNames.values() + .stream() + .map(name -> String.format("/%s", name)) + .collect(Collectors.toSet()); return dockerClient.listContainersCmd() - .withShowAll(true) - .exec().stream() - .filter(container -> Arrays.stream(container.getNames()).anyMatch(name -> - name.startsWith("/" + project))) - .collect(toList()); + .withShowAll(true) + .exec().stream() + .filter( + container -> Arrays.stream(container.getNames()).anyMatch(name -> + names.contains(name) || name.startsWith("/" + project) + ) + ) + .collect(toList()); } private void startAmbassadorContainers() { @@ -368,7 +378,8 @@ public SELF withExposedService(String serviceName, int servicePort, @NonNull Wai int ambassadorPort = nextAmbassadorPort.getAndIncrement(); ambassadorPortMappings.computeIfAbsent(serviceInstanceName, __ -> new ConcurrentHashMap<>()).put(servicePort, ambassadorPort); ambassadorContainer.withTarget(ambassadorPort, serviceInstanceName, servicePort); - ambassadorContainer.addLink(new FutureContainer(this.project + "_" + serviceInstanceName), serviceInstanceName); + String containerName = getContainerName(serviceInstanceName); + ambassadorContainer.addLink(new FutureContainer(containerName), serviceInstanceName); addWaitStrategy(serviceInstanceName, waitStrategy); return self(); } @@ -381,6 +392,17 @@ private String getServiceInstanceName(String serviceName) { return serviceInstanceName; } + private String getContainerName(String serviceName) { + String serviceInstanceName = serviceName; + if (serviceInstanceName.matches(".*_[0-9]+")) { + serviceName = serviceInstanceName.substring(0, serviceInstanceName.indexOf('_')); + if (customContainerNames.containsKey(serviceName)) { + return customContainerNames.get(serviceName); + } + } + return String.format("%s_%s", project, serviceInstanceName); + } + /* * can have multiple wait strategies for a single container, e.g. if waiting on several ports * if no wait strategy is defined, the WaitAllStrategy will return immediately. diff --git a/core/src/main/java/org/testcontainers/containers/ParsedDockerComposeFile.java b/core/src/main/java/org/testcontainers/containers/ParsedDockerComposeFile.java index 8f88724647f..c17075dd893 100644 --- a/core/src/main/java/org/testcontainers/containers/ParsedDockerComposeFile.java +++ b/core/src/main/java/org/testcontainers/containers/ParsedDockerComposeFile.java @@ -12,6 +12,7 @@ import java.io.FileInputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -30,6 +31,8 @@ class ParsedDockerComposeFile { @Getter private Set dependencyImageNames = new HashSet<>(); + @Getter + private Map containerNames = new HashMap<>(); ParsedDockerComposeFile(File composeFile) { Yaml yaml = new Yaml(); @@ -86,19 +89,15 @@ private void parseAndValidate() { final Map serviceDefinitionMap = (Map) serviceDefinition; - validateNoContainerNameSpecified(serviceName, serviceDefinitionMap); + findContainerName(serviceName, serviceDefinitionMap); findServiceImageName(serviceDefinitionMap); findImageNamesInDockerfile(serviceDefinitionMap); } } - private void validateNoContainerNameSpecified(String serviceName, Map serviceDefinitionMap) { + private void findContainerName(String serviceName, Map serviceDefinitionMap) { 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 - )); + this.containerNames.put(serviceName, (String) serviceDefinitionMap.get("container_name")); } } From 4f7fab152e6d494cc77b9a355a9128ccb04ef864 Mon Sep 17 00:00:00 2001 From: reggiemcdonald Date: Thu, 14 May 2020 21:39:07 -0600 Subject: [PATCH 2/2] Add tests for compose file with container name This commit adds tests for compose files with container_name specified. A compose file was added for this purpose. --- ...ParsedDockerComposeFileValidationTest.java | 35 +----------- .../DockerComposeContainerWithNameTest.java | 55 +++++++++++++++++++ .../test/resources/compose-test-with-name.yml | 7 +++ 3 files changed, 65 insertions(+), 32 deletions(-) create mode 100644 core/src/test/java/org/testcontainers/junit/DockerComposeContainerWithNameTest.java create mode 100644 core/src/test/resources/compose-test-with-name.yml diff --git a/core/src/test/java/org/testcontainers/containers/ParsedDockerComposeFileValidationTest.java b/core/src/test/java/org/testcontainers/containers/ParsedDockerComposeFileValidationTest.java index 9a8421086f1..55caac345cb 100644 --- a/core/src/test/java/org/testcontainers/containers/ParsedDockerComposeFileValidationTest.java +++ b/core/src/test/java/org/testcontainers/containers/ParsedDockerComposeFileValidationTest.java @@ -14,39 +14,9 @@ public class ParsedDockerComposeFileValidationTest { @Test - public void shouldValidate() { + public void shouldAcceptContainerName() { File file = new File("src/test/resources/docker-compose-container-name-v1.yml"); - Assertions - .assertThatThrownBy(() -> { - new ParsedDockerComposeFile(file); - }) - .hasMessageContaining(file.getAbsolutePath()) - .hasMessageContaining("'container_name' property set for service 'redis'"); - } - - @Test - public void shouldRejectContainerNameV1() { - Assertions - .assertThatThrownBy(() -> { - new ParsedDockerComposeFile(ImmutableMap.of( - "redis", ImmutableMap.of("container_name", "redis") - )); - }) - .hasMessageContaining("'container_name' property set for service 'redis'"); - } - - @Test - public void shouldRejectContainerNameV2() { - Assertions - .assertThatThrownBy(() -> { - new ParsedDockerComposeFile(ImmutableMap.of( - "version", "2", - "services", ImmutableMap.of( - "redis", ImmutableMap.of("container_name", "redis") - ) - )); - }) - .hasMessageContaining("'container_name' property set for service 'redis'"); + ParsedDockerComposeFile parsedDockerComposeFile = new ParsedDockerComposeFile(file); } @Test @@ -99,4 +69,5 @@ public void shouldObtainImageFromDockerfileBuildWithContext() { ParsedDockerComposeFile parsedFile = new ParsedDockerComposeFile(file); assertEquals("all defined images are found", Sets.newHashSet("redis", "mysql", "alpine:3.2"), parsedFile.getDependencyImageNames()); // redis, mysql from compose file, alpine:3.2 from Dockerfile build } + } diff --git a/core/src/test/java/org/testcontainers/junit/DockerComposeContainerWithNameTest.java b/core/src/test/java/org/testcontainers/junit/DockerComposeContainerWithNameTest.java new file mode 100644 index 00000000000..2118cd6a131 --- /dev/null +++ b/core/src/test/java/org/testcontainers/junit/DockerComposeContainerWithNameTest.java @@ -0,0 +1,55 @@ +package org.testcontainers.junit; + +import org.junit.Rule; +import org.junit.Test; +import org.testcontainers.containers.ContainerState; +import org.testcontainers.containers.DockerComposeContainer; + +import java.io.File; +import java.util.Optional; + +import static org.junit.Assert.assertTrue; +import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals; +import static org.rnorth.visibleassertions.VisibleAssertions.assertNotNull; + + +public class DockerComposeContainerWithNameTest extends BaseDockerComposeTest { + private final String REDIS_SERVICE_NAME = "redis"; + private final String REDIS_INSTANCE_NAME = "redis_1"; + private static final String DB_INSTANCE_NAME = "db_1"; + private static final int DB_PORT = 3306; + + @Rule + public DockerComposeContainer environment = + new DockerComposeContainer(new File("src/test/resources/compose-test-with-name.yml")) + .withExposedService(REDIS_INSTANCE_NAME, REDIS_PORT) + .withExposedService(DB_INSTANCE_NAME, DB_PORT); + + @Override + public DockerComposeContainer getEnvironment() { + return environment; + } + + @Test + public void testGetServicePort() { + int serviceWithInstancePort = environment.getServicePort(REDIS_INSTANCE_NAME, REDIS_PORT); + assertNotNull("Port is set for service with instance number", serviceWithInstancePort); + int serviceWithoutInstancePort = environment.getServicePort(REDIS_SERVICE_NAME, REDIS_PORT); + assertNotNull("Port is set for service with instance number", serviceWithoutInstancePort); + assertEquals("Service ports are the same", serviceWithInstancePort, serviceWithoutInstancePort); + } + + @Test + public void containerNamesShouldBeCorrect() { + String containerName = "/redis_container"; + Optional result = environment.getContainerByServiceName(REDIS_INSTANCE_NAME); + assertTrue(String.format("container should be found for service %s", REDIS_INSTANCE_NAME), result.isPresent()); + ContainerState state = result.get(); + assertEquals("container name should be same as compose file", containerName, state.getContainerInfo().getName()); + + result = environment.getContainerByServiceName(DB_INSTANCE_NAME); + assertTrue(String.format("container should be found for service %s", DB_INSTANCE_NAME), result.isPresent()); + state = result.get(); + assertTrue("container name should contain service name", state.getContainerInfo().getName().contains(DB_INSTANCE_NAME)); + } +} diff --git a/core/src/test/resources/compose-test-with-name.yml b/core/src/test/resources/compose-test-with-name.yml new file mode 100644 index 00000000000..ea4b4d35bf9 --- /dev/null +++ b/core/src/test/resources/compose-test-with-name.yml @@ -0,0 +1,7 @@ +redis: + image: redis + container_name: redis_container +db: + image: mysql:5.7.22 + environment: + MYSQL_RANDOM_ROOT_PASSWORD: "true"