Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions cmd/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,7 @@ Examples:
return fmt.Errorf("checking emulator status: %w", err)
}
if runningName == "" {
sink.Emit(output.ErrorEvent{
Title: fmt.Sprintf("%s is not running", awsContainer.DisplayName()),
Actions: []output.ErrorAction{
{Label: "Start LocalStack:", Value: "lstk"},
{Label: "See help:", Value: "lstk -h"},
},
})
return output.NewSilentError(fmt.Errorf("%s is not running", awsContainer.Name()))
return container.HandleNoRunningContainer(sink, awsContainer)
}

host, _ := endpoint.ResolveHost(cmd.Context(), awsContainer.Port, cfg.LocalStackHost)
Expand Down
9 changes: 1 addition & 8 deletions cmd/az.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,7 @@ func azPreflight(ctx context.Context, cfg *env.Env, sink output.Sink) (string, e
return "", fmt.Errorf("checking emulator status: %w", err)
}
if runningName == "" {
sink.Emit(output.ErrorEvent{
Title: fmt.Sprintf("%s is not running", azureContainer.DisplayName()),
Actions: []output.ErrorAction{
{Label: "Start LocalStack:", Value: "lstk"},
{Label: "See help:", Value: "lstk -h"},
},
})
return "", output.NewSilentError(fmt.Errorf("%s is not running", azureContainer.Name()))
return "", container.HandleNoRunningContainer(sink, azureContainer)
}

resolvedHost, dnsOK := endpoint.ResolveHost(ctx, azureContainer.Port, cfg.LocalStackHost)
Expand Down
9 changes: 1 addition & 8 deletions cmd/iac.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,7 @@ func requireRunningAWSEmulator(ctx context.Context, rt runtime.Runtime, sink out
})
return output.NewSilentError(fmt.Errorf("lstk %s requires the AWS emulator, but the %s is running", cmdLabel, other))
}
sink.Emit(output.ErrorEvent{
Title: fmt.Sprintf("%s is not running", awsContainer.DisplayName()),
Actions: []output.ErrorAction{
{Label: "Start LocalStack:", Value: "lstk"},
{Label: "See help:", Value: "lstk -h"},
},
})
return output.NewSilentError(fmt.Errorf("%s is not running", awsContainer.Name()))
return container.HandleNoRunningContainer(sink, awsContainer)
}

