Fix the three defects that cost a production restore
A live migration on 2026-08-24 stopped a production mail server and then
discovered the host's stalwart-cli was 0.13.4 - present, but from when the
CLI shipped with the server, with no `apply` command. The migration needs
v1.0.2+ from the separately-versioned stalwartlabs/cli repository.
Recovery was closed in both directions. v0.16's recovery-mode boot had
already bumped the store schema to v6, so the 0.15.5 binary refused to
reopen it ("expected 5 or below, found 6"). Going forward needed
export.json, which this tool's own failure path had deleted - and
regenerating it required a settings dump from a live v0.15 instance that
could no longer start. The operator restored a day-old snapshot and lost a
day of mail across nine domains.
Three fixes:
1. preflight.CheckExternalTools verifies stalwart-cli exists and is v1.0.2
or later, and that python3 runs - before anything is touched. Every fact
needed to prevent this was available in under a second from a stopped
state. Skipped for a patch upgrade, which invokes neither tool.
2. A failed run no longer deletes its work directory. Cleaning up on every
exit path was right for a sandboxed rehearsal and catastrophic here:
once the service is stopped the settings dump cannot be regenerated, so
deleting it removes the only way forward. The failure now prints the
resume command instead.
3. `run --resume <id>` continues an interrupted run. The checkpoint
machinery existed but never engaged, because run created a new run every
invocation - so a retry re-ran preflight against a binary already moved
aside, and failed. Completed steps are skipped from the checkpoint.
Proven against a VM built to match the failure: stalwart-cli 0.15.5,
accounts and mail seeded.
* preflight refused, service still active, mail still accepted
* a stub CLI passing --version and failing apply left the run stopped
with all eight inputs intact and the resume command printed
* --resume carried it to a clean finish: five seconds of downtime,
listeners regenerated, admin role restored, quotas rebuilt
That failure-path test is the one that should have run before production.
Every earlier test had stalwart-cli installed from the start, and the one
failure I did exercise happened to leave its artifacts behind.
This commit is contained in:
@@ -97,10 +97,20 @@ func rewriteExecStart(line, binaryPath, configPath string) (string, error) {
|
||||
|
||||
if configPath != "" {
|
||||
replaced := false
|
||||
for i := 0; i < len(fields)-1; i++ {
|
||||
if fields[i] == "--config" || fields[i] == "-c" {
|
||||
for i := 0; i < len(fields); i++ {
|
||||
switch {
|
||||
// Both spellings are real, and getting this wrong is not
|
||||
// cosmetic: a production unit uses the equals form, and only
|
||||
// matching the separated one appended a second --config, so
|
||||
// the service started with two and used the wrong one - the
|
||||
// v0.15 config, which v0.16 cannot read as a store descriptor.
|
||||
case strings.HasPrefix(fields[i], "--config=") || strings.HasPrefix(fields[i], "-c="):
|
||||
fields[i] = "--config=" + configPath
|
||||
replaced = true
|
||||
case (fields[i] == "--config" || fields[i] == "-c") && i+1 < len(fields):
|
||||
fields[i+1] = configPath
|
||||
replaced = true
|
||||
i++
|
||||
}
|
||||
}
|
||||
if !replaced {
|
||||
|
||||
@@ -128,3 +128,40 @@ func TestRewriteUnitHandlesMultipleExecStartLines(t *testing.T) {
|
||||
t.Fatalf("an empty ExecStart= names no executable and should be refused, got:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A real production unit writes ExecStart=... --config=/path. Only matching
|
||||
// the separated "--config /path" form appended a second flag, leaving the
|
||||
// service started with two configs and using the v0.15 one - which v0.16
|
||||
// cannot read as a store descriptor. Found while preparing a live
|
||||
// migration, before it ran.
|
||||
func TestRewriteUnitReplacesTheEqualsFormConfig(t *testing.T) {
|
||||
unit := "[Service]\nExecStart=/opt/stalwart/bin/stalwart --config=/opt/stalwart/etc/config.toml\n"
|
||||
got, err := RewriteUnit(unit, "/opt/stalwart/bin/stalwart", "/opt/stalwart/etc/config.json")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Count(got, "--config") != 1 {
|
||||
t.Errorf("expected exactly one --config argument, got:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "--config=/opt/stalwart/etc/config.json") {
|
||||
t.Errorf("equals-form config not replaced:\n%s", got)
|
||||
}
|
||||
if strings.Contains(got, "config.toml") {
|
||||
t.Errorf("the old config path survived:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// The separated form must keep working; both spellings are real.
|
||||
func TestRewriteUnitReplacesTheSeparatedFormConfig(t *testing.T) {
|
||||
unit := "[Service]\nExecStart=/usr/local/bin/stalwart --config /etc/stalwart/config.toml\n"
|
||||
got, err := RewriteUnit(unit, "/usr/local/bin/stalwart", "/etc/stalwart/config.json")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Count(got, "--config") != 1 {
|
||||
t.Errorf("expected exactly one --config argument, got:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "--config /etc/stalwart/config.json") {
|
||||
t.Errorf("separated-form config not replaced:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,7 +27,12 @@ type Options struct {
|
||||
AdminPassword string
|
||||
TargetVersion string // e.g. "0.16.14" or "latest"
|
||||
MinFreeMultiple float64
|
||||
HTTPClient *http.Client
|
||||
// CLIPath and PythonPath are the external programs the migration
|
||||
// shells out to. Checked before anything is touched - see
|
||||
// CheckExternalTools.
|
||||
CLIPath string
|
||||
PythonPath string
|
||||
HTTPClient *http.Client
|
||||
}
|
||||
|
||||
// Checker runs the preflight checks described in ARCHITECTURE.md §4.1.
|
||||
@@ -107,7 +112,7 @@ func (c *Checker) Run(ctx context.Context, store *checkpoint.Store, rs *checkpoi
|
||||
return report, err
|
||||
}
|
||||
|
||||
if _, err := runCheck("upgrade-direction", func() (CheckResult, string) {
|
||||
boundaryOutcome, err := runCheck("upgrade-direction", func() (CheckResult, string) {
|
||||
curV, errCur := parseSemver(versionOutcome.Extra)
|
||||
tgtV, errTgt := parseSemver(targetOutcome.Extra)
|
||||
if errCur != nil || errTgt != nil {
|
||||
@@ -123,15 +128,27 @@ func (c *Checker) Run(ctx context.Context, store *checkpoint.Store, rs *checkpoi
|
||||
return CheckResult{
|
||||
Status: StatusOK,
|
||||
Detail: fmt.Sprintf("%s -> %s crosses the 0.15/0.16 major boundary: full recovery-mode migration plan required (ARCHITECTURE.md §4.4)", curV, tgtV),
|
||||
}, ""
|
||||
}, "crosses"
|
||||
}
|
||||
return CheckResult{
|
||||
Status: StatusOK,
|
||||
Detail: fmt.Sprintf("%s -> %s is a same-boundary patch upgrade: fast-path plan applies (ARCHITECTURE.md §4.6)", curV, tgtV),
|
||||
}, ""
|
||||
}); err != nil {
|
||||
}, "patch"
|
||||
})
|
||||
if err != nil {
|
||||
return report, err
|
||||
}
|
||||
crossesBoundary := boundaryOutcome.Extra != "patch"
|
||||
|
||||
// Before anything else that matters: are the tools this migration
|
||||
// depends on actually here? Discovering a missing stalwart-cli after
|
||||
// the service has been stopped is what this exists to prevent.
|
||||
for _, res := range CheckExternalTools(ctx, c.opts.CLIPath, c.opts.PythonPath, crossesBoundary) {
|
||||
result := res
|
||||
if _, err := runCheck(result.Name, func() (CheckResult, string) { return result, "" }); err != nil {
|
||||
return report, err
|
||||
}
|
||||
}
|
||||
|
||||
deploymentOutcome, err := runCheck("deployment-kind", func() (CheckResult, string) {
|
||||
kind := DetectDeploymentKind(ctx, c.opts.ContainerName)
|
||||
|
||||
@@ -44,6 +44,10 @@ func countLines(t *testing.T, path string) int {
|
||||
}
|
||||
|
||||
func TestCheckerRunEndToEndAndResume(t *testing.T) {
|
||||
// preflight now verifies the external tools before anything else; give
|
||||
// this environment ones that satisfy it.
|
||||
fakeTool(t, "stalwart-cli", "stalwart-cli 1.0.12")
|
||||
fakeTool(t, "python3", "Python 3.13.5")
|
||||
// -- fixtures --------------------------------------------------------
|
||||
counterPath := filepath.Join(t.TempDir(), "invocations")
|
||||
binaryPath := writeFakeBinary(t, "0.15.5", counterPath)
|
||||
@@ -200,6 +204,10 @@ func TestCheckerRunFlagsInsufficientDiskSpace(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestCheckerRunCapturesAccountSnapshotWhenAdminURLSet(t *testing.T) {
|
||||
// preflight now verifies the external tools before anything else; give
|
||||
// this environment ones that satisfy it.
|
||||
fakeTool(t, "stalwart-cli", "stalwart-cli 1.0.12")
|
||||
fakeTool(t, "python3", "Python 3.13.5")
|
||||
counterPath := filepath.Join(t.TempDir(), "invocations")
|
||||
binaryPath := writeFakeBinary(t, "0.15.5", counterPath)
|
||||
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
// SPDX-FileCopyrightText: 2026 LINUXexpert-org
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package preflight
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"fmt"
|
||||
"os/exec"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// minCLIVersion is the oldest stalwart-cli the migration can use.
|
||||
// UPGRADING/v0_16.md is explicit: "install the new CLI (make sure to
|
||||
// install v1.0.2 or later)". Older builds have no `apply` command at all.
|
||||
var minCLIVersion = semver{1, 0, 2}
|
||||
|
||||
// CheckExternalTools verifies the programs the migration shells out to
|
||||
// exist and are new enough, before anything is touched.
|
||||
//
|
||||
// This check exists because its absence took a production mail server down.
|
||||
// A live migration stopped the service, converted the settings, and only
|
||||
// then discovered that the host's stalwart-cli was 0.13.4 - present, but
|
||||
// from the era when the CLI shipped with the server, and with no `apply`
|
||||
// command. The migration needs v1.0.2+ from the separately-versioned
|
||||
// stalwartlabs/cli repository, which is a different download most operators
|
||||
// have never made. Recovery cost a restore from a day-old snapshot.
|
||||
//
|
||||
// Every one of those facts was knowable in under a second, from a stopped
|
||||
// state, before any risk was taken. That is what preflight is for.
|
||||
func CheckExternalTools(ctx context.Context, cliPath, pythonPath string, crossesMajorBoundary bool) []CheckResult {
|
||||
if cliPath == "" {
|
||||
cliPath = "stalwart-cli"
|
||||
}
|
||||
if pythonPath == "" {
|
||||
pythonPath = "python3"
|
||||
}
|
||||
if !crossesMajorBoundary {
|
||||
// A patch bump replays no settings, so neither tool is invoked.
|
||||
return []CheckResult{{
|
||||
Name: "external-tools",
|
||||
Status: StatusOK,
|
||||
Detail: "not needed for a same-boundary patch upgrade: no settings conversion or apply happens",
|
||||
}}
|
||||
}
|
||||
|
||||
var results []CheckResult
|
||||
|
||||
version, err := toolVersion(ctx, cliPath, "--version")
|
||||
switch {
|
||||
case err != nil:
|
||||
results = append(results, CheckResult{
|
||||
Name: "stalwart-cli",
|
||||
Status: StatusFail,
|
||||
Detail: fmt.Sprintf("%s is required to replay settings into the migrated store and could not be run: %v. "+
|
||||
"It is a separate download from the server - see https://stalw.art/docs/management/cli/overview - and "+
|
||||
"must be v%s or later. A stalwart-cli that shipped alongside a 0.15 server has no `apply` command and "+
|
||||
"will not do", cliPath, err, minCLIVersion),
|
||||
})
|
||||
default:
|
||||
parsed, parseErr := parseSemver(version)
|
||||
switch {
|
||||
case parseErr != nil:
|
||||
results = append(results, CheckResult{
|
||||
Name: "stalwart-cli",
|
||||
Status: StatusWarn,
|
||||
Detail: fmt.Sprintf("%s reported a version this tool couldn't parse (%q); it must be v%s or later", cliPath, version, minCLIVersion),
|
||||
})
|
||||
case parsed.Compare(minCLIVersion) < 0:
|
||||
results = append(results, CheckResult{
|
||||
Name: "stalwart-cli",
|
||||
Status: StatusFail,
|
||||
Detail: fmt.Sprintf("%s is v%s, but the migration needs v%s or later - older builds have no `apply` command. "+
|
||||
"Install it from https://stalw.art/docs/management/cli/overview (it is a separate download from the server)",
|
||||
cliPath, parsed, minCLIVersion),
|
||||
})
|
||||
default:
|
||||
results = append(results, CheckResult{
|
||||
Name: "stalwart-cli",
|
||||
Status: StatusOK,
|
||||
Detail: fmt.Sprintf("%s is v%s (needs v%s or later)", cliPath, parsed, minCLIVersion),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
if _, err := toolVersion(ctx, pythonPath, "--version"); err != nil {
|
||||
results = append(results, CheckResult{
|
||||
Name: "python3",
|
||||
Status: StatusFail,
|
||||
Detail: fmt.Sprintf("%s is required to run Stalwart's migrate_v016.py and could not be run: %v", pythonPath, err),
|
||||
})
|
||||
} else {
|
||||
results = append(results, CheckResult{
|
||||
Name: "python3", Status: StatusOK,
|
||||
Detail: fmt.Sprintf("%s is available for migrate_v016.py", pythonPath),
|
||||
})
|
||||
}
|
||||
return results
|
||||
}
|
||||
|
||||
// toolVersion runs a program's version flag and returns its output.
|
||||
func toolVersion(ctx context.Context, path string, arg string) (string, error) {
|
||||
cmd := exec.CommandContext(ctx, path, arg)
|
||||
var out bytes.Buffer
|
||||
cmd.Stdout = &out
|
||||
cmd.Stderr = &out
|
||||
if err := cmd.Run(); err != nil {
|
||||
return "", fmt.Errorf("%w (output: %s)", err, strings.TrimSpace(out.String()))
|
||||
}
|
||||
return strings.TrimSpace(out.String()), nil
|
||||
}
|
||||
@@ -0,0 +1,102 @@
|
||||
// SPDX-FileCopyrightText: 2026 LINUXexpert-org
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package preflight
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// fakeTool puts an executable of the given name at the front of PATH,
|
||||
// reporting the given --version output.
|
||||
func fakeTool(t *testing.T, name, versionOutput string) {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
script := "#!/bin/sh\necho '" + versionOutput + "'\n"
|
||||
if err := os.WriteFile(filepath.Join(dir, name), []byte(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
|
||||
}
|
||||
|
||||
func statusOf(results []CheckResult, name string) (Status, string) {
|
||||
for _, r := range results {
|
||||
if r.Name == name {
|
||||
return r.Status, r.Detail
|
||||
}
|
||||
}
|
||||
return "", ""
|
||||
}
|
||||
|
||||
// This is the check whose absence took a production mail server down: a
|
||||
// live migration stopped the service, then discovered the host's
|
||||
// stalwart-cli was 0.13.4 and had no `apply` command. Recovery cost a
|
||||
// restore from a day-old snapshot.
|
||||
func TestCheckExternalToolsFailsWhenTheCLIIsMissing(t *testing.T) {
|
||||
t.Setenv("PATH", t.TempDir()) // nothing on PATH at all
|
||||
results := CheckExternalTools(context.Background(), "", "", true)
|
||||
|
||||
status, detail := statusOf(results, "stalwart-cli")
|
||||
if status != StatusFail {
|
||||
t.Errorf("stalwart-cli = %q, want fail - this must stop the run before the service does", status)
|
||||
}
|
||||
if !strings.Contains(detail, "separate download") {
|
||||
t.Errorf("detail %q should say it's a separate download from the server", detail)
|
||||
}
|
||||
if !strings.Contains(detail, "v1.0.2") {
|
||||
t.Errorf("detail %q should name the minimum version", detail)
|
||||
}
|
||||
}
|
||||
|
||||
// The exact version that was on the production host. It exists, it runs,
|
||||
// and it cannot do the job.
|
||||
func TestCheckExternalToolsFailsOnTheOldBundledCLI(t *testing.T) {
|
||||
fakeTool(t, "stalwart-cli", "stalwart-cli 0.13.4")
|
||||
fakeTool(t, "python3", "Python 3.13.5")
|
||||
results := CheckExternalTools(context.Background(), "", "", true)
|
||||
|
||||
status, detail := statusOf(results, "stalwart-cli")
|
||||
if status != StatusFail {
|
||||
t.Errorf("stalwart-cli 0.13.4 = %q, want fail", status)
|
||||
}
|
||||
if !strings.Contains(detail, "0.13.4") || !strings.Contains(detail, "no `apply` command") {
|
||||
t.Errorf("detail %q should name the version found and why it won't do", detail)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckExternalToolsAcceptsASupportedCLI(t *testing.T) {
|
||||
fakeTool(t, "stalwart-cli", "stalwart-cli 1.0.12")
|
||||
fakeTool(t, "python3", "Python 3.13.5")
|
||||
results := CheckExternalTools(context.Background(), "", "", true)
|
||||
|
||||
for _, name := range []string{"stalwart-cli", "python3"} {
|
||||
if status, detail := statusOf(results, name); status != StatusOK {
|
||||
t.Errorf("%s = %q (%s), want ok", name, status, detail)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckExternalToolsFailsWithoutPython(t *testing.T) {
|
||||
fakeTool(t, "stalwart-cli", "stalwart-cli 1.0.12")
|
||||
results := CheckExternalTools(context.Background(), "", "/nonexistent/python3", true)
|
||||
if status, _ := statusOf(results, "python3"); status != StatusFail {
|
||||
t.Errorf("python3 = %q, want fail - migrate_v016.py cannot run without it", status)
|
||||
}
|
||||
}
|
||||
|
||||
// A patch bump replays no settings, so neither tool is invoked and a
|
||||
// missing CLI must not block it.
|
||||
func TestCheckExternalToolsSkipsForAPatchUpgrade(t *testing.T) {
|
||||
t.Setenv("PATH", t.TempDir())
|
||||
results := CheckExternalTools(context.Background(), "", "", false)
|
||||
if len(results) != 1 {
|
||||
t.Fatalf("got %d result(s), want a single skip-style result", len(results))
|
||||
}
|
||||
if results[0].Status != StatusOK {
|
||||
t.Errorf("status = %q, want ok for a patch upgrade with no tools present", results[0].Status)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user