feat(core)!: add support for tc.host and de-prioritise docker:dind - #388
feat(core)!: add support for tc.host and de-prioritise docker:dind#388totallyzen merged 8 commits into
Conversation
Sorry, something went wrong.
| return self.get_docker_client().bridge_ip(self._container.id) | ||
| return gateway_ip | ||
| # # check testcontainers itself runs inside docker container | ||
| # if inside_container() and not os.getenv("DOCKER_HOST") and not host.startswith("http://"): |
There was a problem hiding this comment.
Had to do it, to get things working with Testcontainers Cloud within Codespaces (which runs inside a container). I also thought part of this code duplicate what is done in docker_client.py.
Sorry, something went wrong.
totallyzen
left a comment
There was a problem hiding this comment.
After some cleanup of the "read file" mechanism, it's LGTM.
Also updated the PR body to add details on why this is potentially a breaking change and what to do about it if you are affected.
Sorry, something went wrong.
|
|
||
|
|
||
| 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) |
There was a problem hiding this comment.
private function -> moved to the bottom of this file
Sorry, something went wrong.
🤖 I have created a release *beep* *boop* --- ## [4.0.0](testcontainers-v3.7.1...testcontainers-v4.0.0) (2024-03-06) ### Release Notes The breaking changes are the ones we were able to easily track. If you spot any new issues between `3.7.1` and `4.0.0`, please do report it and we'll do our best to fix everything. The release is now Some kudos from @totallyzen to folks who helped a great deal in starting things again: - kudos to @alexanderankin for his contribution on #426 - kudos to @jankatins for feedback on various PRs including - kudos to @max-pfeiffer and @bearrito for their contributions as well ### ⚠ BREAKING CHANGES * **compose:** implement compose v2 with improved typing ([#426](#426)) * **core:** add support for `tc.host` and de-prioritise `docker:dind` ([#388](#388)) ### Features * **build:** use poetry and organise modules ([#408](#408)) ([6c69583](6c69583)) * **compose:** allow running specific services in compose ([f61dcda](f61dcda)) * **compose:** implement compose v2 with improved typing ([#426](#426)) ([5356caf](5356caf)) * **core:** add support for `tc.host` and de-prioritise `docker:dind` ([#388](#388)) ([2db8e6d](2db8e6d)) * **redis:** support AsyncRedisContainer ([#442](#442)) ([cc4cb37](cc4cb37)) * **release:** automate release via release-please ([#429](#429)) ([30f859e](30f859e)) ### Bug Fixes * Added URLError to exceptions to wait for in elasticsearch ([0f9ad24](0f9ad24)) * **build:** add `pre-commit` as a dev dependency to simplify local dev and CI ([#438](#438)) ([1223583](1223583)) * **build:** early exit strategy for modules ([#437](#437)) ([7358b49](7358b49)) * changed files breaks on main ([#422](#422)) ([3271357](3271357)) * flaky garbage collection resulting in testing errors ([#423](#423)) ([b535ea2](b535ea2)) * rabbitmq readiness probe ([#375](#375)) ([71cb75b](71cb75b)) * **release:** prove that the release process updates the version ([#444](#444)) ([87b5873](87b5873)) * test linting issue ([427c9b8](427c9b8)) ### Documentation * Sphinx - Add title to each doc page ([#443](#443)) ([750e12a](750e12a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
We just got bitten by this change when we upgraded from version 3.5.0 to 4.8.1. We have a unit test which spins up a DockerContainer which has an application that exposes a REST end point over HTTP. We connect to this from the unit test to check it's available. So we used to do something like this: with DockerContainer(container).with_exposed_ports(8083) as container:
url = f'http://{container.get_container_host_ip()}:{container.get_exposed_port(8083)}/'
requests.get(url).raise_for_status()
When we run this in our CI pipeline it is using Docker within Docker and the URL generated above is http://172.17.0.1:32769/ for version 3.5.0. When we upgraded to 4.8.1 the URL changes to be of the form http://localhost:32769 which doesn't resolve and the test fails with a ConnectionRefusedError. I tried various forms of setting a value for tc.host such as tc.host=http://172.17.0.1:32769, tc.host=tcp://172.17.0.1:1234 and they all resulted in connection errors. Eventually I resorted to moving the old behaviour that I found on this PR into my unit test like so if inside_container():
host = container.get_docker_client().gateway_ip(container.get_wrapped_container().id)
else:
host = container.get_container_host_ip()
url = f'http://{host}:{container.get_exposed_port(8083)}/'
and this works. Am I doing something completely wrong or is this expected behaviour? |
Sorry, something went wrong.
|
@massdosage Yeah, sorry for this breaking change. Back then the project was in a bit of a hanging state, with us taking over from the older maintainer, and in general, we were unsure how to go forward with this change. I think for now, your workaround is very clever and thanks for sharing it, we intend to integrate a change like this acutally into the library directly again, to avoid the users having to code around it. Would you be willing to look into contributing the fix to the library, since I think you understand the issue well and also have a good reproducer at hand? (cc @alexanderankin @balint-backmaker ). |
Sorry, something went wrong.
|
Ah sorry @massdosage ,I see the community (@CarliJoy) is already working on it, maybe you can try to stay in the loop there: |
Sorry, something went wrong.
What does this PR do?
Adds support for reading the tc.host property from the ~/.testcontainers.properties file. This config will have the highest priority for configuring the Docker client.
Why is it important
This brings Testcontainers Desktop support to testcontainers-python and aligns this language with the TestcontainersHostStrategy found it testcontainers-java.
Misc
I wasn't able to get a working testcontainers-python development environment set up on my M2 MacBook. This seems to be related to some kind of Cython 3.0.0 issue, that I don't fully understand and trying to fix it breaks the installation of further dependencies (like pymssql). So I was only able to test it in an Ubuntu Codespaces environment (also required some weird Cython workarounds).
Breaking Changes