Use unix socket proxy on OS X instead of netty KQueue route by rnorth · Pull Request #410 · testcontainers/testcontainers-java

rnorth · 2017-07-18T09:23:57Z

For compatibility with OS X 10.11

Fixes #402

looking up configured docker client provider. Add an informative log message instead.

3cce55a

always used on OS X where possible.

bsideup · 2017-07-18T09:26:09Z

CHANGELOG.md

+- Worked around incompatibility between Netty's Unix socket support and OS X 10.11. Reinstated use of TCP-Unix Socket proxy. (Fixes #402)

 ### Changed
+- Updated docker-java to 3.0.12 (#393)


wrong record, was changed in 1.4.0

Bleh, good point. Something went wrong during changelog merge - same as below.

bsideup · 2017-07-18T09:26:34Z

CHANGELOG.md

 - Fixed erroneous version reference used during CI testing of shaded dependencies
 - Fixed leakage of Vibur and Tomcat JDBC test dependencies in `jdbc-test` and `mysql` modules (#382)
 - Added timeout and retries for creation of `RemoteWebDriver` (#381, #373, #257)
+- Fixed double encoding of listNetwork's filter until it's fixed in docker-java (#385)


why? 1.4.1 was released already, we can't change the changelog for it :)

bsideup · 2017-07-18T09:27:09Z

core/pom.xml

+                <!-- replace with junixsocket -->
+                <exclusion>
+                    <groupId>de.gesellix</groupId>
+                    <artifactId>unix-socket-factory</artifactId>


I'm not sure this dependency is there in 3.0.12, probably unnecessary

Good point, will check.

bsideup · 2017-07-18T09:27:40Z

core/src/main/java/org/testcontainers/containers/wait/HostPortWaitStrategy.java


    private boolean shouldCheckWithCommand() {
        // Special case for Docker for Mac, see #160
-        if(!DockerClientFactory.instance().isUsing(DockerMachineClientProviderStrategy.class)


I would keep it, this should apply to both Proxied & direct access

bsideup · 2017-07-18T09:27:46Z

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

-                                    } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
-                                        LOGGER.warn("Can't instantiate a strategy from " + it, e);
+                                    } catch (ClassNotFoundException e) {
+                                        LOGGER.warn("Can't instantiate a strategy from {} (ClassNotFoundException). " +


bsideup · 2017-07-18T09:28:50Z

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

 import java.nio.file.Paths;

 @Slf4j
 public class UnixSocketClientProviderStrategy extends DockerClientProviderStrategy {


I wouldn't change this strategy. It works great for OS X >= 10.12 users, we should use it if OSX's version is >= 10.12, and Proxied if < 10.12

I don't know; the proxied approach seems to be pretty solid and AFAICT the main benefit of the netty KQueue enhancement is design elegance - it lets us remove a hack (albeit a reliable one).

If we can't remove the hack anyway, then do we gain much from having a new potential way of working on OS X?

I need to think about this when I don't have such a headache 😄

I had a feeling that KQueue is faster on my machine, but I might be wrong :)

I ran some simple JMH benchmarks, comparing performance for both creating a container and just retrieving a container listing. TLDR; the docker daemon is the bottleneck for heavy operations, and there's a small performance boost with KQueue, but probably not significant especially given how infrequently we call it:

Benchmark Mode Cnt Score Error Units ClientPerfBenchmark.directClientCreatingContainers thrpt 20 9.839 ± 0.340 ops/s ClientPerfBenchmark.directClientJustListing thrpt 20 435.923 ± 117.758 ops/s ClientPerfBenchmark.proxiedClientCreatingContainers thrpt 20 10.085 ± 0.663 ops/s ClientPerfBenchmark.proxiedClientJustListing thrpt 20 202.696 ± 43.040 ops/s

However, I'm coming around to using the KQueue route on macOS 10.12+, as it will help us establish confidence in this netty component for the future.

Wow! Great approach 👍

bsideup · 2017-07-19T09:42:15Z

CHANGELOG.md

 - Added `getFirstMappedPort` method (#377)
 - Extracted Oracle XE container into a separate repository ([testcontainers/testcontainers-java-module-oracle-xe](https://github.com/testcontainers/testcontainers-java-module-oracle-xe))
 - Added shading tests
- Updated docker-java to 3.0.12 (#393)


bsideup · 2017-07-19T09:43:13Z

core/src/main/java/org/testcontainers/containers/wait/HostPortWaitStrategy.java

        // Special case for Docker for Mac, see #160
-        if(!DockerClientFactory.instance().isUsing(DockerMachineClientProviderStrategy.class)
-                && System.getProperty("os.name").toLowerCase().contains("mac")) {
+        if ((DockerClientFactory.instance().isUsing(ProxiedUnixSocketClientProviderStrategy.class) ||


I think previous code (not docker machine and Mac) should cover both cases. Also, this change has a bug because || has lower priority than &&

I'm afraid I can't see the bug - brackets? Obviously this means the code should be clearer though 😆

bsideup · 2017-07-19T09:45:10Z

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

-        return PRIORITY;
+    protected boolean isApplicable() {
+        return SystemUtils.IS_OS_LINUX ||
+                (SystemUtils.IS_OS_MAC_OSX && !SystemUtils.OS_VERSION.startsWith("10.11"));


we have ComparableVersion, maybe we should use it instead of startsWith?

Of course - I forgot about my own class.

bsideup · 2017-07-19T09:46:09Z

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

+
+    @Override
+    protected boolean isApplicable() {
+        return !SystemUtils.IS_OS_LINUX && socketFile.exists();


We probably shouldn't apply it if OSX and >= 10.12

bsideup

The code looks good, we need to ask someone with OS X 10.11 to test it. Maybe @iNikem

iNikem · 2017-07-24T09:01:58Z

It seems to be working now, thank you! :)

bsideup · 2017-07-24T09:56:02Z

core/src/main/java/org/testcontainers/containers/wait/HostPortWaitStrategy.java

        // Special case for Docker for Mac, see #160
-        if(!DockerClientFactory.instance().isUsing(DockerMachineClientProviderStrategy.class)
-                && System.getProperty("os.name").toLowerCase().contains("mac")) {
+        if (DockerClientFactory.instance().isUsing(DockerMachineClientProviderStrategy.class) &&


ooops, I think we have a bug here, should be "if NOT" (see the diff)

email2liyang · 2017-07-24T12:41:23Z

seems not work for me with the SNAPSHOT.

OS: macOS 10.11.6
Java: java version "1.8.0_111"

testCompile 'com.github.testcontainers.testcontainers-java:testcontainers:use-unix-socket-proxy-SNAPSHOT'

20:37:46.682 [main] DEBUG org.testcontainers.shaded.io.netty.util.internal.PlatformDependent0 - jdk.internal.misc.Unsafe.allocateUninitializedArray(int): unavailable
java.lang.ClassNotFoundException: jdk.internal.misc.Unsafe
	at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.testcontainers.shaded.io.netty.util.internal.PlatformDependent0$6.run(PlatformDependent0.java:295)
	at java.security.AccessController.doPrivileged(Native Method)
	at org.testcontainers.shaded.io.netty.util.internal.PlatformDependent0.<clinit>(PlatformDependent0.java:288)
	at org.testcontainers.shaded.io.netty.util.internal.PlatformDependent.getSystemClassLoader(PlatformDependent.java:895)
	at org.testcontainers.shaded.io.netty.util.internal.PlatformDependent.isAndroid0(PlatformDependent.java:919)
	at org.testcontainers.shaded.io.netty.util.internal.PlatformDependent.<clinit>(PlatformDependent.java:70)
	at org.testcontainers.shaded.io.netty.util.ConstantPool.<init>(ConstantPool.java:32)
	at org.testcontainers.shaded.io.netty.util.Signal$1.<init>(Signal.java:27)
	at org.testcontainers.shaded.io.netty.util.Signal.<clinit>(Signal.java:27)
	at org.testcontainers.shaded.io.netty.util.concurrent.DefaultPromise.<clinit>(DefaultPromise.java:43)
	at org.testcontainers.shaded.io.netty.util.concurrent.MultithreadEventExecutorGroup.<init>(MultithreadEventExecutorGroup.java:36)
	at org.testcontainers.shaded.io.netty.util.concurrent.MultithreadEventExecutorGroup.<init>(MultithreadEventExecutorGroup.java:58)
	at org.testcontainers.shaded.io.netty.util.concurrent.MultithreadEventExecutorGroup.<init>(MultithreadEventExecutorGroup.java:47)
	at org.testcontainers.shaded.io.netty.channel.MultithreadEventLoopGroup.<init>(MultithreadEventLoopGroup.java:59)
	at org.testcontainers.shaded.io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:77)
	at org.testcontainers.shaded.io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:72)
	at org.testcontainers.shaded.io.netty.channel.nio.NioEventLoopGroup.<init>(NioEventLoopGroup.java:59)
	at com.github.dockerjava.netty.NettyDockerCmdExecFactory$InetSocketInitializer.init(NettyDockerCmdExecFactory.java:283)
	at com.github.dockerjava.netty.NettyDockerCmdExecFactory.init(NettyDockerCmdExecFactory.java:206)
	at com.github.dockerjava.core.DockerClientImpl.withDockerCmdExecFactory(DockerClientImpl.java:161)
	at com.github.dockerjava.core.DockerClientBuilder.build(DockerClientBuilder.java:45)
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.getClientForConfig(DockerClientProviderStrategy.java:152)
	at org.testcontainers.dockerclient.UnixSocketClientProviderStrategy.tryConfiguration(UnixSocketClientProviderStrategy.java:62)
	at org.testcontainers.dockerclient.ProxiedUnixSocketClientProviderStrategy.test(ProxiedUnixSocketClientProviderStrategy.java:37)
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.lambda$getFirstValidStrategy$1(DockerClientProviderStrategy.java:96)
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:267)
	at java.util.stream.StreamSpliterators$WrappingSpliterator.tryAdvance(StreamSpliterators.java:302)
	at java.util.stream.Streams$ConcatSpliterator.tryAdvance(Streams.java:728)
	at java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:126)
	at java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:498)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:485)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:152)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.findAny(ReferencePipeline.java:469)
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.getFirstValidStrategy(DockerClientProviderStrategy.java:129)
	at org.testcontainers.DockerClientFactory.client(DockerClientFactory.java:94)
	at org.testcontainers.containers.GenericContainer.<init>(GenericContainer.java:116)

bsideup · 2017-07-24T12:42:34Z

@email2liyang seems like an issue with Netty: netty/netty#6855

bsideup · 2017-07-24T12:43:31Z

Also, what exactly didn't work? Because it seems to be just a noisy logging

email2liyang · 2017-07-24T13:16:23Z

@bsideup , I'm using an elasticsearch image for es related test, seems the snapshot version can't start es container successfully, we could reproduce this issue with steps below

git clone git@github.com:email2liyang/grpc-mate.git
git checkout testcontainer-use-unix-socket-proxy
cd grpc-mate
./gradlew test -Dtest.single=ProductDaoTest

the same command works in master branch where I depends on

testCompile 'org.testcontainers:testcontainers:1.3.1'

bsideup · 2017-07-24T14:12:06Z

@email2liyang yeah, that will be fixed soon (see my last review comment) :)

email2liyang · 2017-07-26T12:35:58Z

@bsideup ,yes confirmed that v1.4.2 works in macOS 10.11, but still get the warning exception like below which related to netty/netty#6855 any idea on when it will get fixed?

20:30:59.253 [main] DEBUG org.testcontainers.shaded.io.netty.util.internal.PlatformDependent0 - jdk.internal.misc.Unsafe.allocateUninitializedArray(int): unavailable
java.lang.ClassNotFoundException: jdk.internal.misc.Unsafe
	at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.testcontainers.shaded.io.netty.util.internal.PlatformDependent0$6.run(PlatformDependent0.java:295)
	at java.security.AccessController.doPrivileged(Native Method)
	at org.testcontainers.shaded.io.netty.util.internal.PlatformDependent0.<clinit>(PlatformDependent0.java:288)

Bumps [amqp-client](https://github.com/rabbitmq/rabbitmq-java-client) from 3.5.3 to 5.5.1. <details> <summary>Release notes</summary> *Sourced from [amqp-client's releases](https://github.com/rabbitmq/rabbitmq-java-client/releases).* > ## 5.5.1 > This is a patch release with a bug fix. All users of the 5.x.x series are encouraged to upgrade to this version. > > # Changes between 5.5.0 and 5.5.1 > > ## Record recoverable entities in RPC channel methods > > GitHub issue: [#425](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/425) > > ## 5.5.0 > This is a maintenance release with 2 new features and a bug fix. All users of the 5.x.x series are encouraged to upgrade to this version. > > # Changes between 5.4.3 and 5.5.0 > > ## Add traffic listener > > GitHub issue: [#411](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/411) > > ## Connection recovery runs into timeouts when using NIO > > GitHub issue: [#413](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/413) > > ## Provide factories to create NIO artefacts > > GitHub issue: [#410](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/410) > > ## 4.9.0 > This is a maintenance release with 2 new features and a bug fix. All users of the 4.x.x and 3.6.x series are encouraged to upgrade to this version. > > # Changes between 4.8.3 and 4.9.0 > > ## Add traffic listener > > GitHub issue: [#411](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/411) > > ## Connection recovery runs into timeouts when using NIO > > GitHub issue: [#413](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/413) > > ## Provide factories to create NIO artefacts > > GitHub issue: [#410](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/410) > > ## 5.5.0.RC1 > This is a pre-release for 5.5.0, a maintenance release with 2 new features and a bug fix. All users of the 5.x.x series are encouraged to test this version. > > # Changes between 5.4.0 and 5.5.0.RC1 > > ## Add traffic listener > ></table> ... (truncated) </details> <details> <summary>Changelog</summary> *Sourced from [amqp-client's changelog](https://github.com/rabbitmq/rabbitmq-java-client/blob/v5.5.1/release-versions.txt).* > RELEASE_VERSION="5.5.1" > DEVELOPMENT_VERSION="5.5.2-SNAPSHOT" </details> <details> <summary>Commits</summary> - See full diff in [compare view](https://github.com/rabbitmq/rabbitmq-java-client/commits/v5.5.1) </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=com.rabbitmq:amqp-client&package-manager=gradle&previous-version=3.5.3&new-version=5.5.1)](https://dependabot.com/compatibility-score.html?dependency-name=com.rabbitmq:amqp-client&package-manager=gradle&previous-version=3.5.3&new-version=5.5.1) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- **Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit. You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com). <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in the `.dependabot/config.yml` file in this repo: - Update frequency (including time of day and day of week) - Automerge options (never/patch/minor, and dev/runtime dependencies) - Pull request limits (per update run and/or open at any time) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) Finally, you can contact us by mentioning @dependabot. </details>

rnorth added 7 commits July 18, 2017 10:01

Prevent showing full stack trace in case of ClassNotFoundException when

98ec10e

looking up configured docker client provider. Add an informative log message instead.

Revert "Update docker-java to 3.0.12 (#393)"

7ecda0e

3cce55a

Upgrade to v3.0.12 of docker-java

3ab1802

Remove workaround CmdExecFactory now that docker-java #857 is fixed

8c130be

Adjust docker client provider strategies so that a proxied connection is

039b6cd

always used on OS X where possible.

Update shading test for changes to docker-java native libs

fbd2938

Update CHANGELOG

b770a3a

rnorth requested a review from bsideup July 18, 2017 09:23

bsideup requested changes Jul 18, 2017

View reviewed changes

rnorth added 2 commits July 19, 2017 10:40

Fixes following code review

27b34db

Fix missing imports broken during merge

21ceabb

bsideup requested changes Jul 19, 2017

View reviewed changes

bsideup added this to the 1.5.0 milestone Jul 19, 2017

Tidy up dependency checks for client provider strategies

b372813

bsideup approved these changes Jul 19, 2017

View reviewed changes

bsideup reviewed Jul 24, 2017

View reviewed changes

rnorth added 2 commits July 24, 2017 21:12

Reinstate correct HostPortWaitStrategy OS detection

56c8d81

Update changelog

88e92d2

rnorth merged commit 51d1d45 into master Jul 24, 2017

rnorth deleted the use-unix-socket-proxy branch October 8, 2018 16:39

Uh oh!

Comments

Conversation

rnorth commented Jul 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bsideup left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iNikem commented Jul 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

email2liyang commented Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bsideup commented Jul 24, 2017

Uh oh!

bsideup commented Jul 24, 2017

Uh oh!

email2liyang commented Jul 24, 2017

Uh oh!

bsideup commented Jul 24, 2017

Uh oh!

email2liyang commented Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bsideup left a comment •

edited

Loading

email2liyang commented Jul 24, 2017 •

edited

Loading

email2liyang commented Jul 26, 2017 •

edited

Loading