diff --git a/internal/container/start.go b/internal/container/start.go index 1242f9b4..813343b2 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -637,17 +637,15 @@ func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink outpu } // Check extra ports required by this emulator (443 for HTTPS, 4510-4559 for - // the service port range). These are singletons: if any is taken, another - // LocalStack instance is likely running and we cannot start a new one. + // the service port range). A running LocalStack was already ruled out above + // by FindRunningByImage, so a conflict here means an unrelated process holds + // the port — we can't publish it, so we stop rather than fail at Docker bind. extraSpecs := make([]string, len(c.ExtraPorts)) for i, ep := range c.ExtraPorts { extraSpecs[i] = ep.HostPort } if conflictPort, err := ports.CheckAvailable(extraSpecs...); err != nil { - sink.Emit(output.ErrorEvent{ - Title: fmt.Sprintf("Port %s is already in use", conflictPort), - Summary: "LocalStack requires this port. Free it before starting.", - }) + emitPortInUseError(sink, conflictPort) tel.EmitEmulatorLifecycleEvent(ctx, telemetry.LifecycleEvent{ EventType: telemetry.LifecycleStartError, Emulator: c.EmulatorType, @@ -678,14 +676,16 @@ func emitLocalStackAlreadyRunningWarning(sink output.Sink, port, runningVersion, } func emitPortInUseError(sink output.Sink, port string) { - actions := []output.ErrorAction{} + actions := []output.ErrorAction{ + {Label: "Identify the process using it:", Value: ports.InspectCommand(port)}, + } configPath, pathErr := config.ConfigFilePath() if pathErr == nil { - actions = append(actions, output.ErrorAction{Label: "Use another port in the configuration:", Value: configPath}) + actions = append(actions, output.ErrorAction{Label: "Or use another port in the configuration:", Value: configPath}) } sink.Emit(output.ErrorEvent{ Title: fmt.Sprintf("Port %s already in use", port), - Summary: "Free the port or configure a different one.", + Summary: "Another process is already using this port.", Actions: actions, }) } diff --git a/internal/ports/ports.go b/internal/ports/ports.go index c10a5e00..f12e2d61 100644 --- a/internal/ports/ports.go +++ b/internal/ports/ports.go @@ -3,6 +3,7 @@ package ports import ( "fmt" "net" + "runtime" "strconv" "strings" "time" @@ -51,3 +52,23 @@ func dial(port string) error { _ = conn.Close() return fmt.Errorf("port %s already in use", port) } + +// InspectCommand returns a shell command the user can run to identify the +// process currently listening on the given port, tailored to the host OS. +// On Windows it uses netstat; elsewhere (macOS, Linux, *BSD) it uses lsof. +func InspectCommand(port string) string { + return inspectCommand(runtime.GOOS, port) +} + +func inspectCommand(goos, port string) string { + if goos == "windows" { + return fmt.Sprintf("netstat -ano | findstr :%s", port) + } + // Privileged ports (<1024, e.g. 443) are typically held by root-owned + // processes such as sshd; lsof only reports other users' sockets under + // sudo, so suggest sudo there to avoid an empty result. + if p, err := strconv.Atoi(port); err == nil && p < 1024 { + return fmt.Sprintf("sudo lsof -i tcp:%s", port) + } + return fmt.Sprintf("lsof -i tcp:%s", port) +} diff --git a/internal/ports/ports_test.go b/internal/ports/ports_test.go index bbcf7df1..bc51eb1b 100644 --- a/internal/ports/ports_test.go +++ b/internal/ports/ports_test.go @@ -73,6 +73,26 @@ func TestCheckAvailable(t *testing.T) { } } +func TestInspectCommand(t *testing.T) { + tests := []struct { + goos string + port string + want string + }{ + {"darwin", "443", "sudo lsof -i tcp:443"}, // privileged port -> sudo + {"linux", "443", "sudo lsof -i tcp:443"}, // privileged port -> sudo + {"linux", "4566", "lsof -i tcp:4566"}, // non-privileged -> plain + {"darwin", "4510", "lsof -i tcp:4510"}, // service range -> plain + {"windows", "443", "netstat -ano | findstr :443"}, + } + + for _, tt := range tests { + t.Run(tt.goos, func(t *testing.T) { + assert.Equal(t, tt.want, inspectCommand(tt.goos, tt.port)) + }) + } +} + // bindPort opens a listener on a random port and keeps it open for the test duration. func bindPort(t *testing.T) string { t.Helper() diff --git a/test/integration/start_test.go b/test/integration/start_test.go index c4056399..93f5df79 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -164,8 +164,10 @@ func TestStartCommandFailsWhenPortInUse(t *testing.T) { require.Error(t, err, "expected lstk start to fail when port is in use") requireExitCode(t, 1, err) assert.Contains(t, stdout, "Port 4566 already in use") - assert.Contains(t, stdout, "Free the port or configure a different one.") - assert.Contains(t, stdout, "Use another port in the configuration:") + assert.Contains(t, stdout, "Another process is already using this port") + assert.Contains(t, stdout, "Identify the process using it:") + assert.Contains(t, stdout, inspectCommandFor("4566")) + assert.Contains(t, stdout, "Or use another port in the configuration:") // Both lstk_lifecycle (start_error) and lstk_command events should be emitted. byName := collectTelemetryByName(t, events, 2) @@ -173,6 +175,47 @@ func TestStartCommandFailsWhenPortInUse(t *testing.T) { assert.Contains(t, byName, "lstk_command") } +// inspectCommandFor mirrors ports.InspectCommand for the current OS so the +// port-conflict tests can assert the diagnostic hint without importing the +// internal package. +func inspectCommandFor(port string) string { + if runtime.GOOS == "windows" { + return "netstat -ano | findstr :" + port + } + if p, err := strconv.Atoi(port); err == nil && p < 1024 { + return "sudo lsof -i tcp:" + port + } + return "lsof -i tcp:" + port +} + +// TestStartCommandFailsWhenExtraPortInUse covers the extra-port branch (443 and +// the 4510-4559 service range) of the pre-flight check. 443 is privileged and +// cannot be bound by a non-root test process, so we occupy a service-range port +// (4510) with the primary edge port (4566) left free. This exercises the same +// branch and asserts it now surfaces the same guidance as the 4566 conflict. +func TestStartCommandFailsWhenExtraPortInUse(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + + ln, err := net.Listen("tcp", ":4510") + require.NoError(t, err, "failed to bind port 4510 for test") + defer func() { _ = ln.Close() }() + + analyticsSrv, events := mockAnalyticsServer(t) + stdout, _, err := runLstk(t, testContext(t), "", env.With(env.AuthToken, "fake-token").With(env.AnalyticsEndpoint, analyticsSrv.URL), "start") + require.Error(t, err, "expected lstk start to fail when an extra port is in use") + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "Port 4510 already in use") + assert.Contains(t, stdout, "Another process is already using this port") + assert.Contains(t, stdout, "Identify the process using it:") + assert.Contains(t, stdout, inspectCommandFor("4510")) + + byName := collectTelemetryByName(t, events, 2) + assert.Contains(t, byName, "lstk_lifecycle") + assert.Contains(t, byName, "lstk_command") +} + func TestStartDoesNotHangWithExternalContainerAndNoCachedLabel(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("PTY not supported on Windows")