Make it possible to run TestContainers inside a container #267

bsideup · 2017-01-15T14:28:51Z

Fixes #264

Known issues:

file mounting will obviously fail, at least if PWD is not the same

bsideup · 2017-01-15T14:42:12Z

.mvn/wrapper/maven-wrapper.properties

@@ -0,0 +1 @@
+distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.3.9/apache-maven-3.3.9-bin.zip


I've added Maven Wrapper ( https://github.com/takari/maven-wrapper ) to make it possible to run TC build inside any container with Java 8. It's also nice to have anyway :)

Yep - good idea!

bsideup · 2017-01-15T14:42:28Z

.travis.yml

@@ -19,8 +19,8 @@ before_install:
  - docker pull mysql:5.6
  - docker pull mysql:5.5
  - docker pull postgres:latest
-  - docker pull selenium/standalone-chrome-debug:2.45.0
-  - docker pull selenium/standalone-firefox-debug:2.45.0
+  - docker pull selenium/standalone-chrome-debug:2.52.0


classpath contains 2.52, but we were pulling 2.45 and then 2.52

bsideup · 2017-01-15T14:43:32Z

circle.yml

+    # Run Docker-in-Docker tests
+    # Yes, we use selenium image, but we use it anyway in BrowserContainer's tests
+    - >
+      docker -H unix:///var/run/docker.sock run --rm \


had to duplicate. I can extract it to .sh file to share, but we don't use CircleCI anymore, right? Probably the config should be removed then

bsideup · 2017-01-15T14:44:22Z

core/src/main/java/org/testcontainers/DockerClientFactory.java

@@ -80,20 +79,29 @@ public DockerClient client() {
        }

        strategy = DockerClientProviderStrategy.getFirstValidStrategy(CONFIGURATION_STRATEGIES);
+
+        log.info("Docker host IP address is {}", strategy.getDockerHostIpAddress());


moved here to avoid the cycling dependency

bsideup · 2017-01-15T14:45:43Z

core/src/main/java/org/testcontainers/DockerClientFactory.java

