Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 378
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
7c09ccc957b863fc30fc1e523dba543ce3ea487dc438a9d5aa214089fdaee73d4ddcba3711fcf89113a3101fd118802f397449d146c1650d3e2c66cc401df7581e38b5b1988e6c2454377251bb4c37960dc66658dd18c011e2f7a1a4827f2839690707d4849c239d5222e03eb6dce010fed8d49a51098351ccbc4a0d8487e9974b3904ae1c74c82afed47f575772dcada6f3d881cb09eFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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.5.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") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| from typing import Optional | ||
| from uuid import uuid4 | ||
| from testcontainers.core.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[str, str]]) -> dict[str, str]: | ||
| if labels is None: | ||
| labels = {} | ||
Comment on lines
+12
to
+13
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you like you can add the following labels by default too: ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| from testcontainers.core import container | ||
| from testcontainers.core.container 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 \d .*", timeout=30) | ||
| Reaper.delete_instance() | ||
| def test_container_without_ryuk(monkeypatch): | ||
| monkeypatch.setattr(container, "RYUK_DISABLED", True) | ||
| with DockerContainer("hello-world") as cont: | ||
| wait_for_logs(cont, "Hello from Docker!") | ||
| assert Reaper._instance is None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we change this default to true to try to preserve backwards compatibility - like in principle we should make it really easy for people to move to the new thing and only really bother everyone else much later on.
like in jvm langs you can do
echo ryuk.disabled=true >> src/test/resources/testcontainers.properties- im not sure of the python equivalent off the bat - maybe we have to stick to some sort of env var stuffThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about setting it to
trueby default and release 4.1 (or 4.2, whichever this one lands in) and then changing it tofalseand release 5.0 straight away? That way we don't force people into using Ryuk on their 4.x builds, but I really think this should be enabled by default, as it is the expected behavior as seen in other implementations.By releasing it in both versions almost simultaneously we give them time to upgrade, and they have the possibility to test out the functionality without major bumping anything.