Skip to content

Commit

Permalink
chore: remove bridge network custom configuration
Browse files Browse the repository at this point in the history
It's not needed at all, as we rely on the underlying container runtime
  • Loading branch information
mdelapenya committed Oct 16, 2024
1 parent b327978 commit 33528c6
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 191 deletions.
6 changes: 3 additions & 3 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand Down
14 changes: 4 additions & 10 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
})
Expand All @@ -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")
Expand Down
10 changes: 0 additions & 10 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

// }
Expand Down Expand Up @@ -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
}

Expand Down
47 changes: 8 additions & 39 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
28 changes: 0 additions & 28 deletions lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
}
}
99 changes: 0 additions & 99 deletions lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"bufio"
"context"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"
Expand All @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion reaper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
}
Expand Down
2 changes: 1 addition & 1 deletion reaper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
Expand Down

0 comments on commit 33528c6

Please sign in to comment.