-        if (!images.stream().anyMatch(it -> it.getRepoTags() != null && asList(it.getRepoTags()).contains("alpine:3.2"))) {
-            PullImageResultCallback callback = client.pullImageCmd("alpine:3.2").exec(new PullImageResultCallback());
-            callback.awaitSuccess();
+    public <T> T runInsideDocker(Consumer<CreateContainerCmd> createContainerCmdConsumer, BiFunction<DockerClient, String, T> block) {


wish I could make it module-level, but I can't because the consumer is in dockerclient subpackage

bsideup · 2017-01-15T14:46:25Z

core/src/main/java/org/testcontainers/dockerclient/DockerClientConfigUtils.java

 public class DockerClientConfigUtils {
+
+    // See https://github.com/docker/docker/blob/a9fa38b1edf30b23cae3eade0be48b3d4b1de14b/daemon/initlayer/setup_unix.go#L25
+    public static final boolean IN_A_CONTAINER = new File("/.dockerenv").exists();


the best way to check I found, libnetwork uses it as well, so it's here to stay

bsideup · 2017-01-15T14:47:14Z

...java/org/testcontainers/dockerclient/EnvironmentAndSystemPropertyClientProviderStrategy.java

@@ -27,7 +27,6 @@ public void test() throws InvalidConfigurationException {
        }

        LOGGER.info("Found docker client settings from environment");
-        LOGGER.info("Docker host IP address is {}", DockerClientConfigUtils.getDockerHostIpAddress(config));


moved to DockerClientFactory

bsideup · 2017-01-15T14:48:28Z

core/src/test/java/org/testcontainers/dockerclient/DockerInDockerTest.java

+    public static TestRule assumption = new TestWatcher() {
+        @Override
+        public Statement apply(Statement base, Description description) {
+            assumeTrue("We're inside a container", DockerClientConfigUtils.IN_A_CONTAINER);


we could add some IP-specific tests here, so this assumption will protect them from running in non-container mode

bsideup · 2017-01-15T15:34:38Z

.travis.yml

+      -v "$(pwd)":"$(pwd)" \
+      -w "$(pwd)" \
+      selenium/standalone-chrome-debug:2.52.0 \
+      ./mvnw -B -pl core test -Dtest=*GenericContainerRuleTest


GenericContainerRuleTest covers most of the cases. Right now we can't use all the tests because some of them make incorrect assumptions

Is the selenium container image just used here because it includes Java? It's potentially a bit misleading..

If so, I wouldn't worry about pulling an openjdk image and using that.

Ok, I'll change it :) Just wanted to save a few seconds of pulling

rnorth · 2017-01-16T08:53:53Z

.mvn/wrapper/maven-wrapper.properties

@@ -0,0 +1 @@
+distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.3.9/apache-maven-3.3.9-bin.zip


Yep - good idea!

rnorth · 2017-01-16T08:54:24Z

.travis.yml

@@ -19,8 +19,8 @@ before_install:
  - docker pull mysql:5.6
  - docker pull mysql:5.5
  - docker pull postgres:latest
-  - docker pull selenium/standalone-chrome-debug:2.45.0
-  - docker pull selenium/standalone-firefox-debug:2.45.0
+  - docker pull selenium/standalone-chrome-debug:2.52.0


rnorth · 2017-01-16T08:59:10Z

.travis.yml

+      -v "$(pwd)":"$(pwd)" \
+      -w "$(pwd)" \
+      selenium/standalone-chrome-debug:2.52.0 \
+      ./mvnw -B -pl core test -Dtest=*GenericContainerRuleTest


Is the selenium container image just used here because it includes Java? It's potentially a bit misleading..

If so, I wouldn't worry about pulling an openjdk image and using that.

rnorth · 2017-01-16T09:09:42Z

core/src/main/java/org/testcontainers/dockerclient/LogToStringContainerCallback.java

+import com.github.dockerjava.api.model.Frame;
+import com.github.dockerjava.core.command.LogContainerResultCallback;
+
+public class LogToStringContainerCallback extends LogContainerResultCallback {


Am I right in thinking that this exists because we can't use our container logging abstraction (with ToStringConsumer) this early in the startup process?

If that's the case, should we reduce visibility of this class from public so as to reduce confusion with our higher-level logging API?

Wish we could :) This class is shared between different packages :(

No problem then - the type system will at least help people not use the wrong thing.

bsideup · 2017-01-16T17:38:28Z

@rnorth applied the fixes. Could you please take a look again?

rnorth

Looks good to me! Thanks for doing this. We probably need to update the docs to mention this - do you have any thoughts on this?

bsideup · 2017-01-21T08:00:33Z

@rnorth yes, I'll update the docs in this branch, thanks for reminding me :)

bsideup · 2017-01-21T08:33:34Z

@rnorth pushed the docs. Would highly appreciate picky review of it :)

rnorth · 2017-01-21T17:04:30Z

@bsideup the docs look fine - I might just tweak some of the wording. Would you mind if I take the branch now? I can rebase+squash, tweak docs, and merge if that's OK with you?

bsideup · 2017-01-21T17:05:22Z

Absolutely! Thanks:)

…

On Sat, 21 Jan 2017 at 19:04, Richard North ***@***.***> wrote: @bsideup <https://github.com/bsideup> the docs look fine - I might just tweak some of the wording. Would you mind if I take the branch now? I can rebase+squash, tweak docs, and merge if that's OK with you? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#267 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABAIiuPA5SUKGcyVr3XdIS-LnaMh2kJjks5rUjqegaJpZM4Lj7iy> .

…cker tests

bsideup commented Jan 15, 2017

View reviewed changes

bsideup requested a review from rnorth January 15, 2017 16:14

bsideup self-assigned this Jan 15, 2017

bsideup added the enhancement label Jan 15, 2017

bsideup added this to the 1.1.8 milestone Jan 15, 2017

bsideup mentioned this pull request Jan 15, 2017

Issues when tests are run inside a container #134

Closed

rnorth reviewed Jan 16, 2017

View reviewed changes

rnorth approved these changes Jan 21, 2017

View reviewed changes

rnorth force-pushed the docker-in-docker branch from f4af27f to 7f36420 Compare January 21, 2017 19:35

bsideup and others added 5 commits January 21, 2017 20:04

Make it possible to run TestContainers inside a container. Fixes #264

398a616

Fix DiD tests pattern using GenericContainerRuleTest for docker-in-do…

55d93ab

…cker tests

use OpenJDK maven image to run docker-in-docker tests

7471ffe

Add the docs

ab9a570

Tweak docs re in-docker usage

e6d8be0

rnorth force-pushed the docker-in-docker branch from 7f36420 to e6d8be0 Compare January 21, 2017 20:04

rnorth merged commit deab822 into master Jan 22, 2017

rnorth deleted the docker-in-docker branch January 22, 2017 07:52

dddpaul mentioned this pull request Jan 24, 2017

Strong coupling with "alpine:3.2" image in DockerClientFactory #281

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make it possible to run TestContainers inside a container #267

Make it possible to run TestContainers inside a container #267

bsideup commented Jan 15, 2017 •

edited

Loading

bsideup Jan 15, 2017

rnorth Jan 16, 2017

bsideup Jan 15, 2017

rnorth Jan 16, 2017

bsideup Jan 15, 2017

bsideup Jan 15, 2017

bsideup Jan 15, 2017

bsideup Jan 15, 2017

bsideup Jan 15, 2017

bsideup Jan 15, 2017

bsideup Jan 15, 2017

rnorth Jan 16, 2017

bsideup Jan 16, 2017

rnorth Jan 16, 2017

rnorth Jan 16, 2017

rnorth Jan 16, 2017

rnorth Jan 16, 2017

bsideup Jan 16, 2017

rnorth Jan 21, 2017

bsideup commented Jan 16, 2017

rnorth left a comment

bsideup commented Jan 21, 2017

bsideup commented Jan 21, 2017

rnorth commented Jan 21, 2017

bsideup commented Jan 21, 2017 via email

		@@ -0,0 +1 @@
		distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.3.9/apache-maven-3.3.9-bin.zip

Make it possible to run TestContainers inside a container #267

Make it possible to run TestContainers inside a container #267

Conversation

bsideup commented Jan 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsideup commented Jan 16, 2017

rnorth left a comment

Choose a reason for hiding this comment

bsideup commented Jan 21, 2017

bsideup commented Jan 21, 2017

rnorth commented Jan 21, 2017

bsideup commented Jan 21, 2017 via email

bsideup commented Jan 15, 2017 •

edited

Loading