From 9865b01cf77472f85d8d699154e65913cc97e826 Mon Sep 17 00:00:00 2001 From: Vemund Santi Date: Wed, 27 Mar 2024 17:28:21 +0100 Subject: [PATCH 1/3] Re-enable tests for Ryuk. Add RYUK_RECONNECTION_TIMEOUT env variable support --- README.md | 13 +++--- core/testcontainers/core/config.py | 1 + core/testcontainers/core/container.py | 9 +++- core/tests/test_ryuk.py | 59 ++++++++++++++++++--------- 4 files changed, 56 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 7f469914..6aa8d698 100644 --- a/README.md +++ b/README.md @@ -25,9 +25,10 @@ The snippet above will spin up a postgres database in a container. The `get_conn ## Configuration -| Env Variable | Example | Description | -| ----------------------------------------- | ----------------------------- | ---------------------------------------- | -| `TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE` | `/var/run/docker.sock` | Path to Docker's socket used by ryuk | -| `TESTCONTAINERS_RYUK_PRIVILEGED` | `false` | Run ryuk as a privileged container | -| `TESTCONTAINERS_RYUK_DISABLED` | `false` | Disable ryuk | -| `RYUK_CONTAINER_IMAGE` | `testcontainers/ryuk:0.5.1` | Custom image for ryuk | +| Env Variable | Example | Description | +| --------------------------------------- | --------------------------- | ---------------------------------------------------------------------------------- | +| `TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE` | `/var/run/docker.sock` | Path to Docker's socket used by ryuk | +| `TESTCONTAINERS_RYUK_PRIVILEGED` | `false` | Run ryuk as a privileged container | +| `TESTCONTAINERS_RYUK_DISABLED` | `false` | Disable ryuk | +| `RYUK_CONTAINER_IMAGE` | `testcontainers/ryuk:0.5.1` | Custom image for ryuk | +| `RYUK_RECONNECTION_TIMEOUT` | `10s` | Reconnection timeout for Ryuk TCP socket before Ryuk reaps all dangling containers | diff --git a/core/testcontainers/core/config.py b/core/testcontainers/core/config.py index 1bf9ad4d..d378b9ec 100644 --- a/core/testcontainers/core/config.py +++ b/core/testcontainers/core/config.py @@ -8,3 +8,4 @@ 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_RECONNECTION_TIMEOUT: str = environ.get("RYUK_RECONNECTION_TIMEOUT", "10s") diff --git a/core/testcontainers/core/container.py b/core/testcontainers/core/container.py index 42f4de52..74c280d5 100644 --- a/core/testcontainers/core/container.py +++ b/core/testcontainers/core/container.py @@ -2,7 +2,13 @@ from socket import socket from typing import TYPE_CHECKING, Optional -from testcontainers.core.config import RYUK_DISABLED, RYUK_DOCKER_SOCKET, RYUK_IMAGE, RYUK_PRIVILEGED +from testcontainers.core.config import ( + RYUK_DISABLED, + RYUK_DOCKER_SOCKET, + RYUK_IMAGE, + RYUK_PRIVILEGED, + RYUK_RECONNECTION_TIMEOUT, +) from testcontainers.core.docker_client import DockerClient from testcontainers.core.exceptions import ContainerStartException from testcontainers.core.labels import LABEL_SESSION_ID, SESSION_ID @@ -194,6 +200,7 @@ def _create_instance(cls) -> "Reaper": .with_exposed_ports(8080) .with_volume_mapping(RYUK_DOCKER_SOCKET, "/var/run/docker.sock", "rw") .with_kwargs(privileged=RYUK_PRIVILEGED, auto_remove=True) + .with_env("RYUK_RECONNECTION_TIMEOUT", RYUK_RECONNECTION_TIMEOUT) .start() ) wait_for_logs(Reaper._container, r".* Started!") diff --git a/core/tests/test_ryuk.py b/core/tests/test_ryuk.py index 4f3b431e..c898f099 100644 --- a/core/tests/test_ryuk.py +++ b/core/tests/test_ryuk.py @@ -1,37 +1,58 @@ -from contextlib import contextmanager - +from time import sleep import pytest +from pytest import MonkeyPatch + +from docker import DockerClient +from docker.errors import NotFound -from testcontainers.core import container +from testcontainers.core import container as container_module from testcontainers.core.container import Reaper from testcontainers.core.container import DockerContainer from testcontainers.core.waiting_utils import wait_for_logs -@pytest.mark.skip("invalid test - ryuk logs 'Removed' right before exiting") -def test_wait_for_reaper(): +def test_wait_for_reaper(monkeypatch: MonkeyPatch): + monkeypatch.setattr(container_module, "RYUK_RECONNECTION_TIMEOUT", "0.1s") + docker_client = DockerClient() container = DockerContainer("hello-world").start() + + container_id = container.get_wrapped_container().short_id + reaper_id = Reaper._container.get_wrapped_container().short_id + + assert docker_client.containers.get(container_id) is not None + assert docker_client.containers.get(reaper_id) is not None + wait_for_logs(container, "Hello from Docker!") - assert Reaper._socket is not None Reaper._socket.close() - assert Reaper._container is not None - wait_for_logs(Reaper._container, r".* Removed \d .*", timeout=30) + sleep(0.6) # Sleep until Ryuk reaps all dangling containers. 0.5 extra seconds for good measure. - Reaper.delete_instance() + with pytest.raises(NotFound): + docker_client.containers.get(container_id) + with pytest.raises(NotFound): + docker_client.containers.get(reaper_id) - -@contextmanager -def reset_reaper_instance(): - old_value = Reaper._instance + # Cleanup Ryuk class fields after manual Ryuk shutdown + Reaper._socket = None Reaper._instance = None - yield - Reaper._instance = old_value + Reaper._container = None -def test_container_without_ryuk(monkeypatch): - monkeypatch.setattr(container, "RYUK_DISABLED", True) - with reset_reaper_instance(), DockerContainer("hello-world") as cont: - wait_for_logs(cont, "Hello from Docker!") +def test_container_without_ryuk(monkeypatch: MonkeyPatch): + monkeypatch.setattr(container_module, "RYUK_DISABLED", True) + with DockerContainer("hello-world") as container: + wait_for_logs(container, "Hello from Docker!") assert Reaper._instance is None + + +def test_ryuk_is_reused_in_same_process(): + with DockerContainer("hello-world") as container: + wait_for_logs(container, "Hello from Docker!") + reaper_instance = Reaper._instance + + assert reaper_instance is not None + + with DockerContainer("hello-world") as container: + wait_for_logs(container, "Hello from Docker!") + assert reaper_instance is Reaper._instance From 15f7434ed083cf58ca269d3bb89c98b6c126a879 Mon Sep 17 00:00:00 2001 From: Vemund Santi Date: Wed, 27 Mar 2024 17:45:36 +0100 Subject: [PATCH 2/3] Bump Ryuk image to latest release --- README.md | 2 +- core/testcontainers/core/config.py | 2 +- index.rst | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6aa8d698..036723d6 100644 --- a/README.md +++ b/README.md @@ -30,5 +30,5 @@ The snippet above will spin up a postgres database in a container. The `get_conn | `TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE` | `/var/run/docker.sock` | Path to Docker's socket used by ryuk | | `TESTCONTAINERS_RYUK_PRIVILEGED` | `false` | Run ryuk as a privileged container | | `TESTCONTAINERS_RYUK_DISABLED` | `false` | Disable ryuk | -| `RYUK_CONTAINER_IMAGE` | `testcontainers/ryuk:0.5.1` | Custom image for ryuk | +| `RYUK_CONTAINER_IMAGE` | `testcontainers/ryuk:0.7.0` | Custom image for ryuk | | `RYUK_RECONNECTION_TIMEOUT` | `10s` | Reconnection timeout for Ryuk TCP socket before Ryuk reaps all dangling containers | diff --git a/core/testcontainers/core/config.py b/core/testcontainers/core/config.py index d378b9ec..0c1b5e0c 100644 --- a/core/testcontainers/core/config.py +++ b/core/testcontainers/core/config.py @@ -4,7 +4,7 @@ SLEEP_TIME = int(environ.get("TC_POOLING_INTERVAL", 1)) TIMEOUT = MAX_TRIES * SLEEP_TIME -RYUK_IMAGE: str = environ.get("RYUK_CONTAINER_IMAGE", "testcontainers/ryuk:0.5.1") +RYUK_IMAGE: str = environ.get("RYUK_CONTAINER_IMAGE", "testcontainers/ryuk:0.7.0") 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") diff --git a/index.rst b/index.rst index d6e7ac7d..756e195c 100644 --- a/index.rst +++ b/index.rst @@ -105,7 +105,7 @@ Configuration +-------------------------------------------+-------------------------------+------------------------------------------+ | ``TESTCONTAINERS_RYUK_DISABLED`` | ``false`` | Disable ryuk | +-------------------------------------------+-------------------------------+------------------------------------------+ -| ``RYUK_CONTAINER_IMAGE`` | ``testcontainers/ryuk:0.5.1`` | Custom image for ryuk | +| ``RYUK_CONTAINER_IMAGE`` | ``testcontainers/ryuk:0.7.0`` | Custom image for ryuk | +-------------------------------------------+-------------------------------+------------------------------------------+ Development and Contributing From b260f8f0ff804999ec5c4883aa5ba94f8c4d0a7d Mon Sep 17 00:00:00 2001 From: Vemund Santi Date: Wed, 27 Mar 2024 18:21:40 +0100 Subject: [PATCH 3/3] fix: Ensure no reaper is present before testing Reaper behaviour --- core/testcontainers/core/container.py | 8 ++++++-- core/tests/test_ryuk.py | 6 +++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/testcontainers/core/container.py b/core/testcontainers/core/container.py index 74c280d5..3e1e1ba1 100644 --- a/core/testcontainers/core/container.py +++ b/core/testcontainers/core/container.py @@ -1,7 +1,10 @@ +import contextlib from platform import system from socket import socket from typing import TYPE_CHECKING, Optional +import docker.errors + from testcontainers.core.config import ( RYUK_DISABLED, RYUK_DOCKER_SOCKET, @@ -183,8 +186,9 @@ def delete_instance(cls) -> None: Reaper._socket.close() Reaper._socket = None - if Reaper._container is not None: - Reaper._container.stop() + if Reaper._container is not None and Reaper._container._container is not None: + with contextlib.suppress(docker.errors.NotFound): + Reaper._container.stop() Reaper._container = None if Reaper._instance is not None: diff --git a/core/tests/test_ryuk.py b/core/tests/test_ryuk.py index c898f099..e21b045b 100644 --- a/core/tests/test_ryuk.py +++ b/core/tests/test_ryuk.py @@ -12,6 +12,7 @@ def test_wait_for_reaper(monkeypatch: MonkeyPatch): + Reaper.delete_instance() monkeypatch.setattr(container_module, "RYUK_RECONNECTION_TIMEOUT", "0.1s") docker_client = DockerClient() container = DockerContainer("hello-world").start() @@ -34,12 +35,11 @@ def test_wait_for_reaper(monkeypatch: MonkeyPatch): docker_client.containers.get(reaper_id) # Cleanup Ryuk class fields after manual Ryuk shutdown - Reaper._socket = None - Reaper._instance = None - Reaper._container = None + Reaper.delete_instance() def test_container_without_ryuk(monkeypatch: MonkeyPatch): + Reaper.delete_instance() monkeypatch.setattr(container_module, "RYUK_DISABLED", True) with DockerContainer("hello-world") as container: wait_for_logs(container, "Hello from Docker!")