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

Extend the builder to support using the DOCKER_HOST environment variable #463

Closed
Xitric opened this issue May 30, 2022 · 6 comments
Closed
Labels
duplicate This issue or pull request already exists enhancement New feature or request
Milestone

Comments

@Xitric
Copy link
Contributor

Xitric commented May 30, 2022

Is your feature request related to a problem? Please describe.

We have begun looking into test containers in our integration and performance testing environments, and for this reason we are collecting some common extensions in a central project shared among our testing projects. Whenever possible, we would prefer such generic extensions to just be a part of this upstream repository.

One such thing is the ability to automatically honour the DOCKER_HOST environment variable. We use this in every test project, since our CI/CD environment exposes the Docker daemon via TCP, while our local development environments do not.

Describe the solution you'd like

We would like if handling the DOCKER_HOST environment variable was something that was baked into the dotnet-testcontainers project, since we think it is a good candidate for an upstream contribution from our internal extension library.

To avoid introducing a breaking change, we considered if it would be best to introduce an additional builder method such as .WithDockerEndpointFromEnvironment() or perhaps .WithDockerHost().

One could also consider supporting a parameterless call to .WithDockerEndpoint() and fall back to using the environment variable if no explicit endpoint is set - although that could be confusing behaviour I guess. I would like some other thoughts on this.

Describe alternatives you've considered

Currently we just have a small chunk of code in an internally shared project that does the magic for us:

var dockerHost = Environment.GetEnvironmentVariable("DOCKER_HOST");
if (dockerHost != null)
{
  builder = builder.WithDockerEndpoint(dockerHost);
}

This is perfectly fine, but we figured it would be a good candidate for something in the upstream repository.

Additional context

I see there was some discussion about the DOCKER_HOST environment variable in #370, but nothing has been contributed so far from what I can tell. Using this environment variable alone is also out of scope for #370, so I guess a separate pull request would be in order.

@HofmeisterAn
Copy link
Collaborator

Using this environment variable alone is also out of scope for #370

Actually, it's part of #370, #457. There will be an interface in the future that offers different ways to authenticate against Docker endpoints. One implementation would just consider the DOCKER_HOST environment variable.

You can change the Docker endpoint for all configurations with TestcontainersSettings.OS too. Set the property right at the start before creating any builder resources: #414 (comment).

Anyway, if it's more convenient for you, we can extend the builder, but I think there isn't much benefit. I really hope we get #370 implemented soon.

@HofmeisterAn HofmeisterAn added the enhancement New feature or request label May 30, 2022
@HofmeisterAn HofmeisterAn added this to the 1.6.0 milestone May 30, 2022
@Xitric
Copy link
Contributor Author

Xitric commented May 31, 2022

Thank you for the clarification. I was not sure what to expect from the state of #370.

What you are saying, is that there will be an "authentication" implementation that simply respects the DOCKER_HOST environment variable (as in, it is planned / underway), or that it could be implemented in that way?

If there is a standard implementation that uses the DOCKER_HOST environment variable to identify and talk to the Docker daemon, that certainly satisfies our requirement.

@HofmeisterAn
Copy link
Collaborator

HofmeisterAn commented Jun 1, 2022

What you are saying, is that there will be an "authentication" implementation that simply respects the DOCKER_HOST environment variable

Yes, there will be an implementation that detects the env variables (even TLS, etc.) and configures the builder automatic. Then, the above suggest code is not necessary anymore. I hope I can bring in myself a bit more and push this feature. I know many ask for this feature. It will still be possible to override the automatic applied configuration.

@Xitric
Copy link
Contributor Author

Xitric commented Jun 2, 2022

@HofmeisterAn In that case, you can go ahead and close this issue if you wish, or leave it open for tracking. I am really happy to at least hear that this support is coming!

@HofmeisterAn HofmeisterAn added the duplicate This issue or pull request already exists label Jun 4, 2022
@HofmeisterAn
Copy link
Collaborator

I prepared most parts: #370 (comment). It shouldn't be much work anymore to set the endpoint automatic from the environment variable (DOCKER_HOST).

@HofmeisterAn
Copy link
Collaborator

DOCKER_HOST is now automatically considered by 1.6.0-beta.2258.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants