diff --git a/buildx.go b/buildx.go index a95bbb3..1b44094 100644 --- a/buildx.go +++ b/buildx.go @@ -14,7 +14,7 @@ const ( remoteDriver = "remote" ) -func cmdSetupBuildx(builder Builder, driverOpts []string, inheritAuth bool) *exec.Cmd { +func cmdSetupBuildx(builder Builder, driverOpts []string, inheritAuth bool, daemonMTU string) *exec.Cmd { args := []string{"buildx", "create", "--use", "--driver", builder.Driver} if builder.Name != "" { args = append(args, "--name", builder.Name) @@ -25,6 +25,8 @@ func cmdSetupBuildx(builder Builder, driverOpts []string, inheritAuth bool) *exe for _, opt := range driverOpts { args = append(args, "--driver-opt", opt) } + + usingHostNetwork := false if harnessHttpProxy := os.Getenv("HARNESS_HTTP_PROXY"); harnessHttpProxy != "" { args = append(args, "--driver-opt", fmt.Sprintf("env.http_proxy=%s", harnessHttpProxy)) @@ -33,6 +35,19 @@ func cmdSetupBuildx(builder Builder, driverOpts []string, inheritAuth bool) *exe } args = append(args, "--driver-opt", "network=host") + usingHostNetwork = true + } + + // Configure MTU for BuildKit container network (skip if using host network) + // Host network inherits MTU automatically, but bridge networks need explicit configuration + if !usingHostNetwork && builder.Driver == dockerContainerDriver { + effectiveMTU := getEffectiveMTU(daemonMTU) + if effectiveMTU != "" { + // Create a custom network with the correct MTU + // Note: We don't use network= driver-opt here because it doesn't support MTU parameter + // Instead, BuildKit container will use the default bridge, which inherits from dockerd's --mtu setting + fmt.Printf("BuildKit will inherit MTU from Docker daemon: %s\n", effectiveMTU) + } } if builder.RemoteConn != "" && builder.Driver == remoteDriver { diff --git a/docker.go b/docker.go index 3ae18f5..2cb062e 100644 --- a/docker.go +++ b/docker.go @@ -166,6 +166,19 @@ const ( // Exec executes the plugin step func (p Plugin) Exec() error { + // Auto-detect and configure MTU if not explicitly set + // This must happen BEFORE starting the daemon + if p.Daemon.MTU == "" && !p.Daemon.Disabled { + if detectedMTU, err := detectNetworkMTU(); err == nil { + fmt.Printf("Auto-detected network MTU: %s (will be applied to Docker daemon and BuildKit containers)\n", detectedMTU) + p.Daemon.MTU = detectedMTU + } else { + fmt.Printf("Note: Could not auto-detect MTU (%s). Docker will use default MTU (1500).\n", err) + fmt.Printf("If you experience network issues, set PLUGIN_MTU explicitly (e.g., PLUGIN_MTU=1400)\n") + } + } else if p.Daemon.MTU != "" { + fmt.Printf("Using explicitly configured MTU: %s\n", p.Daemon.MTU) + } // start the Docker daemon server if !p.Daemon.Disabled { @@ -335,7 +348,7 @@ func (p Plugin) Exec() error { fmt.Printf("Using BuildKit Version with new driver opts: %s\n", p.Builder.BuildkitVersion) updateImageVersion(&p.Builder.DriverOptsNew, p.Builder.BuildkitVersion) } - createCmd := cmdSetupBuildx(p.Builder, p.Builder.DriverOptsNew, p.BuildkitInheritAuth) + createCmd := cmdSetupBuildx(p.Builder, p.Builder.DriverOptsNew, p.BuildkitInheritAuth, p.Daemon.MTU) raw, err = createCmd.Output() if err != nil { fmt.Printf("Unable to setup buildx with new driver opts: %s\n", err) @@ -367,7 +380,7 @@ func (p Plugin) Exec() error { fmt.Printf("Using BuildKit Version: %s\n", version) updateImageVersion(&p.Builder.DriverOpts, version) } - createCmd := cmdSetupBuildx(p.Builder, p.Builder.DriverOpts, p.BuildkitInheritAuth) + createCmd := cmdSetupBuildx(p.Builder, p.Builder.DriverOpts, p.BuildkitInheritAuth, p.Daemon.MTU) raw, err = createCmd.Output() if err != nil { return fmt.Errorf("error while creating buildx builder: %s and err: %s", string(raw), err) diff --git a/mtu_backward_compat_test.go b/mtu_backward_compat_test.go new file mode 100644 index 0000000..c0ed13c --- /dev/null +++ b/mtu_backward_compat_test.go @@ -0,0 +1,192 @@ +package docker + +import ( + "fmt" + "os" + "testing" +) + +// TestBackwardCompatibility_ExplicitMTU verifies that when PLUGIN_MTU is set, +// the auto-detection is skipped and the explicit value is used +func TestBackwardCompatibility_ExplicitMTU(t *testing.T) { + // Simulate the behavior when PLUGIN_MTU is set + daemon := Daemon{ + MTU: "1400", // This would be set from PLUGIN_MTU via app.go + Disabled: false, + } + + // This is what happens in Plugin.Exec() + // The auto-detection should NOT run because MTU is already set + if daemon.MTU == "" && !daemon.Disabled { + t.Error("Auto-detection should NOT run when MTU is explicitly set") + } else if daemon.MTU != "" { + // This path should execute + fmt.Printf("Using explicitly configured MTU: %s\n", daemon.MTU) + } + + // Verify the value hasn't changed + if daemon.MTU != "1400" { + t.Errorf("Expected MTU to remain 1400, got %s", daemon.MTU) + } +} + +// TestBackwardCompatibility_AutoDetect verifies that when PLUGIN_MTU is NOT set, +// auto-detection runs +func TestBackwardCompatibility_AutoDetect(t *testing.T) { + // Simulate the behavior when PLUGIN_MTU is NOT set + daemon := Daemon{ + MTU: "", // Empty, as it would be if PLUGIN_MTU not set + Disabled: false, + } + + // This is what happens in Plugin.Exec() + if daemon.MTU == "" && !daemon.Disabled { + // Auto-detection should run + if detectedMTU, err := detectNetworkMTU(); err == nil { + fmt.Printf("Auto-detected network MTU: %s\n", detectedMTU) + daemon.MTU = detectedMTU + } else { + fmt.Printf("Note: Could not auto-detect MTU (%s). Will use Docker default.\n", err) + } + } + + // Verify that SOME value was attempted to be set (or we got a detection error) + // This is fine either way - the important thing is auto-detection ran + t.Logf("After auto-detection: MTU = %s", daemon.MTU) +} + +// TestBackwardCompatibility_DaemonDisabled verifies that when daemon is disabled, +// no MTU configuration happens +func TestBackwardCompatibility_DaemonDisabled(t *testing.T) { + daemon := Daemon{ + MTU: "", + Disabled: true, // Daemon is disabled (already running) + } + + // This is what happens in Plugin.Exec() + if daemon.MTU == "" && !daemon.Disabled { + t.Error("Auto-detection should NOT run when daemon is disabled") + } + + // MTU should remain empty + if daemon.MTU != "" { + t.Errorf("Expected MTU to remain empty when daemon disabled, got %s", daemon.MTU) + } +} + +// TestBackwardCompatibility_CommandLine verifies the dockerd command is built correctly +func TestBackwardCompatibility_CommandLine(t *testing.T) { + tests := []struct { + name string + mtu string + shouldHaveMTU bool + }{ + { + name: "With explicit MTU", + mtu: "1400", + shouldHaveMTU: true, + }, + { + name: "Without MTU", + mtu: "", + shouldHaveMTU: false, + }, + { + name: "With auto-detected MTU", + mtu: "1450", + shouldHaveMTU: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + daemon := Daemon{ + MTU: tt.mtu, + StoragePath: "/var/lib/docker", + } + + cmd := commandDaemon(daemon) + args := cmd.Args + + foundMTU := false + var mtuValue string + for i, arg := range args { + if arg == "--mtu" { + foundMTU = true + if i+1 < len(args) { + mtuValue = args[i+1] + } + break + } + } + + if tt.shouldHaveMTU { + if !foundMTU { + t.Errorf("Expected --mtu flag in command, but not found. Args: %v", args) + } + if mtuValue != tt.mtu { + t.Errorf("Expected --mtu %s, got %s", tt.mtu, mtuValue) + } + } else { + if foundMTU { + t.Errorf("Expected no --mtu flag in command, but found: --mtu %s", mtuValue) + } + } + }) + } +} + +// TestBackwardCompatibility_EnvVarParsing simulates the full flow from env var to daemon +func TestBackwardCompatibility_EnvVarParsing(t *testing.T) { + // Save original env var + originalMTU := os.Getenv("PLUGIN_MTU") + defer func() { + if originalMTU != "" { + os.Setenv("PLUGIN_MTU", originalMTU) + } else { + os.Unsetenv("PLUGIN_MTU") + } + }() + + tests := []struct { + name string + envValue string + expectedMTU string + }{ + { + name: "PLUGIN_MTU set to 1400", + envValue: "1400", + expectedMTU: "1400", + }, + { + name: "PLUGIN_MTU set to 1450", + envValue: "1450", + expectedMTU: "1450", + }, + { + name: "PLUGIN_MTU empty", + envValue: "", + expectedMTU: "", // Should trigger auto-detection + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envValue != "" { + os.Setenv("PLUGIN_MTU", tt.envValue) + } else { + os.Unsetenv("PLUGIN_MTU") + } + + // In the real app.go, this would be: c.String("daemon.mtu") + // which reads from PLUGIN_MTU + envMTU := os.Getenv("PLUGIN_MTU") + + if envMTU != tt.expectedMTU { + t.Errorf("Expected env MTU %s, got %s", tt.expectedMTU, envMTU) + } + + t.Logf("PLUGIN_MTU=%s correctly read as: %s", tt.envValue, envMTU) + }) + } +} diff --git a/network.go b/network.go new file mode 100644 index 0000000..ed16c6f --- /dev/null +++ b/network.go @@ -0,0 +1,69 @@ +package docker + +import ( + "fmt" + "net" +) + +// detectNetworkMTU detects the MTU of the default network interface +// Returns the MTU value as a string, or an error if detection fails +// Uses Go standard library for cross-platform compatibility (Linux, macOS, Windows) +func detectNetworkMTU() (string, error) { + // Determine the default interface by dialing out to a known external IP + // This doesn't actually send data, just determines routing + conn, err := net.Dial("udp", "8.8.8.8:80") + if err != nil { + return "", fmt.Errorf("failed to determine default network interface: %w", err) + } + defer conn.Close() + + // Get the local address used for this connection + localAddr := conn.LocalAddr().(*net.UDPAddr) + + // Get all network interfaces + interfaces, err := net.Interfaces() + if err != nil { + return "", fmt.Errorf("failed to get network interfaces: %w", err) + } + + // Find the interface that has the local address + for _, iface := range interfaces { + addrs, err := iface.Addrs() + if err != nil { + continue + } + + for _, addr := range addrs { + ipNet, ok := addr.(*net.IPNet) + if ok && ipNet.IP.Equal(localAddr.IP) { + // Found the interface - return its MTU + if iface.MTU <= 0 { + return "", fmt.Errorf("invalid MTU value: %d", iface.MTU) + } + return fmt.Sprintf("%d", iface.MTU), nil + } + } + } + + return "", fmt.Errorf("could not determine MTU for default network interface") +} + +// getEffectiveMTU returns the MTU value to use for BuildKit containers +// Priority: daemon.MTU (explicit) > auto-detected > fallback to empty (docker default) +func getEffectiveMTU(daemonMTU string) string { + // If explicitly set via PLUGIN_MTU, use that + if daemonMTU != "" { + fmt.Printf("Using explicitly configured MTU: %s\n", daemonMTU) + return daemonMTU + } + + // Try to auto-detect + detectedMTU, err := detectNetworkMTU() + if err != nil { + fmt.Printf("Warning: Could not auto-detect MTU (%s), BuildKit will use Docker default\n", err) + return "" + } + + fmt.Printf("Auto-detected network MTU: %s\n", detectedMTU) + return detectedMTU +} diff --git a/network_test.go b/network_test.go new file mode 100644 index 0000000..452ac49 --- /dev/null +++ b/network_test.go @@ -0,0 +1,89 @@ +package docker + +import ( + "fmt" + "testing" +) + +func TestDetectNetworkMTU(t *testing.T) { + mtu, err := detectNetworkMTU() + + if err != nil { + t.Logf("Warning: Could not detect MTU (this may be expected in test environment): %v", err) + // Don't fail the test - MTU detection may not work in all environments + return + } + + if mtu == "" { + t.Error("Expected MTU to be non-empty") + } + + // Basic validation: MTU should be a reasonable value + // Common values: 1280 (minimum IPv6), 1450-1500 (common), up to 9000 (jumbo frames) + t.Logf("Detected MTU: %s", mtu) + + // Parse as integer to validate + var mtuInt int + _, err = fmt.Sscanf(mtu, "%d", &mtuInt) + if err != nil { + t.Errorf("MTU is not a valid integer: %s", mtu) + } + + if mtuInt < 1280 { + t.Errorf("MTU %d is too small (minimum expected: 1280)", mtuInt) + } + + if mtuInt > 9000 { + t.Errorf("MTU %d is unusually large (maximum expected: 9000)", mtuInt) + } +} + +func TestGetEffectiveMTU(t *testing.T) { + tests := []struct { + name string + daemonMTU string + wantExplicit bool + }{ + { + name: "explicit MTU set", + daemonMTU: "1400", + wantExplicit: true, + }, + { + name: "empty MTU triggers auto-detect", + daemonMTU: "", + wantExplicit: false, + }, + { + name: "zero MTU triggers auto-detect", + daemonMTU: "0", + wantExplicit: true, // "0" is explicit, even if unusual + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getEffectiveMTU(tt.daemonMTU) + + if tt.wantExplicit { + if result != tt.daemonMTU { + t.Errorf("Expected explicit MTU %s, got %s", tt.daemonMTU, result) + } + } else { + // For auto-detect, we just verify it returns something + // (or empty if detection fails) + t.Logf("Auto-detected MTU: %s", result) + } + }) + } +} + +func TestGetEffectiveMTU_Priority(t *testing.T) { + // Test that explicit MTU takes priority over auto-detection + explicitMTU := "1350" + result := getEffectiveMTU(explicitMTU) + + if result != explicitMTU { + t.Errorf("Expected explicit MTU %s to take priority, got %s", explicitMTU, result) + } +}