-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Death Note :) #545
Death Note :) #545
Changes from all commits
40d51fe
69dcb17
23c0840
8df3a48
29de66b
dbfc5bb
cf60eba
0ad2797
73206a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,35 +2,32 @@ | |
|
||
import com.github.dockerjava.api.DockerClient; | ||
import com.github.dockerjava.api.command.CreateContainerCmd; | ||
import com.github.dockerjava.api.command.InspectContainerResponse; | ||
import com.github.dockerjava.api.exception.InternalServerErrorException; | ||
import com.github.dockerjava.api.exception.NotFoundException; | ||
import com.github.dockerjava.api.model.*; | ||
import com.github.dockerjava.core.command.ExecStartResultCallback; | ||
import com.github.dockerjava.core.command.PullImageResultCallback; | ||
import com.google.common.annotations.VisibleForTesting; | ||
import com.google.common.collect.ImmutableMap; | ||
import lombok.Synchronized; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.apache.commons.io.IOUtils; | ||
import org.hamcrest.BaseMatcher; | ||
import org.hamcrest.Description; | ||
import org.rnorth.ducttape.unreliables.Unreliables; | ||
import org.rnorth.visibleassertions.VisibleAssertions; | ||
import org.testcontainers.dockerclient.*; | ||
import org.testcontainers.dockerclient.DockerClientProviderStrategy; | ||
import org.testcontainers.dockerclient.DockerMachineClientProviderStrategy; | ||
import org.testcontainers.utility.ComparableVersion; | ||
import org.testcontainers.utility.MountableFile; | ||
import org.testcontainers.utility.ResourceReaper; | ||
import org.testcontainers.utility.TestcontainersConfiguration; | ||
|
||
import java.io.ByteArrayOutputStream; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.net.Socket; | ||
import java.nio.charset.Charset; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.ServiceLoader; | ||
import java.util.concurrent.TimeUnit; | ||
import java.util.UUID; | ||
import java.util.function.BiFunction; | ||
import java.util.function.Consumer; | ||
|
||
|
@@ -42,12 +39,22 @@ | |
@Slf4j | ||
public class DockerClientFactory { | ||
|
||
public static final String TESTCONTAINERS_LABEL = DockerClientFactory.class.getPackage().getName(); | ||
public static final String TESTCONTAINERS_SESSION_ID_LABEL = TESTCONTAINERS_LABEL + ".sessionId"; | ||
|
||
public static final String SESSION_ID = UUID.randomUUID().toString(); | ||
|
||
public static final Map<String, String> DEFAULT_LABELS = ImmutableMap.of( | ||
TESTCONTAINERS_LABEL, "true", | ||
TESTCONTAINERS_SESSION_ID_LABEL, SESSION_ID | ||
); | ||
|
||
private static final String TINY_IMAGE = TestcontainersConfiguration.getInstance().getTinyImage(); | ||
private static DockerClientFactory instance; | ||
|
||
// Cached client configuration | ||
private DockerClientProviderStrategy strategy; | ||
private boolean preconditionsChecked = false; | ||
private boolean initialized = false; | ||
private String activeApiVersion; | ||
private String activeExecutionDriver; | ||
|
||
|
@@ -95,7 +102,7 @@ public DockerClient client() { | |
log.info("Docker host IP address is {}", hostIpAddress); | ||
DockerClient client = strategy.getClient(); | ||
|
||
if (!preconditionsChecked) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. had to change the name because it's not just |
||
if (!initialized) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can encapsulate the following block like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not do it in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem, but why not? 😉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because it's out of scope, and if somebody submits another PR where they change DockerClientFactory, but we didn't merge "Death Note" PR yet, it will create more conflicts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also think about reverts in case we discover some major bug :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes okay, makes sense 🙂 |
||
Info dockerInfo = client.infoCmd().exec(); | ||
Version version = client.versionCmd().exec(); | ||
activeApiVersion = version.getApiVersion(); | ||
|
@@ -106,30 +113,19 @@ public DockerClient client() { | |
" Operating System: " + dockerInfo.getOperatingSystem() + "\n" + | ||
" Total Memory: " + dockerInfo.getMemTotal() / (1024 * 1024) + " MB"); | ||
|
||
if (!TestcontainersConfiguration.getInstance().isDisableChecks()) { | ||
VisibleAssertions.info("Checking the system..."); | ||
|
||
checkDockerVersion(version.getVersion()); | ||
|
||
MountableFile mountableFile = MountableFile.forClasspathResource(this.getClass().getName().replace(".", "/") + ".class"); | ||
String ryukContainerId = ResourceReaper.start(hostIpAddress, client); | ||
log.info("Ryuk started - will monitor and terminate Testcontainers containers on JVM exit"); | ||
|
||
runInsideDocker( | ||
client, | ||
cmd -> cmd | ||
.withCmd("/bin/sh", "-c", "while true; do printf 'hello' | nc -l -p 80; done") | ||
.withBinds(new Bind(mountableFile.getResolvedPath(), new Volume("/dummy"), AccessMode.ro)) | ||
.withExposedPorts(new ExposedPort(80)) | ||
.withPublishAllPorts(true), | ||
(dockerClient, id) -> { | ||
VisibleAssertions.info("Checking the system..."); | ||
|
||
checkDiskSpace(dockerClient, id); | ||
checkMountableFile(dockerClient, id); | ||
checkExposedPort(hostIpAddress, dockerClient, id); | ||
checkDockerVersion(version.getVersion()); | ||
|
||
return null; | ||
}); | ||
if (!TestcontainersConfiguration.getInstance().isDisableChecks()) { | ||
checkDiskSpace(client, ryukContainerId); | ||
checkMountableFile(client, ryukContainerId); | ||
} | ||
preconditionsChecked = true; | ||
|
||
initialized = true; | ||
} | ||
|
||
return client; | ||
|
@@ -178,26 +174,10 @@ private void checkMountableFile(DockerClient dockerClient, String id) { | |
} | ||
} | ||
|
||
private void checkExposedPort(String hostIpAddress, DockerClient dockerClient, String id) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this check is not needed anymore, Ryuk demands a connectivity hence implicitly checks for the exposed port |
||
String response = Unreliables.retryUntilSuccess(3, TimeUnit.SECONDS, () -> { | ||
InspectContainerResponse inspectedContainer = dockerClient.inspectContainerCmd(id).exec(); | ||
|
||
String portSpec = inspectedContainer.getNetworkSettings().getPorts().getBindings().values().iterator().next()[0].getHostPortSpec(); | ||
|
||
try (Socket socket = new Socket(hostIpAddress, Integer.parseInt(portSpec))) { | ||
return IOUtils.toString(socket.getInputStream(), Charset.defaultCharset()); | ||
} catch (IOException e) { | ||
return e.getMessage(); | ||
} | ||
}); | ||
|
||
VisibleAssertions.assertEquals("A port exposed by a docker container should be accessible", "hello", response); | ||
} | ||
|
||
/** | ||
* Check whether the image is available locally and pull it otherwise | ||
*/ | ||
private void checkAndPullImage(DockerClient client, String image) { | ||
public void checkAndPullImage(DockerClient client, String image) { | ||
List<Image> images = client.listImagesCmd().withImageNameFilter(image).exec(); | ||
if (images.isEmpty()) { | ||
client.pullImageCmd(image).exec(new PullImageResultCallback()).awaitSuccess(); | ||
|
@@ -221,7 +201,8 @@ public <T> T runInsideDocker(Consumer<CreateContainerCmd> createContainerCmdCons | |
|
||
private <T> T runInsideDocker(DockerClient client, Consumer<CreateContainerCmd> createContainerCmdConsumer, BiFunction<DockerClient, String, T> block) { | ||
checkAndPullImage(client, TINY_IMAGE); | ||
CreateContainerCmd createContainerCmd = client.createContainerCmd(TINY_IMAGE); | ||
CreateContainerCmd createContainerCmd = client.createContainerCmd(TINY_IMAGE) | ||
.withLabels(DEFAULT_LABELS); | ||
createContainerCmdConsumer.accept(createContainerCmd); | ||
String id = createContainerCmd.exec().getId(); | ||
|
||
|
@@ -263,7 +244,7 @@ DiskSpaceUsage parseAvailableDiskSpace(String dfOutput) { | |
* @return the docker API version of the daemon that we have connected to | ||
*/ | ||
public String getActiveApiVersion() { | ||
if (!preconditionsChecked) { | ||
if (!initialized) { | ||
client(); | ||
} | ||
return activeApiVersion; | ||
|
@@ -273,7 +254,7 @@ public String getActiveApiVersion() { | |
* @return the docker execution driver of the daemon that we have connected to | ||
*/ | ||
public String getActiveExecutionDriver() { | ||
if (!preconditionsChecked) { | ||
if (!initialized) { | ||
client(); | ||
} | ||
return activeExecutionDriver; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
package org.testcontainers.containers; | ||
|
||
import com.github.dockerjava.api.DockerClient; | ||
import com.github.dockerjava.api.exception.DockerException; | ||
import com.github.dockerjava.api.model.Container; | ||
import com.google.common.annotations.VisibleForTesting; | ||
import com.google.common.base.Joiner; | ||
|
@@ -24,16 +23,11 @@ | |
import org.zeroturnaround.exec.stream.slf4j.Slf4jStream; | ||
|
||
import java.io.File; | ||
import java.util.Arrays; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.*; | ||
import java.util.AbstractMap.SimpleEntry; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
import java.util.concurrent.TimeUnit; | ||
import java.util.concurrent.atomic.AtomicInteger; | ||
import java.util.stream.Collectors; | ||
|
||
import static com.google.common.base.Preconditions.checkArgument; | ||
import static com.google.common.base.Preconditions.checkNotNull; | ||
|
@@ -59,6 +53,8 @@ public class DockerComposeContainer<SELF extends DockerComposeContainer<SELF>> e | |
private boolean pull = true; | ||
private boolean tailChildContainers; | ||
|
||
private String project; | ||
|
||
private final AtomicInteger nextAmbassadorPort = new AtomicInteger(2000); | ||
private final Map<String, Map<Integer, Integer>> ambassadorPortMappings = new ConcurrentHashMap<>(); | ||
private final SocatContainer ambassadorContainer = new SocatContainer(); | ||
|
@@ -94,6 +90,7 @@ public DockerComposeContainer(String identifier, List<File> composeFiles) { | |
|
||
// Use a unique identifier so that containers created for this compose environment can be identified | ||
this.identifier = identifier; | ||
project = randomProjectId(); | ||
|
||
this.dockerClient = DockerClientFactory.instance().client(); | ||
} | ||
|
@@ -106,6 +103,7 @@ public void starting(Description description) { | |
profiler.start("Docker Compose container startup"); | ||
|
||
synchronized (MUTEX) { | ||
registerContainersForShutdown(); | ||
if (pull) { | ||
pullImages(); | ||
} | ||
|
@@ -114,7 +112,6 @@ public void starting(Description description) { | |
if (tailChildContainers) { | ||
tailChildContainerLogs(); | ||
} | ||
registerContainersForShutdown(); | ||
startAmbassadorContainers(profiler); | ||
} | ||
} | ||
|
@@ -142,9 +139,9 @@ private void tailChildContainerLogs() { | |
private void runWithCompose(String cmd) { | ||
final DockerCompose dockerCompose; | ||
if (localCompose) { | ||
dockerCompose = new LocalDockerCompose(composeFiles, identifier); | ||
dockerCompose = new LocalDockerCompose(composeFiles, project); | ||
} else { | ||
dockerCompose = new ContainerisedDockerCompose(composeFiles, identifier); | ||
dockerCompose = new ContainerisedDockerCompose(composeFiles, project); | ||
} | ||
|
||
dockerCompose | ||
|
@@ -166,38 +163,17 @@ private void applyScaling() { | |
} | ||
|
||
private void registerContainersForShutdown() { | ||
// Ensure that all service containers that were launched by compose will be killed at shutdown | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome 🗡️ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✂️ 😄 |
||
try { | ||
final List<Container> containers = listChildContainers(); | ||
|
||
// register with ResourceReaper to ensure final shutdown with JVM | ||
containers.forEach(container -> | ||
ResourceReaper.instance().registerContainerForCleanup(container.getId(), container.getNames()[0])); | ||
|
||
// 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().registerNetworkIdForCleanup(network.getId()); | ||
} | ||
}); | ||
|
||
// remember the IDs to allow containers to be killed as soon as we reach stop() | ||
spawnedContainerIds.addAll(containers.stream() | ||
.map(Container::getId) | ||
.collect(Collectors.toSet())); | ||
|
||
} catch (DockerException e) { | ||
logger().debug("Failed to stop a service container with exception", e); | ||
} | ||
ResourceReaper.instance().registerFilterForCleanup(Arrays.asList( | ||
new SimpleEntry<>("label", "com.docker.compose.project=" + project) | ||
)); | ||
} | ||
|
||
private List<Container> listChildContainers() { | ||
return dockerClient.listContainersCmd() | ||
.withShowAll(true) | ||
.exec().stream() | ||
.filter(container -> Arrays.stream(container.getNames()).anyMatch(name -> | ||
name.startsWith("/" + identifier))) | ||
name.startsWith("/" + project))) | ||
.collect(toList()); | ||
} | ||
|
||
|
@@ -216,29 +192,33 @@ public void finished(Description description) { | |
|
||
|
||
synchronized (MUTEX) { | ||
// shut down the ambassador container | ||
ambassadorContainer.stop(); | ||
|
||
// Kill the services using docker-compose | ||
try { | ||
runWithCompose("down -v"); | ||
// shut down the ambassador container | ||
ambassadorContainer.stop(); | ||
|
||
// 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 services using docker-compose | ||
try { | ||
runWithCompose("down -v"); | ||
|
||
// kill the spawned service containers | ||
spawnedContainerIds.forEach(ResourceReaper.instance()::stopAndRemoveContainer); | ||
// 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 | ||
|
||
// remove the networks after removing the containers | ||
spawnedNetworkIds.forEach(ResourceReaper.instance()::removeNetworkById); | ||
} | ||
// kill the spawned service containers | ||
spawnedContainerIds.forEach(ResourceReaper.instance()::stopAndRemoveContainer); | ||
|
||
// remove the networks after removing the containers | ||
spawnedNetworkIds.forEach(ResourceReaper.instance()::removeNetworkById); | ||
} | ||
|
||
spawnedContainerIds.clear(); | ||
spawnedNetworkIds.clear(); | ||
spawnedContainerIds.clear(); | ||
spawnedNetworkIds.clear(); | ||
} finally { | ||
project = randomProjectId(); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -266,7 +246,7 @@ public SELF withExposedService(String serviceName, int servicePort) { | |
int ambassadorPort = nextAmbassadorPort.getAndIncrement(); | ||
ambassadorPortMappings.computeIfAbsent(serviceName, __ -> new ConcurrentHashMap<>()).put(servicePort, ambassadorPort); | ||
ambassadorContainer.withTarget(ambassadorPort, serviceName, servicePort); | ||
ambassadorContainer.addLink(new FutureContainer(this.identifier + "_" + serviceName), serviceName); | ||
ambassadorContainer.addLink(new FutureContainer(this.project + "_" + serviceName), serviceName); | ||
return self(); | ||
} | ||
|
||
|
@@ -351,6 +331,10 @@ public SELF withTailChildContainers(boolean tailChildContainers) { | |
private SELF self() { | ||
return (SELF) this; | ||
} | ||
|
||
private String randomProjectId() { | ||
return identifier + Base58.randomString(6).toLowerCase(); | ||
} | ||
} | ||
|
||
interface DockerCompose { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,10 +211,8 @@ private void tryStart(Profiler profiler) { | |
profiler.start("Create container"); | ||
CreateContainerCmd createCommand = dockerClient.createContainerCmd(dockerImageName); | ||
applyConfiguration(createCommand); | ||
createContainerCmdModifiers.forEach(hook -> hook.accept(createCommand)); | ||
|
||
containerId = createCommand.exec().getId(); | ||
ResourceReaper.instance().registerContainerForCleanup(containerId, dockerImageName); | ||
|
||
logger().info("Starting container with ID: {}", containerId); | ||
profiler.start("Start container"); | ||
|
@@ -465,7 +463,12 @@ private void applyConfiguration(CreateContainerCmd createCommand) { | |
createCommand.withPrivileged(privilegedMode); | ||
} | ||
|
||
createCommand.withLabels(Collections.singletonMap("org.testcontainers", "true")); | ||
createContainerCmdModifiers.forEach(hook -> hook.accept(createCommand)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you have to do this now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just moved the call from another place because we must add our labels after user sets his own |
||
|
||
Map<String, String> labels = createCommand.getLabels(); | ||
labels = new HashMap<>(labels != null ? labels : Collections.emptyMap()); | ||
labels.putAll(DockerClientFactory.DEFAULT_LABELS); | ||
createCommand.withLabels(labels); | ||
} | ||
|
||
private Set<Link> findLinksFromThisContainer(String alias, LinkableContainer linkableContainer) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This boolean is of course false by default, or did you want to make it explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just renamed the variable :) But yes, explicit
false
helps to understand the logic