Skip to content

feat(reliability): integrate the ryuk container for better container cleanup #314

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

Merged
merged 43 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
7c09ccc
Add .with_ryuk method to enable container cleanup with ryuk
santi Mar 4, 2023
957b863
Add tests for ryuk cleanup
santi Mar 4, 2023
fc30fc1
Merge branch 'master' into feat/ryuk-the-reaper
santi Mar 4, 2023
e523dba
Change param name for clarity
santi Mar 4, 2023
543ce3e
Remove unused imports
santi Mar 4, 2023
a487dc4
Add x-tc-sid header to all Docker APi requests from testcontainers
santi Mar 6, 2023
38a9d5a
Combine imports
santi Mar 6, 2023
a214089
Fix circular import by importing from correct script
santi Mar 8, 2023
fdaee73
Fix lint
santi Mar 9, 2023
d4ddcba
Add testcontainer language implementation to default container labels
santi Mar 9, 2023
3711fcf
Merge branch 'main' into feat/ryuk-the-reaper
santi May 23, 2023
89113a3
Rename with_ryuk API to with_auto_remove
santi May 23, 2023
101fd11
Remove unused imports and function
santi May 23, 2023
8802f39
Update Ryuk tests to not rely on __del__ dunder for cleanup
santi May 23, 2023
7449d14
Update if-statement for better readability
santi May 23, 2023
6c1650d
Use __future__ annotations to avoid quotation marks
santi May 23, 2023
3e2c66c
Move Ryuk container image setting into config.py
santi May 23, 2023
c401df7
Move all Ryuk config-from-env logic to config.py. Rename to RYUK_ prefix
santi May 23, 2023
581e38b
Fix lint
santi May 23, 2023
5b1988e
Fix bug where language label was not added when provided custom labels
santi May 23, 2023
6c24543
Bump Ryuk container version
santi May 24, 2023
77251bb
Merge branch 'main' into feat/ryuk-the-reaper
santi Aug 28, 2023
4c37960
Add env variables to Docs. Add env variable for disabling Ryuk
santi Aug 28, 2023
dc66658
Replace programmatic .with_auto_remove() API with env variable for di…
santi Aug 28, 2023
dd18c01
Downgrade PyYAML to 5.3.1 to fix Cython build problem
santi Aug 28, 2023
1e2f7a1
Add dependency restriction to testcontainers-compose to mitigate Cyth…
santi Aug 28, 2023
a4827f2
Allow any single digit of containers to be killed. Some may be dangli…
santi Aug 28, 2023
8396907
Merge branch 'main' into feat/ryuk-the-reaper
santi Feb 17, 2024
07d4849
Remove deprecated setup file
santi Feb 17, 2024
c239d52
Use absolute imports
santi Feb 17, 2024
22e03eb
Fix flake8 lint error
santi Feb 17, 2024
6dce010
Merge remote-tracking branch 'origin/main' into feat/ryuk-the-reaper
santi Mar 11, 2024
fed8d49
Lint
santi Mar 11, 2024
a510983
Add comment on why quotes are used for type annotation
santi Mar 11, 2024
51ccbc4
Shorter comment do make linter happy
santi Mar 11, 2024
a0d8487
Disable ruff rule for prefering X | Y over Optional[X] to support Pyt…
santi Mar 11, 2024
e9974b3
Use absolute imports
santi Mar 11, 2024
904ae1c
Add env variable docs to README
santi Mar 11, 2024
74c82af
Move Reaper class into container.py to avoid circular dependency
santi Mar 11, 2024
ed47f57
Add missing newline to README
santi Mar 12, 2024
5772dca
Remove noqa lint rule exceptions
santi Mar 12, 2024
da6f3d8
fix: linting with ruff, keep runtime typging
totallyzen Mar 13, 2024
81cb09e
Replace future import with quoted type hints
santi Mar 14, 2024
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
5 changes: 5 additions & 0 deletions core/testcontainers/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@
MAX_TRIES = int(environ.get("TC_MAX_TRIES", 120))
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.3.4")
RYUK_PRIVILEGED: bool = environ.get("TESTCONTAINERS_RYUK_PRIVILEGED", "false") == "true"
Copy link

