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.

Sign up for GitHub

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

Jump to bottom

Implement pre-flight checks #363

Merged
merged 7 commits into from
Jun 18, 2017
Merged

Implement pre-flight checks #363

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
909fe0c
Implement pre-flight checks (space, exec, volume mappings, network)
bsideup Jun 10, 2017
367064a
Merge branch 'master' into pre-flight_checks
bsideup Jun 12, 2017
be3264a
CHANGELOG.md entry
bsideup Jun 12, 2017
abedfd0
Split checks into separate methods
bsideup Jun 17, 2017
72af70b
Merge branch 'master' into pre-flight_checks
bsideup Jun 17, 2017
6c2a138
use tinyImage in GenericContainer and change "Boolean disableChecks" …
bsideup Jun 17, 2017
1c11a6a
getDisableChecks -> isDisableChecks
bsideup Jun 17, 2017
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file.
## UNRELEASED
### Changed
- Added `TC_DAEMON` JDBC URL flag to prevent `ContainerDatabaseDriver` from shutting down containers at the time all connections are closed. (#359, #360)
- Added pre-flight checks (can be disabled with `checks.disable` configuration property) (#363)

## [1.3.0] - 2017-06-05
### Fixed
Expand Down
5 changes: 5 additions & 0 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@
<artifactId>slf4j-ext</artifactId>
<version>1.7.25</version>
</dependency>
<dependency>
<groupId>org.rnorth.visible-assertions</groupId>
<artifactId>visible-assertions</artifactId>
<version>1.0.5</version>
</dependency>

<!-- Test dependencies -->
<dependency>
Expand Down
134 changes: 83 additions & 51 deletions core/src/main/java/org/testcontainers/DockerClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,28 @@

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.Image;
import com.github.dockerjava.api.model.Info;
import com.github.dockerjava.api.model.Version;
import com.github.dockerjava.api.model.*;
import com.github.dockerjava.core.command.ExecStartResultCallback;
import com.github.dockerjava.core.command.PullImageResultCallback;

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.visibleassertions.VisibleAssertions;
import org.testcontainers.dockerclient.*;
import org.testcontainers.utility.ComparableVersion;
import org.testcontainers.utility.MountableFile;
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.List;
import java.util.Optional;
import java.util.function.BiFunction;
Expand Down Expand Up @@ -82,7 +92,8 @@ public DockerClient client() {

strategy = DockerClientProviderStrategy.getFirstValidStrategy(CONFIGURATION_STRATEGIES);

log.info("Docker host IP address is {}", strategy.getDockerHostIpAddress());
String hostIpAddress = strategy.getDockerHostIpAddress();
log.info("Docker host IP address is {}", hostIpAddress);
DockerClient client = strategy.getClient();

if (!preconditionsChecked) {
Expand All @@ -96,8 +107,71 @@ public DockerClient client() {
" Operating System: " + dockerInfo.getOperatingSystem() + "\n" +
" Total Memory: " + dockerInfo.getMemTotal() / (1024 * 1024) + " MB");

checkVersion(version.getVersion());
checkDiskSpaceAndHandleExceptions(client);
if (!TestcontainersConfiguration.getInstance().getDisableChecks()) {
VisibleAssertions.info("Checking the system...");

VisibleAssertions.assertThat("Docker version", version.getVersion(), new BaseMatcher<String>() {
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 think it would be possible, to have each check in its own method? Would make the method much more readable and it would directly be visible, which checks are performed.

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, why not

Copy link
Member

Choose a reason for hiding this comment

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

Agree re putting each check in a separate method.

@Override
public boolean matches(Object o) {
return new ComparableVersion(o.toString()).compareTo(new ComparableVersion("1.6.0")) >= 0;
}

@Override
public void describeTo(Description description) {
description.appendText("is newer than 1.6.0");
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't this be a bit shorter by just using an assertTrue method? An anonymous inner class to satisfy assertThat feels a bit heavy..?

Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to use Matcher because in case of a failure it will print current version - helps to debug


MountableFile mountableFile = MountableFile.forClasspathResource(this.getClass().getName().replace(".", "/") + ".class");

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) -> {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();

try {
dockerClient
.execStartCmd(dockerClient.execCreateCmd(id).withAttachStdout(true).withCmd("df", "-P").exec().getId())
.exec(new ExecStartResultCallback(outputStream, null))
.awaitCompletion();
} catch (Exception e) {
log.debug("Can't exec disk checking command", e);
}

DiskSpaceUsage df = parseAvailableDiskSpace(outputStream.toString());

VisibleAssertions.assertTrue(
"Docker environment has more than 2GB free",
df.availableMB.map(it -> it >= 2048).orElse(true)
);

try (InputStream stream = dockerClient.copyArchiveFromContainerCmd(id, "/dummy").exec()) {
stream.read();
VisibleAssertions.pass("File should be mountable");
} catch (Exception e) {
VisibleAssertions.fail("File should be mountable but fails with " + e.getMessage());
}

InspectContainerResponse inspectedContainer = dockerClient.inspectContainerCmd(id).exec();

String portSpec = inspectedContainer.getNetworkSettings().getPorts().getBindings().values().iterator().next()[0].getHostPortSpec();

String response;
try (Socket socket = new Socket(hostIpAddress, Integer.parseInt(portSpec))) {
response = IOUtils.toString(socket.getInputStream(), Charset.defaultCharset());
} catch (IOException e) {
response = e.getMessage();
}
VisibleAssertions.assertEquals("Exposed port is accessible", "hello", response);

return null;
});
}
preconditionsChecked = true;
}

Expand All @@ -121,47 +195,6 @@ public String dockerHostIpAddress() {
return strategy.getDockerHostIpAddress();
}

private void checkVersion(String version) {
String[] splitVersion = version.split("\\.");
if (Integer.valueOf(splitVersion[0]) <= 1 && Integer.valueOf(splitVersion[1]) < 6) {
throw new IllegalStateException("Docker version 1.6.0+ is required, but version " + version + " was found");
}
}

private void checkDiskSpaceAndHandleExceptions(DockerClient client) {
try {
checkDiskSpace(client);
} catch (NotEnoughDiskSpaceException e) {
throw e;
} catch (Exception e) {
log.warn("Encountered and ignored error while checking disk space", e);
}
}

/**
* Check whether this docker installation is likely to have disk space problems
* @param client an active Docker client
*/
private void checkDiskSpace(DockerClient client) {
DiskSpaceUsage df = runInsideDocker(client, cmd -> cmd.withCmd("df", "-P"), (dockerClient, id) -> {
String logResults = dockerClient.logContainerCmd(id)
.withStdOut(true)
.exec(new LogToStringContainerCallback())
.toString();

return parseAvailableDiskSpace(logResults);
});

log.info("Disk utilization in Docker environment is {} ({} )",
df.usedPercent.map(x -> x + "%").orElse("unknown"),
df.availableMB.map(x -> x + " MB available").orElse("unknown available"));

if (df.availableMB.map(it -> it < 2048).orElse(false)) {
log.error("Docker environment has less than 2GB free - execution is unlikely to succeed so will be aborted.");
throw new NotEnoughDiskSpaceException("Not enough disk space in Docker environment");
}
}

public <T> T runInsideDocker(Consumer<CreateContainerCmd> createContainerCmdConsumer, BiFunction<DockerClient, String, T> block) {
if (strategy == null) {
client();
Expand All @@ -176,9 +209,8 @@ private <T> T runInsideDocker(DockerClient client, Consumer<CreateContainerCmd>
createContainerCmdConsumer.accept(createContainerCmd);
String id = createContainerCmd.exec().getId();

client.startContainerCmd(id).exec();

try {
client.startContainerCmd(id).exec();
return block.apply(client, id);
} finally {
try {
Expand All @@ -199,7 +231,7 @@ private DiskSpaceUsage parseAvailableDiskSpace(String dfOutput) {
String[] lines = dfOutput.split("\n");
for (String line : lines) {
String[] fields = line.split("\\s+");
if (fields[5].equals("/")) {
if (fields.length > 5 && fields[5].equals("/")) {
int availableKB = Integer.valueOf(fields[3]);
df.availableMB = Optional.of(availableKB / 1024);
df.usedPercent = Optional.of(Integer.valueOf(fields[4].replace("%", "")));
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/org/testcontainers/utility/TestcontainersConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public class TestcontainersConfiguration {

private String ambassadorContainerImage = "richnorth/ambassador:latest";
private String vncRecordedContainerImage = "richnorth/vnc-recorder:latest";
private String tinyImage = "alpine:3.2";
private String tinyImage = "alpine:3.5";
private Boolean disableChecks = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Curios here: Why Boolean and not boolean?

Copy link
Member Author

Choose a reason for hiding this comment

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

leftover from previous implementation actually :) will change to boolean, thanks :)


private static TestcontainersConfiguration loadConfiguration() {
final TestcontainersConfiguration config = new TestcontainersConfiguration();
Expand All @@ -44,6 +45,7 @@ private static TestcontainersConfiguration loadConfiguration() {
config.ambassadorContainerImage = properties.getProperty("ambassador.container.image", config.ambassadorContainerImage);
config.vncRecordedContainerImage = properties.getProperty("vncrecorder.container.image", config.vncRecordedContainerImage);
config.tinyImage = properties.getProperty("tinyimage.container.image", config.tinyImage);
config.disableChecks = Boolean.parseBoolean(properties.getProperty("checks.disable", config.disableChecks + ""));

log.debug("Testcontainers configuration overrides loaded from {}: {}", configOverrides, config);

Expand Down
6 changes: 0 additions & 6 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,6 @@
<version>1.2.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.rnorth.visible-assertions</groupId>
<artifactId>visible-assertions</artifactId>
<version>1.0.5</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down