Skip to content
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

feat(core)!: add support for tc.host and de-prioritise docker:dind #388

Merged
merged 8 commits into from
Mar 1, 2024

Conversation

kiview
Copy link
Member

@kiview kiview commented Oct 20, 2023

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

  • To prioritise tc.host this PR prioritised having that over true docker:dind use cases
  • This means we stopped trying to automatically infer the container host IP when running inside a docker:dind container
  • When using -v /var/run/docker.sock:/var/run/docker.sock you should be unaffected since your containers run on the original host

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://"):
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

@totallyzen totallyzen changed the title Add support for tc.host feat(core): add support for tc.host Feb 29, 2024
@kiview kiview requested a review from totallyzen March 1, 2024 11:00
@totallyzen totallyzen changed the title feat(core): add support for tc.host feat(core)!: add support for tc.host to support Testcontainers Desktop Mar 1, 2024
@totallyzen totallyzen changed the title feat(core)!: add support for tc.host to support Testcontainers Desktop feat(core)!: add support for tc.host and de-prioritise docker:dind Mar 1, 2024
Copy link
Collaborator

@totallyzen totallyzen left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines -26 to -34


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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

private function -> moved to the bottom of this file

@totallyzen totallyzen merged commit 2db8e6d into main Mar 1, 2024
7 checks passed
@totallyzen totallyzen deleted the tc-host-support branch March 1, 2024 16:48
totallyzen pushed a commit that referenced this pull request Mar 6, 2024
🤖 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants