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

fix(core): Fix retrieval of the docker socket path when using rootless docker #710

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(core): Fix retrieval of the docker socket path when using rootless docker #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 13 additions & 2 deletions core/testcontainers/core/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from os.path import exists
from pathlib import Path
from typing import Optional, Union
from urllib.parse import urlparse

MAX_TRIES = int(environ.get("TC_MAX_TRIES", 120))
SLEEP_TIME = int(environ.get("TC_POOLING_INTERVAL", 1))
Expand All @@ -12,7 +13,7 @@
RYUK_IMAGE: str = environ.get("RYUK_CONTAINER_IMAGE", "testcontainers/ryuk:0.8.1")
RYUK_PRIVILEGED: bool = environ.get("TESTCONTAINERS_RYUK_PRIVILEGED", "false") == "true"
RYUK_DISABLED: bool = environ.get("TESTCONTAINERS_RYUK_DISABLED", "false") == "true"
RYUK_DOCKER_SOCKET: str = environ.get("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", "/var/run/docker.sock")
RYUK_DOCKER_SOCKET: Optional[str] = environ.get("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", None)
RYUK_RECONNECTION_TIMEOUT: str = environ.get("RYUK_RECONNECTION_TIMEOUT", "10s")
TC_HOST_OVERRIDE: Optional[str] = environ.get("TC_HOST", environ.get("TESTCONTAINERS_HOST_OVERRIDE"))

Expand Down Expand Up @@ -49,7 +50,6 @@ class TestcontainersConfiguration:
ryuk_image: str = RYUK_IMAGE
ryuk_privileged: bool = RYUK_PRIVILEGED
ryuk_disabled: bool = RYUK_DISABLED
ryuk_docker_socket: str = RYUK_DOCKER_SOCKET
Copy link
Member

Choose a reason for hiding this comment

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

we need to leave this configurable because otherwise people want to monkeypatch our modules

Copy link
Author

Choose a reason for hiding this comment

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

Can't they just use the environment variable to configure it if someone ever needs it? A quick search on GitHub shows no-one is actually calling TestcontainersConfiguration with custom params

Otherwise we can keep c.ryuk_docker_socket as a direct property for people to configure

And rename the new computed property to something else, e.g. c.docker_socket or c.get_docker_socket() (and use this computed property when needing to provide the docker socket)

Copy link
Member

Choose a reason for hiding this comment

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

see #531

Copy link
Author

Choose a reason for hiding this comment

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

I don't really see where is the problem, it is possible to monkeypatch a computed property:

from testcontainers.core.config import testcontainers_config, TestcontainersConfiguration

def custom_ryuk_docker_socket(self) -> str:
    return "/custom/path/to/docker.sock"

TestcontainersConfiguration.ryuk_docker_socket = property(custom_ryuk_docker_socket)
print(testcontainers_config.ryuk_docker_socket)

Otherwise I proposed another solution with a static property + a computed one, would this fit?

ryuk_reconnection_timeout: str = RYUK_RECONNECTION_TIMEOUT
tc_properties: dict[str, str] = field(default_factory=read_tc_properties)
_docker_auth_config: Optional[str] = field(default_factory=lambda: environ.get("DOCKER_AUTH_CONFIG"))
Expand Down Expand Up @@ -79,6 +79,17 @@ def tc_properties_get_tc_host(self) -> Union[str, None]:
def timeout(self) -> int:
return self.max_tries * self.sleep_time

@property
def docker_host(self) -> Optional[str]:
return self.tc_properties_get_tc_host() or environ.get("DOCKER_HOST")

@property
def ryuk_docker_socket(self) -> str:
if RYUK_DOCKER_SOCKET:
return RYUK_DOCKER_SOCKET
if self.docker_host:
return urlparse(self.docker_host).path
return "/var/run/docker.sock"

testcontainers_config = TestcontainersConfiguration()

Expand Down
9 changes: 2 additions & 7 deletions core/testcontainers/core/docker_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class DockerClient:
"""

def __init__(self, **kwargs) -> None:
docker_host = get_docker_host()
docker_host = c.docker_host

if docker_host:
LOGGER.info(f"using host {docker_host}")
Expand Down Expand Up @@ -89,7 +89,7 @@ def run(
**kwargs,
) -> Container:
# If the user has specified a network, we'll assume the user knows best
if "network" not in kwargs and not get_docker_host():
if "network" not in kwargs and not c.docker_host:
# Otherwise we'll try to find the docker host for dind usage.
host_network = self.find_host_network()
if host_network:
Expand Down Expand Up @@ -218,10 +218,5 @@ def client_networks_create(self, name: str, param: dict):
labels = create_labels("", param.get("labels"))
return self.client.networks.create(name, **{**param, "labels": labels})


def get_docker_host() -> Optional[str]:
return c.tc_properties_get_tc_host() or os.getenv("DOCKER_HOST")


def get_docker_auth_config() -> Optional[str]:
return c.docker_auth_config