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

Improve support for alternative container runtimes #9140

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tjamet
Copy link

@tjamet tjamet commented Aug 22, 2024

In several cases, alternative container runtimes offers a docker compatible API and populates the docker context accordingly.

However, in the current implementation of testcontainers, the context is often ignored as the EnvironmentAndSystemPropertyClientProviderStrategy only considers the DOCKER_HOST override either through testcontainers property or the environment variables.

Improve the support of multiple container runtimes by honoring the current docker context.

In addition, improve the detection of whether the Docker engine runs in a virtual machine without root access for the current user, so it removes the need to configure TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE in standard cases.

This change has been tested with:

  • colima
  • rancher desktop (in non-privileged mode)
  • docker desktop
  • orbstack

@tjamet tjamet requested a review from a team as a code owner August 22, 2024 17:35
@eddumelendez
Copy link
Member

Thanks for your contribution, @tjamet. Can you please run ./gradlew spotlessApply? or give me access to update the branch, please.

kiview
kiview previously approved these changes Sep 20, 2024
Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

LGTM, we only need to check, whether this might accidentally break RootlessDockerClientProviderStrategy, since in this case, the socket might also be in the home directory, but should not resolve to /var/run/docker.sock for getRemoteDockerUnixSocketPath().

docker context use colima
```

For more complex scenarios you should configure the following environment variables:
Copy link
Member

Choose a reason for hiding this comment

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

I think these docs can even be removed, to avoid confusion, right?
Same for the Podman and Rancher docs I guess?

Copy link
Author

Choose a reason for hiding this comment

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

Happy to further remove the docs.
I think the podman one needs to stay. In my experience, rootless podman installation do not configure the Docker context

@tjamet
Copy link
Author

tjamet commented Sep 24, 2024

I will check the rootless docker client strategy and find a way not to break it

In several cases, alternative container runtimes offers a docker
compatible API and populates the docker context accordingly.

However, in the current implementation of testcontainers, the context is
often ignored as the `EnvironmentAndSystemPropertyClientProviderStrategy` only considers
the `DOCKER_HOST` override either through testcontainers property or the
environment variables.

Improve the support of multiple container runtimes by honoring the
current docker context.

In addition, improve the detection of whether the Docker engine runs
in a virtual machine without root access for the current user, so it
removes the need to configure `TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE` in
standard cases.

This change has been tested with:
- colima
- rancher desktop (in non-privileged mode)
- docker desktop
- orbstack
@tjamet
Copy link
Author

tjamet commented Sep 25, 2024

@kiview I've updated the PR taking your thoughts into considerations.

From my trials, the default behaviour for rootless docker containers is to create the docker socket inside XDG_RUNTIME_DIR (usually /var/run/${userID}, while mac os desktop providers creates the socket inside the user home directory.
Both initialised the Docker context.
This means that it is likely that most of the RootlessDockerClientProviderStrategy and DockerDesktopClientProviderStrategy start to use the EnvironmentAndSystemPropertyClientProviderStrategy.

This is what I see on a linux machine:

docker context ls
NAME         DESCRIPTION                               DOCKER ENDPOINT                     ERROR
default      Current DOCKER_HOST based configuration   unix:///var/run/docker.sock
rootless *   Rootless mode                             unix:///run/user/1000/docker.sock

ls -al /run/user/1000/docker.sock
srw-rw---T 1 thibault thibault 0 Sep 25 09:52 /run/user/1000/docker.sock

And on a mac one:

docker context ls
NAME                DESCRIPTION                               DOCKER ENDPOINT                                            ERROR
colima              colima                                    unix:///Users/thibault.jamet/.colima/default/docker.sock
default             Current DOCKER_HOST based configuration   unix:///var/run/docker.sock
desktop-linux       Docker Desktop                            unix:///Users/thibault.jamet/.docker/run/docker.sock
orbstack            OrbStack                                  unix:///Users/thibault.jamet/.orbstack/run/docker.sock
rancher-desktop *   Rancher Desktop moby context              unix:///Users/thibault.jamet/.rd/docker.sock

ls -al /Users/thibault.jamet/.rd/docker.sock
srw-------  1 thibault.jamet  staff  0  9 Sep 13:19 /Users/thibault.jamet/.rd/docker.sock

With this in mind, I changed the approach to consider the socket is virtualised only if it is a unix socket inside the home directory
The previous change checking the user ID would indeed break the rootless Docker approach

@eddumelendez
Copy link
Member

eddumelendez commented Sep 26, 2024

/windows-test

@kiview
Copy link
Member

kiview commented Oct 16, 2024

/windows-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants