Merge pull request #5 from LINUXexpert-org/docker-recovery-launcher-seam

Make how the target version is started a seam

internal/recovery/launcher.go

@@ -0,0 +1,89 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package recovery
+
+import (
+	"context"
+	"time"
+)
+
+// Supervised is one running instance of the target Stalwart version,
+// however it was started. The two phases that bring the target version up
+// against a not-yet-migrated store - the recovery cycle here, and
+// validate.BootCheck's ordinary boot afterwards - need exactly three things
+// from it: to stop it, to read what it printed, and (via WaitForHealthy,
+// which only needs a URL) to know when it is ready.
+//
+// Output is not a nicety. A recovery boot that failed because port 8080 was
+// already in use reported only "connection refused" after a 60-second
+// timeout, while Stalwart had printed "Address already in use" immediately
+// to a pipe nothing was reading. Anything reporting a supervised process
+// failing has to be able to say why, whatever started it.
+type Supervised interface {
+	Stop(gracePeriod time.Duration) error
+	Output() string
+}
+
+// LaunchOptions is everything about running the target version that does
+// not depend on how it is packaged. What differs by packaging - a path on
+// this host, or an image and the mounts to run it against - belongs to the
+// Launcher, which was constructed knowing it.
+type LaunchOptions struct {
+	ConfigPath string
+
+	// RecoveryMode sets STALWART_RECOVERY_MODE=1 and
+	// STALWART_RECOVERY_ADMIN=<AdminUser>:<AdminPassword>, the environment
+	// Stalwart's own upgrade guide uses to bring the new version up against
+	// an unmigrated store. False means an ordinary boot.
+	RecoveryMode  bool
+	AdminUser     string
+	AdminPassword string
+
+	// ExtraEnv is appended after any recovery-mode variables, so a
+	// rehearsal can point ports and paths at a sandbox without those two
+	// becoming the only way to parameterize the launch.
+	ExtraEnv []string
+}
+
+// Launcher starts the staged target version. It exists because that is the
+// one thing in this phase that packaging changes: a systemd install runs a
+// binary this tool downloaded, a container runs an image against the data
+// volume, and everything either of them is started *for* - the recovery
+// cycle, the settings apply, the boot check - is identical afterwards.
+//
+// See ARCHITECTURE.md §4.4. The container implementation is issue #3.
+type Launcher interface {
+	// Launch starts the instance and returns once the OS reports it
+	// started. It does not wait for Stalwart to be ready: callers use
+	// WaitForHealthy for that, because readiness is an HTTP question and
+	// the same one either way.
+	Launch(ctx context.Context, o LaunchOptions) (Supervised, error)
+}
+
+// BinaryLauncher runs the target version as a child process from a path on
+// this host - the systemd deployment's answer, and the only one until
+// container support lands.
+type BinaryLauncher struct {
+	BinaryPath string
+}
+
+func (b BinaryLauncher) Launch(ctx context.Context, o LaunchOptions) (Supervised, error) {
+	p := &Process{}
+	if err := p.Start(ctx, ProcessOptions{
+		BinaryPath:    b.BinaryPath,
+		ConfigPath:    o.ConfigPath,
+		RecoveryMode:  o.RecoveryMode,
+		AdminUser:     o.AdminUser,
+		AdminPassword: o.AdminPassword,
+		ExtraEnv:      o.ExtraEnv,
+	}); err != nil {
+		return nil, err
+	}
+	return p, nil
+}
+
+// Process satisfies Supervised. Asserted here rather than left to the call
+// sites so the compiler catches it if either side drifts.
+var _ Supervised = (*Process)(nil)
+var _ Launcher = BinaryLauncher{}

internal/recovery/launcher_test.go

