diff --git a/internal/cli/stop.go b/internal/cli/stop.go index 5c441e6..d514d49 100644 --- a/internal/cli/stop.go +++ b/internal/cli/stop.go @@ -77,7 +77,11 @@ func addStopFlags(cmd *cobra.Command, opts *stopOpts) { func runStopOrDown(ctx context.Context, out Output, opts stopOpts, remove bool) error { if opts.workspaceFolder == "" { - opts.workspaceFolder, _ = os.Getwd() + wd, err := os.Getwd() + if err != nil { + return writeErrorResult(out, fmt.Sprintf("current directory: %v", err)) + } + opts.workspaceFolder = wd } logger := log.New(log.Options{Writer: out.Stderr(), Format: "text"}) @@ -87,13 +91,20 @@ func runStopOrDown(ctx context.Context, out Output, opts stopOpts, remove bool) } defer engine.Close() - id := resolveWorkspaceContainer(ctx, engine, opts) + id, err := resolveWorkspaceContainer(ctx, engine, opts) + if err != nil { + return writeErrorResult(out, err.Error()) + } if id == "" { return writeSuccessJSON(out, map[string]any{"outcome": "success", "result": "no-container-found"}) } + insp, err := engine.InspectContainer(ctx, id) + if err != nil { + return writeErrorResult(out, fmt.Sprintf("inspect container %s: %v", id, err)) + } project := "" - if insp, ierr := engine.InspectContainer(ctx, id); ierr == nil && insp.Config != nil { + if insp.Config != nil { project = insp.Config.Labels["com.docker.compose.project"] } @@ -129,14 +140,13 @@ func runStopOrDown(ctx context.Context, out Output, opts stopOpts, remove bool) }) } -func resolveWorkspaceContainer(ctx context.Context, engine *docker.EngineClient, opts stopOpts) string { +func resolveWorkspaceContainer(ctx context.Context, engine *docker.EngineClient, opts stopOpts) (string, error) { if opts.containerID != "" { - return opts.containerID + return opts.containerID, nil } configFile := "" if opts.configPath != "" { configFile = resolvePath(opts.configPath) } - id, _ := resolveContainerID(ctx, engine, opts.workspaceFolder, configFile, opts.idLabels) - return id + return resolveContainerID(ctx, engine, opts.workspaceFolder, configFile, opts.idLabels) } diff --git a/internal/cli/stop_held_test.go b/internal/cli/stop_held_test.go index 630aa23..4a09e8c 100644 --- a/internal/cli/stop_held_test.go +++ b/internal/cli/stop_held_test.go @@ -20,6 +20,7 @@ type fakeEngine struct { mu sync.Mutex containers map[string]map[string]string stopFails bool + listFails bool calls []string } @@ -69,6 +70,11 @@ func (e *fakeEngine) serve(w http.ResponseWriter, r *http.Request) { } slices.Sort(want) e.record("list " + strings.Join(want, ",")) + if e.listFails { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"message":"cannot list"}`) + return + } items := []map[string]string{} for id, labels := range e.containers { if hasLabels(labels, want) { @@ -219,6 +225,42 @@ func TestStopWithNoContainerSucceedsWithoutDoingAnything(t *testing.T) { } } +func TestStopOrDownReportsALookupTheEngineRefused(t *testing.T) { + engine := startFakeEngine(t, map[string]map[string]string{}) + engine.listFails = true + cli, _ := fakeDockerCLI(t, false, 0) + + got := runStopOrDownForTest(t, stopOpts{workspaceFolder: t.TempDir(), dockerPath: cli}, true) + + assertResult(t, got, map[string]any{"outcome": "error"}) + if msg, _ := got["message"].(string); !strings.HasPrefix(msg, "Command failed: docker ps ") { + t.Errorf("message = %q, want the failed container lookup", msg) + } + for _, c := range engine.recorded() { + if !strings.HasPrefix(c, "list ") { + t.Errorf("engine call %q after the lookup failed", c) + } + } +} + +func TestStopOrDownReportsAContainerItCannotInspect(t *testing.T) { + engine := startFakeEngine(t, map[string]map[string]string{}) + cli, composeCalls := fakeDockerCLI(t, false, 0) + + got := runStopOrDownForTest(t, stopOpts{containerID: "gone", dockerPath: cli}, false) + + assertResult(t, got, map[string]any{"outcome": "error"}) + if msg, _ := got["message"].(string); !strings.HasPrefix(msg, "inspect container gone: ") { + t.Errorf("message = %q, want the inspect failure", msg) + } + if calls := engine.recorded(); len(calls) != 0 { + t.Errorf("engine calls = %v after the inspect failed", calls) + } + if calls := composeCalls(); calls != nil { + t.Errorf("docker CLI ran %v", calls) + } +} + func TestDownStopsAndRemovesTheContainer(t *testing.T) { engine := startFakeEngine(t, map[string]map[string]string{"c1": {}}) cli, _ := fakeDockerCLI(t, false, 0) diff --git a/internal/docker/argbuilders_test.go b/internal/docker/argbuilders_test.go index b268239..b6786aa 100644 --- a/internal/docker/argbuilders_test.go +++ b/internal/docker/argbuilders_test.go @@ -23,6 +23,7 @@ type cannedResponse struct { stdout []byte stderr []byte code int + err error } func (r *recordingRunner) Run(_ context.Context, stream io.Writer, name string, args ...string) ([]byte, []byte, int, error) { @@ -34,7 +35,7 @@ func (r *recordingRunner) Run(_ context.Context, stream io.Writer, name string, _, _ = stream.Write(resp.stdout) _, _ = stream.Write(resp.stderr) } - return resp.stdout, resp.stderr, resp.code, nil + return resp.stdout, resp.stderr, resp.code, resp.err } return nil, nil, 0, nil } diff --git a/internal/docker/builder_detect_held_test.go b/internal/docker/builder_detect_held_test.go index 1af47a7..f3b0da3 100644 --- a/internal/docker/builder_detect_held_test.go +++ b/internal/docker/builder_detect_held_test.go @@ -1,6 +1,7 @@ package docker import ( + "errors" "testing" "github.com/devcontainers/cli/internal/log" @@ -22,3 +23,14 @@ func TestBuilderDetectionIgnoresAFailedBuildx(t *testing.T) { t.Errorf("FindDockerDriverBuilder after a failed ls = %q, want empty", got) } } + +func TestFindDockerDriverBuilderWhenTheContextCannotBeRead(t *testing.T) { + rr := &recordingRunner{responses: []cannedResponse{ + {err: errors.New("exec docker: no such file")}, + {stdout: []byte("NAME DRIVER\nremote* docker-container running\ndefault docker running\n")}, + }} + c := &Client{DockerPath: "docker", Log: log.Null, Runner: rr} + if got := c.FindDockerDriverBuilder(t.Context()); got != "default" { + t.Errorf("FindDockerDriverBuilder with no current context = %q, want the first docker-driver builder", got) + } +} diff --git a/internal/docker/client.go b/internal/docker/client.go index 10ddf56..7ace159 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -218,8 +218,10 @@ func (c *Client) DetectActiveBuilder(ctx context.Context) *BuilderInfo { } func (c *Client) FindDockerDriverBuilder(ctx context.Context) string { - ctxRes, _ := c.Run(ctx, "context", "inspect", "--format", "{{.Name}}") - currentContext := strings.TrimSpace(string(ctxRes.Stdout)) + currentContext := "" + if ctxRes, err := c.Run(ctx, "context", "inspect", "--format", "{{.Name}}"); err == nil && ctxRes.ExitCode == 0 { + currentContext = strings.TrimSpace(string(ctxRes.Stdout)) + } res, err := c.Run(ctx, "buildx", "ls") if err != nil || res.ExitCode != 0 { diff --git a/internal/lifecycle/probe.go b/internal/lifecycle/probe.go index 02d074a..1643bfb 100644 --- a/internal/lifecycle/probe.go +++ b/internal/lifecycle/probe.go @@ -171,7 +171,10 @@ func ProbedRemoteEnv(ctx context.Context, exec ContainerExecutor, logger log.Log } server := NewShellServer(ctx, exec, containerID, remoteUser) defer server.Close() - env, _ := ProbeRemoteEnv(logger, server, strategy, remoteUser, sessionDataFolder) + env, err := ProbeRemoteEnv(logger, server, strategy, remoteUser, sessionDataFolder) + if err != nil { + logger.Write(log.ToErrorText(err.Error()), log.LevelInfo) + } if env == nil { env = map[string]string{} } diff --git a/internal/lifecycle/shell_test.go b/internal/lifecycle/shell_test.go index 1cb09a4..19c36bd 100644 --- a/internal/lifecycle/shell_test.go +++ b/internal/lifecycle/shell_test.go @@ -1,8 +1,10 @@ package lifecycle import ( + "bytes" "context" "errors" + "strings" "testing" "github.com/devcontainers/cli/internal/log" @@ -119,7 +121,12 @@ func TestProbedRemoteEnv(t *testing.T) { } fe = &fakeExecutor{fn: func(string) (string, string, int, error) { return "", "", 1, errors.New("no shell") }} - if env := ProbedRemoteEnv(t.Context(), fe, log.Null, "cid", "vscode", ProbeLoginShell, ""); env == nil || len(env) != 0 { + var logged bytes.Buffer + logger := log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: &logged}) + if env := ProbedRemoteEnv(t.Context(), fe, logger, "cid", "vscode", ProbeLoginShell, ""); env == nil || len(env) != 0 { t.Fatalf("failed probe: env = %v, want empty", env) } + if !strings.Contains(logged.String(), "no shell") { + t.Errorf("failed probe logged %q, want the probe's error", logged.String()) + } } diff --git a/internal/log/log.go b/internal/log/log.go index 65de9e1..a3e0b4e 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -284,6 +284,14 @@ const ( numberColor = "38;2;86;156;214" ) +func ToErrorText(s string) string { + lines := newlines.Split(s, -1) + for i, l := range lines { + lines[i] = color("31", l) + } + return strings.Join(lines, "\r\n") + "\r\n" +} + func color(c, s string) string { return "\x1b[1m\x1b[" + c + "m" + s + "\x1b[39m\x1b[22m" } diff --git a/internal/log/log_test.go b/internal/log/log_test.go index e3ac32d..8369367 100644 --- a/internal/log/log_test.go +++ b/internal/log/log_test.go @@ -189,3 +189,10 @@ func TestLogLevelFilter_TerminalVsJSON(t *testing.T) { t.Errorf("json must emit all levels, got %q", js) } } + +func TestToErrorTextMatchesTheReference(t *testing.T) { + want := "\x1b[1m\x1b[31mfirst\x1b[39m\x1b[22m\r\n\x1b[1m\x1b[31msecond\x1b[39m\x1b[22m\r\n" + if got := ToErrorText("first\r\nsecond"); got != want { + t.Errorf("ToErrorText = %q, want %q", got, want) + } +}