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

Add support for Docker networks. #372

Merged
merged 13 commits into from
Jun 25, 2017
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ cache:
install:
- echo "MAVEN_OPTS='-XX:+TieredCompilation -XX:TieredStopAtLevel=1'" > ~/.mavenrc
- sudo apt-get update -qq
- sudo apt-get -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confnew install docker-engine
- sudo apt-get -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confnew install docker-ce
- docker info
# Update env vars required by testcontainers
- export DOCKER_HOST=tcp://127.0.0.1:2375
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
- Added pre-flight checks (can be disabled with `checks.disable` configuration property) (#363)
- Removed unused Jersey dependencies (#361)
- Fixed non-POSIX fallback for file attribute reading (#371)
- Added support for the Docker networks (#372)
Copy link
Member

Choose a reason for hiding this comment

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

Trivial grammar note - the is unnecessary here

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! 👍 One day I will learn how to use them properly :D


## [1.3.0] - 2017-06-05
### Fixed
Expand Down
18 changes: 18 additions & 0 deletions core/src/main/java/org/testcontainers/containers/Container.java
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,24 @@ public String getStderr() {
*/
SELF withNetworkMode(String networkMode);

/**
* Set the network for this container, similar to the <code>--network &lt;name&gt;</code>
* option on the docker CLI.
*
* @param network the instance of {@link Network}
* @return this
*/
SELF withNetwork(Network network);

/**
* Set the network aliases for this container, similar to the <code>--network-alias &lt;my-service&gt;</code>
* option on the docker CLI.
*
* @param aliases the list of aliases
* @return this
*/
SELF withNetworkAliases(String... aliases);

/**
* Map a resource (file or directory) on the classpath to a path inside the container.
* This will only work if you are running your tests outside a Docker container.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ public class GenericContainer<SELF extends GenericContainer<SELF>>
@NonNull
private String networkMode;

@NonNull
private Network network;

@NonNull
private List<String> networkAliases = new ArrayList<>();

@NonNull
private Future<String> image;

Expand Down Expand Up @@ -412,7 +418,14 @@ private void applyConfiguration(CreateContainerCmd createCommand) {
.toArray(String[]::new);
createCommand.withExtraHosts(extraHostsArray);

if (networkMode != null) {
if (network != null) {
if (!network.isCreated()) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this likely to happen? If the network hasn't been created, could we infer that the user wants it to be created and trigger creation?

throw new ContainerLaunchException("Aborting attempt to use non-existing network " + network.getName());
}

createCommand.withNetworkMode(network.getName());
createCommand.withAliases(this.networkAliases);
} else if (networkMode != null) {
createCommand.withNetworkMode(networkMode);
}

Expand Down Expand Up @@ -615,6 +628,18 @@ public SELF withNetworkMode(String networkMode) {
return self();
}

@Override
public SELF withNetwork(Network network) {
this.network = network;
return self();
}

@Override
public SELF withNetworkAliases(String... aliases) {
Collections.addAll(this.networkAliases, aliases);
return self();
}

/**
* {@inheritDoc}
*/
Expand Down
142 changes: 142 additions & 0 deletions core/src/main/java/org/testcontainers/containers/Network.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
package org.testcontainers.containers;

import com.github.dockerjava.api.command.CreateNetworkCmd;
import lombok.*;
import lombok.experimental.Delegate;
import org.junit.rules.ExternalResource;
import org.testcontainers.DockerClientFactory;
import org.testcontainers.utility.ResourceReaper;

import java.util.LinkedHashSet;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;

public interface Network extends AutoCloseable {

String getName();

Boolean getEnableIpv6();
Copy link
Member

Choose a reason for hiding this comment

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

Do you really want a Boolean?


String getDriver();

boolean isCreated();

@Override
default void close() {
if (isCreated()) {
ResourceReaper.instance().removeNetworks(getName());
}
}

@SneakyThrows
@SuppressWarnings("unchecked")
default <T extends Network> T as(Class<T> type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious to that purpose of this class, which is related to another question I had: why have an interface? Do we plan on users extending and supplying their own Network implementation?

return type.getDeclaredConstructor(Network.class).newInstance(this);
}

static Network newNetwork() {
return builder().build();
}

static NetworkImpl.NetworkImplBuilder builder() {
return NetworkImpl.builder();
}

@Builder
@Getter
class NetworkImpl implements Network {

private final String name = UUID.randomUUID().toString();

private Boolean enableIpv6;
Copy link
Member

Choose a reason for hiding this comment

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

Same thing with the Boolean here.

Copy link
Member Author

Choose a reason for hiding this comment

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

null here means "the value wasn't provided and withEnableIpv6 should not be called (use a default value from CreateNetworkCmd)


private String driver;

@Singular
private Set<Consumer<CreateNetworkCmd>> createNetworkCmdModifiers = new LinkedHashSet<>();

@Override
public boolean isCreated() {
return false;
}
}

@RequiredArgsConstructor
class Runnable implements Network, java.lang.Runnable {

@Delegate(excludes = Excludes.class)
protected final Network network;

private final AtomicBoolean created = new AtomicBoolean(false);

@Override
public void run() {
if (!created.getAndSet(true)) {
ResourceReaper.instance().registerNetworkForCleanup(getName());

CreateNetworkCmd createNetworkCmd = DockerClientFactory.instance().client().createNetworkCmd();

createNetworkCmd.withName(getName());
createNetworkCmd.withCheckDuplicate(true);

if (getEnableIpv6() != null) {
createNetworkCmd.withEnableIpv6(getEnableIpv6());
}

if (getDriver() != null) {
createNetworkCmd.withDriver(getDriver());
}

if (network instanceof NetworkImpl) {
for (Consumer<CreateNetworkCmd> consumer : ((NetworkImpl) network).getCreateNetworkCmdModifiers()) {
consumer.accept(createNetworkCmd);
}
}

createNetworkCmd.exec();
}
}

@Override
public boolean isCreated() {
return created.get();
}
}

class AutoCreated implements Network {

@Delegate(types = Network.class)
protected final Network.Runnable network;

public AutoCreated(Network network) {
this.network = network.as(Runnable.class);
this.network.run();
}
}

class JUnitRule extends ExternalResource implements Network {

@Delegate(types = Network.class)
protected final Network.Runnable network;

public JUnitRule(Network network) {
this.network = network.as(Runnable.class);
}

@Override
protected void before() throws Throwable {
network.run();
}

@Override
protected void after() {
network.close();
}
}

interface Excludes {
boolean isCreated();
}
}
43 changes: 26 additions & 17 deletions core/src/main/java/org/testcontainers/utility/ResourceReaper.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,12 @@
import com.github.dockerjava.api.exception.DockerException;
import com.github.dockerjava.api.exception.InternalServerErrorException;
import com.github.dockerjava.api.exception.NotFoundException;
import com.github.dockerjava.api.model.Container;
import com.github.dockerjava.api.model.Network;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testcontainers.DockerClientFactory;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;

/**
Expand All @@ -25,7 +22,7 @@ public final class ResourceReaper {
private static ResourceReaper instance;
private final DockerClient dockerClient;
private Map<String, String> registeredContainers = new ConcurrentHashMap<>();
private List<String> registeredNetworks = new ArrayList<>();
private Set<String> registeredNetworks = Collections.newSetFromMap(new ConcurrentHashMap<>());

private ResourceReaper() {
dockerClient = DockerClientFactory.instance().client();
Expand Down Expand Up @@ -145,22 +142,34 @@ public void removeNetworks(String identifier) {
}

private void removeNetwork(String networkName) {
List<Network> networks;
try {
networks = dockerClient.listNetworksCmd().withNameFilter(networkName).exec();
} catch (DockerException e) {
LOGGER.trace("Error encountered when looking up network for removal (name: {}) - it may not have been removed", networkName);
return;
}
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Two tests running with same network name will fail , at least the second create will. They will not understand why since the failure is hidden under trace level. I would keep this at error level or warn.

Copy link
Member Author

Choose a reason for hiding this comment

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

network's name is random (see NetworkImpl)

Copy link
Contributor

Choose a reason for hiding this comment

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

So no way of allowing me to specify a name? I'm Starting to build Testclusters, where I'm reusing containers across runs to shorten test duration. My assumption is same network name across runs. Would love to use the class created here for this.

Copy link
Member

Choose a reason for hiding this comment

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

If your use case would reuse the containers, it would reuse the network as well, I assume?

Copy link
Member Author

Choose a reason for hiding this comment

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

@asafm we don't allow users to set names of containers (and now networks) because it might lead to the names conflict (especially with parallel execution).

As @kiview said: if you reuse things, reuse the networks also

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm reusing across builds, on same machine. Quite similar to docker compose. I can code it with the client my self, but I think it's harmless to leave the opening to set the name to who ever needs it. I'm ok with any direction you take.

Copy link
Member

Choose a reason for hiding this comment

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

Let's think about whether we could allow reuse between runs separately. It's probably quite doable, but for the sake of this PR let's resume later.

}

for (Network network : networks) {
List<Network> networks;
try {
dockerClient.removeNetworkCmd(network.getId()).exec();
registeredNetworks.remove(network.getId());
LOGGER.debug("Removed network: {}", networkName);
} catch (DockerException e) {
LOGGER.trace("Error encountered removing network (name: {}) - it may not have been removed", network.getName());
// Then try to list all networks with the same name
networks = dockerClient.listNetworksCmd().withNameFilter(networkName).exec();
} catch (Exception e) {
LOGGER.trace("Error encountered when looking up network for removal (name: {}) - it may not have been removed", networkName);
return;
}

for (Network network : networks) {
try {
dockerClient.removeNetworkCmd(network.getId()).exec();
registeredNetworks.remove(network.getId());
LOGGER.debug("Removed network: {}", networkName);
} catch (Exception e) {
LOGGER.trace("Error encountered removing network (name: {}) - it may not have been removed", network.getName());
}
}
} finally {
registeredNetworks.remove(networkName);
}
}
}
Loading