@teodor-t-tenev teodor-t-tenev Apr 17, 2024

Choose a reason for hiding this comment

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

This gets evaluated on import and can't be manupulated afterwards better make it a method for better evaluation. Currently in order for this to work I need:

os.environ['TESTCONTAINERS_RYUK_DISABLED'] = 'true'
from testcontainers.core.container import DockerContainer

This setup

Copy link
Member

Choose a reason for hiding this comment

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

@teodor-t-tenev - #532 solves this, and its included since 4.3.2 - i think it should work if you have regular imports at the top of your file and later do

from testcontainers.core.config import testcontainers_config as config
config.ryuk_disabled = True

Choose a reason for hiding this comment

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

Thanks for the reply. That did the work for 4.3.2.

RYUK_DOCKER_SOCKET: str = environ.get("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE",
"/var/run/docker.sock")
21 changes: 11 additions & 10 deletions core/testcontainers/core/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
import os
from typing import Iterable, Optional, Tuple


from .reaper import Reaper
from .config import RYUK_IMAGE
from .waiting_utils import wait_container_is_ready
from .docker_client import DockerClient
from .exceptions import ContainerStartException
Expand All @@ -26,6 +29,7 @@ def __init__(self, image: str, docker_client_kw: Optional[dict] = None, **kwargs
self.env = {}
self.ports = {}
self.volumes = {}
self.auto_remove = True
self.image = image
self._docker = DockerClient(**(docker_client_kw or {}))
self._container = None
Expand All @@ -46,6 +50,10 @@ def with_exposed_ports(self, *ports: Iterable[int]) -> 'DockerContainer':
self.ports[port] = None
return self

def with_auto_remove(self, enabled: bool) -> 'DockerContainer':
self.auto_remove = enabled
return self

def with_kwargs(self, **kwargs) -> 'DockerContainer':
self._kwargs = kwargs
return self
Expand All @@ -56,6 +64,9 @@ def maybe_emulate_amd64(self) -> 'DockerContainer':
return self

def start(self) -> 'DockerContainer':
if self.auto_remove and self.image != RYUK_IMAGE:
logger.debug("Creating Ryuk container")
Reaper.get_instance()
logger.info("Pulling image %s", self.image)
docker_client = self.get_docker_client()
self._container = docker_client.run(
Expand All @@ -74,16 +85,6 @@ def __enter__(self) -> 'DockerContainer':
def __exit__(self, exc_type, exc_val, exc_tb) -> None:
self.stop()

def __del__(self) -> None:
"""
Try to remove the container in all circumstances
"""
if self._container is not None:
try:
self.stop()
except: # noqa: E722
pass

def get_container_host_ip(self) -> str:
# infer from docker host
host = self.get_docker_client().host()
Expand Down
23 changes: 7 additions & 16 deletions core/testcontainers/core/docker_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,49 +10,40 @@
# 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 atexit
import docker
from docker.errors import NotFound
from docker.models.containers import Container, ContainerCollection
import functools as ft
import os
from typing import List, Optional, Union
import urllib


from .labels import create_labels, SESSION_ID
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)


class DockerClient:
"""
Thin wrapper around :class:`docker.DockerClient` for a more functional interface.
"""

def __init__(self, **kwargs) -> None:
self.client = docker.from_env(**kwargs)
self.client.api.headers["x-tc-sid"] = SESSION_ID

@ft.wraps(ContainerCollection.run)
def run(self, image: str, command: Union[str, List[str]] = None,
environment: Optional[dict] = None, ports: Optional[dict] = None,
detach: bool = False, stdout: bool = True, stderr: bool = False, remove: bool = False,
labels: Optional[dict] = None, detach: bool = False, stdout: bool = True,
stderr: bool = False, remove: bool = False,
**kwargs) -> Container:
container = self.client.containers.run(
image, command=command, stdout=stdout, stderr=stderr, remove=remove, detach=detach,
environment=environment, ports=ports, **kwargs
environment=environment, ports=ports, labels=create_labels(image, labels), **kwargs
)
if detach:
atexit.register(_stop_container, container)
return container

def port(self, container_id: str, port: int) -> int:
Expand Down
21 changes: 21 additions & 0 deletions core/testcontainers/core/labels.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from uuid import uuid4
from typing import Optional

from .config import RYUK_IMAGE


SESSION_ID: str = str(uuid4())
LABEL_SESSION_ID = "org.testcontainers.session-id"
LABEL_LANG = "org.testcontainers.lang"


def create_labels(image: str, labels: Optional[dict]) -> dict:
if labels is None:
labels = {}
Comment on lines +12 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

If you like you can add the following labels by default too:

org.testcontainers.lang=python
org.testcontainers.version=0.1.0 // The corresponding version of Testcontainers for Python

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the org.testcontainers.lang=python label in a commit, but reading the actual version is a bit harder, because of how this repo is structured. The core package is bundled together with the other packages published from this repo, and is still on a static v0.0.1rc1. Finding out which package is actually using the core package during runtime is quite brittle and prone to errors, so I'll leave that as a later task.

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK, for the other languages we just set the core version too. At least for Java and .NET, there are no individual versions.

labels[LABEL_LANG] = "python"

if image == RYUK_IMAGE:
return labels

labels[LABEL_SESSION_ID] = SESSION_ID
return labels
69 changes: 69 additions & 0 deletions core/testcontainers/core/reaper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
from __future__ import annotations

from socket import socket
from typing import TYPE_CHECKING, Optional


from .utils import setup_logger
from .config import RYUK_IMAGE, RYUK_DOCKER_SOCKET, RYUK_PRIVILEGED
from .waiting_utils import wait_for_logs
from .labels import LABEL_SESSION_ID, SESSION_ID

if TYPE_CHECKING:
from .container import DockerContainer


logger = setup_logger(__name__)


class Reaper:
_instance: Optional[Reaper] = None
_container: "Optional[DockerContainer]" = None
_socket: Optional[socket] = None

@classmethod
def get_instance(cls) -> Reaper:
if not Reaper._instance:
Reaper._instance = Reaper._create_instance()

return Reaper._instance

@classmethod
def delete_instance(cls) -> None:
if Reaper._socket is not None:
Reaper._socket.close()
Reaper._socket = None

if Reaper._container is not None:
Reaper._container.stop()
Reaper._container = None

if Reaper._instance is not None:
Reaper._instance = None

@classmethod
def _create_instance(cls) -> Reaper:
from .container import DockerContainer

logger.debug(f"Creating new Reaper for session: {SESSION_ID}")

Reaper._container = (
DockerContainer(RYUK_IMAGE)
.with_name(f"testcontainers-ryuk-{SESSION_ID}")
.with_exposed_ports(8080)
.with_volume_mapping(RYUK_DOCKER_SOCKET, "/var/run/docker.sock", "rw")
.with_kwargs(privileged=RYUK_PRIVILEGED)
.start()
)
wait_for_logs(Reaper._container, r".* Started!")

container_host = Reaper._container.get_container_host_ip()
container_port = int(Reaper._container.get_exposed_port(8080))

Reaper._socket = socket()
Reaper._socket.connect((container_host, container_port))
Reaper._socket.send(f"label={LABEL_SESSION_ID}={SESSION_ID}\r\n".encode())

Reaper._instance = Reaper()

return Reaper._instance
22 changes: 22 additions & 0 deletions core/tests/test_ryuk.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from testcontainers.core.reaper import Reaper
from testcontainers.core.container import DockerContainer
from testcontainers.core.waiting_utils import wait_for_logs


def test_wait_for_reaper():
container = DockerContainer("hello-world").start()
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 1 .*", timeout=15)

Reaper.delete_instance()


def test_container_without_ryuk():
with DockerContainer("hello-world").with_auto_remove(False) as container:
wait_for_logs(container, "Hello from Docker!")
assert Reaper._instance is None