From 6647ab92eac103c262cdead4cbb30ce183ceae5e Mon Sep 17 00:00:00 2001 From: carole-lavillonniere Date: Tue, 17 Feb 2026 15:58:48 +0100 Subject: [PATCH 1/4] handle container already running --- internal/container/start.go | 47 ++++++++++++++++++++++++++++++++ test/integration/license_test.go | 10 +++---- test/integration/main_test.go | 26 ++++++++++++++++++ test/integration/start_test.go | 42 ++++++++++++++++++++++++++-- test/integration/stop_test.go | 22 --------------- 5 files changed, 117 insertions(+), 30 deletions(-) diff --git a/internal/container/start.go b/internal/container/start.go index a370a3e..7a54892 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -3,6 +3,7 @@ package container import ( "context" "fmt" + "net" "net/http" "os" stdruntime "runtime" @@ -54,6 +55,15 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformCl } } + // Check state before proceeding: skip already-running containers, reject port conflicts. + containers, cfg.Containers, err = selectContainersToStart(ctx, rt, sink, containers, cfg.Containers) + if err != nil { + return err + } + if len(containers) == 0 { + return nil + } + // Pull all images first for _, config := range containers { // Remove any existing stopped container with the same name @@ -101,6 +111,43 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformCl return nil } +func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink output.Sink, containers []runtime.ContainerConfig, cfgContainers []config.ContainerConfig) ([]runtime.ContainerConfig, []config.ContainerConfig, error) { + var filtered []runtime.ContainerConfig + var filteredCfg []config.ContainerConfig + for i, c := range containers { + running, err := rt.IsRunning(ctx, c.Name) + if err != nil && !errdefs.IsNotFound(err) { + return nil, nil, fmt.Errorf("failed to check container status: %w", err) + } + if running { + output.EmitLog(sink, fmt.Sprintf("%s is already running", c.Name)) + continue + } + if err := checkPortAvailable(c.Port); err != nil { + configPath, pathErr := config.ConfigFilePath() + if pathErr != nil { + return nil, nil, err + } + return nil, nil, fmt.Errorf("%w\nTo use a different port, edit %s", err, configPath) + } + filtered = append(filtered, c) + filteredCfg = append(filteredCfg, cfgContainers[i]) + } + return filtered, filteredCfg, nil +} + +func checkPortAvailable(port string) error { + conn, err := net.DialTimeout("tcp", "localhost:"+port, time.Second) + if err != nil { + return nil + } + err = conn.Close() + if err != nil { + return nil + } + return fmt.Errorf("port %s already in use", port) +} + func validateLicense(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformClient api.PlatformAPI, containerConfig runtime.ContainerConfig, cfgContainer *config.ContainerConfig, token string) error { version := cfgContainer.Tag if version == "" || version == "latest" { diff --git a/test/integration/license_test.go b/test/integration/license_test.go index a22232f..f04e460 100644 --- a/test/integration/license_test.go +++ b/test/integration/license_test.go @@ -16,8 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -const licenseContainerName = "localstack-aws" - func TestLicenseValidationSuccess(t *testing.T) { requireDocker(t) authToken := env.Require(t, env.AuthToken) @@ -74,7 +72,7 @@ func TestLicenseValidationSuccess(t *testing.T) { require.NoError(t, err, "lstk start failed: %s", output) - inspect, err := dockerClient.ContainerInspect(ctx, licenseContainerName) + inspect, err := dockerClient.ContainerInspect(ctx, containerName) require.NoError(t, err, "failed to inspect container") assert.True(t, inspect.State.Running, "container should be running") } @@ -99,12 +97,12 @@ func TestLicenseValidationFailure(t *testing.T) { assert.Contains(t, string(output), "invalid, inactive, or expired") // Verify container was not started - _, err = dockerClient.ContainerInspect(ctx, licenseContainerName) + _, err = dockerClient.ContainerInspect(ctx, containerName) assert.Error(t, err, "container should not exist after license failure") } func cleanupLicense() { ctx := context.Background() - _ = dockerClient.ContainerStop(ctx, licenseContainerName, container.StopOptions{}) - _ = dockerClient.ContainerRemove(ctx, licenseContainerName, container.RemoveOptions{Force: true}) + _ = dockerClient.ContainerStop(ctx, containerName, container.StopOptions{}) + _ = dockerClient.ContainerRemove(ctx, containerName, container.RemoveOptions{Force: true}) } diff --git a/test/integration/main_test.go b/test/integration/main_test.go index 82c9a4e..02a78d5 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "io" "net/http" "net/http/httptest" "os" @@ -14,8 +15,11 @@ import ( "testing" "github.com/99designs/keyring" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/image" "github.com/docker/docker/client" "github.com/localstack/lstk/test/integration/env" + "github.com/stretchr/testify/require" ) // syncBuffer is a thread-safe buffer for concurrent read/write access. @@ -137,6 +141,28 @@ func DeleteAuthTokenFromKeyring() error { return err } +const ( + containerName = "localstack-aws" + testImage = "alpine:latest" +) + +func startTestContainer(t *testing.T, ctx context.Context) { + t.Helper() + + reader, err := dockerClient.ImagePull(ctx, testImage, image.PullOptions{}) + require.NoError(t, err, "failed to pull test image") + _, _ = io.Copy(io.Discard, reader) + _ = reader.Close() + + resp, err := dockerClient.ContainerCreate(ctx, &container.Config{ + Image: testImage, + Cmd: []string{"sleep", "infinity"}, + }, nil, nil, nil, containerName) + require.NoError(t, err, "failed to create test container") + err = dockerClient.ContainerStart(ctx, resp.ID, container.StartOptions{}) + require.NoError(t, err, "failed to start test container") +} + func createMockLicenseServer(success bool) *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == "POST" && r.URL.Path == "/v1/license/request" { diff --git a/test/integration/start_test.go b/test/integration/start_test.go index 2826ab4..272a408 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -2,6 +2,8 @@ package integration_test import ( "context" + "net" + "os" "os/exec" "testing" "time" @@ -12,8 +14,6 @@ import ( "github.com/stretchr/testify/require" ) -const containerName = "localstack-aws" - func TestStartCommandSucceedsWithValidToken(t *testing.T) { requireDocker(t) _ = env.Require(t, env.AuthToken) @@ -85,6 +85,44 @@ func TestStartCommandFailsWithInvalidToken(t *testing.T) { assert.Contains(t, string(output), "license validation failed") } +func TestStartCommandDoesNothingWhenAlreadyRunning(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + startTestContainer(t, ctx) + + cmd := exec.CommandContext(ctx, binaryPath(), "start") + cmd.Env = append(os.Environ(), "LOCALSTACK_AUTH_TOKEN=fake-token") + output, err := cmd.CombinedOutput() + + require.NoError(t, err, "lstk start should succeed when container is already running: %s", output) + assert.Contains(t, string(output), "already running") +} + +func TestStartCommandFailsWhenPortInUse(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + + ln, err := net.Listen("tcp", ":4566") + require.NoError(t, err, "failed to bind port 4566 for test") + defer func() { _ = ln.Close() }() + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binaryPath(), "start") + cmd.Env = append(os.Environ(), "LOCALSTACK_AUTH_TOKEN=fake-token") + output, err := cmd.CombinedOutput() + + require.Error(t, err, "expected lstk start to fail when port is in use") + assert.Contains(t, string(output), "port 4566 already in use") +} + func cleanup() { ctx := context.Background() _ = dockerClient.ContainerStop(ctx, containerName, container.StopOptions{}) diff --git a/test/integration/stop_test.go b/test/integration/stop_test.go index 01cad28..6f1288b 100644 --- a/test/integration/stop_test.go +++ b/test/integration/stop_test.go @@ -2,36 +2,14 @@ package integration_test import ( "context" - "io" "os/exec" "testing" "time" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/image" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -const testImage = "alpine:latest" - -func startTestContainer(t *testing.T, ctx context.Context) { - t.Helper() - - reader, err := dockerClient.ImagePull(ctx, testImage, image.PullOptions{}) - require.NoError(t, err, "failed to pull test image") - _, _ = io.Copy(io.Discard, reader) - _ = reader.Close() - - resp, err := dockerClient.ContainerCreate(ctx, &container.Config{ - Image: testImage, - Cmd: []string{"sleep", "infinity"}, - }, nil, nil, nil, containerName) - require.NoError(t, err, "failed to create test container") - err = dockerClient.ContainerStart(ctx, resp.ID, container.StartOptions{}) - require.NoError(t, err, "failed to start test container") -} - func TestStopCommandSucceeds(t *testing.T) { requireDocker(t) cleanup() From 73f3a09992109513653565ecf5522e7350926c3c Mon Sep 17 00:00:00 2001 From: carole-lavillonniere Date: Tue, 17 Feb 2026 16:07:17 +0100 Subject: [PATCH 2/4] refactoring --- internal/container/start.go | 58 +++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/internal/container/start.go b/internal/container/start.go index 7a54892..01a74b5 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -55,7 +55,6 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformCl } } - // Check state before proceeding: skip already-running containers, reject port conflicts. containers, cfg.Containers, err = selectContainersToStart(ctx, rt, sink, containers, cfg.Containers) if err != nil { return err @@ -64,50 +63,65 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformCl return nil } - // Pull all images first - for _, config := range containers { + // TODO validate license for tag "latest" without resolving the actual image version, + // and avoid pulling all images first + if err := pullImages(ctx, rt, sink, containers); err != nil { + return err + } + + if err := validateLicenses(ctx, rt, sink, platformClient, containers, cfg.Containers, token); err != nil { + return err + } + + return startContainers(ctx, rt, sink, containers) +} + +func pullImages(ctx context.Context, rt runtime.Runtime, sink output.Sink, containers []runtime.ContainerConfig) error { + for _, c := range containers { // Remove any existing stopped container with the same name - if err := rt.Remove(ctx, config.Name); err != nil && !errdefs.IsNotFound(err) { - return fmt.Errorf("failed to remove existing container %s: %w", config.Name, err) + if err := rt.Remove(ctx, c.Name); err != nil && !errdefs.IsNotFound(err) { + return fmt.Errorf("failed to remove existing container %s: %w", c.Name, err) } - output.EmitStatus(sink, "pulling", config.Image, "") + output.EmitStatus(sink, "pulling", c.Image, "") progress := make(chan runtime.PullProgress) go func() { for p := range progress { - output.EmitProgress(sink, config.Image, p.LayerID, p.Status, p.Current, p.Total) + output.EmitProgress(sink, c.Image, p.LayerID, p.Status, p.Current, p.Total) } }() - if err := rt.PullImage(ctx, config.Image, progress); err != nil { - return fmt.Errorf("failed to pull image %s: %w", config.Image, err) + if err := rt.PullImage(ctx, c.Image, progress); err != nil { + return fmt.Errorf("failed to pull image %s: %w", c.Image, err) } } + return nil +} - // TODO validate license for tag "latest" without resolving the actual image version, - // and avoid pulling all images first - for i, c := range cfg.Containers { +func validateLicenses(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformClient api.PlatformAPI, containers []runtime.ContainerConfig, cfgContainers []config.ContainerConfig, token string) error { + for i, c := range cfgContainers { if err := validateLicense(ctx, rt, sink, platformClient, containers[i], &c, token); err != nil { return err } } + return nil +} - // Start containers - for _, config := range containers { - output.EmitStatus(sink, "starting", config.Name, "") - containerID, err := rt.Start(ctx, config) +func startContainers(ctx context.Context, rt runtime.Runtime, sink output.Sink, containers []runtime.ContainerConfig) error { + for _, c := range containers { + output.EmitStatus(sink, "starting", c.Name, "") + containerID, err := rt.Start(ctx, c) if err != nil { - return fmt.Errorf("failed to start %s: %w", config.Name, err) + return fmt.Errorf("failed to start %s: %w", c.Name, err) } - output.EmitStatus(sink, "waiting", config.Name, "") - healthURL := fmt.Sprintf("http://localhost:%s%s", config.Port, config.HealthPath) - if err := awaitStartup(ctx, rt, sink, containerID, config.Name, healthURL); err != nil { + output.EmitStatus(sink, "waiting", c.Name, "") + healthURL := fmt.Sprintf("http://localhost:%s%s", c.Port, c.HealthPath) + if err := awaitStartup(ctx, rt, sink, containerID, c.Name, healthURL); err != nil { return err } - output.EmitStatus(sink, "ready", config.Name, fmt.Sprintf("containerId: %s", containerID[:12])) + output.EmitStatus(sink, "ready", c.Name, fmt.Sprintf("containerId: %s", containerID[:12])) } - return nil } From 02272426ebf76f9291e7da87e8eca06a447cf1dc Mon Sep 17 00:00:00 2001 From: carole-lavillonniere Date: Thu, 19 Feb 2026 15:16:09 +0100 Subject: [PATCH 3/4] add Tag and ProductName to runtime.ContainerConfig --- internal/container/start.go | 52 ++++++++++++++++++------------------- internal/runtime/runtime.go | 12 +++++---- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/internal/container/start.go b/internal/container/start.go index 01a74b5..b6ad258 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -44,18 +44,24 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformCl if err != nil { return err } + productName, err := c.ProductName() + if err != nil { + return err + } env := append(c.Env, "LOCALSTACK_AUTH_TOKEN="+token) containers[i] = runtime.ContainerConfig{ - Image: image, - Name: c.Name(), - Port: c.Port, - HealthPath: healthPath, - Env: env, + Image: image, + Name: c.Name(), + Port: c.Port, + HealthPath: healthPath, + Env: env, + Tag: c.Tag, + ProductName: productName, } } - containers, cfg.Containers, err = selectContainersToStart(ctx, rt, sink, containers, cfg.Containers) + containers, err = selectContainersToStart(ctx, rt, sink, containers) if err != nil { return err } @@ -69,7 +75,7 @@ func Start(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformCl return err } - if err := validateLicenses(ctx, rt, sink, platformClient, containers, cfg.Containers, token); err != nil { + if err := validateLicenses(ctx, rt, sink, platformClient, containers, token); err != nil { return err } @@ -97,9 +103,9 @@ func pullImages(ctx context.Context, rt runtime.Runtime, sink output.Sink, conta return nil } -func validateLicenses(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformClient api.PlatformAPI, containers []runtime.ContainerConfig, cfgContainers []config.ContainerConfig, token string) error { - for i, c := range cfgContainers { - if err := validateLicense(ctx, rt, sink, platformClient, containers[i], &c, token); err != nil { +func validateLicenses(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformClient api.PlatformAPI, containers []runtime.ContainerConfig, token string) error { + for _, c := range containers { + if err := validateLicense(ctx, rt, sink, platformClient, c, token); err != nil { return err } } @@ -125,13 +131,12 @@ func startContainers(ctx context.Context, rt runtime.Runtime, sink output.Sink, return nil } -func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink output.Sink, containers []runtime.ContainerConfig, cfgContainers []config.ContainerConfig) ([]runtime.ContainerConfig, []config.ContainerConfig, error) { +func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink output.Sink, containers []runtime.ContainerConfig) ([]runtime.ContainerConfig, error) { var filtered []runtime.ContainerConfig - var filteredCfg []config.ContainerConfig - for i, c := range containers { + for _, c := range containers { running, err := rt.IsRunning(ctx, c.Name) if err != nil && !errdefs.IsNotFound(err) { - return nil, nil, fmt.Errorf("failed to check container status: %w", err) + return nil, fmt.Errorf("failed to check container status: %w", err) } if running { output.EmitLog(sink, fmt.Sprintf("%s is already running", c.Name)) @@ -140,14 +145,13 @@ func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink outpu if err := checkPortAvailable(c.Port); err != nil { configPath, pathErr := config.ConfigFilePath() if pathErr != nil { - return nil, nil, err + return nil, err } - return nil, nil, fmt.Errorf("%w\nTo use a different port, edit %s", err, configPath) + return nil, fmt.Errorf("%w\nTo use a different port, edit %s", err, configPath) } filtered = append(filtered, c) - filteredCfg = append(filteredCfg, cfgContainers[i]) } - return filtered, filteredCfg, nil + return filtered, nil } func checkPortAvailable(port string) error { @@ -162,8 +166,8 @@ func checkPortAvailable(port string) error { return fmt.Errorf("port %s already in use", port) } -func validateLicense(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformClient api.PlatformAPI, containerConfig runtime.ContainerConfig, cfgContainer *config.ContainerConfig, token string) error { - version := cfgContainer.Tag +func validateLicense(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformClient api.PlatformAPI, containerConfig runtime.ContainerConfig, token string) error { + version := containerConfig.Tag if version == "" || version == "latest" { actualVersion, err := rt.GetImageVersion(ctx, containerConfig.Image) if err != nil { @@ -172,16 +176,12 @@ func validateLicense(ctx context.Context, rt runtime.Runtime, sink output.Sink, version = actualVersion } - productName, err := cfgContainer.ProductName() - if err != nil { - return err - } output.EmitStatus(sink, "validating license", containerConfig.Name, version) hostname, _ := os.Hostname() licenseReq := &api.LicenseRequest{ Product: api.ProductInfo{ - Name: productName, + Name: containerConfig.ProductName, Version: version, }, Credentials: api.CredentialsInfo{ @@ -195,7 +195,7 @@ func validateLicense(ctx context.Context, rt runtime.Runtime, sink output.Sink, } if err := platformClient.GetLicense(ctx, licenseReq); err != nil { - return fmt.Errorf("license validation failed for %s:%s: %w", productName, version, err) + return fmt.Errorf("license validation failed for %s:%s: %w", containerConfig.ProductName, version, err) } return nil diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 4f83f39..46981c6 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -3,11 +3,13 @@ package runtime import "context" type ContainerConfig struct { - Image string - Name string - Port string - HealthPath string - Env []string // e.g., ["KEY=value", "FOO=bar"] + Image string + Name string + Port string + HealthPath string + Env []string // e.g., ["KEY=value", "FOO=bar"] + Tag string + ProductName string } type PullProgress struct { From f4f420e4f11c2b0ef781681041080e233e447392 Mon Sep 17 00:00:00 2001 From: carole-lavillonniere Date: Thu, 19 Feb 2026 15:27:06 +0100 Subject: [PATCH 4/4] move checkPortAvailable to its own package --- internal/container/start.go | 16 ++-------------- internal/ports/ports.go | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 14 deletions(-) create mode 100644 internal/ports/ports.go diff --git a/internal/container/start.go b/internal/container/start.go index b6ad258..4f18d6e 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -3,7 +3,6 @@ package container import ( "context" "fmt" - "net" "net/http" "os" stdruntime "runtime" @@ -14,6 +13,7 @@ import ( "github.com/localstack/lstk/internal/auth" "github.com/localstack/lstk/internal/config" "github.com/localstack/lstk/internal/output" + "github.com/localstack/lstk/internal/ports" "github.com/localstack/lstk/internal/runtime" ) @@ -142,7 +142,7 @@ func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink outpu output.EmitLog(sink, fmt.Sprintf("%s is already running", c.Name)) continue } - if err := checkPortAvailable(c.Port); err != nil { + if err := ports.CheckAvailable(c.Port); err != nil { configPath, pathErr := config.ConfigFilePath() if pathErr != nil { return nil, err @@ -154,18 +154,6 @@ func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink outpu return filtered, nil } -func checkPortAvailable(port string) error { - conn, err := net.DialTimeout("tcp", "localhost:"+port, time.Second) - if err != nil { - return nil - } - err = conn.Close() - if err != nil { - return nil - } - return fmt.Errorf("port %s already in use", port) -} - func validateLicense(ctx context.Context, rt runtime.Runtime, sink output.Sink, platformClient api.PlatformAPI, containerConfig runtime.ContainerConfig, token string) error { version := containerConfig.Tag if version == "" || version == "latest" { diff --git a/internal/ports/ports.go b/internal/ports/ports.go new file mode 100644 index 0000000..e155b5e --- /dev/null +++ b/internal/ports/ports.go @@ -0,0 +1,16 @@ +package ports + +import ( + "fmt" + "net" + "time" +) + +func CheckAvailable(port string) error { + conn, err := net.DialTimeout("tcp", "localhost:"+port, time.Second) + if err != nil { + return nil + } + _ = conn.Close() + return fmt.Errorf("port %s already in use", port) +}