From 33528c667838d2bb926960510427478014fd134c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 16 Oct 2024 13:12:01 +0200 Subject: [PATCH] chore: remove bridge network custom configuration It's not needed at all, as we rely on the underlying container runtime --- docker.go | 6 +-- docker_test.go | 14 ++--- internal/config/config.go | 10 ---- internal/config/config_test.go | 47 +++------------- lifecycle.go | 28 ---------- lifecycle_test.go | 99 ---------------------------------- reaper.go | 2 +- reaper_test.go | 2 +- 8 files changed, 17 insertions(+), 191 deletions(-) diff --git a/docker.go b/docker.go index a50b4ed92e..395dc169ce 100644 --- a/docker.go +++ b/docker.go @@ -43,8 +43,8 @@ import ( var _ Container = (*DockerContainer)(nil) const ( - Bridge = "bridge" // Bridge network driver and name - ReaperDefault = "reaper_default" // Deprecated: use Bridge instead. Default network name when bridge is not available + Bridge = "bridge" // Deprecated, it will removed in the next major release. Bridge network driver and name + ReaperDefault = "reaper_default" // Deprecated: it will removed in the next major release. Default network name when bridge is not available packagePath = "github.com/testcontainers/testcontainers-go" ) @@ -1504,7 +1504,7 @@ func (p *DockerProvider) GetNetwork(ctx context.Context, req NetworkRequest) (ne } func (p *DockerProvider) GetGatewayIP(ctx context.Context) (string, error) { - nw, err := p.GetNetwork(ctx, NetworkRequest{Name: config.Read().TestcontainersBridgeName}) + nw, err := p.GetNetwork(ctx, NetworkRequest{Name: Bridge}) if err != nil { return "", err } diff --git a/docker_test.go b/docker_test.go index 281d010bd9..ee60b7e957 100644 --- a/docker_test.go +++ b/docker_test.go @@ -25,7 +25,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/wait" ) @@ -509,7 +508,9 @@ func TestContainerCreationWithName(t *testing.T) { }, WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort), Name: creationName, - Networks: []string{config.Read().TestcontainersBridgeName}, + // no network means it will be connected to the default bridge network + // of any given container runtime + Networks: []string{}, }, Started: true, }) @@ -530,14 +531,7 @@ func TestContainerCreationWithName(t *testing.T) { t.Fatal(err) } if len(networks) != 1 { - t.Errorf("Expected networks 1. Got '%d'.", len(networks)) - } - - expectedBridge := config.Read().TestcontainersBridgeName - - network := networks[0] - if network != expectedBridge { - t.Errorf("Expected network name '%s'. Got '%s'.", expectedBridge, network) + t.Errorf("Expected networks 1, the default bridge network. Got '%d'.", len(networks)) } endpoint, err := nginxC.PortEndpoint(ctx, nginxDefaultPort, "http") diff --git a/internal/config/config.go b/internal/config/config.go index 36f5e36991..4dcf766d58 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -85,11 +85,6 @@ type Config struct { // // Environment variable: TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE TestcontainersHost string `properties:"tc.host,default="` - - // TestcontainersBridgeName is the name of the bridge network used by Testcontainers. - // - // Environment variable: TESTCONTAINERS_BRIDGE_NAME - TestcontainersBridgeName string `properties:"tc.bridge.name,default=bridge"` } // } @@ -150,11 +145,6 @@ func applyEnvironmentConfiguration(config Config) Config { config.RyukConnectionTimeout = timeout } - testcontainersBridgeName := os.Getenv("TESTCONTAINERS_BRIDGE_NAME") - if testcontainersBridgeName != "" { - config.TestcontainersBridgeName = testcontainersBridgeName - } - return config } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2a683b8fd7..341b62c550 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -129,17 +129,15 @@ func TestReadTCConfig(t *testing.T) { t.Setenv("TESTCONTAINERS_RYUK_VERBOSE", "true") t.Setenv("TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT", "13s") t.Setenv("TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT", "12s") - t.Setenv("TESTCONTAINERS_BRIDGE_NAME", "testbridge") config := read() expected := Config{ - HubImageNamePrefix: defaultHubPrefix, - RyukDisabled: true, - RyukPrivileged: true, - RyukVerbose: true, - RyukReconnectionTimeout: 13 * time.Second, - RyukConnectionTimeout: 12 * time.Second, - TestcontainersBridgeName: "testbridge", + HubImageNamePrefix: defaultHubPrefix, + RyukDisabled: true, + RyukPrivileged: true, + RyukVerbose: true, + RyukReconnectionTimeout: 13 * time.Second, + RyukConnectionTimeout: 12 * time.Second, } assert.Equal(t, expected, config) @@ -149,9 +147,8 @@ func TestReadTCConfig(t *testing.T) { defaultRyukConnectionTimeout := 60 * time.Second defaultRyukReconnectionTimeout := 10 * time.Second defaultCfg := Config{ - RyukConnectionTimeout: defaultRyukConnectionTimeout, - RyukReconnectionTimeout: defaultRyukReconnectionTimeout, - TestcontainersBridgeName: "bridge", + RyukConnectionTimeout: defaultRyukConnectionTimeout, + RyukReconnectionTimeout: defaultRyukReconnectionTimeout, } tests := []struct { @@ -477,34 +474,6 @@ func TestReadTCConfig(t *testing.T) { HubImageNamePrefix: defaultHubPrefix + "/env/", }, }, - { - "bridge-name/property", - `tc.bridge.name=props`, - map[string]string{}, - Config{ - TestcontainersBridgeName: "props", - }, - }, - { - "bridge-name/env-var", - ``, - map[string]string{ - "TESTCONTAINERS_BRIDGE_NAME": "env", - }, - Config{ - TestcontainersBridgeName: "env", - }, - }, - { - "bridge-name/env-var-wins", - `tc.bridge.name=props`, - map[string]string{ - "TESTCONTAINERS_BRIDGE_NAME": "env", - }, - Config{ - TestcontainersBridgeName: "env", - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/lifecycle.go b/lifecycle.go index c49d8a60f0..9270a790b2 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -12,8 +12,6 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" "github.com/docker/go-connections/nat" - - "github.com/testcontainers/testcontainers-go/internal/config" ) // ContainerRequestHook is a hook that will be called before a container is created. @@ -509,16 +507,6 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain networkingConfig.EndpointsConfig = endpointSettings - // Move the bridge network to the user-defined bridge network, if configured. - // This is needed because different container runtimes might use different a bridge network name. - // As an example, when listing the networks, the Docker client always retrives the default - // bridge network as "bridge", but when attaching a container to that bridge network, - // the container runtime can fail because the bridge network is not named "bridge". - // For that reason we need to move the bridge network to the user-defined bridge network, - // that's why we are offering an extension point to configure the bridge network name - // at the Testcontainers configuration level. - bridgeNetworModifier(networkingConfig.EndpointsConfig) - exposedPorts := req.ExposedPorts // this check must be done after the pre-creation Modifiers are called, so the network mode is already set if len(exposedPorts) == 0 && !hostConfig.NetworkMode.IsContainer() { @@ -640,19 +628,3 @@ func defaultHostConfigModifier(req ContainerRequest) func(hostConfig *container. hostConfig.Resources = req.Resources } } - -// bridgeNetworModifier moves the bridge network to the user-defined bridge network, if configured. -func bridgeNetworModifier(endpointSettings map[string]*network.EndpointSettings) { - userDefinedBridge := config.Read().TestcontainersBridgeName - - if userDefinedBridge == Bridge { - return - } - - // If the map contains a bridge network, use the configured bridge network. - if _, ok := endpointSettings[Bridge]; ok { - nw := endpointSettings[Bridge] - delete(endpointSettings, Bridge) - endpointSettings[userDefinedBridge] = nw - } -} diff --git a/lifecycle_test.go b/lifecycle_test.go index 44c11a334d..04da1df835 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -4,8 +4,6 @@ import ( "bufio" "context" "fmt" - "os" - "path/filepath" "strings" "testing" "time" @@ -18,7 +16,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/testcontainers/testcontainers-go/internal/config" "github.com/testcontainers/testcontainers-go/wait" ) @@ -297,102 +294,6 @@ func TestPreCreateModifierHook(t *testing.T) { ) }) - t.Run("endpoint-settings-modifier/custom-bridge-network", func(t *testing.T) { - // set testcontainers properties including a custom bridge network name - tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) - t.Setenv("USERPROFILE", tmpDir) // windows - - const bridgeNetworkName = "test-bridge" - content := "tc.bridge.name=" + bridgeNetworkName - if err := os.WriteFile(filepath.Join(tmpDir, ".testcontainers.properties"), []byte(content), 0o600); err != nil { - t.Errorf("Failed to create the file: %v", err) - return - } - config.Reset() // reset the config to reload the properties for testing purposes - - req := ContainerRequest{ - Image: nginxAlpineImage, // alpine image does expose port 80 - } - - // define empty inputs to be overwritten by the pre create hook - inputConfig := &container.Config{ - Image: req.Image, - } - inputHostConfig := &container.HostConfig{} - inputNetworkingConfig := &network.NetworkingConfig{ - EndpointsConfig: map[string]*network.EndpointSettings{ - "bridge": { - MacAddress: "bridge-mac", - Aliases: []string{"bridge-alias"}, - }, - }, - } - - err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) - require.NoError(t, err) - - // assertions - - require.Equal( - t, - "bridge-mac", - inputNetworkingConfig.EndpointsConfig[bridgeNetworkName].MacAddress, - ) - require.Equal( - t, - []string{"bridge-alias"}, - inputNetworkingConfig.EndpointsConfig[bridgeNetworkName].Aliases, - ) - }) - - t.Run("endpoint-settings-modifier/default-bridge-network", func(t *testing.T) { - // set testcontainers properties including a custom bridge network name - tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) - t.Setenv("USERPROFILE", tmpDir) // windows - - if err := os.WriteFile(filepath.Join(tmpDir, ".testcontainers.properties"), []byte(""), 0o600); err != nil { - t.Errorf("Failed to create the file: %v", err) - return - } - config.Reset() // reset the config to reload the properties for testing purposes - - req := ContainerRequest{ - Image: nginxAlpineImage, // alpine image does expose port 80 - } - - // define empty inputs to be overwritten by the pre create hook - inputConfig := &container.Config{ - Image: req.Image, - } - inputHostConfig := &container.HostConfig{} - inputNetworkingConfig := &network.NetworkingConfig{ - EndpointsConfig: map[string]*network.EndpointSettings{ - Bridge: { - MacAddress: "bridge-mac", - Aliases: []string{"bridge-alias"}, - }, - }, - } - - err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) - require.NoError(t, err) - - // assertions - - require.Equal( - t, - "bridge-mac", - inputNetworkingConfig.EndpointsConfig[Bridge].MacAddress, - ) - require.Equal( - t, - []string{"bridge-alias"}, - inputNetworkingConfig.EndpointsConfig[Bridge].Aliases, - ) - }) - t.Run("Request contains exposed port modifiers without protocol", func(t *testing.T) { req := ContainerRequest{ Image: nginxAlpineImage, // alpine image does expose port 80 diff --git a/reaper.go b/reaper.go index d0132fb809..9d74f573e2 100644 --- a/reaper.go +++ b/reaper.go @@ -254,7 +254,7 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider) ( HostConfigModifier: func(hc *container.HostConfig) { hc.AutoRemove = true hc.Binds = []string{dockerHostMount + ":/var/run/docker.sock"} - hc.NetworkMode = Bridge + hc.NetworkMode = "bridge" }, Env: map[string]string{}, } diff --git a/reaper_test.go b/reaper_test.go index e285600465..cd0e1ee7d8 100644 --- a/reaper_test.go +++ b/reaper_test.go @@ -438,7 +438,7 @@ func Test_NewReaper(t *testing.T) { assert.Equal(t, test.req.Env, provider.req.Env, "expected env doesn't match the submitted request") // checks for reaper's preCreationCallback fields - assert.Equal(t, container.NetworkMode(Bridge), provider.hostConfig.NetworkMode, "expected networkMode doesn't match the submitted request") + assert.Equal(t, container.NetworkMode("bridge"), provider.hostConfig.NetworkMode, "expected networkMode doesn't match the submitted request") assert.True(t, provider.hostConfig.AutoRemove, "expected networkMode doesn't match the submitted request") }) }