-
Notifications
You must be signed in to change notification settings - Fork 374
feat: reusable containers #636
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
Open
matthiasschaub
wants to merge
27
commits into
testcontainers:main
Choose a base branch
from
GIScience:reusable_containers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+225
−3
Open
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
a933873
feat: reusable containers
matthiasschaub f0e2bc7
docs: add documentation about reusable containers
matthiasschaub 08e33ba
test: additional testcase for reusable containers
matthiasschaub d2a83bc
test: add newlines for better readability
matthiasschaub c781606
warn user if ryuk is disabled but with_reuse used
matthiasschaub dd429e7
docs: fix code highlighting
matthiasschaub e87e782
fix: use Union instead of | for type hint
matthiasschaub c656660
refactor: remove TODO comment
matthiasschaub efb1265
docs: update section on reusable containers
matthiasschaub d4445d6
feat(reuse): do not change contract of stop method
matthiasschaub 1ea9ed1
feat(reuse): do not create Ryuk cleanup instance
matthiasschaub ea6fec7
refactor: move hash generation into if clause
matthiasschaub 7c1e8e7
Merge remote-tracking branch 'origin/main' into reusable_containers
matthiasschaub 2113561
Merge branch 'main' into reusable_containers
matthiasschaub 0615c29
Merge branch 'main' into reusable_containers
matthiasschaub 23e436a
Merge branch 'main' into reusable_containers
matthiasschaub 2bfb36d
fix: revert accidental removal of @property
matthiasschaub 6796a9a
Merge remote-tracking branch 'testcontainers/main' into reusable_cont…
matthiasschaub 3050fee
Merge remote-tracking branch 'testcontainers/main' into reusable_cont…
matthiasschaub beac036
Merge branch 'main' into reusable_containers
matthiasschaub 35e0599
tests: access tc properties through function call
matthiasschaub e84fe02
refactor: use crc32 instead of sha1 to compute hash
matthiasschaub 3a31883
refactor: remove remnant code from past merge
matthiasschaub 4c0ba18
docs: more detailed explanation on how to use reusable containers
matthiasschaub 3894eb1
fix: check if self._kwargs is None before using it
matthiasschaub e690ae9
Merge remote-tracking branch 'origin/main' into reusable_containers
matthiasschaub 3367a80
fix: restore deleted method after resolving merge conflict
matthiasschaub File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,6 +134,10 @@ def docker_auth_config(self, value: str) -> None: | |
| def tc_properties_get_tc_host(self) -> Union[str, None]: | ||
| return self.tc_properties.get("tc.host") | ||
|
|
||
| def tc_properties_testcontainers_reuse_enable(self) -> bool: | ||
| enabled = self.tc_properties.get("testcontainers.reuse.enable") | ||
|
Author
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. Mirroring the Java implementation, I would like to make this configurable via environment variable as well: https://java.testcontainers.org/features/reuse/#how-to-use-it What do you think? |
||
| return enabled == "true" | ||
|
|
||
| @property | ||
| def ryuk_privileged(self) -> bool: | ||
| if self._ryuk_privileged is not None: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| from time import sleep | ||
|
|
||
| from docker.models.containers import Container | ||
|
|
||
| from testcontainers.core.config import testcontainers_config | ||
| from testcontainers.core.container import DockerContainer | ||
| from testcontainers.core.docker_client import DockerClient | ||
| from testcontainers.core.waiting_utils import wait_for_logs | ||
| from testcontainers.core.container import Reaper | ||
|
|
||
|
|
||
| def test_docker_container_reuse_default(): | ||
| # Make sure Ryuk cleanup is not active from previous test runs | ||
| Reaper.delete_instance() | ||
|
|
||
| container = DockerContainer("hello-world").start() | ||
| wait_for_logs(container, "Hello from Docker!") | ||
|
|
||
| assert container._reuse == False | ||
| assert testcontainers_config.tc_properties_testcontainers_reuse_enable() == False | ||
| assert Reaper._socket is not None | ||
|
|
||
| container.stop() | ||
| containers = DockerClient().client.containers.list(all=True) | ||
| assert container._container is not None | ||
| assert container._container.id not in [container.id for container in containers] | ||
|
|
||
|
|
||
| def test_docker_container_with_reuse_reuse_disabled(caplog): | ||
| # Make sure Ryuk cleanup is not active from previous test runs | ||
| Reaper.delete_instance() | ||
|
|
||
| container = DockerContainer("hello-world").with_reuse().start() | ||
| wait_for_logs(container, "Hello from Docker!") | ||
|
|
||
| assert container._reuse == True | ||
| assert testcontainers_config.tc_properties_testcontainers_reuse_enable() == False | ||
| assert ("Reuse was requested (`with_reuse`) but the environment does not support the ") in caplog.text | ||
| assert Reaper._socket is not None | ||
|
|
||
| container.stop() | ||
| containers = DockerClient().client.containers.list(all=True) | ||
| assert container._container is not None | ||
| assert container._container.id not in [container.id for container in containers] | ||
|
|
||
|
|
||
| def test_docker_container_without_reuse_reuse_enabled(monkeypatch): | ||
| # Make sure Ryuk cleanup is not active from previous test runs | ||
| Reaper.delete_instance() | ||
|
|
||
| tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"} | ||
| monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock) | ||
|
|
||
| container = DockerContainer("hello-world").start() | ||
| wait_for_logs(container, "Hello from Docker!") | ||
|
|
||
| assert container._reuse == False | ||
| assert testcontainers_config.tc_properties_testcontainers_reuse_enable() == True | ||
| assert Reaper._socket is not None | ||
|
|
||
| container.stop() | ||
| containers = DockerClient().client.containers.list(all=True) | ||
| assert container._container is not None | ||
| assert container._container.id not in [container.id for container in containers] | ||
|
|
||
|
|
||
| def test_docker_container_with_reuse_reuse_enabled(monkeypatch): | ||
| # Make sure Ryuk cleanup is not active from previous test runs | ||
| Reaper.delete_instance() | ||
|
|
||
| tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"} | ||
| monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock) | ||
|
|
||
| container = DockerContainer("hello-world").with_reuse().start() | ||
| wait_for_logs(container, "Hello from Docker!") | ||
|
|
||
| assert Reaper._socket is None | ||
|
|
||
| containers = DockerClient().client.containers.list(all=True) | ||
| assert container._container is not None | ||
| assert container._container.id in [container.id for container in containers] | ||
|
|
||
| # Cleanup after keeping container alive (with_reuse) | ||
| container.stop() | ||
|
|
||
|
|
||
| def test_docker_container_with_reuse_reuse_enabled_same_id(monkeypatch): | ||
| # Make sure Ryuk cleanup is not active from previous test runs | ||
| Reaper.delete_instance() | ||
|
|
||
| tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"} | ||
| monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock) | ||
|
|
||
| container_1 = DockerContainer("hello-world").with_reuse().start() | ||
| assert container_1._container is not None | ||
| id_1 = container_1._container.id | ||
| container_2 = DockerContainer("hello-world").with_reuse().start() | ||
| assert container_2._container is not None | ||
| id_2 = container_2._container.id | ||
| assert Reaper._socket is None | ||
| assert id_1 == id_2 | ||
| # Cleanup after keeping container alive (with_reuse) | ||
| container_1.stop() | ||
| # container_2.stop() is not needed since it is the same as container_1 | ||
|
|
||
|
|
||
| def test_docker_container_labels_hash_default(): | ||
| # w/out reuse | ||
| with DockerContainer("hello-world") as container: | ||
| assert container._container is not None | ||
| assert "hash" not in container._container.labels.keys() | ||
|
|
||
|
|
||
| def test_docker_container_labels_hash(monkeypatch): | ||
| tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"} | ||
| monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock) | ||
| expected_hash = "380705419" | ||
| with DockerContainer("hello-world").with_reuse() as container: | ||
| assert container._container is not None | ||
| assert container._container.labels["hash"] == expected_hash | ||
|
|
||
|
|
||
| def test_docker_client_find_container_by_hash_not_existing(): | ||
| with DockerContainer("hello-world"): | ||
| assert DockerClient().find_container_by_hash("foo") == None | ||
|
|
||
|
|
||
| def test_docker_client_find_container_by_hash_existing(monkeypatch): | ||
| tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"} | ||
| monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock) | ||
| with DockerContainer("hello-world").with_reuse() as container: | ||
| assert container._container is not None | ||
| hash_ = container._container.labels["hash"] | ||
| found_container = DockerClient().find_container_by_hash(hash_) | ||
| assert isinstance(found_container, Container) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should the user be warned that by using this feature, containers need to be removed manually? (That this feature should not be used in a CI)
Also, do we need to make clear how this feature works (explaining the hash in use). -> If a container's run configuration changes, the hash changes and a new container will be used.
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.
seems like you have added these comments to the doc, i think that is fine. the hash would be great to add as users would benefit from knowing exactly what is hashed.
self.image,self._command,self.env,self.ports,self._name,self.volumes,str(tuple(sorted(self._kwargs.items()))),- this may fail and why i want to have this be tucked away inside an obviously readableifblockUh oh!
There was an error while loading. Please reload this page.
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.
Note to self: Maybe use something like https://github.com/knowsuchagency/picocache/blob/main/picocache/utils.py#L9 for making the hash key
Uh oh!
There was an error while loading. Please reload this page.
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.
Before running
str(tuple(sorted(self._kwargs.items()))),self._kwargsis now beeing checked forNone-> 3894eb1