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

fix(core): running testcontainer inside container #714

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 21 additions & 8 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
ARG PYTHON_VERSION
FROM python:${version}-slim-bookworm
ARG PYTHON_VERSION=3.10
FROM python:${PYTHON_VERSION}-slim-bookworm

ENV POETRY_NO_INTERACTION=1 \
POETRY_VIRTUALENVS_IN_PROJECT=1 \
POETRY_VIRTUALENVS_CREATE=1 \
POETRY_CACHE_DIR=/tmp/poetry_cache

WORKDIR /workspace
RUN pip install --upgrade pip \
&& apt-get update \
&& apt-get install -y \
freetds-dev \
&& rm -rf /var/lib/apt/lists/*
&& apt-get install -y freetds-dev \
&& apt-get install -y make \
# no real need for keeping this image small at the moment
&& :; # rm -rf /var/lib/apt/lists/*

# install poetry
RUN bash -c 'python -m venv /opt/poetry-venv && source $_/bin/activate && pip install poetry && ln -s $(which poetry) /usr/bin'

# install requirements we exported from poetry
COPY build/requirements.txt requirements.txt
RUN pip install -r requirements.txt
# install dependencies with poetry
COPY pyproject.toml .
COPY poetry.lock .
RUN poetry install --all-extras --with dev --no-root

# copy project source
COPY . .

# install project with poetry
RUN poetry install --all-extras --with dev
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ lint: ## Lint all files in the project, which we also run in pre-commit
poetry run pre-commit run -a

image: ## Make the docker image for dind tests
poetry export -f requirements.txt -o build/requirements.txt
docker build --build-arg PYTHON_VERSION=${PYTHON_VERSION} -t ${IMAGE} .

DOCKER_RUN = docker run --rm -v /var/run/docker.sock:/var/run/docker.sock
Expand Down
34 changes: 34 additions & 0 deletions core/testcontainers/core/config.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
from dataclasses import dataclass, field
from enum import Enum
from logging import warning
from os import environ
from os.path import exists
from pathlib import Path
from typing import Optional, Union


class ConnectionMode(Enum):
bridge_ip = "bridge_ip"
gateway_ip = "gateway_ip"
docker_host = "docker_host"

@property
def use_mapped_port(self) -> bool:
"""
Return true if we need to use mapped port for this connection

This is true for everything but bridge mode.
"""
if self == self.bridge_ip:
return False
return True


MAX_TRIES = int(environ.get("TC_MAX_TRIES", 120))
SLEEP_TIME = int(environ.get("TC_POOLING_INTERVAL", 1))
TIMEOUT = MAX_TRIES * SLEEP_TIME
Expand All @@ -20,6 +39,19 @@
TC_GLOBAL = Path.home() / TC_FILE


def get_user_overwritten_connection_mode() -> Optional[ConnectionMode]:
"""
Return the user overwritten connection mode.
"""
connection_mode: str | None = environ.get("TESTCONTAINERS_CONNECTION_MODE")
if connection_mode:
try:
return ConnectionMode(connection_mode)
except ValueError as e:
raise ValueError(f"Error parsing TESTCONTAINERS_CONNECTION_MODE: {e}") from e
return None


def read_tc_properties() -> dict[str, str]:
"""
Read the .testcontainers.properties for settings. (see the Java implementation for details)
Expand Down Expand Up @@ -54,6 +86,8 @@ class TestcontainersConfiguration:
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"))
tc_host_override: Optional[str] = TC_HOST_OVERRIDE
connection_mode_override: Optional[ConnectionMode] = None

"""
https://github.com/testcontainers/testcontainers-go/blob/dd76d1e39c654433a3d80429690d07abcec04424/docker.go#L644
if os env TC_HOST is set, use it
Expand Down
51 changes: 18 additions & 33 deletions core/testcontainers/core/container.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import contextlib
from platform import system
from socket import socket
from typing import TYPE_CHECKING, Optional, Union

import docker.errors
from docker import version
from docker.types import EndpointConfig
from typing_extensions import Self
from typing_extensions import Self, assert_never

from testcontainers.core.config import ConnectionMode
from testcontainers.core.config import testcontainers_config as c
from testcontainers.core.docker_client import DockerClient
from testcontainers.core.exceptions import ContainerStartException
from testcontainers.core.labels import LABEL_SESSION_ID, SESSION_ID
from testcontainers.core.network import Network
from testcontainers.core.utils import inside_container, is_arm, setup_logger
from testcontainers.core.utils import is_arm, setup_logger
from testcontainers.core.waiting_utils import wait_container_is_ready, wait_for_logs

if TYPE_CHECKING:
Expand Down Expand Up @@ -129,38 +129,23 @@
self.stop()

def get_container_host_ip(self) -> str:
# infer from docker host
host = self.get_docker_client().host()
if not host:
return "localhost"
# see https://github.com/testcontainers/testcontainers-python/issues/415
if host == "localnpipe" and system() == "Windows":
return "localhost"

# # check testcontainers itself runs inside docker container
# if inside_container() and not os.getenv("DOCKER_HOST") and not host.startswith("http://"):
# # 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
connection_mode: ConnectionMode
connection_mode = self.get_docker_client().get_connection_mode()
if connection_mode == ConnectionMode.docker_host:
return self.get_docker_client().host()
Comment on lines 131 to +135
Copy link
Contributor Author

@CarliJoy CarliJoy Oct 23, 2024

Choose a reason for hiding this comment

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

Btw: one could argue that this is incorrect.

self.get_docker_client().host() does return a DNS name instead of an IP especially localhost.
Indeed for some containers this has a negative effect: I tried to create a MariaDB container that failed. Using localhost instead of 127.0.0.1 it was trying to connect through the unix socket.

But this is the old behaviour so I kept it the way it is.
We might want to change this in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah yes, as it says "container_ip" in the method name but it returns a domain name instead, hm.

elif connection_mode == ConnectionMode.gateway_ip:
return self.get_docker_client().gateway_ip(self._container.id)
elif connection_mode == ConnectionMode.bridge_ip:
return self.get_docker_client().bridge_ip(self._container.id)
else:
# ensure that we covered all possible connection_modes
assert_never(connection_mode)

Check warning on line 142 in core/testcontainers/core/container.py

View check run for this annotation

Codecov / codecov/patch

core/testcontainers/core/container.py#L142

Added line #L142 was not covered by tests

@wait_container_is_ready()
def get_exposed_port(self, port: int) -> str:
mapped_port = self.get_docker_client().port(self._container.id, port)
if inside_container():
gateway_ip = self.get_docker_client().gateway_ip(self._container.id)
host = self.get_docker_client().host()

if gateway_ip == host:
return port
return mapped_port
def get_exposed_port(self, port: int) -> int:
if self.get_docker_client().get_connection_mode().use_mapped_port:
return self.get_docker_client().port(self._container.id, port)
return port

def with_command(self, command: str) -> Self:
self._command = command
Expand Down
53 changes: 45 additions & 8 deletions core/testcontainers/core/docker_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import contextlib
import functools as ft
import importlib.metadata
import ipaddress
import os
import socket
import urllib
import urllib.parse
from collections.abc import Iterable
Expand All @@ -24,12 +26,13 @@
from docker.models.images import Image, ImageCollection
from typing_extensions import ParamSpec

from testcontainers.core import utils
from testcontainers.core.auth import DockerAuthInfo, parse_docker_auth_config
from testcontainers.core.config import ConnectionMode
from testcontainers.core.config import testcontainers_config as c
from testcontainers.core.labels import SESSION_ID, create_labels
from testcontainers.core.utils import default_gateway_ip, inside_container, setup_logger

LOGGER = setup_logger(__name__)
LOGGER = utils.setup_logger(__name__)

_P = ParamSpec("_P")
_T = TypeVar("_T")
Expand Down Expand Up @@ -127,8 +130,18 @@ def find_host_network(self) -> Optional[str]:
"""
# If we're docker in docker running on a custom network, we need to inherit the
# network settings, so we can access the resulting container.

# first to try to find the network the container runs in, if we can determine
container_id = utils.get_running_in_container_id()
if container_id:
with contextlib.suppress(Exception):
return self.network_name(container_id)

# if this results nothing, try to determine the network based on the
# docker_host
try:
docker_host = ipaddress.IPv4Address(self.host())
host_ip = socket.gethostbyname(self.host())
docker_host = ipaddress.IPv4Address(host_ip)
# See if we can find the host on our networks
for network in self.client.networks.list(filters={"type": "custom"}):
if "IPAM" in network.attrs:
Expand All @@ -139,7 +152,7 @@ def find_host_network(self) -> Optional[str]:
continue
if docker_host in subnet:
return network.name
except ipaddress.AddressValueError:
except (ipaddress.AddressValueError, OSError):
pass
return None

Expand Down Expand Up @@ -187,6 +200,28 @@ def gateway_ip(self, container_id: str) -> str:
network_name = self.network_name(container_id)
return container["NetworkSettings"]["Networks"][network_name]["Gateway"]

def get_connection_mode(self) -> ConnectionMode:
"""
Determine the connection mode.

See https://github.com/testcontainers/testcontainers-python/issues/475#issuecomment-2407250970
"""
if c.connection_mode_override:
return c.connection_mode_override
localhosts = {"localhost", "127.0.0.1", "::1"}
if not utils.inside_container() or self.host() not in localhosts:
# if running not inside a container or with a non-local docker client,
# connect ot the docker host per default
return ConnectionMode.docker_host
elif self.find_host_network():
# a host network could be determined, indicator for DooD,
# so we should connect to the bridge_ip as the container we run in
# and the one we started are connected to the same network
# that might have no access to either docker_host or the gateway
return ConnectionMode.bridge_ip
# default for DinD
return ConnectionMode.gateway_ip

def host(self) -> str:
"""
Get the hostname or ip address of the docker host.
Expand All @@ -196,13 +231,15 @@ def host(self) -> str:
return host
try:
url = urllib.parse.urlparse(self.client.api.base_url)

except ValueError:
return "localhost"
if "http" in url.scheme or "tcp" in url.scheme:
if "http" in url.scheme or "tcp" in url.scheme and url.hostname:
# see https://github.com/testcontainers/testcontainers-python/issues/415
if url.hostname == "localnpipe" and utils.is_windows():
return "localhost"
return url.hostname
if inside_container() and ("unix" in url.scheme or "npipe" in url.scheme):
ip_address = default_gateway_ip()
if utils.inside_container() and ("unix" in url.scheme or "npipe" in url.scheme):
ip_address = utils.default_gateway_ip()
if ip_address:
return ip_address
return "localhost"
Expand Down
20 changes: 19 additions & 1 deletion core/testcontainers/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import platform
import subprocess
import sys
from typing import Any, Optional
from pathlib import Path
from typing import Any, Final, Optional

LINUX = "linux"
MAC = "mac"
Expand Down Expand Up @@ -80,3 +81,20 @@ def raise_for_deprecated_parameter(kwargs: dict[Any, Any], name: str, replacemen
if kwargs.pop(name, None):
raise ValueError(f"Use `{replacement}` instead of `{name}`")
return kwargs


CGROUP_FILE: Final[Path] = Path("/proc/self/cgroup")


def get_running_in_container_id() -> Optional[str]:
"""
Get the id of the currently running container
"""
if not CGROUP_FILE.is_file():
return None
cgroup = CGROUP_FILE.read_text()
for line in cgroup.splitlines(keepends=False):
path = line.rpartition(":")[2]
if path.startswith("/docker"):
return path.removeprefix("/docker/")
return None
29 changes: 29 additions & 0 deletions core/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,35 @@
from pathlib import Path

import pytest
from typing import Callable
import subprocess
from testcontainers.core.container import DockerClient
import sys

PROJECT_DIR = Path(__file__).parent.parent.parent.resolve()


def pytest_configure(config: pytest.Config) -> None:
"""
Add configuration for custom pytest markers.
"""
config.addinivalue_line(
"markers",
"inside_docker_check: test used to validate DinD/DooD are working as expected",
)
Comment on lines +12 to +19
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this just for filtering in/out dind/dod tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is to filter out tests that run within the test_dind and test_dood tests.
Inside this tests now the docker container is build and executed in a dind and dood environment.
This way we have proper test that dind and dood actually works.

I left the old make test-dind (which is not correct because it's acutall DooD) untouched.
It would actually be better to remove it not only because of the wrong name.
It also doesn't succeed because some things as some auth tests seem to have troubles with TLS settings.
I don't think the complete test suit has to pass inside a dind/dood env.
In the end these test are integration tests and should be treated as such.

Copy link
Collaborator

Choose a reason for hiding this comment

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

lets remove the things that don't make sense

Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps in a separate PR maybe to make the history clearer



@pytest.fixture(scope="session")
def python_testcontainer_image() -> str:
"""Build an image with test containers python for DinD and DooD tests"""
py_version = ".".join(map(str, sys.version_info[:2]))
image_name = f"testcontainers-python:{py_version}"
subprocess.run(
[*("docker", "build"), *("--build-arg", f"PYTHON_VERSION={py_version}"), *("-t", image_name), "."],
cwd=PROJECT_DIR,
check=True,
)
return image_name


@pytest.fixture
Expand Down
26 changes: 25 additions & 1 deletion core/tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
from testcontainers.core.config import TestcontainersConfiguration as TCC, TC_FILE
import pytest

from testcontainers.core.config import (
TestcontainersConfiguration as TCC,
TC_FILE,
get_user_overwritten_connection_mode,
ConnectionMode,
)

from pytest import MonkeyPatch, mark, LogCaptureFixture

Expand Down Expand Up @@ -60,3 +67,20 @@ def test_timeout() -> None:
config.max_tries = 2
config.sleep_time = 3
assert config.timeout == 6


def test_invalid_connection_mode(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("TESTCONTAINERS_CONNECTION_MODE", "FOOBAR")
with pytest.raises(ValueError, match="Error parsing TESTCONTAINERS_CONNECTION_MODE.*FOOBAR.*"):
get_user_overwritten_connection_mode()


@pytest.mark.parametrize("mode, use_mapped", (("bridge_ip", False), ("gateway_ip", True), ("docker_host", True)))
def test_valid_connection_mode(monkeypatch: pytest.MonkeyPatch, mode: str, use_mapped: bool) -> None:
monkeypatch.setenv("TESTCONTAINERS_CONNECTION_MODE", mode)
assert get_user_overwritten_connection_mode().use_mapped_port is use_mapped


def test_no_connection_mode_given(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.delenv("TESTCONTAINERS_CONNECTION_MODE", raising=False)
assert get_user_overwritten_connection_mode() is None
Loading