Skip to content
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

Implement dependsOn for cross-container dependencies #1404

Merged
merged 7 commits into from Jul 23, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -31,6 +31,7 @@
import org.testcontainers.images.RemoteDockerImage;
import org.testcontainers.images.builder.Transferable;
import org.testcontainers.lifecycle.Startable;
import org.testcontainers.lifecycle.Startables;
import org.testcontainers.lifecycle.TestDescription;
import org.testcontainers.lifecycle.TestLifecycleAware;
import org.testcontainers.utility.*;
Expand All @@ -40,6 +41,7 @@
import java.nio.file.Path;
import java.time.Duration;
import java.util.*;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
Expand All @@ -54,7 +56,6 @@
* Base class for that allows a container to be launched and controlled.
*/
@Data
@EqualsAndHashCode(callSuper = false)
public class GenericContainer<SELF extends GenericContainer<SELF>>
extends FailureDetectingExternalResource
implements Container<SELF>, AutoCloseable, WaitStrategyTarget, Startable {
Expand Down Expand Up @@ -131,6 +132,8 @@ public class GenericContainer<SELF extends GenericContainer<SELF>>

private Map<MountableFile, String> copyToFileContainerPathMap = new HashMap<>();

protected final Set<Startable> dependencies = new HashSet<>();

/*
* Unique instance of DockerClient for use by this container object.
*/
Expand Down Expand Up @@ -198,6 +201,26 @@ public GenericContainer(@NonNull final Future<String> image) {
this.image = image;
}

/**
* @see #dependsOn(List)
*/
public SELF dependsOn(Startable... startables) {
rnorth marked this conversation as resolved.
Show resolved Hide resolved
Collections.addAll(dependencies, startables);
return self();
}

/**
* Delays this container's creation and start until provided {@link Startable}s start first.
* Note that the circular dependencies are not supported.
*
* @param startables a list of {@link Startable} to depend on
* @see Startables#deepStart(Collection)
*/
public SELF dependsOn(List<Startable> startables) {
dependencies.addAll(startables);
return self();
}

public String getContainerId() {
return containerId;
}
Expand All @@ -206,10 +229,12 @@ public String getContainerId() {
* Starts the container using docker, pulling an image if necessary.
*/
@Override
@SneakyThrows({InterruptedException.class, ExecutionException.class})
public void start() {
if (containerId != null) {
return;
}
Startables.deepStart(dependencies).get();
doStart();
}

Expand Down Expand Up @@ -1167,6 +1192,16 @@ public SELF withTmpFs(Map<String, String> mapping) {
return self();
}

@Override
public boolean equals(Object o) {
return this == o;
}

@Override
public int hashCode() {
return System.identityHashCode(this);
}

/**
* Convenience class with access to non-public members of GenericContainer.
*
Expand Down
@@ -1,7 +1,14 @@
package org.testcontainers.lifecycle;

import java.util.Collections;
import java.util.Set;

public interface Startable extends AutoCloseable {

default Set<Startable> getDependencies() {
return Collections.emptySet();
}

void start();

void stop();
Expand Down
76 changes: 76 additions & 0 deletions core/src/main/java/org/testcontainers/lifecycle/Startables.java
@@ -0,0 +1,76 @@
package org.testcontainers.lifecycle;

import lombok.experimental.UtilityClass;

import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Stream;

@UtilityClass
public class Startables {

private static final Executor EXECUTOR = Executors.newCachedThreadPool(new ThreadFactory() {

private final AtomicLong COUNTER = new AtomicLong(0);

@Override
public Thread newThread(Runnable r) {
Thread thread = new Thread(r, "testcontainers-lifecycle-" + COUNTER.getAndIncrement());
thread.setDaemon(true);
return thread;
}
});

/**
* @see #deepStart(Stream)
*/
public CompletableFuture<Void> deepStart(Collection<Startable> startables) {
return deepStart(startables.stream());
}

/**
* Start every {@link Startable} recursively and asynchronously and join on the result.
*
* Performance note:
* The method uses and returns {@link CompletableFuture}s to resolve as many {@link Startable}s at once as possible.
* This way, for the following graph:
* / b \
* a e
* c /
* d /
* "a", "c" and "d" will resolve in parallel, then "b".
*
* If we would call blocking {@link Startable#start()}, "e" would wait for "b", "b" for "a", and only then "c", and then "d".
* But, since "c" and "d" are independent from "a", there is no point in waiting for "a" to be resolved first.
*
* @param startables a {@link Stream} of {@link Startable}s to start and scan for transitive dependencies.
* @return a {@link CompletableFuture} that resolves once all {@link Startable}s have started.
*/
public CompletableFuture<Void> deepStart(Stream<Startable> startables) {
return deepStart(new HashMap<>(), startables);
}

/**
*
* @param started an intermediate storage for already started {@link Startable}s to prevent multiple starts.
* @param startables a {@link Stream} of {@link Startable}s to start and scan for transitive dependencies.
*/
private CompletableFuture<Void> deepStart(Map<Startable, CompletableFuture<Void>> started, Stream<Startable> startables) {
rnorth marked this conversation as resolved.
Show resolved Hide resolved
CompletableFuture[] futures = startables
.map(it -> {
// avoid a recursive update in `computeIfAbsent`
Map<Startable, CompletableFuture<Void>> subStarted = new HashMap<>(started);
CompletableFuture<Void> future = started.computeIfAbsent(it, startable -> {
return deepStart(subStarted, startable.getDependencies().stream()).thenRunAsync(startable::start, EXECUTOR);
});
started.putAll(subStarted);
return future;
})
.toArray(CompletableFuture[]::new);

return CompletableFuture.allOf(futures);
}
}
142 changes: 142 additions & 0 deletions core/src/test/java/org/testcontainers/junit/DependenciesTest.java
@@ -0,0 +1,142 @@
package org.testcontainers.junit;

import lombok.Getter;
import org.junit.Test;
import org.rnorth.visibleassertions.VisibleAssertions;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.containers.startupcheck.OneShotStartupCheckStrategy;
import org.testcontainers.lifecycle.Startable;
import org.testcontainers.lifecycle.Startables;

import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Stream;

public class DependenciesTest {

@Test
public void shouldWorkWithSimpleDependency() {
InvocationCountingStartable startable = new InvocationCountingStartable();

try (
GenericContainer container = new GenericContainer()
.withStartupCheckStrategy(new OneShotStartupCheckStrategy())
.dependsOn(startable)
) {
container.start();
}

VisibleAssertions.assertEquals("Started once", 1, startable.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("Does not trigger .stop()", 0, startable.getStopInvocationCount().intValue());
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure there's a good reason, but why do we not propagate a stop() call through the dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

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

because there is no guarantee that the dependencies will be a part of the same "group".

We may introduce deepStop in future

Choose a reason for hiding this comment

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

I think a deepStop would be useful for when you know these are the semantics you want. I end up having to manually create one in several projects. Thoughts on adding an API?

Btw, are both stop and start required to be idempotent? I think the current implementation of deepStart depends on this.

}

@Test
public void shouldWorkWithMutlipleDependencies() {
InvocationCountingStartable startable1 = new InvocationCountingStartable();
InvocationCountingStartable startable2 = new InvocationCountingStartable();

try (
GenericContainer container = new GenericContainer()
.withStartupCheckStrategy(new OneShotStartupCheckStrategy())
.dependsOn(startable1, startable2)
) {
container.start();
}

VisibleAssertions.assertEquals("Startable1 started once", 1, startable1.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("Startable2 started once", 1, startable2.getStartInvocationCount().intValue());
}

@Test
public void shouldStartEveryTime() {
InvocationCountingStartable startable = new InvocationCountingStartable();

try (
GenericContainer container = new GenericContainer()
.withStartupCheckStrategy(new OneShotStartupCheckStrategy())
.dependsOn(startable)
) {
container.start();
container.stop();

container.start();
container.stop();

container.start();
}

VisibleAssertions.assertEquals("Started multiple times", 3, startable.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("Does not trigger .stop()", 0, startable.getStopInvocationCount().intValue());
}

@Test
public void shouldStartTransitiveDependencies() {
Copy link
Member

Choose a reason for hiding this comment

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

Just eyeballing the code, it looks like we're safe from circular dependencies, and also 'diamond' dependencies should be OK too.

Do you think it might be good to have tests for these scenarios in case a regression is introduced later?

Copy link
Member Author

Choose a reason for hiding this comment

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

Circular dependencies won't work (we have "hard depends on")

I will add a test for the diamond case 👍

InvocationCountingStartable transitiveOfTransitiveStartable = new InvocationCountingStartable();
InvocationCountingStartable transitiveStartable = new InvocationCountingStartable();
transitiveStartable.getDependencies().add(transitiveOfTransitiveStartable);

InvocationCountingStartable startable = new InvocationCountingStartable();
startable.getDependencies().add(transitiveStartable);

try (
GenericContainer container = new GenericContainer()
.withStartupCheckStrategy(new OneShotStartupCheckStrategy())
.dependsOn(startable)
) {
container.start();
container.stop();
}

VisibleAssertions.assertEquals("Root started", 1, startable.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("Transitive started", 1, transitiveStartable.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("Transitive of transitive started", 1, transitiveOfTransitiveStartable.getStartInvocationCount().intValue());
}

@Test
public void shouldHandleDiamondDependencies() throws Exception {
InvocationCountingStartable a = new InvocationCountingStartable();
InvocationCountingStartable b = new InvocationCountingStartable();
InvocationCountingStartable c = new InvocationCountingStartable();
InvocationCountingStartable d = new InvocationCountingStartable();
// / b \
// a d
// \ c /
b.getDependencies().add(a);
c.getDependencies().add(a);

d.getDependencies().add(b);
d.getDependencies().add(c);

Startables.deepStart(Stream.of(d)).get(1, TimeUnit.SECONDS);

VisibleAssertions.assertEquals("A started", 1, a.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("B started", 1, b.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("C started", 1, c.getStartInvocationCount().intValue());
VisibleAssertions.assertEquals("D started", 1, d.getStartInvocationCount().intValue());
}

private static class InvocationCountingStartable implements Startable {

@Getter
rnorth marked this conversation as resolved.
Show resolved Hide resolved
Set<Startable> dependencies = new HashSet<>();

@Getter
AtomicLong startInvocationCount = new AtomicLong(0);

@Getter
AtomicLong stopInvocationCount = new AtomicLong(0);

@Override
public void start() {
startInvocationCount.getAndIncrement();

}

@Override
public void stop() {
stopInvocationCount.getAndIncrement();
}
}
}