@@ -0,0 +1,123 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package recovery
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint"
+)
+
+// recordingLauncher stands in for a deployment that is not a local binary -
+// a container, when that lands. It records what it was asked for and
+// delegates the actual process to BinaryLauncher, so the recovery cycle
+// still has something real to talk to.
+type recordingLauncher struct {
+	inner  Launcher
+	calls  int
+	lastOp LaunchOptions
+}
+
+func (r *recordingLauncher) Launch(ctx context.Context, o LaunchOptions) (Supervised, error) {
+	r.calls++
+	r.lastOp = o
+	return r.inner.Launch(ctx, o)
+}
+
+// The whole point of the seam: a deployment that is not a host binary can
+// supply its own way of starting the target version, and everything the
+// recovery cycle does afterwards is unchanged.
+func TestRunUsesTheSuppliedLauncher(t *testing.T) {
+	port := freePort(t)
+	configPath := filepath.Join(t.TempDir(), "config.json")
+	if err := os.WriteFile(configPath, []byte("{}"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	applyDir := t.TempDir()
+	withFakeExecutable(t, "stalwart-cli", fmt.Sprintf("#!/bin/sh\necho \"$@\" >> %q\nexit 0\n", argsFile(t, applyDir)))
+	exportFile := filepath.Join(t.TempDir(), "export.json")
+	if err := os.WriteFile(exportFile, []byte("{}"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	store := checkpoint.NewStore(t.TempDir())
+	rs, err := store.Create("0.15.5", "0.16.14")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	launcher := &recordingLauncher{inner: BinaryLauncher{BinaryPath: testBinaryPath(t)}}
+	_, err = Run(context.Background(), store, rs, Options{
+		// Deliberately empty: a supplied Launcher owns how the target
+		// version is started, so BinaryPath must not be consulted at all.
+		BinaryPath:     "",
+		ConfigPath:     configPath,
+		ListenURL:      fmt.Sprintf("http://127.0.0.1:%d/", port),
+		AdminUser:      "admin",
+		ApplyFiles:     []string{exportFile},
+		ExtraEnv:       helperProcessEnv(port),
+		StartupTimeout: 5 * time.Second,
+		StopGrace:      5 * time.Second,
+		Launcher:       launcher,
+	})
+	if err != nil {
+		t.Fatalf("Run with a supplied launcher: %v", err)
+	}
+	if launcher.calls != 1 {
+		t.Errorf("launcher called %d times, want 1", launcher.calls)
+	}
+	if !launcher.lastOp.RecoveryMode {
+		t.Error("recovery cycle launched without RecoveryMode set")
+	}
+	if launcher.lastOp.AdminPassword == "" {
+		t.Error("recovery cycle launched without a generated admin password")
+	}
+	if launcher.lastOp.ConfigPath != configPath {
+		t.Errorf("ConfigPath = %q, want %q", launcher.lastOp.ConfigPath, configPath)
+	}
+}
+
+// A nil Launcher has to keep meaning exactly what it meant before this
+// seam existed, or every existing caller changes behaviour silently.
+func TestNilLauncherStillRunsTheBinaryPath(t *testing.T) {
+	port := freePort(t)
+	configPath := filepath.Join(t.TempDir(), "config.json")
+	if err := os.WriteFile(configPath, []byte("{}"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	applyDir := t.TempDir()
+	applyLog := argsFile(t, applyDir)
+	withFakeExecutable(t, "stalwart-cli", fmt.Sprintf("#!/bin/sh\necho \"$@\" >> %q\nexit 0\n", applyLog))
+	exportFile := filepath.Join(t.TempDir(), "export.json")
+	if err := os.WriteFile(exportFile, []byte("{}"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	store := checkpoint.NewStore(t.TempDir())
+	rs, err := store.Create("0.15.5", "0.16.14")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := Run(context.Background(), store, rs, Options{
+		BinaryPath:     testBinaryPath(t),
+		ConfigPath:     configPath,
+		ListenURL:      fmt.Sprintf("http://127.0.0.1:%d/", port),
+		AdminUser:      "admin",
+		ApplyFiles:     []string{exportFile},
+		ExtraEnv:       helperProcessEnv(port),
+		StartupTimeout: 5 * time.Second,
+		StopGrace:      5 * time.Second,
+		// Launcher deliberately unset.
+	}); err != nil {
+		t.Fatalf("Run with a nil launcher: %v", err)
+	}
+	if got := readArgsFile(t, applyLog); got == "" {
+		t.Fatal("stalwart-cli apply was never invoked, so the cycle did not run")
+	}
+}

internal/recovery/recovery.go

@@ -29,6 +29,11 @@ type Options struct {
 	StartupTimeout time.Duration
 	StopGrace      time.Duration
 	HTTPClient     *http.Client
+
+	// Launcher starts the target version. Nil means BinaryPath as a child
+	// process, which is what every caller wants today; a container
+	// deployment supplies its own (issue #3).
+	Launcher Launcher
 }
 
 // GenerateRecoveryPassword returns a fresh random one-time password for
@@ -64,12 +69,16 @@ func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState,
 			return checkpoint.StepOutcome{}, err
 		}
 
-		proc := &Process{}
+		launcher := opts.Launcher
-		if startErr := proc.Start(ctx, ProcessOptions{
+		if launcher == nil {
-			BinaryPath: opts.BinaryPath, ConfigPath: opts.ConfigPath,
+			launcher = BinaryLauncher{BinaryPath: opts.BinaryPath}
+		}
+		proc, startErr := launcher.Launch(ctx, LaunchOptions{
+			ConfigPath:   opts.ConfigPath,
 			RecoveryMode: true, AdminUser: opts.AdminUser, AdminPassword: password,
 			ExtraEnv: opts.ExtraEnv,
-		}); startErr != nil {
+		})
+		if startErr != nil {
 			return checkpoint.StepOutcome{}, startErr
 		}
 
@@ -115,7 +124,7 @@ func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState,
 // words are usually the whole diagnosis - a bind conflict, a rejected
 // config value - and without them the caller is left guessing at a
 // timeout.
-func outputSuffix(proc *Process) string {
+func outputSuffix(proc Supervised) string {
 	out := strings.TrimSpace(proc.Output())
 	if out == "" {
 		return " (the process produced no output)"

internal/validate/bootcheck.go

@@ -29,6 +29,10 @@ type BootCheckOptions struct {
 	StopGrace  time.Duration
 	HTTPClient *http.Client
 
+	// Launcher starts the instance this boots. Nil means BinaryPath as a
+	// child process - see recovery.Launcher.
+	Launcher recovery.Launcher
+
 	// ContentIntegrityBefore, if non-nil, is the pre-migration snapshot
 	// preflight captured (checkpoint.RunState.PreflightSnapshot). When set,
 	// BootCheck captures a fresh snapshot from the instance it just booted
@@ -59,10 +63,14 @@ type BootCheckOptions struct {
 // so a retry just redoes the whole cycle - see recovery.Run's doc comment
 // for the full reasoning, which applies identically here.
 func BootCheck(ctx context.Context, o BootCheckOptions) (detail string, result *ContentIntegrityResult, err error) {
-	proc := &recovery.Process{}
+	launcher := o.Launcher
-	if startErr := proc.Start(ctx, recovery.ProcessOptions{
+	if launcher == nil {
-		BinaryPath: o.BinaryPath, ConfigPath: o.ConfigPath, RecoveryMode: false, ExtraEnv: o.ExtraEnv,
+		launcher = recovery.BinaryLauncher{BinaryPath: o.BinaryPath}
-	}); startErr != nil {
+	}
+	proc, startErr := launcher.Launch(ctx, recovery.LaunchOptions{
+		ConfigPath: o.ConfigPath, RecoveryMode: false, ExtraEnv: o.ExtraEnv,
+	})
+	if startErr != nil {
 		return "", nil, fmt.Errorf("validate: start normal boot: %w", startErr)
 	}