Make how the target version is started a seam

Both phases that bring the target version up against a not-yet-migrated
store -- the recovery cycle, and the ordinary boot validate does after it --
constructed a child process from a path on this host directly. That is the
one thing about them packaging changes: a container runs an image against
the data volume instead. Everything either of them is started *for* is
identical afterwards.

So starting it is now a Launcher, returning a Supervised the callers stop
and read output from. BinaryLauncher is today's behaviour and the default
when Options.Launcher is nil, so every existing caller is unchanged -- no
test needed editing, which is the evidence for that rather than a claim
about it.

Deliberately narrower than the interface sketched in issue #3. Stage and
cutover also differ by packaging, but designing their interfaces now would
be designing against a guess: there is no second implementation yet to
shape them, and the shape a container needs is what PR 3 and PR 4 find out.
This seam is different because it already had two callers doing the same
thing for the same reason, so extracting it describes the code rather than
predicting it.

outputSuffix now takes Supervised. It only ever needed Output(), and the
diagnosis it exists to preserve -- the server's own words about a bind
conflict or a rejected config value, which a bare timeout loses -- matters
whatever started the process.
This commit is contained in:
2026-08-28 17:02:48 -07:00
parent bb257892e7
commit 5a7a126960
4 changed files with 238 additions and 9 deletions
+89
View File
@@ -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{}
+123
View File
@@ -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")
}
}
+14 -5
View File
@@ -29,6 +29,11 @@ type Options struct {
StartupTimeout time.Duration StartupTimeout time.Duration
StopGrace time.Duration StopGrace time.Duration
HTTPClient *http.Client 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 // 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 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, RecoveryMode: true, AdminUser: opts.AdminUser, AdminPassword: password,
ExtraEnv: opts.ExtraEnv, ExtraEnv: opts.ExtraEnv,
}); startErr != nil { })
if startErr != nil {
return checkpoint.StepOutcome{}, startErr 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 // words are usually the whole diagnosis - a bind conflict, a rejected
// config value - and without them the caller is left guessing at a // config value - and without them the caller is left guessing at a
// timeout. // timeout.
func outputSuffix(proc *Process) string { func outputSuffix(proc Supervised) string {
out := strings.TrimSpace(proc.Output()) out := strings.TrimSpace(proc.Output())
if out == "" { if out == "" {
return " (the process produced no output)" return " (the process produced no output)"
+12 -4
View File
@@ -29,6 +29,10 @@ type BootCheckOptions struct {
StopGrace time.Duration StopGrace time.Duration
HTTPClient *http.Client 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 // ContentIntegrityBefore, if non-nil, is the pre-migration snapshot
// preflight captured (checkpoint.RunState.PreflightSnapshot). When set, // preflight captured (checkpoint.RunState.PreflightSnapshot). When set,
// BootCheck captures a fresh snapshot from the instance it just booted // 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 // so a retry just redoes the whole cycle - see recovery.Run's doc comment
// for the full reasoning, which applies identically here. // for the full reasoning, which applies identically here.
func BootCheck(ctx context.Context, o BootCheckOptions) (detail string, result *ContentIntegrityResult, err error) { 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) return "", nil, fmt.Errorf("validate: start normal boot: %w", startErr)
} }