// runningNonAWSEmulator returns the display name of a running non-AWS emulator
Expand Down
10 changes: 9 additions & 1 deletion internal/container/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,18 @@ func Logs(ctx context.Context, rt runtime.Runtime, sink output.Sink, containers
// TODO: handle logs per container
c := containers[0]

name, err := ResolveRunningContainerName(ctx, rt, c)
if err != nil {
return fmt.Errorf("checking %s running: %w", c.Name(), err)
}
if name == "" {
return HandleNoRunningContainer(sink, c)
}
Comment on lines +46 to +48

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: this is the 4th copy of this code, the first three are in aws.go, az.go and status.go. Maybe we can extract it to a handleNoRunningContainer in, let's say, running.go?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in b1eb7e4, good point


pr, pw := io.Pipe()
errCh := make(chan error, 1)
go func() {
err := rt.StreamLogs(ctx, c.Name(), pw, follow)
err := rt.StreamLogs(ctx, name, pw, follow)
pw.CloseWithError(err)
errCh <- err
}()
Expand Down
21 changes: 21 additions & 0 deletions internal/container/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func runLogs(t *testing.T, content string, verbose bool) *captureSink {
ctrl := gomock.NewController(t)
mockRT := runtime.NewMockRuntime(ctrl)
mockRT.EXPECT().IsHealthy(gomock.Any()).Return(nil)
mockRT.EXPECT().IsRunning(gomock.Any(), "localstack-aws").Return(true, nil)
mockRT.EXPECT().
StreamLogs(gomock.Any(), "localstack-aws", gomock.Any(), false).
DoAndReturn(func(_ context.Context, _ string, out io.Writer, _ bool) error {
Expand Down Expand Up @@ -97,3 +98,23 @@ func TestLogs_FilteredLinesDropped(t *testing.T) {
require.Len(t, sink.lines, 1)
assert.Contains(t, sink.lines[0].Line, "keep")
}

// When no matching emulator container is running, Logs must fail with a
// silent error instead of trying to stream from a nonexistent container.
func TestLogs_EmulatorNotRunning_ReturnsSilentError(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockRT := runtime.NewMockRuntime(ctrl)
mockRT.EXPECT().IsHealthy(gomock.Any()).Return(nil)
mockRT.EXPECT().IsRunning(gomock.Any(), "localstack-aws").Return(false, nil)
mockRT.EXPECT().
FindRunningByImage(gomock.Any(), []string{"localstack/localstack-pro", "localstack/localstack"}, "4566/tcp").
Return(nil, nil)

sink := &captureSink{}
containers := []config.ContainerConfig{{Type: config.EmulatorAWS}}
err := Logs(context.Background(), mockRT, sink, containers, false, false)

require.Error(t, err)
assert.True(t, output.IsSilent(err), "error must be silent since the sink already surfaced a 'not running' message to the user")
}
17 changes: 17 additions & 0 deletions internal/container/running.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/localstack/lstk/internal/config"
"github.com/localstack/lstk/internal/output"
"github.com/localstack/lstk/internal/runtime"
)

Expand Down Expand Up @@ -58,3 +59,19 @@ func ResolveRunningContainerName(ctx context.Context, rt runtime.Runtime, c conf

return "", nil
}

// HandleNoRunningContainer emits the standard "not running" error for c
// through sink (naming it and pointing the user at how to start it), then
// returns a silent error for the caller to propagate. Callers that already
// resolved ResolveRunningContainerName to "" should use this instead of
// duplicating the ErrorEvent/Actions boilerplate.
func HandleNoRunningContainer(sink output.Sink, c config.ContainerConfig) error {
sink.Emit(output.ErrorEvent{
Title: fmt.Sprintf("%s is not running", c.DisplayName()),
Actions: []output.ErrorAction{
{Label: "Start LocalStack:", Value: "lstk"},
{Label: "See help:", Value: "lstk -h"},
},
})
return output.NewSilentError(fmt.Errorf("%s is not running", c.Name()))
}
9 changes: 1 addition & 8 deletions internal/container/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,7 @@ func Status(ctx context.Context, rt runtime.Runtime, containers []config.Contain
return fmt.Errorf("checking %s running: %w", c.Name(), err)
}
if name == "" {
sink.Emit(output.ErrorEvent{
Title: fmt.Sprintf("%s is not running", c.DisplayName()),
Actions: []output.ErrorAction{
{Label: "Start LocalStack:", Value: "lstk"},
{Label: "See help:", Value: "lstk -h"},
},
})
return output.NewSilentError(fmt.Errorf("%s is not running", c.Name()))
return HandleNoRunningContainer(sink, c)
}

// status makes direct HTTP calls to LocalStack, so it needs the actual host port.
Expand Down
32 changes: 30 additions & 2 deletions test/integration/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package integration_test

import (
"bufio"
"context"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -53,13 +54,40 @@ func TestLogsCommandFailsWhenNotRunning(t *testing.T) {

configFile := writeAwsConfig(t)
analyticsSrv, events := mockAnalyticsServer(t)
_, stderr, err := runLstk(t, testContext(t), "", env.With(env.AnalyticsEndpoint, analyticsSrv.URL), "--config", configFile, "logs", "--follow")
stdout, _, err := runLstk(t, testContext(t), "", env.With(env.AnalyticsEndpoint, analyticsSrv.URL), "--config", configFile, "logs", "--follow")
require.Error(t, err, "expected lstk logs --follow to fail when container not running")
requireExitCode(t, 1, err)
assert.Contains(t, stderr, "emulator is not running")
assert.Contains(t, stdout, "LocalStack AWS Emulator is not running")
assertCommandTelemetry(t, events, "logs", 1)
}

// lstk logs must find the emulator even when it's running under a container
// name other than the config-derived canonical name (e.g. started outside
// lstk), the same way lstk status/stop already do.
func TestLogsWorksWithExternalContainer(t *testing.T) {
requireDocker(t)
cleanup()
t.Cleanup(cleanup)

ctx := testContext(t)

const fakeImage = "localstack/localstack-pro:test-fake"
_, err := dockerClient.ImageTag(ctx, client.ImageTagOptions{Source: testImage, Target: fakeImage})
require.NoError(t, err)
t.Cleanup(func() {
_, _ = dockerClient.ImageRemove(context.Background(), fakeImage, client.ImageRemoveOptions{})
})

startExternalContainer(t, ctx, fakeImage, "localstack-main", "4566")

configFile := writeAwsConfig(t)
analyticsSrv, events := mockAnalyticsServer(t)
_, stderr, err := runLstk(t, ctx, "", env.With(env.AnalyticsEndpoint, analyticsSrv.URL), "--config", configFile, "logs")
require.NoError(t, err, "lstk logs should work with externally-named container: %s", stderr)
requireExitCode(t, 0, err)
assertCommandTelemetry(t, events, "logs", 0)
}

func TestLogsFollowStreamsOutput(t *testing.T) {
requireDocker(t)
cleanup()
Expand Down
Loading