Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion internal/core/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package core

import (
"context"
"fmt"
"path/filepath"

"github.com/docker/docker/client"
Expand All @@ -14,7 +15,10 @@ import (
func NewClient(ctx context.Context, ops ...client.Opt) (*client.Client, error) {
tcConfig := config.Read()

dockerHost := MustExtractDockerHost(ctx)
dockerHost, err := extractDockerHost(ctx)
if err != nil {
return nil, fmt.Errorf("extract docker host: %w", err)
}

opts := []client.Opt{client.FromEnv, client.WithAPIVersionNegotiation()}
if dockerHost != "" {
Expand Down
35 changes: 31 additions & 4 deletions internal/core/docker_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var (
ErrSocketNotFoundInPath = errors.New("docker socket not found in " + DockerSocketPath)
// ErrTestcontainersHostNotSetInProperties this error is specific to Testcontainers
ErrTestcontainersHostNotSetInProperties = errors.New("tc.host not set in ~/.testcontainers.properties")
ErrDockerSocketNotSetInDockerContext = errors.New("socket not found in docker context")
)

var (
Expand Down Expand Up @@ -73,7 +74,7 @@ var dockerHostCheck = func(ctx context.Context, host string) error {
return nil
}

// MustExtractDockerHost Extracts the docker host from the different alternatives, caching the result to avoid unnecessary
// MustExtractDockerHost extracts the docker host from the different alternatives, caching the result to avoid unnecessary
// calculations. Use this function to get the actual Docker host. This function does not consider Windows containers at the moment.
// The possible alternatives are:
//
Expand Down Expand Up @@ -118,13 +119,26 @@ func MustExtractDockerSocket(ctx context.Context) string {
return dockerSocketPathCache
}

// extractDockerHost Extracts the docker host from the different alternatives, without caching the result.
// This internal method is handy for testing purposes.
// ExtractDockerHost Extracts the docker host from the different alternatives, without caching the result.
// Use this function to get the actual Docker host. This function does not consider Windows containers at the moment.
// The possible alternatives are:
//
// 1. Docker host from the "tc.host" property in the ~/.testcontainers.properties file.
// 2. DOCKER_HOST environment variable.
// 3. Docker host from context.
// 4. Docker host from the default docker socket path, without the unix schema.
// 5. Docker host from the "docker.host" property in the ~/.testcontainers.properties file.
// 6. Rootless docker socket path.
func ExtractDockerHost(ctx context.Context) (string, error) {
return extractDockerHost(ctx)
}

func extractDockerHost(ctx context.Context) (string, error) {
dockerHostFns := []func(context.Context) (string, error){
testcontainersHostFromProperties,
dockerHostFromEnv,
dockerHostFromContext,
dockerHostFromDockerContext,
dockerSocketPath,
dockerHostFromProperties,
rootlessDockerSocketPath,
Expand Down Expand Up @@ -227,12 +241,14 @@ func isHostNotSet(err error) bool {
case errors.Is(err, ErrTestcontainersHostNotSetInProperties),
errors.Is(err, ErrDockerHostNotSet),
errors.Is(err, ErrDockerSocketNotSetInContext),
errors.Is(err, ErrDockerSocketNotSetInDockerContext),
errors.Is(err, ErrDockerSocketNotSetInProperties),
errors.Is(err, ErrSocketNotFoundInPath),
errors.Is(err, ErrXDGRuntimeDirNotSet),
errors.Is(err, ErrRootlessDockerNotFoundHomeRunDir),
errors.Is(err, ErrRootlessDockerNotFoundHomeDesktopDir),
errors.Is(err, ErrRootlessDockerNotFoundRunDir):
errors.Is(err, ErrRootlessDockerNotFoundRunDir),
errors.Is(err, ErrRootlessDockerNotFound):
return true
default:
return false
Expand Down Expand Up @@ -262,6 +278,17 @@ func dockerHostFromContext(ctx context.Context) (string, error) {
return "", ErrDockerSocketNotSetInContext
}

// dockerHostFromDockerContext returns the docker host from the DOCKER_CONTEXT environment variable, if it's not empty
func dockerHostFromDockerContext(_ context.Context) (string, error) {
// exec `docker context inspect -f='{{.Endpoints.docker.Host}}'`
cmd := exec.Command("docker", "context", "inspect", "-f", "{{.Endpoints.docker.Host}}")
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: we have an ongoing PR addressing the docker context support. Although shelling out to the docker CLI is easy, we disregard its usage to avoid handling exit codes, and parsing responses.

Copy link
Author

Choose a reason for hiding this comment

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

That is an wild PR my guy! 😂.

The user-facing panic is unfortunate but secondary to not respecting the current docker context. If you have another fix for this incoming I can just close this PR.

I don't completely understand that rationale -- I'd buy not wanting to rely on on a docker binary on PATH but this package has lots of passing on failures if things don't work -- but that's fine you can do what you think is best.

I find layers of complexity on top of test code need to be abundantly clear clear that they are worth the complexity.
Obtaining the current docker host from the current context being more than a 30 line change is a big red flag IMO.
Regardless, I don't really mind how this gets fixed but it impacts me and folks contributing things I'm working on.

So far my experience with this package has driven me to write first a wrapper and then a (IMO) both much smaller, simpler, and better implementation that is much more natively tied into the go testing library. I haven't made it public but it will be at http://github.com/tmc/testctr at some point soon -- it has a (completely optional) testcontainers-go backend implementation.

output, err := cmd.CombinedOutput()
if err != nil {
return "", fmt.Errorf("%w: %w", ErrDockerSocketNotSetInDockerContext, err)
}
return strings.TrimSpace(string(output)), nil
}

// dockerHostFromProperties returns the docker host from the ~/.testcontainers.properties file, if it's not empty
func dockerHostFromProperties(_ context.Context) (string, error) {
cfg := config.Read()
Expand Down
6 changes: 5 additions & 1 deletion provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,13 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error
return nil, err
}

host, err := core.ExtractDockerHost(ctx)
if err != nil {
return nil, fmt.Errorf("failed to extract docker host: %w", err)
}
return &DockerProvider{
DockerProviderOptions: o,
host: core.MustExtractDockerHost(ctx),
host: host,
client: c,
config: config.Read(),
}, nil
Expand Down
Loading