diff --git a/pkg/container/docker/client.go b/pkg/container/docker/client.go index a0909fcfc7..e05c5155ea 100644 --- a/pkg/container/docker/client.go +++ b/pkg/container/docker/client.go @@ -1638,9 +1638,11 @@ func mergeEnvVars(base, extra map[string]string) map[string]string { // the workload and returns the host-side port it is bound on. func (c *Client) setupIngressContainer(ctx context.Context, containerName string, upstreamPort int, attachStdio bool, externalEndpointsConfig map[string]*network.EndpointSettings, networkPermissions *permissions.NetworkPermissions) (int, error) { - squidPort, err := networking.FindOrUsePort(upstreamPort + 1) + // A random port avoids every same-image workload racing to bind the same + // preferred port when starting concurrently (see #6063). + squidPort, err := networking.FindOrUsePort(0) if err != nil { - return 0, fmt.Errorf("failed to find or use port %d: %w", squidPort, err) + return 0, fmt.Errorf("failed to find an available port: %w", err) } squidExposedPorts := map[string]struct{}{ fmt.Sprintf("%d/tcp", squidPort): {}, diff --git a/pkg/container/docker/envoy.go b/pkg/container/docker/envoy.go index 60b0be40d9..38decd6176 100644 --- a/pkg/container/docker/envoy.go +++ b/pkg/container/docker/envoy.go @@ -886,7 +886,9 @@ func (e *envoyProxy) SetupIngress(ctx context.Context, spec proxySpec, _ egressR var ingressPort int if spec.TransportType != "stdio" && spec.UpstreamPort > 0 { - port, err := networking.FindOrUsePort(spec.UpstreamPort + 1) + // A random port avoids every same-image workload racing to bind the + // same preferred port when starting concurrently (see #6063). + port, err := networking.FindOrUsePort(0) if err != nil { return 0, fmt.Errorf("failed to find ingress port: %w", err) } diff --git a/pkg/container/docker/envoy_test.go b/pkg/container/docker/envoy_test.go index e5bfafdbd3..a45c8b0bef 100644 --- a/pkg/container/docker/envoy_test.go +++ b/pkg/container/docker/envoy_test.go @@ -6,10 +6,12 @@ package docker import ( "context" "encoding/json" + "fmt" "os" "os/exec" "regexp" "strings" + "sync" "testing" "time" @@ -821,6 +823,70 @@ func TestEnvoyProxy_SetupOrchestration(t *testing.T) { } } +// TestEnvoyProxy_SetupIngress_ConcurrentSameUpstreamPort guards against a +// deterministic ingress port collision: two workloads built from the same +// image share the same fixed UpstreamPort, so deriving the ingress port from +// UpstreamPort+1 made every concurrently-starting instance request the +// identical host port. The fix picks a random port instead, so concurrent +// SetupIngress calls sharing UpstreamPort must land on different ports. +func TestEnvoyProxy_SetupIngress_ConcurrentSameUpstreamPort(t *testing.T) { + t.Parallel() + + const upstreamPort = 8080 + const workloadCount = 4 + + ports := make([]int, workloadCount) + errs := make([]error, workloadCount) + + var wg sync.WaitGroup + for i := range workloadCount { + wg.Add(1) + go func(idx int) { + defer wg.Done() + api := &fakeDockerAPI{ + createFunc: func(_ context.Context, _ *container.Config, _ *container.HostConfig, + _ *network.NetworkingConfig, _ *v1.Platform, _ string) (container.CreateResponse, error) { + return container.CreateResponse{ID: "cid-envoy"}, nil + }, + startFunc: func(_ context.Context, _ string, _ mobyclient.ContainerStartOptions) error { return nil }, + } + e := &envoyProxy{client: &Client{ + api: api, + imageManager: &fakeImageManager{availableImages: map[string]struct{}{defaultEnvoyImage: {}}}, + }} + spec := proxySpec{ + WorkloadName: fmt.Sprintf("app-%d", idx), + TransportType: "streamable-http", + UpstreamPort: upstreamPort, + GatewayIP: dockerDefaultBridgeGatewayIP, + Endpoints: map[string]*network.EndpointSettings{}, + } + port, err := e.SetupIngress(t.Context(), spec, egressResult{}) + ports[idx] = port + errs[idx] = err + }(i) + } + + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for concurrent SetupIngress calls") + } + + seen := make(map[int]bool, workloadCount) + for i, port := range ports { + require.NoError(t, errs[i]) + assert.NotEqual(t, upstreamPort+1, port, "ingress port must not be derived from UpstreamPort+1") + assert.False(t, seen[port], "port %d was reserved by more than one concurrent workload", port) + seen[port] = true + } +} + // TestListenerTimeoutsDisabledForStreaming guards against Envoy's 15s default // RouteAction.timeout (a hard total-response cap) and 5m stream_idle_timeout // truncating long-lived MCP streams (SSE / streamable-http). Both the ingress