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

fix(core): DinD issues #141, #329 #368

Merged
merged 14 commits into from
Mar 20, 2024

Conversation

deeninetyone
Copy link
Contributor

Fix #141 - find IP from custom network if the container is not using the default network
Close #329 - This seems fixed in the underlying docker libraries.
Improve support for Docker in Docker running on a custom network, by attempting to find the right custom network and use it for new containers. This adds support for using testcontainers-python running the GitHub Actions Runner Controller to run self-hosted actions runners on prem, when you run your workflows in containers.

@sean-hernon
Copy link

Any movement on merging this? I am doing something similar, to index by the custom network id instead of bridge. Would be nice to delete my own workarounds.

@damianoct
Copy link

any updates? we need to have testcontainers working properly in DIND environments

@VerdantForge
Copy link

Also interested in updates on this issue

@covatic-john
Copy link

covatic-john commented Dec 8, 2023

Trying to get docker container in container running in bitbucket and struggling with it
I am sure that this is to do with the sock or internal host. Any pointers please

self = <docker.transport.unixconn.UnixHTTPConnection object at 0x7f4b610548e0>

def connect(self):

    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)

    sock.settimeout(self.timeout)
  sock.connect(self.unix_socket)

E FileNotFoundError: [Errno 2] No such file or directory

/usr/local/lib/python3.10/site-packages/docker/transport/unixconn.py:27: FileNotFoundError

@alexanderankin
Copy link
Member

i was able to run the test suite in dind while testing compose. what does dind mean for folks here? pardon my ignorance - but want to clarify so i can understand how to manually test. i do see that there is a test here. i am adding 4.1.0 because since we have code here, we can test and merge with relatively low consequences

@alexanderankin
Copy link
Member

Maintainers are allowed to edit this pull request.

thank you, i will try to rebase this to get it to apply cleanly if i am able to make sense of the changes.

@deeninetyone
Copy link
Contributor Author

Maintainers are allowed to edit this pull request.

thank you, i will try to rebase this to get it to apply cleanly if i am able to make sense of the changes.

It fixes the case where the container you are running in has a custom network (not the default one). In this case, the library needs to try to find the correct network and join the test container to the same network, or the container you're running in cannot contact the test container.

@alexanderankin alexanderankin changed the title 141 329 dind fixes fix(core): DinD issues #141, #329 Mar 20, 2024
@totallyzen
Copy link
Collaborator

totallyzen commented Mar 20, 2024

@deeninetyone thanks for the contrib! Looks promising and makes a lot of sense!

I have two asks:

  • can you include tc.host in the "user knows best" check? It's been attempted in the init function so you might want to touch that too to make sure we're doing this in a single place
  • can you make the new addition more structured around functions? these nested if/for/if/for make the code hard to read.

For the latter, my suggestion would be that you isolate the try-catch block into a function and instead of modifying kwargs from deep within the loop

  • your new function would be something like def find_host_network() -> Optional[str]:
  • then you'd do "user knows best, OR try to find host network, and if found one, alter the kwargs

EDIT: if you don't have the time, just say so, I'll backport my suggestion (I have no intention of stealing your thunder)

EDIT 2:

Maintainers are allowed to edit this pull request.

Right 😂

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.

LGTM, thanks for the fix! ❤️

@totallyzen totallyzen merged commit b10d916 into testcontainers:main Mar 20, 2024
7 checks passed
alexanderankin pushed a commit that referenced this pull request Mar 24, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.2.0](testcontainers-v4.1.0...testcontainers-v4.2.0)
(2024-03-24)


### Features

* support influxdb
([#413](#413))
([13742a5](13742a5))


### Bug Fixes

* **arangodb:** tests to pass on ARM CPUs - change default image to
3.11.x where ARM image is published
([#479](#479))
([7b58a50](7b58a50))
* **core:** DinD issues
[#141](#141),
[#329](#329)
([#368](#368))
([b10d916](b10d916))
* **core:** raise an exception when docker compose fails to start
[#258](#258)
([#485](#485))
([d61af38](d61af38))
* **core:** use auto_remove=True with reaper instance
([#499](#499))
([274a400](274a400))
* **docs:** update the non-existent main.yml badge
([#493](#493))
([1d10c1c](1d10c1c))
* Fix the return type of `DockerContainer.get_logs`
([#487](#487))
([cd72f68](cd72f68))
* **keycloak:** tests on aarch64, use image from [jboss -&gt; quay],
change supported version [16+ -> 18+]
([#480](#480))
([5758310](5758310))
* **postgres:** doctest
([#473](#473))
([c9c6f92](c9c6f92))
* read the docs build works again
([#496](#496))
([dfd1781](dfd1781))
* readthedocs build - take 1
([#495](#495))
([b3b9901](b3b9901))

---
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>
bearrito pushed a commit to bearrito/testcontainers-python that referenced this pull request Mar 30, 2024
…ntainers#368)

Fix testcontainers#141 - find IP from custom network if the container is not using the
default network
Close testcontainers#329 - This seems fixed in the underlying docker libraries.
Improve support for Docker in Docker running on a custom network, by
attempting to find the right custom network and use it for new
containers. This adds support for using testcontainers-python running
the GitHub Actions Runner Controller to run self-hosted actions runners
on prem, when you run your workflows in containers.

---------

Co-authored-by: Dee Moore <[email protected]>
Co-authored-by: David Ankin <[email protected]>
Co-authored-by: Balint Bartha <[email protected]>
bearrito pushed a commit to bearrito/testcontainers-python that referenced this pull request Mar 30, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.2.0](testcontainers/testcontainers-python@testcontainers-v4.1.0...testcontainers-v4.2.0)
(2024-03-24)


### Features

* support influxdb
([testcontainers#413](testcontainers#413))
([13742a5](testcontainers@13742a5))


### Bug Fixes

* **arangodb:** tests to pass on ARM CPUs - change default image to
3.11.x where ARM image is published
([testcontainers#479](testcontainers#479))
([7b58a50](testcontainers@7b58a50))
* **core:** DinD issues
[testcontainers#141](testcontainers#141),
[testcontainers#329](testcontainers#329)
([testcontainers#368](testcontainers#368))
([b10d916](testcontainers@b10d916))
* **core:** raise an exception when docker compose fails to start
[testcontainers#258](testcontainers#258)
([testcontainers#485](testcontainers#485))
([d61af38](testcontainers@d61af38))
* **core:** use auto_remove=True with reaper instance
([testcontainers#499](testcontainers#499))
([274a400](testcontainers@274a400))
* **docs:** update the non-existent main.yml badge
([testcontainers#493](testcontainers#493))
([1d10c1c](testcontainers@1d10c1c))
* Fix the return type of `DockerContainer.get_logs`
([testcontainers#487](testcontainers#487))
([cd72f68](testcontainers@cd72f68))
* **keycloak:** tests on aarch64, use image from [jboss -&gt; quay],
change supported version [16+ -> 18+]
([testcontainers#480](testcontainers#480))
([5758310](testcontainers@5758310))
* **postgres:** doctest
([testcontainers#473](testcontainers#473))
([c9c6f92](testcontainers@c9c6f92))
* read the docs build works again
([testcontainers#496](testcontainers#496))
([dfd1781](testcontainers@dfd1781))
* readthedocs build - take 1
([testcontainers#495](testcontainers#495))
([b3b9901](testcontainers@b3b9901))

---
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
Projects
None yet
8 participants