Fix bind mounts for ResourceReaper/ryuk and ContainerisedDockerCompose on macOS

[gesellix]

Ryuk fails with "can't connect to Ryuk" on Docker Desktop for macOS, while everything's fine on Linux. I didn't check Windows, yet.

Removing the leading slash / on non-Windows like proposed with this pull request helps. Due to refactorings the leading slash has been there for a while and I don't understand, why there aren't more open issues regarding Ryuk. So, this fix is more a best guess :)

I assume that something has changed in recent releases of Docker Desktop for macOS. The diff of Ryuk's container mounts shows the effect with and without leading slash:

        "Mounts": [
            {
                "Type": "bind",
<                "Source": "/host_mnt/Users/gesellix/Library/Containers/com.docker.docker/Data/docker.sock",
>                "Source": "/run/host-services/docker.proxy.sock",
                "Destination": "/var/run/docker.sock",
                "Mode": "rw",
                "RW": true,
                "Propagation": "rprivate"
            }
        ],

Mount source /run/host-services/docker.proxy.sock is the same when manually creating a container via docker run -v /var/run/docker.sock:....

My setup:
Docker Desktop for Mac 2.3.5.0 (47376) (edge Channel)
Docker Engine 19.03.13-beta2

Relates to #545
Relates to #2998

[rnorth]

@gesellix you're right - I've just switched to the Edge version on MacOS and also see a failure:

    13:00:02.863 WARN  org.testcontainers.utility.ResourceReaper - Can not connect to Ryuk at localhost:32768
    java.net.SocketException: Broken pipe (Write failed)
        at java.base/java.net.SocketOutputStream.socketWrite0(Native Method)
        at java.base/java.net.SocketOutputStream.socketWrite(SocketOutputStream.java:110)
        at java.base/java.net.SocketOutputStream.write(SocketOutputStream.java:129)
        at org.testcontainers.utility.ResourceReaper$FilterRegistry.register(ResourceReaper.java:426)
        at org.testcontainers.utility.ResourceReaper.lambda$start$1(ResourceReaper.java:147)
        at org.rnorth.ducttape.ratelimits.RateLimiter.doWhenReady(RateLimiter.java:27)
        at org.testcontainers.utility.ResourceReaper.lambda$start$2(ResourceReaper.java:131)
        at java.base/java.lang.Thread.run(Thread.java:834)

We very nearly removed the extra / prefix in a previous PR, but in the end left it in place to avoid needing a Windows-specific variant (the extra slash used to do no harm on MacOS). I think you're right to amend as you have in this PR.

Until we manage to release a fix the workaround will be to use the Stable, not Edge, version of Docker.

I'll test locally and get back to you.

[rnorth]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[bsideup]

@gesellix good catch!

I wonder if Docker consider it a regression (given that edge isn't the release version).

[gesellix]

I assume that the change is related to their efforts of improving file system (share) performance.

[bsideup]

Could be, yeah. Although I still don't get why a valid Linux path no longer works

[gesellix]

From https://unix.stackexchange.com/a/1919/115690:

Multiple slashes are allowed and are equivalent to a single slash. From the Single Unix specification (version 4), base definitions §3.271 pathname: “Multiple successive slashes are considered to be the same as one slash.”

There is one exception: If a pathname begins with two successive characters, the first component following the leading characters may be interpreted in an implementation-defined manner. (ref: base definitions §4.13 pathname resolution). Linux itself doesn't do this, though some applications might, and other unix-ish system do (e.g. Cygwin).

So, the double slash is allowed/tolerated, but not necessarily valid on every *nix platform. It seems to be required as kind of a workaround, to make the socket bind work on Windows.

Why can't we have nice things 😬

I don't want to say that the Docker Edge release should be considered like a preview/release candidate. My impression is more like "let's apply the workaround with the leading slash only to the platform where the workaround is required".

[bsideup]

@gesellix oh, TIL :D Thanks!

BTW I believe this path has to be updated as well:

testcontainers-java/modules/localstack/src/main/java/org/testcontainers/containers/localstack/LocalStackContainer.java

Line 87 in 39c4208

withFileSystemBind("//var/run/docker.sock", "/var/run/docker.sock");

(plus also use DockerClientFactory.instance().getRemoteDockerUnixSocketPath() instead of a hardcoded path)

[gesellix]

this path has to be updated as well

will do 👍

[gesellix]

One check fails, but seems unrelated. Maybe it's enough to retry or do you see the root cause?

[bsideup]

@gesellix retried 👍

[rnorth]

Coming back to this just now - thanks for this and the additional updates @gesellix.
I'll merge.

[gesellix]

Welcome :)
Thanks for merging!

[aheritier]

thanks a lot @bsideup for pointing me to this issue. I wasn't able to find it.

[mjustin]

This change to the Docker Desktop default was just released today in 2.4.0.0, so this issue is no longer just specific to the Edge version: https://docs.docker.com/docker-for-mac/release-notes/