testcontainers-python icon indicating copy to clipboard operation
testcontainers-python copied to clipboard

fix DinD host detection when using local socket

Open ikravets opened this issue 2 years ago • 7 comments

When using IPC socket (unix/npipe) to communicate with local docker server docker-py implementation rewrites the base url to HTTP, see https://github.com/docker/docker-py/blob/e901eac7a8c5f29c7720eafb9f58c8356cca2324/docker/api/client.py#L143-L168

We cannot rely on URL scheme in order to detect such connections. Instead, we detect connection adapters added by docker-py api client:

  • UnixHTTPAdapter https://github.com/docker/docker-py/blob/e901eac7a8c5f29c7720eafb9f58c8356cca2324/docker/transport/unixconn.py

  • NpipeHTTPAdapter https://github.com/docker/docker-py/blob/e901eac7a8c5f29c7720eafb9f58c8356cca2324/docker/transport/npipeconn.py

As NpipeHTTPAdapter type may not be available outside of Windows we rely on adapter-specific attributes.

ikravets avatar Dec 26 '22 17:12 ikravets

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@928af5a). Click here to learn what that means. The diff coverage is n/a.

:exclamation: Current head afa057e differs from pull request most recent head f9d4508. Consider uploading reports for the commit f9d4508 to get more accurate results

@@           Coverage Diff           @@
##             main     #283   +/-   ##
=======================================
  Coverage        ?   87.30%           
=======================================
  Files           ?       30           
  Lines           ?      843           
  Branches        ?       57           
=======================================
  Hits            ?      736           
  Misses          ?       76           
  Partials        ?       31           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Dec 27 '22 22:12 codecov-commenter

Rebased to the master. @tillahoffmann, any chance for a review?

ikravets avatar Jan 01 '23 08:01 ikravets

rebased to master

ikravets avatar Jan 10 '23 15:01 ikravets

Thank you for these improvements! Are there tests that we could add that fail before the change is introduced to avoid future regressions?

tillahoffmann avatar Jan 10 '23 16:01 tillahoffmann

@tillahoffmann Hi, it's been more than half a year since I've opened this PR. Can we maybe merge this?

ikravets avatar Jul 05 '23 12:07 ikravets

@tillahoffmann a kind ping

ikravets avatar Jul 17 '23 08:07 ikravets