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: avoid double lock in DockerProvider.DaemonHost() #2900

Merged
merged 11 commits into from
Dec 20, 2024
12 changes: 11 additions & 1 deletion docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,11 @@ func (p *DockerProvider) daemonHostLocked(ctx context.Context) (string, error) {
p.hostCache = daemonURL.Hostname()
case "unix", "npipe":
if core.InAContainer() {
ip, err := p.GetGatewayIP(ctx)
defaultNetwork, err := p.ensureDefaultNetworkLocked(ctx)
if err != nil {
return "", fmt.Errorf("ensure default network: %w", err)
}
ip, err := p.getGatewayIP(ctx, defaultNetwork)
if err != nil {
ip, err = core.DefaultGatewayIP()
if err != nil {
Expand Down Expand Up @@ -1598,7 +1602,10 @@ func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) {
if err != nil {
return "", fmt.Errorf("ensure default network: %w", err)
}
return p.getGatewayIP(ctx, defaultNetwork)
}

func (p *DockerProvider) getGatewayIP(ctx context.Context, defaultNetwork string) (string, error) {
stevenh marked this conversation as resolved.
Show resolved Hide resolved
nw, err := p.GetNetwork(ctx, NetworkRequest{Name: defaultNetwork})
if err != nil {
return "", err
Expand All @@ -1624,7 +1631,10 @@ func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) {
func (p *DockerProvider) ensureDefaultNetwork(ctx context.Context) (string, error) {
p.mtx.Lock()
defer p.mtx.Unlock()
return p.ensureDefaultNetworkLocked(ctx)
}

func (p *DockerProvider) ensureDefaultNetworkLocked(ctx context.Context) (string, error) {
if p.defaultNetwork != "" {
// Already set.
return p.defaultNetwork, nil
Expand Down
38 changes: 38 additions & 0 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/testcontainers/testcontainers-go/internal/core"
"github.com/testcontainers/testcontainers-go/wait"
)

Expand All @@ -35,6 +36,7 @@ const (
nginxAlpineImage = "nginx:alpine"
nginxDefaultPort = "80/tcp"
nginxHighPort = "8080/tcp"
golangImage = "golang"
daemonMaxVersion = "1.41"
)

Expand Down Expand Up @@ -2125,3 +2127,39 @@ func TestCustomPrefixTrailingSlashIsProperlyRemovedIfPresent(t *testing.T) {
dockerContainer := c.(*DockerContainer)
require.Equal(t, fmt.Sprintf("%s%s", hubPrefixWithTrailingSlash, dockerImage), dockerContainer.Image)
}

// TODO: remove this skip check when context rework is merged alongside [core.DockerEnvFile] removal.
func Test_Provider_DaemonHost_Issue2897(t *testing.T) {
ctx := context.Background()
provider, err := NewDockerProvider()
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, provider.Close())
})

orig := core.DockerEnvFile
core.DockerEnvFile = filepath.Join(t.TempDir(), ".dockerenv")
t.Cleanup(func() {
core.DockerEnvFile = orig
})

f, err := os.Create(core.DockerEnvFile)
require.NoError(t, err)
require.NoError(t, f.Close())
t.Cleanup(func() {
require.NoError(t, os.Remove(f.Name()))
})

errCh := make(chan error, 1)
go func() {
_, err := provider.DaemonHost(ctx)
errCh <- err
}()

select {
case <-time.After(1 * time.Second):
t.Fatal("timeout waiting for DaemonHost")
case err := <-errCh:
require.NoError(t, err)
}
}
7 changes: 6 additions & 1 deletion internal/core/docker_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,15 @@ func testcontainersHostFromProperties(ctx context.Context) (string, error) {
return "", ErrTestcontainersHostNotSetInProperties
}

// DockerEnvFile is the file that is created when running inside a container.
// It's a variable to allow testing.
// TODO: Remove this once context rework is done, which eliminates need for the default network creation.
var DockerEnvFile = "/.dockerenv"

// InAContainer returns true if the code is running inside a container
// See https://github.com/docker/docker/blob/a9fa38b1edf30b23cae3eade0be48b3d4b1de14b/daemon/initlayer/setup_unix.go#L25
func InAContainer() bool {
return inAContainer("/.dockerenv")
return inAContainer(DockerEnvFile)
}

func inAContainer(path string) bool {
Expand Down
Loading