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(registry): compatibility with WSL #2705

Merged
merged 4 commits into from
Aug 19, 2024

Conversation

stevenh
Copy link
Collaborator

@stevenh stevenh commented Aug 8, 2024

Use local IP instead of localhost to access registry container to address failures under WSL where the docker daemon can't access localhost.

Refactor tests to use helpers and require reducing code and simplifying the flow.

Add a default image which can be used be consumers.

Add HostAddress which returns host : port compatible with WSL.

Add SetDockerAuthConfig and DockerAuthConfig methods that can be used to easily configure authentication via DOCKER_AUTH_CONFIG environment variable.

Fix container clean up which was missed in a number of cases.

@stevenh stevenh requested a review from a team as a code owner August 8, 2024 18:32
Copy link

netlify bot commented Aug 8, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit c1ad9da
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/66c3346958982d0008145909
😎 Deploy Preview https://deploy-preview-2705--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Added some comments, this is almost ready to be merged, although I'd like to discuss more with the team about the credentialStore being pinned to "desktop".

In any case, thanks in advance for this fix!

modules/registry/examples_test.go Outdated Show resolved Hide resolved
modules/registry/registry.go Outdated Show resolved Hide resolved
modules/registry/registry.go Outdated Show resolved Hide resolved
modules/registry/registry.go Outdated Show resolved Hide resolved
modules/registry/registry.go Outdated Show resolved Hide resolved
modules/registry/registry.go Outdated Show resolved Hide resolved
@stevenh
Copy link
Collaborator Author

stevenh commented Aug 14, 2024

Depends on #2727

@stevenh stevenh marked this pull request as draft August 14, 2024 17:28
@stevenh stevenh force-pushed the fix/registry-wsl branch 3 times, most recently from bc83838 to 6334467 Compare August 16, 2024 14:29
Use local IP instead of localhost to access registry container
to address failures under WSL where the docker daemon can't
access localhost.

Refactor tests to use helpers and require reducing code and
simplifying the flow.

Add a default image which can be used be consumers.

Add HostAddress which returns host : port compatible with
WSL.

Add SetDockerAuthConfig and DockerAuthConfig methods that can
be used to easily configure authentication via DOCKER_AUTH_CONFIG
environment variable.

Fix container clean up which was missed in a number of case.
@stevenh stevenh marked this pull request as ready for review August 16, 2024 15:58
@stevenh stevenh requested a review from mdelapenya August 16, 2024 15:58
Copy link
Collaborator Author

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Needs a few tweaks

docs/modules/registry.md Outdated Show resolved Hide resolved
docs/modules/registry.md Outdated Show resolved Hide resolved
docs/modules/registry.md Outdated Show resolved Hide resolved
Co-authored-by: Steven Hartland <stevenmhartland@gmail.com>
Copy link
Member

@mdelapenya mdelapenya 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!

@mdelapenya mdelapenya merged commit aaa88d2 into testcontainers:main Aug 19, 2024
112 checks passed
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Aug 19, 2024
@mdelapenya mdelapenya self-assigned this Aug 19, 2024
mdelapenya added a commit that referenced this pull request Aug 22, 2024
* main:
  chore(deps): bump github/codeql-action from 3.24.9 to 3.25.15 (#2677)
  fix: use of log.Fatal in main (#2739)
  chore: prepare for next minor development cycle (0.34.0)
  chore: use new version (v0.33.0) in modules and examples
  fix: authentication tests on WSL (#2706)
  fix(registry): compatibility with WSL (#2705)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Changes that do not impact the existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants