From 19fb55bcf1f906655a6cbd5d64b8bb297a996b1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 21 Aug 2024 12:15:18 +0200 Subject: [PATCH 01/25] fix: check if the discovered docker socket responds --- internal/core/docker_host.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index b9d2d60d51..beb959a712 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -116,6 +116,22 @@ func extractDockerHost(ctx context.Context) string { continue } + // check if the docker host responds to an Info request + // we are going to use the default Docker client to check if the host is reachable + // which will avoid recursive calls to this function + cli, err := client.NewClientWithOpts(client.WithHost(dockerHost)) + if err != nil { + outerErr = fmt.Errorf("%w: %w", outerErr, err) + continue + } + defer cli.Close() + + _, err = cli.Info(ctx) + if err != nil { + outerErr = fmt.Errorf("%w: %w", outerErr, err) + continue + } + return dockerHost } From fbadada0697c9ac3c69049b77c68e4e231a507fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 21 Aug 2024 12:30:19 +0200 Subject: [PATCH 02/25] fix: update tests --- internal/core/docker_host.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index beb959a712..e8db3fe182 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -116,22 +116,6 @@ func extractDockerHost(ctx context.Context) string { continue } - // check if the docker host responds to an Info request - // we are going to use the default Docker client to check if the host is reachable - // which will avoid recursive calls to this function - cli, err := client.NewClientWithOpts(client.WithHost(dockerHost)) - if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) - continue - } - defer cli.Close() - - _, err = cli.Info(ctx) - if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) - continue - } - return dockerHost } @@ -157,7 +141,8 @@ func extractDockerSocket(ctx context.Context) string { // and receiving an instance of the Docker API client interface. // This internal method is handy for testing purposes, passing a mock type simulating the desired behaviour. func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { - // check that the socket is not a tcp or unix socket + // check that the socket is not a tcp or unix socket, + // and that it is reachable. checkDockerSocketFn := func(socket string) string { // this use case will cover the case when the docker host is a tcp socket if strings.HasPrefix(socket, TCPSchema) { @@ -168,6 +153,11 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st return strings.Replace(socket, DockerSocketSchema, "", 1) } + _, err := cli.Info(ctx) + if err != nil { + return DockerSocketPath + } + return socket } From c6c4832a78138890cfd52543f3658a8d5ee62fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 21 Aug 2024 12:57:25 +0200 Subject: [PATCH 03/25] chore: add test --- internal/core/docker_host_test.go | 32 ++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 23d8e1e1fa..cc0b36bc38 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -2,12 +2,14 @@ package core import ( "context" + "fmt" "os" "path/filepath" "testing" "github.com/docker/docker/api/types/system" "github.com/docker/docker/client" + "github.com/docker/docker/errdefs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -271,12 +273,17 @@ func TestExtractDockerHost(t *testing.T) { // different operating systems. type mockCli struct { client.APIClient - OS string + OS string + notReachable bool } // Info returns a mock implementation of types.Info, which is handy for detecting the operating system, // which is used to determine the default docker socket path. func (m mockCli) Info(ctx context.Context) (system.Info, error) { + if m.notReachable { + return system.Info{}, errdefs.Unavailable(fmt.Errorf("Docker is not reachable")) + } + return system.Info{ OperatingSystem: m.OS, }, nil @@ -411,6 +418,29 @@ func TestExtractDockerSocketFromClient(t *testing.T) { assert.Equal(t, "/this/is/a/sample.sock", socket) }) + + t.Run("Unix Docker Socket is not reachable, so Info panics", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Errorf("The code did not panic") + } + }() + + content := "docker.host=" + DockerSocketSchema + "/this/is/a/sample.sock" + setupTestcontainersProperties(t, content) + setupDockerSocketNotFound(t) + + t.Cleanup(resetSocketOverrideFn) + + ctx := context.Background() + os.Unsetenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE") + os.Unsetenv("DOCKER_HOST") + + // force a non + socket := extractDockerSocketFromClient(ctx, mockCli{notReachable: true}) + + assert.Equal(t, "/this/is/a/sample.sock", socket) + }) } func TestInAContainer(t *testing.T) { From ff83affc9351cc42d19c6c0273c77bc6d3a10efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 21 Aug 2024 14:02:58 +0200 Subject: [PATCH 04/25] Revert "chore: add test" This reverts commit c6c4832a78138890cfd52543f3658a8d5ee62fd9. --- internal/core/docker_host_test.go | 32 +------------------------------ 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index cc0b36bc38..23d8e1e1fa 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -2,14 +2,12 @@ package core import ( "context" - "fmt" "os" "path/filepath" "testing" "github.com/docker/docker/api/types/system" "github.com/docker/docker/client" - "github.com/docker/docker/errdefs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -273,17 +271,12 @@ func TestExtractDockerHost(t *testing.T) { // different operating systems. type mockCli struct { client.APIClient - OS string - notReachable bool + OS string } // Info returns a mock implementation of types.Info, which is handy for detecting the operating system, // which is used to determine the default docker socket path. func (m mockCli) Info(ctx context.Context) (system.Info, error) { - if m.notReachable { - return system.Info{}, errdefs.Unavailable(fmt.Errorf("Docker is not reachable")) - } - return system.Info{ OperatingSystem: m.OS, }, nil @@ -418,29 +411,6 @@ func TestExtractDockerSocketFromClient(t *testing.T) { assert.Equal(t, "/this/is/a/sample.sock", socket) }) - - t.Run("Unix Docker Socket is not reachable, so Info panics", func(t *testing.T) { - defer func() { - if r := recover(); r == nil { - t.Errorf("The code did not panic") - } - }() - - content := "docker.host=" + DockerSocketSchema + "/this/is/a/sample.sock" - setupTestcontainersProperties(t, content) - setupDockerSocketNotFound(t) - - t.Cleanup(resetSocketOverrideFn) - - ctx := context.Background() - os.Unsetenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE") - os.Unsetenv("DOCKER_HOST") - - // force a non - socket := extractDockerSocketFromClient(ctx, mockCli{notReachable: true}) - - assert.Equal(t, "/this/is/a/sample.sock", socket) - }) } func TestInAContainer(t *testing.T) { From 270c2b63117e0347144ef6b6b6a5fe5c2cded29d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 21 Aug 2024 14:03:03 +0200 Subject: [PATCH 05/25] Revert "fix: update tests" This reverts commit fbadada0697c9ac3c69049b77c68e4e231a507fb. --- internal/core/docker_host.go | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index e8db3fe182..beb959a712 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -116,6 +116,22 @@ func extractDockerHost(ctx context.Context) string { continue } + // check if the docker host responds to an Info request + // we are going to use the default Docker client to check if the host is reachable + // which will avoid recursive calls to this function + cli, err := client.NewClientWithOpts(client.WithHost(dockerHost)) + if err != nil { + outerErr = fmt.Errorf("%w: %w", outerErr, err) + continue + } + defer cli.Close() + + _, err = cli.Info(ctx) + if err != nil { + outerErr = fmt.Errorf("%w: %w", outerErr, err) + continue + } + return dockerHost } @@ -141,8 +157,7 @@ func extractDockerSocket(ctx context.Context) string { // and receiving an instance of the Docker API client interface. // This internal method is handy for testing purposes, passing a mock type simulating the desired behaviour. func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { - // check that the socket is not a tcp or unix socket, - // and that it is reachable. + // check that the socket is not a tcp or unix socket checkDockerSocketFn := func(socket string) string { // this use case will cover the case when the docker host is a tcp socket if strings.HasPrefix(socket, TCPSchema) { @@ -153,11 +168,6 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st return strings.Replace(socket, DockerSocketSchema, "", 1) } - _, err := cli.Info(ctx) - if err != nil { - return DockerSocketPath - } - return socket } From cdb3825ad22903d4faad38a8985fb0ce996ee65b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 21 Aug 2024 14:03:07 +0200 Subject: [PATCH 06/25] Revert "fix: check if the discovered docker socket responds" This reverts commit 19fb55bcf1f906655a6cbd5d64b8bb297a996b1a. --- internal/core/docker_host.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index beb959a712..b9d2d60d51 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -116,22 +116,6 @@ func extractDockerHost(ctx context.Context) string { continue } - // check if the docker host responds to an Info request - // we are going to use the default Docker client to check if the host is reachable - // which will avoid recursive calls to this function - cli, err := client.NewClientWithOpts(client.WithHost(dockerHost)) - if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) - continue - } - defer cli.Close() - - _, err = cli.Info(ctx) - if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) - continue - } - return dockerHost } From 42a77ddea89ce317fc7cac860e5ee4010f0adcb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 21 Aug 2024 14:53:58 +0200 Subject: [PATCH 07/25] chore: support passing callback checks to the docker host/socket path resolution This way the tests are able to verify if the socket/host is reachable by calling a mock client. The production code will use the default callback check, which calls a vanilla docker client using the discovered host as host --- internal/core/docker_host.go | 47 ++++++++++++++++++++++++++----- internal/core/docker_host_test.go | 42 +++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index b9d2d60d51..3b0015efdd 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -56,6 +56,23 @@ func DefaultGatewayIP() (string, error) { return ip, nil } +// Use a vanilla Docker client to check if the Docker host is reachable. +// It will avoid recursive calls to this function. +var defaultCallbackCheckFn = func(host string) error { + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host)) + if err != nil { + return err + } + + _, err = cli.Info(context.Background()) + if err != nil { + return err + } + defer cli.Close() + + return nil +} + // ExtractDockerHost 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: @@ -67,9 +84,16 @@ func DefaultGatewayIP() (string, error) { // 5. Docker host from the "docker.host" property in the ~/.testcontainers.properties file. // 6. Rootless docker socket path. // 7. Else, the default Docker socket including schema will be returned. -func ExtractDockerHost(ctx context.Context) string { +func ExtractDockerHost(ctx context.Context, callbackChecks ...func(host string) error) string { dockerHostOnce.Do(func() { - dockerHostCache = extractDockerHost(ctx) + // the tests can pass a custom callback check to avoid the Docker client creation + // and to avoid changing the code base, passing zero callbacks will mean the default one, + // which checks that the host is reachable. + if len(callbackChecks) == 0 { + callbackChecks = append(callbackChecks, defaultCallbackCheckFn) + } + + dockerHostCache = extractDockerHost(ctx, callbackChecks...) }) return dockerHostCache @@ -98,7 +122,7 @@ func ExtractDockerSocket(ctx context.Context) string { // extractDockerHost Extracts the docker host from the different alternatives, without caching the result. // This internal method is handy for testing purposes. -func extractDockerHost(ctx context.Context) string { +func extractDockerHost(ctx context.Context, callbackChecks ...func(host string) error) string { dockerHostFns := []func(context.Context) (string, error){ testcontainersHostFromProperties, dockerHostFromEnv, @@ -109,6 +133,7 @@ func extractDockerHost(ctx context.Context) string { } outerErr := ErrSocketNotFound +BEGIN: for _, dockerHostFn := range dockerHostFns { dockerHost, err := dockerHostFn(ctx) if err != nil { @@ -116,6 +141,14 @@ func extractDockerHost(ctx context.Context) string { continue } + for _, callbackCheck := range callbackChecks { + err = callbackCheck(dockerHost) + if err != nil { + outerErr = fmt.Errorf("%w: %w", outerErr, err) + continue BEGIN // retry with the next docker host function + } + } + return dockerHost } @@ -123,7 +156,7 @@ func extractDockerHost(ctx context.Context) string { return DockerSocketPathWithSchema } -// extractDockerHost Extracts the docker socket from the different alternatives, without caching the result. +// extractDockerSocket Extracts the docker socket from the different alternatives, without caching the result. // It will internally use the default Docker client, calling the internal method extractDockerSocketFromClient with it. // This internal method is handy for testing purposes. // If a Docker client cannot be created, the program will panic. @@ -134,13 +167,13 @@ func extractDockerSocket(ctx context.Context) string { } defer cli.Close() - return extractDockerSocketFromClient(ctx, cli) + return extractDockerSocketFromClient(ctx, cli, defaultCallbackCheckFn) } // extractDockerSocketFromClient Extracts the docker socket from the different alternatives, without caching the result, // and receiving an instance of the Docker API client interface. // This internal method is handy for testing purposes, passing a mock type simulating the desired behaviour. -func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { +func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient, callbackChecks ...func(host string) error) string { // check that the socket is not a tcp or unix socket checkDockerSocketFn := func(socket string) string { // this use case will cover the case when the docker host is a tcp socket @@ -179,7 +212,7 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st return DockerSocketPath } - dockerHost := extractDockerHost(ctx) + dockerHost := extractDockerHost(ctx, callbackChecks...) return checkDockerSocketFn(dockerHost) } diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 23d8e1e1fa..2a868bbff1 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -2,6 +2,7 @@ package core import ( "context" + "fmt" "os" "path/filepath" "testing" @@ -40,6 +41,14 @@ var resetSocketOverrideFn = func() { os.Setenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", originalDockerSocketOverride) } +var testCallbackCheckPassing = func(_ string) error { + return nil +} + +var testCallbackCheckError = func(_ string) error { + return fmt.Errorf("could not check the Docker host") +} + func TestExtractDockerHost(t *testing.T) { setupDockerHostNotFound(t) // do not mess with local .testcontainers.properties @@ -47,10 +56,11 @@ func TestExtractDockerHost(t *testing.T) { t.Setenv("HOME", tmpDir) t.Setenv("USERPROFILE", tmpDir) // Windows support - t.Run("Docker Host as extracted just once", func(t *testing.T) { + t.Run("Docker Host is extracted just once", func(t *testing.T) { expected := "/path/to/docker.sock" t.Setenv("DOCKER_HOST", expected) - host := ExtractDockerHost(context.Background()) + + host := ExtractDockerHost(context.Background(), testCallbackCheckPassing) assert.Equal(t, expected, host) @@ -71,6 +81,18 @@ func TestExtractDockerHost(t *testing.T) { assert.Equal(t, testRemoteHost, host) }) + t.Run("Testcontainers Host is resolved first but not reachable", func(t *testing.T) { + t.Setenv("DOCKER_HOST", "/path/to/docker.sock") + content := "tc.host=" + testRemoteHost + + setupTestcontainersProperties(t, content) + + host := extractDockerHost(context.Background(), testCallbackCheckError) + + // the default Docker host with schema is returned because we cannot any other host + assert.Equal(t, DockerSocketPathWithSchema, host) + }) + t.Run("Docker Host as environment variable", func(t *testing.T) { t.Setenv("DOCKER_HOST", "/path/to/docker.sock") host := extractDockerHost(context.Background()) @@ -411,6 +433,22 @@ func TestExtractDockerSocketFromClient(t *testing.T) { assert.Equal(t, "/this/is/a/sample.sock", socket) }) + + t.Run("Unix Docker Socket is passed as docker.host property but not reachable", func(t *testing.T) { + content := "docker.host=" + DockerSocketSchema + "/this/is/a/sample.sock" + setupTestcontainersProperties(t, content) + + t.Cleanup(resetSocketOverrideFn) + + ctx := context.Background() + os.Unsetenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE") + os.Unsetenv("DOCKER_HOST") + + socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}, testCallbackCheckError) + + // the default Docker host without schema is returned because we cannot any other host + assert.Equal(t, DockerSocketPath, socket) + }) } func TestInAContainer(t *testing.T) { From 64c4d2d7880ef084a62e186027e9c538a5db5582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 22 Aug 2024 08:01:56 +0200 Subject: [PATCH 08/25] chore: convert var into function --- internal/core/docker_host.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 3b0015efdd..ded6ee7a77 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -56,9 +56,9 @@ func DefaultGatewayIP() (string, error) { return ip, nil } -// Use a vanilla Docker client to check if the Docker host is reachable. +// defaultCallbackCheck Use a vanilla Docker client to check if the Docker host is reachable. // It will avoid recursive calls to this function. -var defaultCallbackCheckFn = func(host string) error { +func defaultCallbackCheck(host string) error { cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host)) if err != nil { return err @@ -90,7 +90,7 @@ func ExtractDockerHost(ctx context.Context, callbackChecks ...func(host string) // and to avoid changing the code base, passing zero callbacks will mean the default one, // which checks that the host is reachable. if len(callbackChecks) == 0 { - callbackChecks = append(callbackChecks, defaultCallbackCheckFn) + callbackChecks = append(callbackChecks, defaultCallbackCheck) } dockerHostCache = extractDockerHost(ctx, callbackChecks...) @@ -167,7 +167,7 @@ func extractDockerSocket(ctx context.Context) string { } defer cli.Close() - return extractDockerSocketFromClient(ctx, cli, defaultCallbackCheckFn) + return extractDockerSocketFromClient(ctx, cli, defaultCallbackCheck) } // extractDockerSocketFromClient Extracts the docker socket from the different alternatives, without caching the result, From e8eeff38be456f9d531c449dbf5fac6272cff88c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 22 Aug 2024 16:42:44 +0200 Subject: [PATCH 09/25] chore: mock callback check instead --- internal/core/docker_host.go | 36 +++++++++++-------------------- internal/core/docker_host_test.go | 28 +++++++++++++++++++----- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index ded6ee7a77..62ff7310c3 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -56,15 +56,15 @@ func DefaultGatewayIP() (string, error) { return ip, nil } -// defaultCallbackCheck Use a vanilla Docker client to check if the Docker host is reachable. +// defaultCallbackCheckFn Use a vanilla Docker client to check if the Docker host is reachable. // It will avoid recursive calls to this function. -func defaultCallbackCheck(host string) error { +var defaultCallbackCheckFn = func(ctx context.Context, host string) error { cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host)) if err != nil { return err } - _, err = cli.Info(context.Background()) + _, err = cli.Info(ctx) if err != nil { return err } @@ -84,16 +84,9 @@ func defaultCallbackCheck(host string) error { // 5. Docker host from the "docker.host" property in the ~/.testcontainers.properties file. // 6. Rootless docker socket path. // 7. Else, the default Docker socket including schema will be returned. -func ExtractDockerHost(ctx context.Context, callbackChecks ...func(host string) error) string { +func ExtractDockerHost(ctx context.Context) string { dockerHostOnce.Do(func() { - // the tests can pass a custom callback check to avoid the Docker client creation - // and to avoid changing the code base, passing zero callbacks will mean the default one, - // which checks that the host is reachable. - if len(callbackChecks) == 0 { - callbackChecks = append(callbackChecks, defaultCallbackCheck) - } - - dockerHostCache = extractDockerHost(ctx, callbackChecks...) + dockerHostCache = extractDockerHost(ctx) }) return dockerHostCache @@ -122,7 +115,7 @@ func ExtractDockerSocket(ctx context.Context) string { // extractDockerHost Extracts the docker host from the different alternatives, without caching the result. // This internal method is handy for testing purposes. -func extractDockerHost(ctx context.Context, callbackChecks ...func(host string) error) string { +func extractDockerHost(ctx context.Context) string { dockerHostFns := []func(context.Context) (string, error){ testcontainersHostFromProperties, dockerHostFromEnv, @@ -133,7 +126,6 @@ func extractDockerHost(ctx context.Context, callbackChecks ...func(host string) } outerErr := ErrSocketNotFound -BEGIN: for _, dockerHostFn := range dockerHostFns { dockerHost, err := dockerHostFn(ctx) if err != nil { @@ -141,12 +133,10 @@ BEGIN: continue } - for _, callbackCheck := range callbackChecks { - err = callbackCheck(dockerHost) - if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) - continue BEGIN // retry with the next docker host function - } + err = defaultCallbackCheckFn(ctx, dockerHost) + if err != nil { + outerErr = fmt.Errorf("%w: %w", outerErr, err) + continue } return dockerHost @@ -167,13 +157,13 @@ func extractDockerSocket(ctx context.Context) string { } defer cli.Close() - return extractDockerSocketFromClient(ctx, cli, defaultCallbackCheck) + return extractDockerSocketFromClient(ctx, cli) } // extractDockerSocketFromClient Extracts the docker socket from the different alternatives, without caching the result, // and receiving an instance of the Docker API client interface. // This internal method is handy for testing purposes, passing a mock type simulating the desired behaviour. -func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient, callbackChecks ...func(host string) error) string { +func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { // check that the socket is not a tcp or unix socket checkDockerSocketFn := func(socket string) string { // this use case will cover the case when the docker host is a tcp socket @@ -212,7 +202,7 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient, ca return DockerSocketPath } - dockerHost := extractDockerHost(ctx, callbackChecks...) + dockerHost := extractDockerHost(ctx) return checkDockerSocketFn(dockerHost) } diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 2a868bbff1..75b121ec55 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -41,14 +41,22 @@ var resetSocketOverrideFn = func() { os.Setenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", originalDockerSocketOverride) } -var testCallbackCheckPassing = func(_ string) error { +var testCallbackCheckPassing = func(_ context.Context, _ string) error { return nil } -var testCallbackCheckError = func(_ string) error { +var testCallbackCheckError = func(_ context.Context, _ string) error { return fmt.Errorf("could not check the Docker host") } +func mockCallbackCheck(t *testing.T, fn func(_ context.Context, _ string) error) { + oldCheck := defaultCallbackCheckFn + defaultCallbackCheckFn = fn + t.Cleanup(func() { + defaultCallbackCheckFn = oldCheck + }) +} + func TestExtractDockerHost(t *testing.T) { setupDockerHostNotFound(t) // do not mess with local .testcontainers.properties @@ -56,11 +64,14 @@ func TestExtractDockerHost(t *testing.T) { t.Setenv("HOME", tmpDir) t.Setenv("USERPROFILE", tmpDir) // Windows support + // apply the passing check to all sub-tests + mockCallbackCheck(t, testCallbackCheckPassing) + t.Run("Docker Host is extracted just once", func(t *testing.T) { expected := "/path/to/docker.sock" t.Setenv("DOCKER_HOST", expected) - host := ExtractDockerHost(context.Background(), testCallbackCheckPassing) + host := ExtractDockerHost(context.Background()) assert.Equal(t, expected, host) @@ -87,7 +98,10 @@ func TestExtractDockerHost(t *testing.T) { setupTestcontainersProperties(t, content) - host := extractDockerHost(context.Background(), testCallbackCheckError) + // mock the callback check to return an error + mockCallbackCheck(t, testCallbackCheckError) + + host := extractDockerHost(context.Background()) // the default Docker host with schema is returned because we cannot any other host assert.Equal(t, DockerSocketPathWithSchema, host) @@ -307,6 +321,8 @@ func (m mockCli) Info(ctx context.Context) (system.Info, error) { func TestExtractDockerSocketFromClient(t *testing.T) { setupDockerHostNotFound(t) + mockCallbackCheck(t, testCallbackCheckPassing) + t.Run("Docker socket from Testcontainers host defined in properties", func(t *testing.T) { content := "tc.host=" + testRemoteHost @@ -444,7 +460,9 @@ func TestExtractDockerSocketFromClient(t *testing.T) { os.Unsetenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE") os.Unsetenv("DOCKER_HOST") - socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}, testCallbackCheckError) + mockCallbackCheck(t, testCallbackCheckError) + + socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) // the default Docker host without schema is returned because we cannot any other host assert.Equal(t, DockerSocketPath, socket) From da6d2b1aac9ea5464a164899fea8d803731a558d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 22 Aug 2024 17:18:09 +0200 Subject: [PATCH 10/25] chore: simplify --- internal/core/docker_host.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 62ff7310c3..e84b462b97 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -133,9 +133,7 @@ func extractDockerHost(ctx context.Context) string { continue } - err = defaultCallbackCheckFn(ctx, dockerHost) - if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) + if err = defaultCallbackCheckFn(ctx, dockerHost); err != nil { continue } From 8dd29791da15df4124d9460b2bde4b20f322031e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 26 Aug 2024 20:14:52 +0200 Subject: [PATCH 11/25] chore: raise error when extracting the docker host --- internal/core/docker_host.go | 21 ++++-- internal/core/docker_host_test.go | 103 ++++++++++++++++-------------- 2 files changed, 69 insertions(+), 55 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index e84b462b97..147f6c47e5 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -83,10 +83,15 @@ var defaultCallbackCheckFn = func(ctx context.Context, host string) error { // 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. -// 7. Else, the default Docker socket including schema will be returned. +// 7. Else, because the Docker host is not set, it panics. func ExtractDockerHost(ctx context.Context) string { dockerHostOnce.Do(func() { - dockerHostCache = extractDockerHost(ctx) + cache, err := extractDockerHost(ctx) + if err != nil { + panic(err) + } + + dockerHostCache = cache }) return dockerHostCache @@ -115,7 +120,7 @@ func ExtractDockerSocket(ctx context.Context) string { // extractDockerHost Extracts the docker host from the different alternatives, without caching the result. // This internal method is handy for testing purposes. -func extractDockerHost(ctx context.Context) string { +func extractDockerHost(ctx context.Context) (string, error) { dockerHostFns := []func(context.Context) (string, error){ testcontainersHostFromProperties, dockerHostFromEnv, @@ -137,11 +142,10 @@ func extractDockerHost(ctx context.Context) string { continue } - return dockerHost + return dockerHost, nil } - // We are not supporting Windows containers at the moment - return DockerSocketPathWithSchema + return "", outerErr } // extractDockerSocket Extracts the docker socket from the different alternatives, without caching the result. @@ -200,7 +204,10 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st return DockerSocketPath } - dockerHost := extractDockerHost(ctx) + dockerHost, err := extractDockerHost(ctx) + if err != nil { + panic(err) // Docker host is required to get the Docker socket + } return checkDockerSocketFn(dockerHost) } diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 75b121ec55..8f282f594d 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -78,7 +78,7 @@ func TestExtractDockerHost(t *testing.T) { t.Setenv("DOCKER_HOST", "/path/to/another/docker.sock") host = ExtractDockerHost(context.Background()) - assert.Equal(t, expected, host) + require.Equal(t, expected, host) }) t.Run("Testcontainers Host is resolved first", func(t *testing.T) { @@ -87,9 +87,9 @@ func TestExtractDockerHost(t *testing.T) { setupTestcontainersProperties(t, content) - host := extractDockerHost(context.Background()) - - assert.Equal(t, testRemoteHost, host) + host, err := extractDockerHost(context.Background()) + require.NoError(t, err) + require.Equal(t, testRemoteHost, host) }) t.Run("Testcontainers Host is resolved first but not reachable", func(t *testing.T) { @@ -101,17 +101,17 @@ func TestExtractDockerHost(t *testing.T) { // mock the callback check to return an error mockCallbackCheck(t, testCallbackCheckError) - host := extractDockerHost(context.Background()) - - // the default Docker host with schema is returned because we cannot any other host - assert.Equal(t, DockerSocketPathWithSchema, host) + host, err := extractDockerHost(context.Background()) + require.ErrorIs(t, err, ErrDockerSocketNotSetInContext) + require.ErrorIs(t, err, ErrDockerSocketNotSetInProperties) + require.Equal(t, "", host) }) t.Run("Docker Host as environment variable", func(t *testing.T) { t.Setenv("DOCKER_HOST", "/path/to/docker.sock") - host := extractDockerHost(context.Background()) - - assert.Equal(t, "/path/to/docker.sock", host) + host, err := extractDockerHost(context.Background()) + require.NoError(t, err) + require.Equal(t, "/path/to/docker.sock", host) }) t.Run("Malformed Docker Host is passed in context", func(t *testing.T) { @@ -120,9 +120,9 @@ func TestExtractDockerHost(t *testing.T) { ctx := context.Background() - host := extractDockerHost(context.WithValue(ctx, DockerHostContextKey, "path-to-docker-sock")) - - assert.Equal(t, DockerSocketPathWithSchema, host) + host, err := extractDockerHost(context.WithValue(ctx, DockerHostContextKey, "path-to-docker-sock")) + require.Error(t, err) + require.Equal(t, "", host) }) t.Run("Malformed Schema Docker Host is passed in context", func(t *testing.T) { @@ -130,17 +130,17 @@ func TestExtractDockerHost(t *testing.T) { setupRootlessNotFound(t) ctx := context.Background() - host := extractDockerHost(context.WithValue(ctx, DockerHostContextKey, "http://path to docker sock")) - - assert.Equal(t, DockerSocketPathWithSchema, host) + host, err := extractDockerHost(context.WithValue(ctx, DockerHostContextKey, "http://path to docker sock")) + require.Error(t, err) + require.Equal(t, "", host) }) t.Run("Unix Docker Host is passed in context", func(t *testing.T) { ctx := context.Background() - host := extractDockerHost(context.WithValue(ctx, DockerHostContextKey, DockerSocketSchema+"/this/is/a/sample.sock")) - - assert.Equal(t, "/this/is/a/sample.sock", host) + host, err := extractDockerHost(context.WithValue(ctx, DockerHostContextKey, DockerSocketSchema+"/this/is/a/sample.sock")) + require.NoError(t, err) + require.Equal(t, "/this/is/a/sample.sock", host) }) t.Run("Unix Docker Host is passed as docker.host", func(t *testing.T) { @@ -150,26 +150,26 @@ func TestExtractDockerHost(t *testing.T) { setupTestcontainersProperties(t, content) - host := extractDockerHost(context.Background()) - - assert.Equal(t, DockerSocketSchema+"/this/is/a/sample.sock", host) + host, err := extractDockerHost(context.Background()) + require.NoError(t, err) + require.Equal(t, DockerSocketSchema+"/this/is/a/sample.sock", host) }) t.Run("Default Docker socket", func(t *testing.T) { setupRootlessNotFound(t) tmpSocket := setupDockerSocket(t) - host := extractDockerHost(context.Background()) - - assert.Equal(t, tmpSocket, host) + host, err := extractDockerHost(context.Background()) + require.NoError(t, err) + require.Equal(t, tmpSocket, host) }) - t.Run("Default Docker Host when empty", func(t *testing.T) { + t.Run("Error when empty", func(t *testing.T) { setupDockerSocketNotFound(t) setupRootlessNotFound(t) - host := extractDockerHost(context.Background()) - - assert.Equal(t, DockerSocketPathWithSchema, host) + host, err := extractDockerHost(context.Background()) + require.Error(t, err) + require.Equal(t, "", host) }) t.Run("Extract Docker socket", func(t *testing.T) { @@ -183,7 +183,7 @@ func TestExtractDockerHost(t *testing.T) { socket, err := testcontainersHostFromProperties(context.Background()) require.NoError(t, err) - assert.Equal(t, testRemoteHost, socket) + require.Equal(t, testRemoteHost, socket) }) t.Run("Testcontainers host is not defined in properties", func(t *testing.T) { @@ -205,7 +205,7 @@ func TestExtractDockerHost(t *testing.T) { socket, err := dockerHostFromEnv(context.Background()) require.NoError(t, err) - assert.Equal(t, tmpSocket, socket) + require.Equal(t, tmpSocket, socket) }) t.Run("DOCKER_HOST is not set", func(t *testing.T) { @@ -227,7 +227,7 @@ func TestExtractDockerHost(t *testing.T) { socket, err := dockerSocketOverridePath() require.NoError(t, err) - assert.Equal(t, tmpSocket, socket) + require.Equal(t, tmpSocket, socket) }) t.Run("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE is not set", func(t *testing.T) { @@ -245,7 +245,7 @@ func TestExtractDockerHost(t *testing.T) { socket, err := dockerHostFromContext(context.WithValue(ctx, DockerHostContextKey, DockerSocketSchema+"/this/is/a/sample.sock")) require.NoError(t, err) - assert.Equal(t, "/this/is/a/sample.sock", socket) + require.Equal(t, "/this/is/a/sample.sock", socket) }) t.Run("Context sets a malformed Docker socket", func(t *testing.T) { @@ -269,7 +269,7 @@ func TestExtractDockerHost(t *testing.T) { socket, err := dockerSocketPath(context.Background()) require.NoError(t, err) - assert.Equal(t, tmpSocket, socket) + require.Equal(t, tmpSocket, socket) }) t.Run("Docker host is defined in properties", func(t *testing.T) { @@ -280,7 +280,7 @@ func TestExtractDockerHost(t *testing.T) { socket, err := dockerHostFromProperties(context.Background()) require.NoError(t, err) - assert.Equal(t, tmpSocket, socket) + require.Equal(t, tmpSocket, socket) }) t.Run("Docker host is not defined in properties", func(t *testing.T) { @@ -329,7 +329,7 @@ func TestExtractDockerSocketFromClient(t *testing.T) { setupTestcontainersProperties(t, content) socket := extractDockerSocketFromClient(context.Background(), mockCli{OS: "foo"}) - assert.Equal(t, DockerSocketPath, socket) + require.Equal(t, DockerSocketPath, socket) }) t.Run("Docker socket from Testcontainers host takes precedence over TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", func(t *testing.T) { @@ -341,7 +341,7 @@ func TestExtractDockerSocketFromClient(t *testing.T) { t.Setenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", "/path/to/docker.sock") socket := extractDockerSocketFromClient(context.Background(), mockCli{OS: "foo"}) - assert.Equal(t, DockerSocketPath, socket) + require.Equal(t, DockerSocketPath, socket) }) t.Run("Docker Socket as Testcontainers environment variable", func(t *testing.T) { @@ -352,7 +352,7 @@ func TestExtractDockerSocketFromClient(t *testing.T) { t.Setenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", "/path/to/docker.sock") host := extractDockerSocketFromClient(context.Background(), mockCli{OS: "foo"}) - assert.Equal(t, "/path/to/docker.sock", host) + require.Equal(t, "/path/to/docker.sock", host) }) t.Run("Docker Socket as Testcontainers environment variable, removes prefixes", func(t *testing.T) { @@ -362,11 +362,11 @@ func TestExtractDockerSocketFromClient(t *testing.T) { t.Setenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", DockerSocketSchema+"/path/to/docker.sock") host := extractDockerSocketFromClient(context.Background(), mockCli{OS: "foo"}) - assert.Equal(t, "/path/to/docker.sock", host) + require.Equal(t, "/path/to/docker.sock", host) t.Setenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", testRemoteHost) host = extractDockerSocketFromClient(context.Background(), mockCli{OS: "foo"}) - assert.Equal(t, DockerSocketPath, host) + require.Equal(t, DockerSocketPath, host) }) t.Run("Unix Docker Socket is passed as DOCKER_HOST variable (Docker Desktop on non-Windows)", func(t *testing.T) { @@ -385,7 +385,7 @@ func TestExtractDockerSocketFromClient(t *testing.T) { socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Docker Desktop"}) - assert.Equal(t, DockerSocketPath, socket) + require.Equal(t, DockerSocketPath, socket) }) t.Run("Unix Docker Socket is passed as DOCKER_HOST variable (Docker Desktop for Windows)", func(t *testing.T) { @@ -400,7 +400,7 @@ func TestExtractDockerSocketFromClient(t *testing.T) { socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Docker Desktop"}) - assert.Equal(t, WindowsDockerSocketPath, socket) + require.Equal(t, WindowsDockerSocketPath, socket) }) t.Run("Unix Docker Socket is passed as DOCKER_HOST variable (Not Docker Desktop)", func(t *testing.T) { @@ -414,7 +414,7 @@ func TestExtractDockerSocketFromClient(t *testing.T) { socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) - assert.Equal(t, "/this/is/a/sample.sock", socket) + require.Equal(t, "/this/is/a/sample.sock", socket) }) t.Run("Unix Docker Socket is passed as DOCKER_HOST variable (Not Docker Desktop), removes prefixes", func(t *testing.T) { @@ -427,11 +427,11 @@ func TestExtractDockerSocketFromClient(t *testing.T) { t.Setenv("DOCKER_HOST", DockerSocketSchema+"/this/is/a/sample.sock") socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) - assert.Equal(t, "/this/is/a/sample.sock", socket) + require.Equal(t, "/this/is/a/sample.sock", socket) t.Setenv("DOCKER_HOST", testRemoteHost) socket = extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) - assert.Equal(t, DockerSocketPath, socket) + require.Equal(t, DockerSocketPath, socket) }) t.Run("Unix Docker Socket is passed as docker.host property", func(t *testing.T) { @@ -447,7 +447,7 @@ func TestExtractDockerSocketFromClient(t *testing.T) { socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) - assert.Equal(t, "/this/is/a/sample.sock", socket) + require.Equal(t, "/this/is/a/sample.sock", socket) }) t.Run("Unix Docker Socket is passed as docker.host property but not reachable", func(t *testing.T) { @@ -462,10 +462,17 @@ func TestExtractDockerSocketFromClient(t *testing.T) { mockCallbackCheck(t, testCallbackCheckError) + // check that panic is raised + defer func() { + if r := recover(); r == nil { + t.Errorf("The code did not panic") + } + }() + socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) // the default Docker host without schema is returned because we cannot any other host - assert.Equal(t, DockerSocketPath, socket) + require.Equal(t, "", socket) }) } From 94b661df608819ff677c435a92568d22435cbce0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 10:29:57 +0200 Subject: [PATCH 12/25] docs: document that the extract functions panics --- docs/features/configuration.md | 4 ++-- internal/core/docker_host.go | 5 +++-- testcontainers.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/features/configuration.md b/docs/features/configuration.md index aa5a0d0a61..ee5b6a4d69 100644 --- a/docs/features/configuration.md +++ b/docs/features/configuration.md @@ -85,7 +85,7 @@ See [Docker environment variables](https://docs.docker.com/engine/reference/comm 3. `${HOME}/.docker/desktop/docker.sock`. 4. `/run/user/${UID}/docker.sock`, where `${UID}` is the user ID of the current user. -7. The default Docker socket including schema will be returned if none of the above are set. +7. The library panics if none of the above are set, meaning that the Docker host was not detected. ## Docker socket path detection @@ -109,4 +109,4 @@ Path to Docker's socket. Used by Ryuk, Docker Compose, and a few other container 6. Else, the default location of the docker socket is used: `/var/run/docker.sock` -In any case, if the docker socket schema is `tcp://`, the default docker socket path will be returned. +The library panics if the Docker host cannot be discovered. diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 147f6c47e5..87db9df605 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -109,7 +109,7 @@ func ExtractDockerHost(ctx context.Context) string { // 5. If the socket contains the unix schema, the schema is removed (e.g. unix:///var/run/docker.sock -> /var/run/docker.sock) // 6. Else, the default location of the docker socket is used (/var/run/docker.sock) // -// In any case, if the docker socket schema is "tcp://", the default docker socket path will be returned. +// It panics if a Docker client cannot be created, or the Docker host cannot be discovered. func ExtractDockerSocket(ctx context.Context) string { dockerSocketPathOnce.Do(func() { dockerSocketPathCache = extractDockerSocket(ctx) @@ -151,7 +151,7 @@ func extractDockerHost(ctx context.Context) (string, error) { // extractDockerSocket Extracts the docker socket from the different alternatives, without caching the result. // It will internally use the default Docker client, calling the internal method extractDockerSocketFromClient with it. // This internal method is handy for testing purposes. -// If a Docker client cannot be created, the program will panic. +// It panics if a Docker client cannot be created, or the Docker host is not discovered. func extractDockerSocket(ctx context.Context) string { cli, err := NewClient(ctx) if err != nil { @@ -165,6 +165,7 @@ func extractDockerSocket(ctx context.Context) string { // extractDockerSocketFromClient Extracts the docker socket from the different alternatives, without caching the result, // and receiving an instance of the Docker API client interface. // This internal method is handy for testing purposes, passing a mock type simulating the desired behaviour. +// It panics if the Docker Info call errors, or the Docker host is not discovered. func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) string { // check that the socket is not a tcp or unix socket checkDockerSocketFn := func(socket string) string { diff --git a/testcontainers.go b/testcontainers.go index 5b52e09a22..2b2ead0248 100644 --- a/testcontainers.go +++ b/testcontainers.go @@ -18,7 +18,7 @@ import ( // 5. If the socket contains the unix schema, the schema is removed (e.g. unix:///var/run/docker.sock -> /var/run/docker.sock) // 6. Else, the default location of the docker socket is used (/var/run/docker.sock) // -// In any case, if the docker socket schema is "tcp://", the default docker socket path will be returned. +// It panics if a Docker client cannot be created, or the Docker host cannot be discovered. func ExtractDockerSocket() string { return core.ExtractDockerSocket(context.Background()) } From d29de5e09f55e307d13b8dc40adb9945149bbf44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 10:42:02 +0200 Subject: [PATCH 13/25] chore: simplify error handler --- internal/core/docker_host.go | 33 ++++++++++++++++++++++++--- internal/core/docker_host_test.go | 3 +-- internal/core/docker_rootless.go | 12 +++++++--- internal/core/docker_rootless_test.go | 8 +------ 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 87db9df605..b70477c664 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -130,22 +130,29 @@ func extractDockerHost(ctx context.Context) (string, error) { rootlessDockerSocketPath, } - outerErr := ErrSocketNotFound + var errs []error for _, dockerHostFn := range dockerHostFns { dockerHost, err := dockerHostFn(ctx) if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) + if !isHostNotFound(err) { + errs = append(errs, err) + } continue } if err = defaultCallbackCheckFn(ctx, dockerHost); err != nil { + errs = append(errs, fmt.Errorf("check host %q: %w", dockerHost, err)) continue } return dockerHost, nil } - return "", outerErr + if len(errs) > 0 { + return "", errors.Join(errs...) + } + + return "", ErrSocketNotFound } // extractDockerSocket Extracts the docker socket from the different alternatives, without caching the result. @@ -213,6 +220,26 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st return checkDockerSocketFn(dockerHost) } +// isHostNotSet returns true if the error is related to the Docker host +// not being set, false otherwise. +func isHostNotFound(err error) bool { + switch { + case errors.Is(err, ErrTestcontainersHostNotSetInProperties), + errors.Is(err, ErrDockerHostNotSet), + errors.Is(err, ErrDockerSocketNotSetInContext), + errors.Is(err, ErrDockerSocketNotSetInProperties), + errors.Is(err, ErrSocketNotFoundInPath), + errors.Is(err, ErrDockerSocketNotSetInProperties), + errors.Is(err, ErrXDGRuntimeDirNotSet), + errors.Is(err, ErrRootlessDockerNotFoundHomeRunDir), + errors.Is(err, ErrRootlessDockerNotFoundHomeDesktopDir), + errors.Is(err, ErrRootlessDockerNotFoundRunDir): + return true + default: + return false + } +} + // dockerHostFromEnv returns the docker host from the DOCKER_HOST environment variable, if it's not empty func dockerHostFromEnv(ctx context.Context) (string, error) { if dockerHostPath := os.Getenv("DOCKER_HOST"); dockerHostPath != "" { diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 8f282f594d..b12efd82ac 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -102,8 +102,7 @@ func TestExtractDockerHost(t *testing.T) { mockCallbackCheck(t, testCallbackCheckError) host, err := extractDockerHost(context.Background()) - require.ErrorIs(t, err, ErrDockerSocketNotSetInContext) - require.ErrorIs(t, err, ErrDockerSocketNotSetInProperties) + require.Error(t, err) require.Equal(t, "", host) }) diff --git a/internal/core/docker_rootless.go b/internal/core/docker_rootless.go index 44782d31b3..aa40ba822f 100644 --- a/internal/core/docker_rootless.go +++ b/internal/core/docker_rootless.go @@ -53,18 +53,24 @@ func rootlessDockerSocketPath(_ context.Context) (string, error) { rootlessSocketPathFromRunDir, } - outerErr := ErrRootlessDockerNotFound + var errs []error for _, socketPathFn := range socketPathFns { s, err := socketPathFn() if err != nil { - outerErr = fmt.Errorf("%w: %w", outerErr, err) + if !isHostNotFound(err) { + errs = append(errs, err) + } continue } return DockerSocketSchema + s, nil } - return "", outerErr + if len(errs) > 0 { + return "", errors.Join(errs...) + } + + return "", ErrRootlessDockerNotFound } func fileExists(f string) bool { diff --git a/internal/core/docker_rootless_test.go b/internal/core/docker_rootless_test.go index ef018eda53..5b22a196f6 100644 --- a/internal/core/docker_rootless_test.go +++ b/internal/core/docker_rootless_test.go @@ -178,14 +178,8 @@ func TestRootlessDockerSocketPath(t *testing.T) { setupRootlessNotFound(t) socketPath, err := rootlessDockerSocketPath(context.Background()) - require.ErrorIs(t, err, ErrRootlessDockerNotFound) + require.Error(t, err) assert.Empty(t, socketPath) - - // the wrapped error includes all the locations that were checked - require.ErrorContains(t, err, ErrRootlessDockerNotFoundXDGRuntimeDir.Error()) - require.ErrorContains(t, err, ErrRootlessDockerNotFoundHomeRunDir.Error()) - require.ErrorContains(t, err, ErrRootlessDockerNotFoundHomeDesktopDir.Error()) - require.ErrorContains(t, err, ErrRootlessDockerNotFoundRunDir.Error()) }) } From c4546f693e2c6afa61bc4241bda7f374600f1b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 12:02:46 +0200 Subject: [PATCH 14/25] chore: use require.Panics --- internal/core/docker_host_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index b12efd82ac..54fc6b9106 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -461,14 +461,10 @@ func TestExtractDockerSocketFromClient(t *testing.T) { mockCallbackCheck(t, testCallbackCheckError) - // check that panic is raised - defer func() { - if r := recover(); r == nil { - t.Errorf("The code did not panic") - } - }() - - socket := extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) + var socket string + require.Panics(t, func() { + socket = extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) + }) // the default Docker host without schema is returned because we cannot any other host require.Equal(t, "", socket) From b493c72659c07c8566e7038a5aab9b9e1f323b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 12:08:27 +0200 Subject: [PATCH 15/25] fix: remove duplicated case --- internal/core/docker_host.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index b70477c664..a7697e8547 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -229,7 +229,6 @@ func isHostNotFound(err error) bool { errors.Is(err, ErrDockerSocketNotSetInContext), errors.Is(err, ErrDockerSocketNotSetInProperties), errors.Is(err, ErrSocketNotFoundInPath), - errors.Is(err, ErrDockerSocketNotSetInProperties), errors.Is(err, ErrXDGRuntimeDirNotSet), errors.Is(err, ErrRootlessDockerNotFoundHomeRunDir), errors.Is(err, ErrRootlessDockerNotFoundHomeDesktopDir), From 5a234fe88eb1fa031306b67118ca42f495dfea8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 12:36:24 +0200 Subject: [PATCH 16/25] fix: negotiate API version in the plain docker client call --- internal/core/docker_host.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index a7697e8547..3b173dcd06 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -59,7 +59,7 @@ func DefaultGatewayIP() (string, error) { // defaultCallbackCheckFn Use a vanilla Docker client to check if the Docker host is reachable. // It will avoid recursive calls to this function. var defaultCallbackCheckFn = func(ctx context.Context, host string) error { - cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host)) + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host), client.WithAPIVersionNegotiation()) if err != nil { return err } From 38720a32b19996407717a8f8f209f1fed60062c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 14:33:04 +0200 Subject: [PATCH 17/25] fix: defer closing the client earlier --- internal/core/docker_host.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 3b173dcd06..3d3e27b1b4 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -63,12 +63,12 @@ var defaultCallbackCheckFn = func(ctx context.Context, host string) error { if err != nil { return err } + defer cli.Close() _, err = cli.Info(ctx) if err != nil { return err } - defer cli.Close() return nil } From 1da780c46401a3d5610cbb8ae61f599b25e86274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 14:34:18 +0200 Subject: [PATCH 18/25] chore: better function name --- internal/core/docker_host.go | 4 ++-- internal/core/docker_rootless.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 3d3e27b1b4..ce9bc1c73f 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -134,7 +134,7 @@ func extractDockerHost(ctx context.Context) (string, error) { for _, dockerHostFn := range dockerHostFns { dockerHost, err := dockerHostFn(ctx) if err != nil { - if !isHostNotFound(err) { + if !isHostNotSet(err) { errs = append(errs, err) } continue @@ -222,7 +222,7 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st // isHostNotSet returns true if the error is related to the Docker host // not being set, false otherwise. -func isHostNotFound(err error) bool { +func isHostNotSet(err error) bool { switch { case errors.Is(err, ErrTestcontainersHostNotSetInProperties), errors.Is(err, ErrDockerHostNotSet), diff --git a/internal/core/docker_rootless.go b/internal/core/docker_rootless.go index aa40ba822f..b8e0f6e17e 100644 --- a/internal/core/docker_rootless.go +++ b/internal/core/docker_rootless.go @@ -57,7 +57,7 @@ func rootlessDockerSocketPath(_ context.Context) (string, error) { for _, socketPathFn := range socketPathFns { s, err := socketPathFn() if err != nil { - if !isHostNotFound(err) { + if !isHostNotSet(err) { errs = append(errs, err) } continue From 2472045f04fae032d228df70abd39d1dac665f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 14:35:32 +0200 Subject: [PATCH 19/25] chore: convert vars into functions --- internal/core/docker_host_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 54fc6b9106..2c03f93bd9 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -41,11 +41,11 @@ var resetSocketOverrideFn = func() { os.Setenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE", originalDockerSocketOverride) } -var testCallbackCheckPassing = func(_ context.Context, _ string) error { +func testCallbackCheckPassing(_ context.Context, _ string) error { return nil } -var testCallbackCheckError = func(_ context.Context, _ string) error { +func testCallbackCheckError(_ context.Context, _ string) error { return fmt.Errorf("could not check the Docker host") } From 1f3c6d9f50f8b4a9883ca6950b4489fe2197c2d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 14:37:35 +0200 Subject: [PATCH 20/25] chore: no need to assert as panic should occur --- internal/core/docker_host_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 2c03f93bd9..8ce0d5527e 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -461,13 +461,10 @@ func TestExtractDockerSocketFromClient(t *testing.T) { mockCallbackCheck(t, testCallbackCheckError) - var socket string require.Panics(t, func() { - socket = extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) + // no need to check for the returned socket, as it must panic + _ = extractDockerSocketFromClient(ctx, mockCli{OS: "Ubuntu"}) }) - - // the default Docker host without schema is returned because we cannot any other host - require.Equal(t, "", socket) }) } From 311507eb41fa1273349832b4b1234d755c3dd8d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 14:56:47 +0200 Subject: [PATCH 21/25] chor: rename check function --- docker_client.go | 4 ++-- image_test.go | 4 ++-- internal/core/client.go | 2 +- internal/core/docker_host.go | 18 +++++++++--------- internal/core/docker_host_test.go | 10 +++++----- modules/localstack/localstack.go | 2 +- provider.go | 2 +- provider_test.go | 2 +- reaper.go | 2 +- reaper_test.go | 4 ++-- testcontainers.go | 9 +++++++-- 11 files changed, 32 insertions(+), 27 deletions(-) diff --git a/docker_client.go b/docker_client.go index c8e8e825b0..04df712916 100644 --- a/docker_client.go +++ b/docker_client.go @@ -79,8 +79,8 @@ func (c *DockerClient) Info(ctx context.Context) (system.Info, error) { dockerInfo.OperatingSystem, dockerInfo.MemTotal/1024/1024, infoLabels, internal.Version, - core.ExtractDockerHost(ctx), - core.ExtractDockerSocket(ctx), + core.MustExtractDockerHost(ctx), + core.MustExtractDockerSocket(ctx), core.SessionID(), core.ProcessID(), ) diff --git a/image_test.go b/image_test.go index 17595b6590..795a521b29 100644 --- a/image_test.go +++ b/image_test.go @@ -10,7 +10,7 @@ import ( ) func TestImageList(t *testing.T) { - t.Setenv("DOCKER_HOST", core.ExtractDockerHost(context.Background())) + t.Setenv("DOCKER_HOST", core.MustExtractDockerHost(context.Background())) provider, err := ProviderDocker.GetProvider() if err != nil { @@ -54,7 +54,7 @@ func TestImageList(t *testing.T) { } func TestSaveImages(t *testing.T) { - t.Setenv("DOCKER_HOST", core.ExtractDockerHost(context.Background())) + t.Setenv("DOCKER_HOST", core.MustExtractDockerHost(context.Background())) provider, err := ProviderDocker.GetProvider() if err != nil { diff --git a/internal/core/client.go b/internal/core/client.go index 64af509b9f..04a54bcbc5 100644 --- a/internal/core/client.go +++ b/internal/core/client.go @@ -14,7 +14,7 @@ import ( func NewClient(ctx context.Context, ops ...client.Opt) (*client.Client, error) { tcConfig := config.Read() - dockerHost := ExtractDockerHost(ctx) + dockerHost := MustExtractDockerHost(ctx) opts := []client.Opt{client.FromEnv, client.WithAPIVersionNegotiation()} if dockerHost != "" { diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index ce9bc1c73f..3785dc0fc5 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -56,24 +56,24 @@ func DefaultGatewayIP() (string, error) { return ip, nil } -// defaultCallbackCheckFn Use a vanilla Docker client to check if the Docker host is reachable. +// dockerHostCheck Use a vanilla Docker client to check if the Docker host is reachable. // It will avoid recursive calls to this function. -var defaultCallbackCheckFn = func(ctx context.Context, host string) error { +var dockerHostCheck = func(ctx context.Context, host string) error { cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host), client.WithAPIVersionNegotiation()) if err != nil { - return err + return fmt.Errorf("new client: %w", err) } defer cli.Close() _, err = cli.Info(ctx) if err != nil { - return err + return fmt.Errorf("docker info: %w", err) } return nil } -// ExtractDockerHost 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: // @@ -84,7 +84,7 @@ var defaultCallbackCheckFn = func(ctx context.Context, host string) error { // 5. Docker host from the "docker.host" property in the ~/.testcontainers.properties file. // 6. Rootless docker socket path. // 7. Else, because the Docker host is not set, it panics. -func ExtractDockerHost(ctx context.Context) string { +func MustExtractDockerHost(ctx context.Context) string { dockerHostOnce.Do(func() { cache, err := extractDockerHost(ctx) if err != nil { @@ -97,7 +97,7 @@ func ExtractDockerHost(ctx context.Context) string { return dockerHostCache } -// ExtractDockerSocket Extracts the docker socket from the different alternatives, removing the socket schema and +// MustExtractDockerSocket Extracts the docker socket from the different alternatives, removing the socket schema and // caching the result to avoid unnecessary calculations. Use this function to get the docker socket path, // not the host (e.g. mounting the socket in a container). This function does not consider Windows containers at the moment. // The possible alternatives are: @@ -110,7 +110,7 @@ func ExtractDockerHost(ctx context.Context) string { // 6. Else, the default location of the docker socket is used (/var/run/docker.sock) // // It panics if a Docker client cannot be created, or the Docker host cannot be discovered. -func ExtractDockerSocket(ctx context.Context) string { +func MustExtractDockerSocket(ctx context.Context) string { dockerSocketPathOnce.Do(func() { dockerSocketPathCache = extractDockerSocket(ctx) }) @@ -140,7 +140,7 @@ func extractDockerHost(ctx context.Context) (string, error) { continue } - if err = defaultCallbackCheckFn(ctx, dockerHost); err != nil { + if err = dockerHostCheck(ctx, dockerHost); err != nil { errs = append(errs, fmt.Errorf("check host %q: %w", dockerHost, err)) continue } diff --git a/internal/core/docker_host_test.go b/internal/core/docker_host_test.go index 8ce0d5527e..eceee573fc 100644 --- a/internal/core/docker_host_test.go +++ b/internal/core/docker_host_test.go @@ -50,10 +50,10 @@ func testCallbackCheckError(_ context.Context, _ string) error { } func mockCallbackCheck(t *testing.T, fn func(_ context.Context, _ string) error) { - oldCheck := defaultCallbackCheckFn - defaultCallbackCheckFn = fn + oldCheck := dockerHostCheck + dockerHostCheck = fn t.Cleanup(func() { - defaultCallbackCheckFn = oldCheck + dockerHostCheck = oldCheck }) } @@ -71,13 +71,13 @@ func TestExtractDockerHost(t *testing.T) { expected := "/path/to/docker.sock" t.Setenv("DOCKER_HOST", expected) - host := ExtractDockerHost(context.Background()) + host := MustExtractDockerHost(context.Background()) assert.Equal(t, expected, host) t.Setenv("DOCKER_HOST", "/path/to/another/docker.sock") - host = ExtractDockerHost(context.Background()) + host = MustExtractDockerHost(context.Background()) require.Equal(t, expected, host) }) diff --git a/modules/localstack/localstack.go b/modules/localstack/localstack.go index f06054e83b..49414a5a52 100644 --- a/modules/localstack/localstack.go +++ b/modules/localstack/localstack.go @@ -74,7 +74,7 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize // Run creates an instance of the LocalStack container type // - overrideReq: a function that can be used to override the default container request, usually used to set the image version, environment variables for localstack, etc. func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustomizer) (*LocalStackContainer, error) { - dockerHost := testcontainers.ExtractDockerSocket() + dockerHost := testcontainers.MustExtractDockerSocket() req := testcontainers.ContainerRequest{ Image: img, diff --git a/provider.go b/provider.go index c563503036..b5e5ffa997 100644 --- a/provider.go +++ b/provider.go @@ -147,7 +147,7 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error return &DockerProvider{ DockerProviderOptions: o, - host: core.ExtractDockerHost(ctx), + host: core.MustExtractDockerHost(ctx), client: c, config: config.Read(), }, nil diff --git a/provider_test.go b/provider_test.go index 2448d84c07..097c83a02c 100644 --- a/provider_test.go +++ b/provider_test.go @@ -8,7 +8,7 @@ import ( ) func TestProviderTypeGetProviderAutodetect(t *testing.T) { - dockerHost := core.ExtractDockerHost(context.Background()) + dockerHost := core.MustExtractDockerHost(context.Background()) const podmanSocket = "unix://$XDG_RUNTIME_DIR/podman/podman.sock" tests := []struct { diff --git a/reaper.go b/reaper.go index c17b4f329e..c41520b5b7 100644 --- a/reaper.go +++ b/reaper.go @@ -233,7 +233,7 @@ func reuseReaperContainer(ctx context.Context, sessionID string, provider Reaper // newReaper creates a Reaper with a sessionID to identify containers and a // provider to use. Do not call this directly, use reuseOrCreateReaper instead. func newReaper(ctx context.Context, sessionID string, provider ReaperProvider) (*Reaper, error) { - dockerHostMount := core.ExtractDockerSocket(ctx) + dockerHostMount := core.MustExtractDockerSocket(ctx) reaper := &Reaper{ Provider: provider, diff --git a/reaper_test.go b/reaper_test.go index c757fbb3ea..e526e8ec9a 100644 --- a/reaper_test.go +++ b/reaper_test.go @@ -91,7 +91,7 @@ func createContainerRequest(customize func(ContainerRequest) ContainerRequest) C ExposedPorts: []string{"8080/tcp"}, Labels: core.DefaultLabels(testSessionID), HostConfigModifier: func(hostConfig *container.HostConfig) { - hostConfig.Binds = []string{core.ExtractDockerSocket(context.Background()) + ":/var/run/docker.sock"} + hostConfig.Binds = []string{core.MustExtractDockerSocket(context.Background()) + ":/var/run/docker.sock"} }, WaitingFor: wait.ForListeningPort(nat.Port("8080/tcp")), Env: map[string]string{ @@ -376,7 +376,7 @@ func Test_NewReaper(t *testing.T) { name: "docker-host in context", req: createContainerRequest(func(req ContainerRequest) ContainerRequest { req.HostConfigModifier = func(hostConfig *container.HostConfig) { - hostConfig.Binds = []string{core.ExtractDockerSocket(context.Background()) + ":/var/run/docker.sock"} + hostConfig.Binds = []string{core.MustExtractDockerSocket(context.Background()) + ":/var/run/docker.sock"} } return req }), diff --git a/testcontainers.go b/testcontainers.go index 2b2ead0248..fd30eed957 100644 --- a/testcontainers.go +++ b/testcontainers.go @@ -6,6 +6,11 @@ import ( "github.com/testcontainers/testcontainers-go/internal/core" ) +// Deprecated: use MustExtractDockerHost instead. +func ExtractDockerSocket() string { + return MustExtractDockerSocket() +} + // ExtractDockerSocket Extracts the docker socket from the different alternatives, removing the socket schema. // Use this function to get the docker socket path, not the host (e.g. mounting the socket in a container). // This function does not consider Windows containers at the moment. @@ -19,8 +24,8 @@ import ( // 6. Else, the default location of the docker socket is used (/var/run/docker.sock) // // It panics if a Docker client cannot be created, or the Docker host cannot be discovered. -func ExtractDockerSocket() string { - return core.ExtractDockerSocket(context.Background()) +func MustExtractDockerSocket() string { + return core.MustExtractDockerSocket(context.Background()) } // SessionID returns a unique session ID for the current test session. Because each Go package From 50d424fbeccd9d38366352a2a661e8330a84a061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 16:58:56 +0200 Subject: [PATCH 22/25] chore: pass ctx to new function --- testcontainers.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/testcontainers.go b/testcontainers.go index fd30eed957..16b7eeca0e 100644 --- a/testcontainers.go +++ b/testcontainers.go @@ -8,10 +8,10 @@ import ( // Deprecated: use MustExtractDockerHost instead. func ExtractDockerSocket() string { - return MustExtractDockerSocket() + return MustExtractDockerSocket(context.Background()) } -// ExtractDockerSocket Extracts the docker socket from the different alternatives, removing the socket schema. +// MustExtractDockerSocket Extracts the docker socket from the different alternatives, removing the socket schema. // Use this function to get the docker socket path, not the host (e.g. mounting the socket in a container). // This function does not consider Windows containers at the moment. // The possible alternatives are: @@ -24,8 +24,8 @@ func ExtractDockerSocket() string { // 6. Else, the default location of the docker socket is used (/var/run/docker.sock) // // It panics if a Docker client cannot be created, or the Docker host cannot be discovered. -func MustExtractDockerSocket() string { - return core.MustExtractDockerSocket(context.Background()) +func MustExtractDockerSocket(ctx context.Context) string { + return core.MustExtractDockerSocket(ctx) } // SessionID returns a unique session ID for the current test session. Because each Go package From f030b5c65fd57eecde25b58d7fd54ce4c454efa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 17:05:31 +0200 Subject: [PATCH 23/25] chore: more exhaustive error check in tests --- internal/core/docker_rootless_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/docker_rootless_test.go b/internal/core/docker_rootless_test.go index 5b22a196f6..7897f35783 100644 --- a/internal/core/docker_rootless_test.go +++ b/internal/core/docker_rootless_test.go @@ -178,7 +178,7 @@ func TestRootlessDockerSocketPath(t *testing.T) { setupRootlessNotFound(t) socketPath, err := rootlessDockerSocketPath(context.Background()) - require.Error(t, err) + require.ErrorIs(t, err, ErrRootlessDockerNotFoundXDGRuntimeDir) assert.Empty(t, socketPath) }) } From 8cbe4e65159eb3d582f5293bee96911534cad249 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 17:07:58 +0200 Subject: [PATCH 24/25] docs: typo --- internal/core/docker_host.go | 2 +- testcontainers.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/core/docker_host.go b/internal/core/docker_host.go index 3785dc0fc5..3088a3742b 100644 --- a/internal/core/docker_host.go +++ b/internal/core/docker_host.go @@ -105,7 +105,7 @@ func MustExtractDockerHost(ctx context.Context) string { // 1. Docker host from the "tc.host" property in the ~/.testcontainers.properties file. // 2. The TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE environment variable. // 3. Using a Docker client, check if the Info().OperativeSystem is "Docker Desktop" and return the default docker socket path for rootless docker. -// 4. Else, Get the current Docker Host from the existing strategies: see ExtractDockerHost. +// 4. Else, Get the current Docker Host from the existing strategies: see MustExtractDockerHost. // 5. If the socket contains the unix schema, the schema is removed (e.g. unix:///var/run/docker.sock -> /var/run/docker.sock) // 6. Else, the default location of the docker socket is used (/var/run/docker.sock) // diff --git a/testcontainers.go b/testcontainers.go index 16b7eeca0e..7ae4a40c14 100644 --- a/testcontainers.go +++ b/testcontainers.go @@ -19,7 +19,7 @@ func ExtractDockerSocket() string { // 1. Docker host from the "tc.host" property in the ~/.testcontainers.properties file. // 2. The TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE environment variable. // 3. Using a Docker client, check if the Info().OperativeSystem is "Docker Desktop" and return the default docker socket path for rootless docker. -// 4. Else, Get the current Docker Host from the existing strategies: see ExtractDockerHost. +// 4. Else, Get the current Docker Host from the existing strategies: see MustExtractDockerHost. // 5. If the socket contains the unix schema, the schema is removed (e.g. unix:///var/run/docker.sock -> /var/run/docker.sock) // 6. Else, the default location of the docker socket is used (/var/run/docker.sock) // From 9dfd4fba06310529c908a5416de2ca8ed3f2218a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 28 Aug 2024 18:26:05 +0200 Subject: [PATCH 25/25] fix: update usage --- modules/localstack/localstack.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/localstack/localstack.go b/modules/localstack/localstack.go index 49414a5a52..961527cd3e 100644 --- a/modules/localstack/localstack.go +++ b/modules/localstack/localstack.go @@ -74,7 +74,7 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize // Run creates an instance of the LocalStack container type // - overrideReq: a function that can be used to override the default container request, usually used to set the image version, environment variables for localstack, etc. func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustomizer) (*LocalStackContainer, error) { - dockerHost := testcontainers.MustExtractDockerSocket() + dockerHost := testcontainers.MustExtractDockerSocket(ctx) req := testcontainers.ContainerRequest{ Image: img,