Stop burying JSON output, and use the image already present when a pull fails

cmd/engine_mock_test.go

@@ -18,6 +18,8 @@ type mockEngine struct {
 	containerExists    map[string]bool
 	containerExistsErr error
 
+	imageExists bool
+
 	volumes           map[string]bool
 	volumeExistsErr   error
 	createVolumeErr   error
@@ -88,6 +90,8 @@ func (m *mockEngine) RemoveVolume(name string) error {
 }
 func (m *mockEngine) ExportVolume(string, io.Writer) error { return m.exportVolumeErr }
 
+func (m *mockEngine) ImageExists(string) (bool, error) { return m.imageExists, nil }
+
 func (m *mockEngine) PullImage(_ string, msgs chan<- string) error {
 	if msgs != nil {
 		for _, line := range m.pullMsgs {

cmd/root.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/signal"
@@ -46,6 +47,15 @@ var (
 Run omnideck without a command. It will open the right screen for first setup,
 repair, or managing an existing installation.`,
 		PersistentPreRun: persistentPreRun,
+		// A failing command has already said what went wrong, in the shape the
+		// caller asked for. Printing the whole usage block after it buries that
+		// message, and under --json it puts a wall of prose on stderr behind a
+		// single line of JSON.
+		SilenceUsage: true,
+		// Errors are printed by Execute instead, so the one error that carries
+		// no message can exit quietly rather than leaving a bare "Error:"
+		// behind the output the caller actually asked for.
+		SilenceErrors: true,
 	}
 )
 
@@ -65,6 +75,9 @@ func Execute() {
 	defer stop()
 	engine.SetCancelContext(ctx)
 	if err := rootCmd.Execute(); err != nil {
+		if !errors.Is(err, errAborted) {
+			fmt.Fprintln(os.Stderr, "Error:", err)
+		}
 		os.Exit(1)
 	}
 }

engine/docker.go

@@ -44,6 +44,15 @@ func (e *DockerEngine) ContainerExists(name string) (bool, error) {
 	return strings.TrimSpace(string(out)) == name, nil
 }
 
+func (e *DockerEngine) ImageExists(image string) (bool, error) {
+	// Docker has no "image exists"; inspecting one that is absent fails.
+	cmd := buildCmd("docker", "image", "inspect", image)
+	if err := cmd.Run(); err != nil {
+		return false, nil
+	}
+	return true, nil
+}
+
 func (e *DockerEngine) CreateVolume(name string) error {
 	cmd := buildCmd("docker", "volume", "create", name)
 	out, err := cmd.CombinedOutput()

engine/engine.go

@@ -43,6 +43,10 @@ type Engine interface {
 	RemoveVolume(name string) error
 	ExportVolume(name string, w io.Writer) error
 	PullImage(image string, msgs chan<- string) error
+	// ImageExists reports whether the image is already on this machine, so a
+	// registry that cannot be reached need not stop work that has everything
+	// it needs locally.
+	ImageExists(image string) (bool, error)
 	RunContainer(opts RunOptions) error
 	// CheckOllamaConnection tests the same container-to-host address Omnideck
 	// receives. It runs from inside an existing Omnideck container.

engine/podman.go

@@ -35,6 +35,15 @@ func (e *PodmanEngine) ContainerExists(name string) (bool, error) {
 	return strings.TrimSpace(string(out)) == name, nil
 }
 
+func (e *PodmanEngine) ImageExists(image string) (bool, error) {
+	cmd := buildCmd("podman", "image", "exists", image)
+	if err := cmd.Run(); err != nil {
+		// Absent is reported by exit status, and is not a failure to ask.
+		return false, nil
+	}
+	return true, nil
+}
+
 func (e *PodmanEngine) CreateVolume(name string) error {
 	return createVolumeIfMissing(name, e.VolumeExists, func() error {
 		cmd := buildCmd("podman", "volume", "create", name)

tui/test_helpers_test.go

@@ -43,6 +43,7 @@ func (m *mockEngine) IsAvailable() bool                     { return true }
 func (m *mockEngine) HasPermission() bool                   { return true }
 func (m *mockEngine) Version() string                       { return "1.0" }
 func (m *mockEngine) ImageDigest(string) string             { return "" }
+func (m *mockEngine) ImageExists(string) (bool, error)      { return true, nil }
 func (m *mockEngine) PullImage(string, chan<- string) error { return nil }
 func (m *mockEngine) RunContainer(opts engine.RunOptions) error {
 	m.lastRunOptions = opts

workflow/create_instance.go

@@ -16,6 +16,7 @@ type InstanceCreationEngine interface {
 	CreateVolume(name string) error
 	RemoveVolume(name string) error
 	PullImage(image string, msgs chan<- string) error
+	ImageExists(image string) (bool, error)
 	RunContainer(opts engine.RunOptions) error
 	RemoveContainer(name string) error
 }
@@ -109,7 +110,17 @@ func CreateInstance(eng InstanceCreationEngine, cfg *config.Config, save func()
 	close(msgs)
 	<-forwarded
 	if pullErr != nil {
-		return pullErr
+		// A registry that cannot be reached is only fatal when the image is not
+		// already here. Setting up again offline, or repairing an installation
+		// whose container is gone, needs nothing downloaded — and an image
+		// named by digest cannot have changed since it was fetched.
+		present, existsErr := eng.ImageExists(cfg.Image)
+		if existsErr != nil || !present {
+			return pullErr
+		}
+		if opts.OnPullProgress != nil {
+			opts.OnPullProgress("Using the copy already on this computer")
+		}
 	}
 
 	opts.reportStage("run_container")

workflow/create_instance_test.go

@@ -3,6 +3,7 @@ package workflow
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 
 	"github.com/omnideck-dev/cli/config"
@@ -16,11 +17,16 @@ type fakeCreationEngine struct {
 	createdVolumes  []string
 	removedVolumes  []string
 	pullErr         error
+	imagePresent    bool
 	pullMsgs        []string
 	runErr          error
 	removedContaner bool
 }
 
+func (f *fakeCreationEngine) ImageExists(string) (bool, error) {
+	return f.imagePresent, nil
+}
+
 func (f *fakeCreationEngine) ContainerExists(string) (bool, error) {
 	return f.containerExists, f.existsErr
 }
@@ -179,3 +185,38 @@ func TestCreateInstanceFailureDuringCancellationReportsContextCanceled(t *testin
 		t.Fatalf("removedVolumes = %v, want both volumes cleaned up despite cancellation", eng.removedVolumes)
 	}
 }
+
+func TestCreateInstanceUsesTheLocalImageWhenTheRegistryCannotBeReached(t *testing.T) {
+	// Setting up again offline, or repairing an installation whose container is
+	// gone, needs nothing downloaded. An image named by digest cannot have
+	// changed since it was fetched, so the copy already here is the right one.
+	eng := &fakeCreationEngine{pullErr: errors.New("pinging container registry: Forbidden"), imagePresent: true}
+	cfg := testCreateConfig()
+
+	var progress []string
+	err := CreateInstance(eng, cfg, func() error { return nil }, CreateInstanceOptions{
+		OnPullProgress: func(line string) { progress = append(progress, line) },
+	})
+	if err != nil {
+		t.Fatalf("expected the local image to be used, got %v", err)
+	}
+	if len(eng.removedVolumes) != 0 || eng.removedContaner {
+		t.Fatal("nothing failed, so nothing should have been cleaned up")
+	}
+	if len(progress) == 0 || !strings.Contains(progress[len(progress)-1], "already on this computer") {
+		t.Fatalf("expected the fallback to be reported, got %v", progress)
+	}
+}
+
+func TestCreateInstanceFailsWhenTheImageIsNeitherFetchableNorPresent(t *testing.T) {
+	eng := &fakeCreationEngine{pullErr: errors.New("pinging container registry: Forbidden")}
+	cfg := testCreateConfig()
+
+	err := CreateInstance(eng, cfg, func() error { return nil }, CreateInstanceOptions{})
+	if err == nil {
+		t.Fatal("expected the unreachable registry to fail when nothing is here to use")
+	}
+	if len(eng.removedVolumes) != 2 {
+		t.Fatalf("removedVolumes = %v, want both volumes cleaned up", eng.removedVolumes)
+	}
+}