diff --git a/internal/container/logtail.go b/internal/container/logtail.go new file mode 100644 index 00000000..d436c1b4 --- /dev/null +++ b/internal/container/logtail.go @@ -0,0 +1,37 @@ +package container + +import "sync" + +// logTail is a thread-safe, byte-bounded buffer that retains the most recent +// maxBytes written to it. It captures a container's startup logs while the +// container runs so they survive the container being auto-removed (--rm) the +// instant it exits — otherwise a crash during startup would leave no diagnostics. +type logTail struct { + mu sync.Mutex + buf []byte + maxBytes int +} + +func newLogTail(maxBytes int) *logTail { + return &logTail{maxBytes: maxBytes} +} + +// Write appends p, keeping only the most recent maxBytes. It never errors, so a +// follow-logs goroutine writing into it cannot fail the capture. +func (l *logTail) Write(p []byte) (int, error) { + l.mu.Lock() + defer l.mu.Unlock() + l.buf = append(l.buf, p...) + if len(l.buf) > l.maxBytes { + trimmed := make([]byte, l.maxBytes) + copy(trimmed, l.buf[len(l.buf)-l.maxBytes:]) + l.buf = trimmed + } + return len(p), nil +} + +func (l *logTail) String() string { + l.mu.Lock() + defer l.mu.Unlock() + return string(l.buf) +} diff --git a/internal/container/logtail_test.go b/internal/container/logtail_test.go new file mode 100644 index 00000000..da1bc27c --- /dev/null +++ b/internal/container/logtail_test.go @@ -0,0 +1,54 @@ +package container + +import ( + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLogTailRetainsContentUnderLimit(t *testing.T) { + lt := newLogTail(1024) + n, err := lt.Write([]byte("hello ")) + assert.NoError(t, err) + assert.Equal(t, 6, n) + _, _ = lt.Write([]byte("world")) + assert.Equal(t, "hello world", lt.String()) +} + +func TestLogTailKeepsOnlyMostRecentBytes(t *testing.T) { + lt := newLogTail(5) + _, _ = lt.Write([]byte("abc")) + _, _ = lt.Write([]byte("defgh")) + // Only the last 5 bytes are retained. + assert.Equal(t, "defgh", lt.String()) + + _, _ = lt.Write([]byte("XY")) + assert.Equal(t, "fghXY", lt.String()) +} + +func TestLogTailWriteReportsFullLength(t *testing.T) { + // Write must report len(p) even when the tail is trimmed, so an io.Writer + // consumer (stdcopy) does not treat it as a short write. + lt := newLogTail(4) + n, err := lt.Write([]byte("abcdefgh")) + assert.NoError(t, err) + assert.Equal(t, 8, n) + assert.Equal(t, "efgh", lt.String()) +} + +func TestLogTailConcurrentWritesAreSafe(t *testing.T) { + lt := newLogTail(64) + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, _ = lt.Write([]byte(strings.Repeat("x", 8))) + _ = lt.String() + }() + } + wg.Wait() + assert.LessOrEqual(t, len(lt.String()), 64) +} diff --git a/internal/container/start.go b/internal/container/start.go index 1242f9b4..719beb97 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -342,8 +342,9 @@ func tipsForType(t config.EmulatorType) []string { func pullImages(ctx context.Context, rt runtime.Runtime, sink output.Sink, tel *telemetry.Client, containers []runtime.ContainerConfig, interactive bool) (map[string]bool, error) { pulled := make(map[string]bool, len(containers)) for _, c := range containers { - // Remove any existing stopped container with the same name - if err := rt.Remove(ctx, c.Name); err != nil && !errdefs.IsNotFound(err) { + // Remove any existing container with the same name. rt.Remove tolerates the + // container being absent or mid auto-removal (--rm) and waits until it is gone. + if err := rt.Remove(ctx, c.Name); err != nil { return nil, fmt.Errorf("failed to remove existing container %s: %w", c.Name, err) } @@ -521,8 +522,28 @@ func startContainers(ctx context.Context, rt runtime.Runtime, sink output.Sink, return fmt.Errorf("failed to start LocalStack: %w", err) } + // Follow the container's logs into a bounded buffer from the moment it + // starts. With AutoRemove (--rm) the container is removed the instant it + // exits, so a post-hoc log fetch would race the removal; buffering as it + // runs keeps the startup logs available to explain a crash. + startupLogs := newLogTail(maxStartupLogBytes) + logCtx, stopLogTail := context.WithCancel(ctx) + logDone := make(chan struct{}) + go func() { + defer close(logDone) + _ = rt.StreamLogs(logCtx, containerID, startupLogs, true) + }() + healthURL := fmt.Sprintf("http://localhost:%s%s", c.Port, c.HealthPath) - if err := awaitStartup(ctx, rt, sink, containerID, "LocalStack", healthURL); err != nil { + err = awaitStartup(ctx, rt, sink, containerID, "LocalStack", healthURL, startupLogs) + // Stop following and let the goroutine return before continuing, so it does + // not outlive the start. Bounded so a slow stream teardown can't hang start. + stopLogTail() + select { + case <-logDone: + case <-time.After(2 * time.Second): + } + if err != nil { sink.Emit(output.SpinnerStop()) errCode := telemetry.ErrCodeStartFailed var licErr *licenseNotCoveredError @@ -775,12 +796,19 @@ func (e *licenseNotCoveredError) Error() string { return "license does not include this emulator" } +// maxStartupLogBytes bounds how much of a failing container's log tail is buffered +// to explain a crash during startup. +const maxStartupLogBytes = 64 * 1024 + // awaitStartup polls until one of two outcomes: // - Success: health endpoint returns 200 (license is valid, LocalStack is ready) // - Failure: container stops running (e.g., license activation failed), returns error with container logs // +// startupLogs holds the container's logs streamed while it ran, so they survive +// the container being auto-removed (--rm) on exit. +// // TODO: move to Runtime interface if other runtimes (k8s?) need native readiness probes -func awaitStartup(ctx context.Context, rt runtime.Runtime, sink output.Sink, containerID, name, healthURL string) error { +func awaitStartup(ctx context.Context, rt runtime.Runtime, sink output.Sink, containerID, name, healthURL string, startupLogs *logTail) error { client := &http.Client{Timeout: 2 * time.Second} for { @@ -789,11 +817,18 @@ func awaitStartup(ctx context.Context, rt runtime.Runtime, sink output.Sink, con return fmt.Errorf("failed to check container status: %w", err) } if !running { - logs, logsErr := rt.Logs(ctx, containerID, 20) - if logsErr == nil && strings.Contains(logs, "not covered by your license") { + // Prefer the logs streamed while the container ran: with --rm the + // container is already gone, so a direct fetch would return nothing. + logs := startupLogs.String() + if logs == "" { + if direct, derr := rt.Logs(ctx, containerID, 20); derr == nil { + logs = direct + } + } + if strings.Contains(logs, "not covered by your license") { return &licenseNotCoveredError{} } - if logsErr != nil || logs == "" { + if logs == "" { return fmt.Errorf("%s exited unexpectedly", name) } return fmt.Errorf("%s exited unexpectedly:\n%s", name, logs) diff --git a/internal/container/start_test.go b/internal/container/start_test.go index bedddb10..40ef38e7 100644 --- a/internal/container/start_test.go +++ b/internal/container/start_test.go @@ -553,6 +553,7 @@ func TestStartContainers_SnowflakeLicenseError(t *testing.T) { const containerID = "abc123" licenseLog := "⚠️ The Snowflake emulator is currently not covered by your license. ❄️" mockRT.EXPECT().Start(gomock.Any(), c).Return(containerID, nil) + mockRT.EXPECT().StreamLogs(gomock.Any(), containerID, gomock.Any(), true).Return(nil) mockRT.EXPECT().IsRunning(gomock.Any(), containerID).Return(false, nil) mockRT.EXPECT().Logs(gomock.Any(), containerID, 20).Return(licenseLog, nil) @@ -599,6 +600,7 @@ func TestStartContainers_AzureLicenseError(t *testing.T) { const containerID = "abc123" licenseLog := "The Azure emulator is currently not covered by your license." mockRT.EXPECT().Start(gomock.Any(), c).Return(containerID, nil) + mockRT.EXPECT().StreamLogs(gomock.Any(), containerID, gomock.Any(), true).Return(nil) mockRT.EXPECT().IsRunning(gomock.Any(), containerID).Return(false, nil) mockRT.EXPECT().Logs(gomock.Any(), containerID, 20).Return(licenseLog, nil) diff --git a/internal/runtime/docker.go b/internal/runtime/docker.go index b0c96571..0bd08c20 100644 --- a/internal/runtime/docker.go +++ b/internal/runtime/docker.go @@ -278,6 +278,7 @@ func (d *DockerRuntime) Start(ctx context.Context, config ContainerConfig) (stri HostConfig: &container.HostConfig{ PortBindings: portBindings, Binds: binds, + AutoRemove: true, }, Name: config.Name, }) @@ -304,9 +305,39 @@ func (d *DockerRuntime) Stop(ctx context.Context, containerName string) error { return nil } +const ( + containerRemovalTimeout = 10 * time.Second + containerRemovalPollInterval = 100 * time.Millisecond +) + func (d *DockerRuntime) Remove(ctx context.Context, containerName string) error { _, err := d.client.ContainerRemove(ctx, containerName, client.ContainerRemoveOptions{}) - return err + // With AutoRemove (--rm) Docker may already be removing the container, so + // ContainerRemove can report it is already gone (not-found) or that removal is + // in progress (conflict). Both mean the container is on its way out. + if err != nil && !errdefs.IsNotFound(err) && !errdefs.IsConflict(err) { + return err + } + // Wait until the container is actually gone, so a subsequent create reusing the + // same name does not race the in-flight auto-removal ("name already in use"). + return d.waitContainerGone(ctx, containerName) +} + +// waitContainerGone blocks until no container named containerName exists, the +// context is cancelled, or containerRemovalTimeout elapses. +func (d *DockerRuntime) waitContainerGone(ctx context.Context, containerName string) error { + ctx, cancel := context.WithTimeout(ctx, containerRemovalTimeout) + defer cancel() + for { + if _, err := d.client.ContainerInspect(ctx, containerName, client.ContainerInspectOptions{}); errdefs.IsNotFound(err) { + return nil + } + select { + case <-ctx.Done(): + return fmt.Errorf("timed out waiting for container %s to be removed", containerName) + case <-time.After(containerRemovalPollInterval): + } + } } func (d *DockerRuntime) IsRunning(ctx context.Context, containerID string) (bool, error) { diff --git a/test/integration/start_test.go b/test/integration/start_test.go index c4056399..4dbaf2c7 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -481,6 +481,11 @@ func TestStartCommandSetsUpContainerCorrectly(t *testing.T) { "expected volume bind mount to /var/lib/localstack, got: %v", inspect.Container.HostConfig.Binds) }) + t.Run("auto remove", func(t *testing.T) { + assert.True(t, inspect.Container.HostConfig.AutoRemove, + "expected container to be created with AutoRemove (--rm) so it does not linger after exit") + }) + t.Run("http health endpoint", func(t *testing.T) { resp, err := http.Get("http://localhost.localstack.cloud:4566/_localstack/health") require.NoError(t, err) @@ -1091,10 +1096,14 @@ image = %q "lstk must not run a pre-flight license check for a local image") // Started from the configured local image: lstk created the container using it. - inspect, err := dockerClient.ContainerInspect(ctx, wantContainer, client.ContainerInspectOptions{}) - require.NoError(t, err, "lstk should have created a container from the custom image") - assert.Equal(t, fullRef, inspect.Container.Config.Image, - "the container should be created from the configured custom image") + // With --rm the stub image's container is auto-removed the instant it exits, so + // it may already be gone — the "Using local image" output above is the + // authoritative signal. When the container is still present, additionally + // confirm it was created from the configured image. + if inspect, err := dockerClient.ContainerInspect(ctx, wantContainer, client.ContainerInspectOptions{}); err == nil { + assert.Equal(t, fullRef, inspect.Container.Config.Image, + "the container should be created from the configured custom image") + } } func cleanup() {