Skip to content

Commit

Permalink
feat(core)!: add support for tc.host and de-prioritise docker:dind (
Browse files Browse the repository at this point in the history
#388)

## What does this PR do?

Adds support for reading the `tc.host` property from the
`~/.testcontainers.properties` file. This config will have the highest
priority for configuring the Docker client.

## Why is it important

This brings [Testcontainers
Desktop](https://testcontainers.com/desktop/) support to
testcontainers-python and aligns this language with the
`TestcontainersHostStrategy` found it testcontainers-java.

## Misc

I wasn't able to get a working testcontainers-python development
environment set up on my M2 MacBook. This seems to be related to some
kind of Cython 3.0.0 issue, that I don't fully understand and trying to
fix it breaks the installation of further dependencies (like pymssql).
So I was only able to test it in an Ubuntu Codespaces environment (also
required some weird Cython workarounds).

##  Breaking Changes

- To prioritise `tc.host` this PR prioritised having that over true
`docker:dind` use cases
- This means we stopped trying to automatically infer the container host
IP when running inside a `docker:dind` container
- When using `-v /var/run/docker.sock:/var/run/docker.sock` you should
be unaffected since your containers run on the original host

---------

Co-authored-by: Bálint Bartha <39852431+totallyzen@users.noreply.github.com>
Co-authored-by: David Ankin <daveankin@gmail.com>
  • Loading branch information
3 people authored Mar 1, 2024
1 parent 9e240d0 commit 2db8e6d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 23 deletions.
25 changes: 12 additions & 13 deletions core/testcontainers/core/container.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import contextlib
import os
from platform import system
from typing import Optional

Expand Down Expand Up @@ -102,18 +101,18 @@ def get_container_host_ip(self) -> str:
if host == "localnpipe" and system() == "Windows":
return "localhost"

# check testcontainers itself runs inside docker container
if inside_container() and not os.getenv("DOCKER_HOST"):
# If newly spawned container's gateway IP address from the docker
# "bridge" network is equal to detected host address, we should use
# container IP address, otherwise fall back to detected host
# address. Even it's inside container, we need to double check,
# because docker host might be set to docker:dind, usually in CI/CD environment
gateway_ip = self.get_docker_client().gateway_ip(self._container.id)

if gateway_ip == host:
return self.get_docker_client().bridge_ip(self._container.id)
return gateway_ip
# # check testcontainers itself runs inside docker container
# if inside_container() and not os.getenv("DOCKER_HOST") and not host.startswith("http://"):

This comment has been minimized.

Copy link
@malltshik

malltshik Sep 10, 2024

Is it possible to add and not os.getenv("TC_HOST") here instead of commenting everything out?

This comment has been minimized.

Copy link
@alexanderankin

alexanderankin Sep 10, 2024

Author Collaborator

yes, even better with dedicated env var, this was the plan for #622

# # If newly spawned container's gateway IP address from the docker
# # "bridge" network is equal to detected host address, we should use
# # container IP address, otherwise fall back to detected host
# # address. Even it's inside container, we need to double check,
# # because docker host might be set to docker:dind, usually in CI/CD environment
# gateway_ip = self.get_docker_client().gateway_ip(self._container.id)

# if gateway_ip == host:
# return self.get_docker_client().bridge_ip(self._container.id)
# return gateway_ip
return host

@wait_container_is_ready()
Expand Down
51 changes: 41 additions & 10 deletions core/testcontainers/core/docker_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import functools as ft
import os
import urllib
from os.path import exists
from pathlib import Path
from typing import Optional, Union

import docker
Expand All @@ -23,15 +25,8 @@
from .utils import default_gateway_ip, inside_container, setup_logger

LOGGER = setup_logger(__name__)


def _stop_container(container: Container) -> None:
try:
container.stop()
except NotFound:
pass
except Exception as ex:
LOGGER.warning("failed to shut down container %s with image %s: %s", container.id, container.image, ex)
TC_FILE = ".testcontainers.properties"
TC_GLOBAL = Path.home() / TC_FILE


class DockerClient:
Expand All @@ -40,7 +35,13 @@ class DockerClient:
"""

def __init__(self, **kwargs) -> None:
self.client = docker.from_env(**kwargs)
docker_host = read_tc_properties().get("tc.host")

if docker_host:
LOGGER.info(f"using host {docker_host}")
self.client = docker.DockerClient(base_url=docker_host)
else:
self.client = docker.from_env(**kwargs)

@ft.wraps(ContainerCollection.run)
def run(
Expand Down Expand Up @@ -123,3 +124,33 @@ def host(self) -> str:
if ip_address:
return ip_address
return "localhost"


@ft.cache
def read_tc_properties() -> dict[str, str]:
"""
Read the .testcontainers.properties for settings. (see the Java implementation for details)
Currently we only support the ~/.testcontainers.properties but may extend to per-project variables later.
:return: the merged properties from the sources.
"""
tc_files = [item for item in [TC_GLOBAL] if exists(item)]
if not tc_files:
return {}
settings = {}

for file in tc_files:
tuples = []
with open(file) as contents:
tuples = [line.split("=") for line in contents.readlines() if "=" in line]
settings = {**settings, **{item[0]: item[1] for item in tuples}}
return settings


def _stop_container(container: Container) -> None:
try:
container.stop()
except NotFound:
pass
except Exception as ex:
LOGGER.warning("failed to shut down container %s with image %s: %s", container.id, container.image, ex)

0 comments on commit 2db8e6d

Please sign in to comment.