diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 940012d..6957dd1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -308,10 +308,32 @@ attempt reuses the existing backup and dumps rather than re-capturing (faster retry, and one fewer chance for the retry's own backup step to fail). -**Status: not yet implemented.** `stalwart-migrate run` without `--dry-run` -currently refuses to proceed, precisely because this phase doesn't exist yet -— committing to a real cutover without a working rollback would violate the -one guarantee this tool exists to provide. §4.9 covers what does work today. +**Status: implemented** (`internal/rollback`, plus `internal/service` for +the systemd/Docker control it needs) for the manual trigger. Departures from +the procedure above, and what it still doesn't cover: + +- Only the manual trigger exists. The automatic one fires on validation + failure during a real cutover, and there is no real cutover to fail yet. +- The procedure gains a step 0 this design didn't call out: the + backup is re-verified against its manifest *before* the service is + stopped. Finding a corrupt backup is survivable while the failed + instance is still up, and unsurvivable once the data directory has been + moved aside. +- FoundationDB is refused rather than attempted: §4.2's backup step only + *starts* an `fdbbackup` job, and restoring one means `fdbrestore` + against a quiesced cluster. Refusing up front beats a rollback that + reports success without restoring anything. +- Step 3 (restore the old unit/Compose config) is wired but inert: it + restores a preserved service definition if the run recorded one, and + nothing records one yet because cutover — the phase that would rewrite + it — doesn't exist. It reports as an explicit skip, not a silent pass. +- For an external SQL store, step 2 replays the critical-table dump in + place. Unlike the filesystem path, the current contents are *not* + preserved first; the plan the command prints says so before it acts. + +`stalwart-migrate run` without `--dry-run` still refuses, but the reason +has narrowed: what's missing now is §4.5 cutover itself, not the ability to +undo it. ### 4.9 Dry run @@ -399,13 +421,16 @@ stalwart-migrate run --dry-run [--target-binary PATH] ... # implemented [--keep-artifacts] (without --dry-run: refused today — see §4.8's status note) stalwart-migrate status [run-id] # implemented -stalwart-migrate rollback # not yet implemented (§4.8) +stalwart-migrate rollback [--yes] # implemented — see §4.8 stalwart-migrate confirm # not yet implemented stalwart-migrate report [--json] # not yet implemented ``` -`run` is the only command that mutates anything and it always starts with -preflight. Once rollback exists, `confirm` will be a separate, explicit step +`run` is the only command that mutates anything on a *successful* path, and +it always starts with preflight. `rollback` mutates too, by design — it's +the one command that stops a running mail server and overwrites a live data +directory — so it prints the plan it resolved from the run's checkpoint and +refuses to act without `--yes`. Once rollback exists, `confirm` will be a separate, explicit step so backups aren't pruned just because validation passed automatically — the operator gets a beat to actually use the migrated server before disk space is reclaimed. Default retention if never confirmed: configurable TTL, warns @@ -416,7 +441,7 @@ run `stalwart-migrate -h` for the actual current flag set. ``` stalwart-migrator/ - cmd/stalwart-migrate/ main.go, preflight.go, run.go, status.go — CLI entry + wiring + cmd/stalwart-migrate/ main.go, preflight.go, run.go, status.go, rollback.go — CLI entry + wiring internal/plan/ version-boundary → ordered phase list (§4.6) [done] internal/checkpoint/ run-id, state.json read/write, resume logic (§5) [done] internal/preflight/ §4.1 checks [done] @@ -437,7 +462,14 @@ its own package, pending a real cutover phase to generalize it against. `internal/stalwartapi` is deliberately the only thing that speaks JMAP/HTTP to Stalwart — every other package depends on it, not on `net/http` directly, -so auth handling and retry/backoff live in one place. +so auth handling and retry/backoff live in one place. `internal/service` is +the same idea for the other external surface: it is the only thing that +shells out to `systemctl` or `docker`, so the commands that can take mail +delivery down sit in one auditable file rather than in each phase that +happens to need them. Rollback needs it today and cutover will need exactly +the same operations, which is why it's its own package rather than living +inside `internal/rollback`. `preflight.DeploymentKind` is a type alias for +`service.Kind`, so detection and control can't drift apart. ## 8. Open questions for the next pass @@ -504,15 +536,31 @@ so auth handling and retry/backoff live in one place. mailbox snapshotting all work end-to-end, and dry-run's comparison is now the closest thing to §4.7's actual no-data-loss guarantee this tool has — the remaining major gap is `internal/rollback` and real cutover (below). -- **No real cutover or rollback yet**: `internal/rollback` doesn't exist, - and neither does any systemd/Docker service control. `run` without - `--dry-run` refuses for exactly this reason. Building rollback first - - before wiring a real cutover - is deliberate: this tool should never be - able to commit to a change it can't undo. -- **Dry-run's un-stopped backup snapshot** (§4.9 step 2): without service - control, a dry-run backs up a live, in-use store unless the operator stops - it manually first. Worth revisiting once service control exists, so - dry-run can offer to do this safely itself. +- **Rollback: done. Real cutover: still missing.** `internal/rollback` and + `internal/service` are implemented and tested (§4.8), so `stalwart-migrate + rollback ` can now undo a run: it re-verifies the backup, stops the + service, moves the failed attempt aside without deleting it, restores the + data directory (re-verifying every restored file against the manifest) or + replays the SQL dump, reinstalls the preserved binary, restarts, and runs a + reduced validation suite against the *restored* instance rather than + assuming it worked. `run` without `--dry-run` still refuses, but now for + the narrower reason that §4.5 cutover itself isn't built - the thing that + would swap the binary, rewrite the unit, and switch the service over. Doing + rollback first was deliberate: this tool should never be able to commit to + a change it can't undo. +- **`confirm` still has no implementation**, so nothing can set + `RollbackWindowClosed` - rollback honours the flag and refuses when it's + set, but only a hand-edited state.json can currently set it. Closing the + window is the point of no return for the backups this restores from, so it + should land together with the retention/TTL policy §6 describes, not + before it. +- **Cutover must preserve the service definition it rewrites**, recording it + as a `service-unit` artifact; rollback's restore step already reads that + contract and reports an explicit skip until something writes one. +- **Dry-run's un-stopped backup snapshot** (§4.9 step 2): a dry-run still + backs up a live, in-use store unless the operator stops it manually first. + `internal/service` now makes doing this properly possible - dry-run just + hasn't been wired to offer it yet. ## Sources diff --git a/README.md b/README.md index 2bc4dab..8238dca 100644 --- a/README.md +++ b/README.md @@ -8,9 +8,10 @@ Go, standard library only — no external dependencies. ## Status -Partially implemented. Roughly 6,000 lines of tested code across backup, -preflight, checkpointing, validation and recovery; two packages are still -stubs, and that gap is what gates the rest. +Partially implemented. Roughly 8,400 lines of tested code across backup, +preflight, checkpointing, validation, recovery and rollback. What's missing +now is the real cutover phase — the step that actually swaps the binary and +switches the live service over. | Command | State | |---|---| @@ -18,29 +19,35 @@ stubs, and that gap is what gates the rest. | `stalwart-migrate run --dry-run` | **Works** — preflight, real backup, sandboxed trial conversion | | `stalwart-migrate run` | **Refuses on purpose** — see below | | `stalwart-migrate status ` | **Works** | -| `stalwart-migrate rollback ` | Not implemented | +| `stalwart-migrate rollback ` | **Works** — prints its plan; acts only with `--yes` | | `stalwart-migrate confirm ` | Not implemented | | `stalwart-migrate report ` | Not implemented | -**`run` without `--dry-run` deliberately refuses to proceed.** A real cutover -needs `internal/rollback` — currently a `doc.go` and nothing else — plus real -systemd/Docker service control. Committing to a migration with no working -rollback would break the single guarantee the tool exists to make, so it -stops rather than going partway. That refusal is the correct behaviour today, -not a bug. +**`run` without `--dry-run` deliberately refuses to proceed.** Rollback and +service control now exist, so the reason has narrowed: what's still missing +is cutover itself (ARCHITECTURE.md §4.5) — installing the new binary, +rewriting the service definition, and starting the migrated instance for +real. `run` stops rather than going partway. That refusal is the correct +behaviour today, not a bug. + +Rollback was built before cutover on purpose: this tool should never be able +to commit to a change it can't undo. `stalwart-migrate rollback ` +resolves what it would do from the run's own checkpoint, prints that plan, +and touches nothing without `--yes`. Package state: | Package | Lines | Tests | |---|---|---| +| `internal/rollback` | 1807 | yes | | `internal/backup` | 1805 | yes | -| `internal/preflight` | 1324 | yes | +| `internal/preflight` | 1329 | yes | | `internal/validate` | 792 | yes | | `internal/stalwartapi` | 716 | yes | | `internal/recovery` | 702 | yes | | `internal/checkpoint` | 559 | yes | +| `internal/service` | 467 | yes | | `internal/plan` | 196 | yes | -| `internal/rollback` | stub | — | | `internal/config` | stub | — | ## Why not a shell script @@ -92,3 +99,39 @@ directory — read the caveat the command prints before using it on anything you care about. Where the plan crosses the 0.15/0.16 boundary it clones that verified backup into a disposable sandbox and converts the copy, leaving the original untouched. + +## Rolling back + +`rollback` is the one command that stops a running mail server and +overwrites a live data directory, so it never acts on its own reading of the +situation without showing you that reading first: + +```sh +stalwart-migrate rollback # prints the plan, touches nothing +stalwart-migrate rollback --yes # performs it +``` + +Everything in the plan — which backup, which manifest, which preserved +binary — comes from that run's own checkpoint, so rolling back days later +doesn't depend on remembering any of it. What the checkpoint deliberately +doesn't store (database credentials for an external SQL backend) is what the +flags are for. + +The order matters and is not negotiable: the backup is re-verified against +its manifest **before** the service is stopped, because a corrupt backup is +survivable while the failed instance is still up and unsurvivable once its +data directory has been moved aside. Nothing from the failed attempt is +deleted — the half-migrated data directory and the displaced binary are +moved to `.failed-` names. Afterwards a reduced validation suite runs +against the *restored* instance (version, reachability, directory counts) +rather than assuming the restore worked; pass `--admin-url` to get the last +two, which are skipped without it. + +Every step is checkpointed, so a rollback interrupted partway — which is +exactly when a machine is most likely to be rebooted out from under it — +resumes where it stopped instead of restarting a destructive sequence from +the top. Re-running a completed rollback is inert. + +FoundationDB installs are refused rather than attempted: the backup phase +only *starts* an `fdbbackup` job, and restoring one needs `fdbrestore` +against a quiesced cluster. diff --git a/cmd/stalwart-migrate/main.go b/cmd/stalwart-migrate/main.go index 4d69fe8..c83d719 100644 --- a/cmd/stalwart-migrate/main.go +++ b/cmd/stalwart-migrate/main.go @@ -24,7 +24,7 @@ func main() { case "status": err = runStatus(os.Args[2:]) case "rollback": - err = fmt.Errorf("not implemented yet: see internal/rollback") + err = runRollback(os.Args[2:]) case "confirm": err = fmt.Errorf("not implemented yet: see internal/checkpoint") case "report": @@ -47,7 +47,7 @@ commands: preflight run read-only checks and print the migration plan run --dry-run: simulate and validate against a sandbox (real cutover isn't implemented yet) status show the state of an in-progress or completed run - rollback restore the pre-migration backup for a given run + rollback restore the pre-migration backup for a given run (prints the plan; --yes to act) confirm close the rollback window for a completed run report print the validation report for a run`) } diff --git a/cmd/stalwart-migrate/rollback.go b/cmd/stalwart-migrate/rollback.go new file mode 100644 index 0000000..038a06b --- /dev/null +++ b/cmd/stalwart-migrate/rollback.go @@ -0,0 +1,159 @@ +package main + +import ( + "context" + "flag" + "fmt" + "net/http" + "os" + "strings" + "time" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" + "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" + "github.com/LINUXexpert-org/stalwart-migrator/internal/rollback" + "github.com/LINUXexpert-org/stalwart-migrator/internal/service" +) + +// splitRunID pulls the run-id out of args wherever it appears, so +// `rollback --data-dir X` works as naturally as +// `rollback --data-dir X `. Go's flag package stops parsing at the +// first positional argument, which would otherwise make the obvious +// invocation order silently drop every flag after the run-id - on a command +// whose flags decide what gets overwritten, that is not a failure mode +// worth living with. Tokens consumed as a flag's value are skipped by +// asking the FlagSet itself which flags take one, rather than by +// maintaining a second list of them here. +func splitRunID(fs *flag.FlagSet, args []string) (runID string, rest []string) { + rest = make([]string, 0, len(args)) + for i := 0; i < len(args); i++ { + arg := args[i] + if arg == "--" { + rest = append(rest, args[i:]...) + break + } + if strings.HasPrefix(arg, "-") { + rest = append(rest, arg) + name := strings.TrimLeft(arg, "-") + if !strings.Contains(arg, "=") && takesValue(fs, name) && i+1 < len(args) { + i++ + rest = append(rest, args[i]) + } + continue + } + if runID == "" { + runID = arg + continue + } + rest = append(rest, arg) // a second positional: let Parse report it + } + return runID, rest +} + +func takesValue(fs *flag.FlagSet, name string) bool { + f := fs.Lookup(name) + if f == nil { + return false + } + boolFlag, ok := f.Value.(interface{ IsBoolFlag() bool }) + return !ok || !boolFlag.IsBoolFlag() +} + +// runRollback implements `stalwart-migrate rollback `: put the +// instance back the way it was before the named run touched it +// (ARCHITECTURE.md §4.8). +// +// This is the only command that stops a running mail server and overwrites +// a live data directory, so it always prints the resolved plan first and +// refuses to act without --yes. Everything the plan needs comes from the +// run's own checkpoint - which backup, which manifest, which preserved +// binary - so an operator rolling back days after the fact doesn't have to +// remember any of it. The flags below exist to supply what the checkpoint +// deliberately doesn't store (database credentials) or to override a +// detection that was wrong. +func runRollback(args []string) error { + fs := flag.NewFlagSet("rollback", flag.ExitOnError) + stateDir := fs.String("state-dir", checkpoint.DefaultBaseDir, "directory runs are checkpointed in") + dataDir := fs.String("data-dir", "/var/lib/stalwart", "stalwart data directory to restore into (embedded backends)") + binaryPath := fs.String("binary", "/usr/local/bin/stalwart", "path the preserved old binary is reinstalled to") + serviceUnitPath := fs.String("service-unit", "", "path a preserved systemd unit or Compose file is restored to (only used if the run recorded one)") + + deployment := fs.String("deployment", "", `override how the service is controlled: "systemd" or "docker" (default: whatever preflight detected for this run)`) + unitName := fs.String("unit", "stalwart", "systemd unit name") + containerName := fs.String("container", "stalwart", "docker container name") + stopTimeout := fs.Duration("stop-timeout", 60*time.Second, "how long to wait for the service to actually stop") + startTimeout := fs.Duration("start-timeout", 60*time.Second, "how long to wait for the restored service to come back") + + adminURL := fs.String("admin-url", "", "base URL for the restored instance's admin/JMAP API, for post-rollback verification") + adminUser := fs.String("admin-user", "", "admin username") + adminPassword := fs.String("admin-password", os.Getenv("STALWART_MIGRATE_ADMIN_PASSWORD"), + "admin password (or set STALWART_MIGRATE_ADMIN_PASSWORD)") + verifyTimeout := fs.Duration("verify-timeout", 60*time.Second, "how long to wait for the restored instance to answer") + + dbHost := fs.String("db-host", "", "external database host (postgresql/mysql backends)") + dbPort := fs.String("db-port", "", "external database port") + dbName := fs.String("db-name", "", "external database name") + dbUser := fs.String("db-user", "", "external database user") + dbPassword := fs.String("db-password", os.Getenv("STALWART_MIGRATE_DB_PASSWORD"), + "external database password (or set STALWART_MIGRATE_DB_PASSWORD)") + sqlDump := fs.String("sql-dump", "", "override the dump file to replay (default: the one this run recorded)") + + yes := fs.Bool("yes", false, "actually perform the rollback; without it, the plan is printed and nothing is touched") + + runID, rest := splitRunID(fs, args) + if err := fs.Parse(rest); err != nil { + return err + } + if runID == "" || fs.NArg() != 0 { + return fmt.Errorf("usage: stalwart-migrate rollback [flags]") + } + + store := checkpoint.NewStore(*stateDir) + rs, err := store.Load(runID) + if err != nil { + return fmt.Errorf("load run %s: %w", runID, err) + } + + opts := rollback.Options{ + Deployment: service.Options{ + Kind: service.Kind(*deployment), UnitName: *unitName, ContainerName: *containerName, + }, + StopTimeout: *stopTimeout, StartTimeout: *startTimeout, + DataDir: *dataDir, + BinaryPath: *binaryPath, + ServiceUnitPath: *serviceUnitPath, + SQL: backup.SQLOptions{ + Host: *dbHost, Port: *dbPort, Database: *dbName, User: *dbUser, Password: *dbPassword, OutPath: *sqlDump, + }, + AdminURL: *adminURL, AdminUser: *adminUser, AdminPassword: *adminPassword, + HTTPClient: &http.Client{}, VerifyTimeout: *verifyTimeout, + } + + plan, err := rollback.BuildPlan(rs, opts) + if err != nil { + return err + } + fmt.Print(plan.String()) + + if !*yes { + fmt.Println("\nnothing has been touched. Re-run with --yes to perform this rollback.") + return nil + } + if *adminURL == "" { + fmt.Println("\nnote: without --admin-url, the reachability and directory-count checks after the restart are skipped - " + + "the rollback will report success on the restore mechanics alone.") + } + + fmt.Println("\n--- rollback ---") + report, err := rollback.Run(context.Background(), store, rs, opts) + fmt.Print(report.String()) + if err != nil { + return fmt.Errorf("rollback did not complete: %w", err) + } + + fmt.Printf("\nROLLBACK COMPLETE for run %s: the instance is back on %s. "+ + "The failed attempt's data and binary are preserved under .failed-%s names, and this run's backups are untouched, "+ + "so a retry after the underlying issue is fixed doesn't have to re-capture anything.\n", + rs.RunID, rs.SourceVersion, rs.RunID) + return nil +} diff --git a/cmd/stalwart-migrate/run.go b/cmd/stalwart-migrate/run.go index 7b6e750..df96a51 100644 --- a/cmd/stalwart-migrate/run.go +++ b/cmd/stalwart-migrate/run.go @@ -17,12 +17,10 @@ import ( ) // runRun implements `stalwart-migrate run`. Only --dry-run is available -// today: a real cutover needs internal/rollback (so a failed migration can -// actually be undone) and real systemd/Docker service control, neither of -// which exist yet - see ARCHITECTURE.md §8. Committing to a real migration -// without a working rollback would violate the one thing this whole tool -// exists to guarantee, so `run` without --dry-run refuses rather than doing -// it partway. +// today: the cutover phase itself (§4.5) isn't built. internal/rollback and +// internal/service now exist, so the missing piece is no longer "this can't +// be undone" but "there's nothing here to undo" - see ARCHITECTURE.md §8. +// `run` without --dry-run refuses rather than doing a migration partway. // // --dry-run runs preflight and a real backup (see the caveat printed below // about why backup still touches the live data directory), then - if the @@ -70,8 +68,9 @@ func runRun(args []string) (err error) { if !*dryRun { return fmt.Errorf( - "real (non-dry-run) migrations aren't available yet: internal/rollback and real systemd/Docker service control " + - "aren't implemented, so this tool can't yet guarantee it can undo a failed cutover. Run with --dry-run to " + + "real (non-dry-run) migrations aren't available yet: the cutover phase - installing the new binary, rewriting the " + + "service definition, and starting the migrated instance for real (ARCHITECTURE.md §4.5) - isn't implemented. " + + "Rollback is, so a future cutover will be undoable; there just isn't one to undo yet. Run with --dry-run to " + "validate the migration mechanics against a disposable sandbox copy of your data - nothing in production is touched", ) } @@ -136,9 +135,9 @@ func runRun(args []string) (err error) { fmt.Printf("\nplan: %s\n", p.Reason) fmt.Println("\n--- backup ---") - fmt.Println("(dry-run does not stop the live Stalwart service itself - no service control is implemented yet. " + - "For a guaranteed-consistent snapshot, stop stalwart before running this; otherwise the filesystem copy may " + - "reflect a live, in-use store. This is unrelated to whether production gets touched - it never does.)") + fmt.Println("(dry-run does not stop the live Stalwart service itself - internal/service can now do that, but dry-run " + + "isn't wired to offer it. For a guaranteed-consistent snapshot, stop stalwart before running this; otherwise the " + + "filesystem copy may reflect a live, in-use store. This is unrelated to whether production gets touched - it never does.)") backupDir := filepath.Join(runWorkDir, "backup") scriptDest := filepath.Join(runWorkDir, "migrate_v016.py") diff --git a/cmd/stalwart-migrate/status.go b/cmd/stalwart-migrate/status.go index 77e2959..1124cec 100644 --- a/cmd/stalwart-migrate/status.go +++ b/cmd/stalwart-migrate/status.go @@ -10,13 +10,21 @@ import ( func runStatus(args []string) error { fs := flag.NewFlagSet("status", flag.ExitOnError) stateDir := fs.String("state-dir", checkpoint.DefaultBaseDir, "directory runs are checkpointed in") - if err := fs.Parse(args); err != nil { + + // Same treatment as `rollback`: without this, `status + // --state-dir X` would look up the run in the default directory and + // report it missing, since flag parsing stops at the run-id. + runID, rest := splitRunID(fs, args) + if err := fs.Parse(rest); err != nil { return err } + if fs.NArg() != 0 { + return fmt.Errorf("usage: stalwart-migrate status [run-id] [flags]") + } store := checkpoint.NewStore(*stateDir) - if fs.NArg() == 0 { + if runID == "" { ids, err := store.List() if err != nil { return fmt.Errorf("list runs: %w", err) @@ -32,7 +40,6 @@ func runStatus(args []string) error { return nil } - runID := fs.Arg(0) rs, err := store.Load(runID) if err != nil { return fmt.Errorf("load run %s: %w", runID, err) diff --git a/internal/preflight/deployment.go b/internal/preflight/deployment.go index 867e9a9..b474675 100644 --- a/internal/preflight/deployment.go +++ b/internal/preflight/deployment.go @@ -4,16 +4,21 @@ import ( "context" "os" "os/exec" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/service" ) // DeploymentKind is how a Stalwart instance appears to be run, which -// determines how cutover and rollback restart it. -type DeploymentKind string +// determines how cutover and rollback restart it. It's an alias for +// service.Kind rather than a parallel type: detection here and control +// there have to agree on the same vocabulary, and one definition can't +// drift from itself. +type DeploymentKind = service.Kind const ( - DeploymentSystemd DeploymentKind = "systemd" - DeploymentDocker DeploymentKind = "docker" - DeploymentUnknown DeploymentKind = "unknown" + DeploymentSystemd = service.Systemd + DeploymentDocker = service.Docker + DeploymentUnknown = service.Unknown ) var systemdUnitPaths = []string{ diff --git a/internal/rollback/exectest_test.go b/internal/rollback/exectest_test.go new file mode 100644 index 0000000..db65fdd --- /dev/null +++ b/internal/rollback/exectest_test.go @@ -0,0 +1,92 @@ +package rollback + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" +) + +// withFakeExecutable puts a fake executable named `name` at the front of +// PATH for the duration of the test, so code that shells out to a +// real-world tool (psql, mysql, the stalwart binary's --version) can be +// exercised without that tool being installed. t.Setenv restores PATH +// automatically and marks the test non-parallel. +func withFakeExecutable(t *testing.T, name, script string) (dir string) { + t.Helper() + dir = t.TempDir() + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + return dir +} + +func argsFile(t *testing.T, dir string) string { + t.Helper() + return filepath.Join(dir, "invoked-args.log") +} + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return "" + } + if err != nil { + t.Fatal(err) + } + return string(data) +} + +func fakeScriptLoggingArgs(logPath string, extraBody string) string { + return fmt.Sprintf("#!/bin/sh\necho \"$@\" >> %q\n%s\n", logPath, extraBody) +} + +// fakeController stands in for a real systemd unit or Docker container, so +// the orchestration in Run can be tested without either. It records the +// order it was called in - which is the thing that actually matters here: +// restoring a data directory while the service is still running would be a +// silent corruption bug, and call order is how that's caught. +type fakeController struct { + calls []string + active bool + stopErr error + startErr error + + // onStop runs after a successful Stop, letting a test assert on the + // state of the world at the exact moment the service went down. + onStop func() +} + +func (f *fakeController) Stop(context.Context) error { + f.calls = append(f.calls, "stop") + if f.stopErr != nil { + return f.stopErr + } + f.active = false + if f.onStop != nil { + f.onStop() + } + return nil +} + +func (f *fakeController) Start(context.Context) error { + f.calls = append(f.calls, "start") + if f.startErr != nil { + return f.startErr + } + f.active = true + return nil +} + +func (f *fakeController) Active(context.Context) (bool, error) { return f.active, nil } + +func (f *fakeController) ReloadConfig(context.Context) error { + f.calls = append(f.calls, "reload") + return nil +} + +func (f *fakeController) Target() string { return "test service" } diff --git a/internal/rollback/report.go b/internal/rollback/report.go new file mode 100644 index 0000000..5c196e4 --- /dev/null +++ b/internal/rollback/report.go @@ -0,0 +1,44 @@ +package rollback + +import ( + "fmt" + "strings" +) + +type Status string + +const ( + StatusOK Status = "ok" + StatusSkipped Status = "skip" + StatusFail Status = "fail" +) + +type CheckResult struct { + Name string + Status Status + Detail string +} + +type Report struct { + Results []CheckResult +} + +// Blocking reports whether anything failed. A rollback report that isn't +// clean means the instance is in an unknown state - never quietly "rolled +// back". +func (r Report) Blocking() bool { + for _, res := range r.Results { + if res.Status == StatusFail { + return true + } + } + return false +} + +func (r Report) String() string { + var b strings.Builder + for _, res := range r.Results { + fmt.Fprintf(&b, "[%-4s] %-22s %s\n", strings.ToUpper(string(res.Status)), res.Name, res.Detail) + } + return b.String() +} diff --git a/internal/rollback/restore.go b/internal/rollback/restore.go new file mode 100644 index 0000000..09492f2 --- /dev/null +++ b/internal/rollback/restore.go @@ -0,0 +1,223 @@ +package rollback + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" +) + +// PreserveFailedState moves a path aside to ".failed-" instead +// of deleting it, so a rollback never destroys the failed attempt's own +// state (ARCHITECTURE.md §4.8) - if the rollback itself turns out to have +// been the wrong call, or the failure needs diagnosing afterward, the +// half-migrated data is still there under a name that says which run +// produced it. +// +// It's idempotent for a resumed rollback: if the preserved path already +// exists, a previous attempt already did this, and the function reports +// that rather than clobbering the earlier rescue with whatever is at path +// now. +func PreserveFailedState(path, runID string) (preservedPath string, moved bool, err error) { + preservedPath = fmt.Sprintf("%s.failed-%s", path, runID) + + if _, statErr := os.Stat(preservedPath); statErr == nil { + return preservedPath, false, nil + } else if !os.IsNotExist(statErr) { + return "", false, fmt.Errorf("rollback: stat %s: %w", preservedPath, statErr) + } + + if _, statErr := os.Stat(path); os.IsNotExist(statErr) { + return preservedPath, false, nil // nothing there to preserve + } else if statErr != nil { + return "", false, fmt.Errorf("rollback: stat %s: %w", path, statErr) + } + + if err := os.Rename(path, preservedPath); err != nil { + return "", false, fmt.Errorf("rollback: move %s aside to %s: %w", path, preservedPath, err) + } + return preservedPath, true, nil +} + +// RestoreDataDir copies a verified backup back over the original data +// directory and then re-verifies what it wrote against the same manifest. +// The second verification is the point: a restore that silently truncated +// or corrupted a file would otherwise be indistinguishable from a good one +// until Stalwart failed to open its store, long after this tool reported +// success. +// +// The caller must have moved any existing dataDir aside first (see +// PreserveFailedState) - CopyDataDir clears its destination, and clearing a +// live data directory is not a decision this function should be making on +// its own. +func RestoreDataDir(backupDir, dataDir string, m *backup.Manifest) error { + if _, err := backup.CopyDataDir(backupDir, dataDir); err != nil { + return fmt.Errorf("rollback: restore %s from %s: %w", dataDir, backupDir, err) + } + if err := backup.VerifyDataDirBackup(dataDir, m); err != nil { + return fmt.Errorf("rollback: the restored data directory doesn't match the backup manifest: %w", err) + } + return nil +} + +// RestoreBinary puts the preserved old binary back at binaryPath, moving +// whatever is there now aside first (never deleting it - the new-version +// binary is what a retry after the underlying issue is fixed will want). +// It verifies the preserved binary's checksum against the one recorded when +// it was preserved before installing it, so a rollback can't restore a +// binary that was corrupted or swapped since backup ran. +func RestoreBinary(preservedPath, binaryPath, wantSHA256, runID string) (displaced string, err error) { + sum, _, err := hashFile(preservedPath) + if err != nil { + return "", err + } + if wantSHA256 != "" && sum != wantSHA256 { + return "", fmt.Errorf( + "rollback: preserved binary %s has sha256 %s but the checkpoint recorded %s when it was preserved - refusing to install a binary that changed since then", + preservedPath, sum, wantSHA256) + } + + displaced, moved, err := PreserveFailedState(binaryPath, runID) + if err != nil { + return "", err + } + if !moved { + displaced = "" + } + + if err := os.Rename(preservedPath, binaryPath); err != nil { + return "", fmt.Errorf("rollback: restore %s to %s: %w", preservedPath, binaryPath, err) + } + return displaced, nil +} + +// RestoreFile copies src over dst (used for a preserved systemd unit or +// Compose file), preserving dst's own mode if it exists and falling back to +// src's otherwise. It writes to a temp file in the destination directory +// and renames it into place, so a service definition is never left +// half-written - the same reason checkpoint.Store.Save does it. +func RestoreFile(src, dst string) error { + perm := os.FileMode(0o644) + if info, err := os.Stat(dst); err == nil { + perm = info.Mode().Perm() + } else if info, err := os.Stat(src); err == nil { + perm = info.Mode().Perm() + } + + data, err := os.ReadFile(src) + if err != nil { + return fmt.Errorf("rollback: read %s: %w", src, err) + } + tmp, err := os.CreateTemp(filepath.Dir(dst), filepath.Base(dst)+".tmp-*") + if err != nil { + return fmt.Errorf("rollback: create temp file next to %s: %w", dst, err) + } + tmpPath := tmp.Name() + defer os.Remove(tmpPath) // no-op once the rename below succeeds + + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return fmt.Errorf("rollback: write %s: %w", tmpPath, err) + } + if err := tmp.Chmod(perm); err != nil { + tmp.Close() + return fmt.Errorf("rollback: chmod %s: %w", tmpPath, err) + } + if err := tmp.Sync(); err != nil { + tmp.Close() + return fmt.Errorf("rollback: sync %s: %w", tmpPath, err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("rollback: close %s: %w", tmpPath, err) + } + if err := os.Rename(tmpPath, dst); err != nil { + return fmt.Errorf("rollback: move %s into place at %s: %w", tmpPath, dst, err) + } + return nil +} + +// BuildPsqlArgs returns psql's argv for restoring the critical-table dump +// backup.RunPgDump produced, without the leading "psql". ON_ERROR_STOP=1 is +// not optional here: psql's default is to report an error and carry on, +// which would let a restore that only half-applied exit zero and be +// reported as a successful rollback. +func BuildPsqlArgs(o backup.SQLOptions) []string { + args := []string{"-v", "ON_ERROR_STOP=1", "-U", o.User, "-d", o.Database} + if o.Host != "" { + args = append(args, "-h", o.Host) + } + if o.Port != "" { + args = append(args, "-p", o.Port) + } + return append(args, "-f", o.OutPath) +} + +// BuildMySQLRestoreArgs returns mysql's argv for the same restore. mysql +// reads the dump from stdin rather than taking a file flag, so RunMySQLRestore +// redirects it. +func BuildMySQLRestoreArgs(o backup.SQLOptions) []string { + args := []string{"-u", o.User} + if o.Host != "" { + args = append(args, "-h", o.Host) + } + if o.Port != "" { + args = append(args, "-P", o.Port) + } + return append(args, o.Database) +} + +// RunPsqlRestore replays a pg_dump file, passing the password via +// PGPASSWORD exactly as backup.RunPgDump does rather than on the command +// line where `ps` could read it. +func RunPsqlRestore(ctx context.Context, o backup.SQLOptions) error { + cmd := exec.CommandContext(ctx, "psql", BuildPsqlArgs(o)...) + cmd.Env = append(os.Environ(), "PGPASSWORD="+o.Password) + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("rollback: psql restore failed: %w (output: %s)", err, strings.TrimSpace(string(out))) + } + return nil +} + +// RunMySQLRestore replays a mysqldump file from stdin, with the password +// passed via MYSQL_PWD. +func RunMySQLRestore(ctx context.Context, o backup.SQLOptions) error { + f, err := os.Open(o.OutPath) + if err != nil { + return fmt.Errorf("rollback: open dump %s: %w", o.OutPath, err) + } + defer f.Close() + + cmd := exec.CommandContext(ctx, "mysql", BuildMySQLRestoreArgs(o)...) + cmd.Env = append(os.Environ(), "MYSQL_PWD="+o.Password) + cmd.Stdin = f + var stderr strings.Builder + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("rollback: mysql restore failed: %w (stderr: %s)", err, strings.TrimSpace(stderr.String())) + } + return nil +} + +// hashFile returns a file's SHA256 and size, for checking a preserved +// artifact against what the checkpoint recorded for it. +func hashFile(path string) (sha256Hex string, size int64, err error) { + f, err := os.Open(path) + if err != nil { + return "", 0, fmt.Errorf("rollback: hash %s: %w", path, err) + } + defer f.Close() + h := sha256.New() + n, err := io.Copy(h, f) + if err != nil { + return "", 0, fmt.Errorf("rollback: hash %s: %w", path, err) + } + return hex.EncodeToString(h.Sum(nil)), n, nil +} diff --git a/internal/rollback/restore_test.go b/internal/rollback/restore_test.go new file mode 100644 index 0000000..cbba04a --- /dev/null +++ b/internal/rollback/restore_test.go @@ -0,0 +1,300 @@ +package rollback + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" +) + +// writeTree creates a small directory tree and returns its path, standing +// in for a Stalwart data directory. +func writeTree(t *testing.T, root string, files map[string]string) string { + t.Helper() + for rel, content := range files { + path := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o640); err != nil { + t.Fatal(err) + } + } + return root +} + +func TestPreserveFailedStateMovesRatherThanDeletes(t *testing.T) { + dir := t.TempDir() + dataDir := writeTree(t, filepath.Join(dir, "data"), map[string]string{"blobs/a": "half-migrated"}) + + preserved, moved, err := PreserveFailedState(dataDir, "run-1") + if err != nil { + t.Fatal(err) + } + if !moved { + t.Error("moved = false, want true") + } + if want := dataDir + ".failed-run-1"; preserved != want { + t.Errorf("preserved path = %q, want %q", preserved, want) + } + if _, err := os.Stat(dataDir); !os.IsNotExist(err) { + t.Errorf("original path still exists after being moved aside: %v", err) + } + if got := readFile(t, filepath.Join(preserved, "blobs/a")); got != "half-migrated" { + t.Errorf("preserved content = %q, want the failed attempt's data intact", got) + } +} + +// A resumed rollback re-runs steps that were interrupted. If this clobbered +// the earlier rescue with whatever is at the original path the second time +// around, the failed attempt's state - the thing §4.8 promises never to +// delete - would be lost precisely when a rollback got interrupted. +func TestPreserveFailedStateDoesNotClobberAnEarlierRescue(t *testing.T) { + dir := t.TempDir() + dataDir := filepath.Join(dir, "data") + writeTree(t, dataDir, map[string]string{"a": "first"}) + if _, _, err := PreserveFailedState(dataDir, "run-1"); err != nil { + t.Fatal(err) + } + writeTree(t, dataDir, map[string]string{"a": "second"}) + + preserved, moved, err := PreserveFailedState(dataDir, "run-1") + if err != nil { + t.Fatal(err) + } + if moved { + t.Error("moved = true on the second call, want false") + } + if got := readFile(t, filepath.Join(preserved, "a")); got != "first" { + t.Errorf("preserved content = %q, want %q - the first rescue must survive", got, "first") + } +} + +func TestPreserveFailedStateIsFineWhenThereIsNothingToPreserve(t *testing.T) { + preserved, moved, err := PreserveFailedState(filepath.Join(t.TempDir(), "absent"), "run-1") + if err != nil { + t.Fatalf("want no error when the path doesn't exist, got %v", err) + } + if moved { + t.Error("moved = true, want false") + } + if preserved == "" { + t.Error("preserved path should still be reported") + } +} + +func TestRestoreDataDirRestoresAndReverifies(t *testing.T) { + dir := t.TempDir() + src := writeTree(t, filepath.Join(dir, "data"), map[string]string{ + "config": "settings", "blobs/one": "hello", "blobs/two": "world", + }) + backupDir := filepath.Join(dir, "backup") + manifest, err := backup.CopyDataDir(src, backupDir) + if err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(src); err != nil { + t.Fatal(err) + } + + if err := RestoreDataDir(backupDir, src, manifest); err != nil { + t.Fatalf("RestoreDataDir: %v", err) + } + for rel, want := range map[string]string{"config": "settings", "blobs/one": "hello", "blobs/two": "world"} { + if got := readFile(t, filepath.Join(src, rel)); got != want { + t.Errorf("restored %s = %q, want %q", rel, got, want) + } + } +} + +// A restore that put back corrupt bytes and reported success would be worse +// than one that failed: the operator would believe the rollback worked and +// only find out when Stalwart couldn't open its store. +func TestRestoreDataDirFailsWhenTheBackupNoLongerMatchesItsManifest(t *testing.T) { + dir := t.TempDir() + src := writeTree(t, filepath.Join(dir, "data"), map[string]string{"blobs/one": "hello"}) + backupDir := filepath.Join(dir, "backup") + manifest, err := backup.CopyDataDir(src, backupDir) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(backupDir, "blobs/one"), []byte("corrupted"), 0o640); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(src); err != nil { + t.Fatal(err) + } + + err = RestoreDataDir(backupDir, src, manifest) + if err == nil { + t.Fatal("RestoreDataDir: want error for a backup that no longer matches its manifest, got nil") + } + if !strings.Contains(err.Error(), "doesn't match the backup manifest") { + t.Errorf("error %q should say the restored directory didn't match the manifest", err) + } +} + +func TestRestoreBinaryReinstallsOldAndPreservesCurrent(t *testing.T) { + dir := t.TempDir() + binaryPath := filepath.Join(dir, "stalwart") + preserved := binaryPath + ".v0.15.5" + if err := os.WriteFile(preserved, []byte("old binary"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(binaryPath, []byte("new binary"), 0o755); err != nil { + t.Fatal(err) + } + sum, _, err := hashFile(preserved) + if err != nil { + t.Fatal(err) + } + + displaced, err := RestoreBinary(preserved, binaryPath, sum, "run-1") + if err != nil { + t.Fatalf("RestoreBinary: %v", err) + } + if got := readFile(t, binaryPath); got != "old binary" { + t.Errorf("binary at %s = %q, want the old one back", binaryPath, got) + } + if got := readFile(t, displaced); got != "new binary" { + t.Errorf("displaced binary = %q, want the new one preserved for a retry", got) + } +} + +func TestRestoreBinaryRefusesAChangedBinary(t *testing.T) { + dir := t.TempDir() + binaryPath := filepath.Join(dir, "stalwart") + preserved := binaryPath + ".v0.15.5" + if err := os.WriteFile(preserved, []byte("swapped out from under us"), 0o755); err != nil { + t.Fatal(err) + } + + _, err := RestoreBinary(preserved, binaryPath, "0000000000000000000000000000000000000000000000000000000000000000", "run-1") + if err == nil { + t.Fatal("RestoreBinary: want error when the preserved binary's checksum doesn't match, got nil") + } + if _, statErr := os.Stat(binaryPath); !os.IsNotExist(statErr) { + t.Error("a binary failing its checksum must not be installed") + } +} + +func TestRestoreFileKeepsDestinationPermissions(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "stalwart.service.preserved") + dst := filepath.Join(dir, "stalwart.service") + if err := os.WriteFile(src, []byte("[Service]\nExecStart=/usr/local/bin/stalwart\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(dst, []byte("[Service]\nExecStart=/usr/local/bin/stalwart-new\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := RestoreFile(src, dst); err != nil { + t.Fatalf("RestoreFile: %v", err) + } + if got := readFile(t, dst); !strings.Contains(got, "/usr/local/bin/stalwart\n") { + t.Errorf("restored unit = %q, want the preserved one", got) + } + info, err := os.Stat(dst) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o644 { + t.Errorf("restored unit mode = %v, want 0644 (the destination's own mode)", info.Mode().Perm()) + } + // The temp file it writes through must not be left behind next to a + // service definition directory. + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 2 { + t.Errorf("directory has %d entries, want 2 - a temp file was left behind", len(entries)) + } +} + +// psql's default is to report an error and keep going, which would let a +// half-applied restore exit zero and be reported as a successful rollback. +func TestBuildPsqlArgsStopsOnError(t *testing.T) { + args := BuildPsqlArgs(backup.SQLOptions{User: "stalwart", Database: "mail", Host: "db", Port: "5432", OutPath: "/tmp/dump.sql"}) + joined := strings.Join(args, " ") + if !strings.Contains(joined, "-v ON_ERROR_STOP=1") { + t.Errorf("psql args %q must set ON_ERROR_STOP=1", joined) + } + for _, want := range []string{"-U stalwart", "-d mail", "-h db", "-p 5432", "-f /tmp/dump.sql"} { + if !strings.Contains(joined, want) { + t.Errorf("psql args %q missing %q", joined, want) + } + } +} + +func TestBuildMySQLRestoreArgsOmitsUnsetConnectionFields(t *testing.T) { + args := BuildMySQLRestoreArgs(backup.SQLOptions{User: "stalwart", Database: "mail"}) + if got, want := strings.Join(args, " "), "-u stalwart mail"; got != want { + t.Errorf("mysql args = %q, want %q", got, want) + } +} + +func TestRunPsqlRestorePassesPasswordViaEnvironmentNotArgv(t *testing.T) { + dir := t.TempDir() + log := argsFile(t, dir) + withFakeExecutable(t, "psql", fakeScriptLoggingArgs(log, "echo \"PGPASSWORD=$PGPASSWORD\" >> "+log)) + + err := RunPsqlRestore(context.Background(), backup.SQLOptions{ + User: "stalwart", Database: "mail", Password: "hunter2", OutPath: filepath.Join(dir, "dump.sql"), + }) + if err != nil { + t.Fatal(err) + } + logged := readFile(t, log) + if !strings.Contains(logged, "PGPASSWORD=hunter2") { + t.Errorf("psql invocation %q should receive the password via PGPASSWORD", logged) + } + argv := strings.SplitN(logged, "\n", 2)[0] + if strings.Contains(argv, "hunter2") { + t.Errorf("psql argv %q contains the password - anything running `ps` could read it", argv) + } +} + +func TestRunPsqlRestoreSurfacesFailureOutput(t *testing.T) { + dir := t.TempDir() + withFakeExecutable(t, "psql", "#!/bin/sh\necho 'ERROR: relation \"s\" already exists' >&2\nexit 1\n") + err := RunPsqlRestore(context.Background(), backup.SQLOptions{User: "u", Database: "d", OutPath: filepath.Join(dir, "dump.sql")}) + if err == nil { + t.Fatal("want error when psql fails, got nil") + } + if !strings.Contains(err.Error(), "already exists") { + t.Errorf("error %q should carry psql's own output", err) + } +} + +func TestRunMySQLRestoreFeedsTheDumpOnStdin(t *testing.T) { + dir := t.TempDir() + log := argsFile(t, dir) + dump := filepath.Join(dir, "dump.sql") + if err := os.WriteFile(dump, []byte("INSERT INTO s VALUES (1);\n"), 0o640); err != nil { + t.Fatal(err) + } + withFakeExecutable(t, "mysql", fakeScriptLoggingArgs(log, "cat >> "+log)) + + err := RunMySQLRestore(context.Background(), backup.SQLOptions{ + User: "stalwart", Database: "mail", Password: "hunter2", OutPath: dump, + }) + if err != nil { + t.Fatal(err) + } + if got := readFile(t, log); !strings.Contains(got, "INSERT INTO s VALUES (1);") { + t.Errorf("mysql stdin = %q, want the dump's contents", got) + } +} + +func TestRunMySQLRestoreFailsOnMissingDump(t *testing.T) { + withFakeExecutable(t, "mysql", "#!/bin/sh\nexit 0\n") + err := RunMySQLRestore(context.Background(), backup.SQLOptions{User: "u", Database: "d", OutPath: filepath.Join(t.TempDir(), "absent.sql")}) + if err == nil { + t.Fatal("want error when the dump file doesn't exist, got nil") + } +} diff --git a/internal/rollback/rollback.go b/internal/rollback/rollback.go new file mode 100644 index 0000000..8e1dbdf --- /dev/null +++ b/internal/rollback/rollback.go @@ -0,0 +1,460 @@ +package rollback + +import ( + "context" + "fmt" + "net/http" + "strings" + "time" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" + "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" + "github.com/LINUXexpert-org/stalwart-migrator/internal/service" +) + +// Artifact names this phase reads out of the checkpoint. backup.Run writes +// the first three as it captures them; ArtifactServiceUnit is the contract +// a future cutover phase has to honour when it rewrites a systemd unit or +// Compose file, so this phase can put the original back. +const ( + ArtifactOldBinary = "old-binary" + ArtifactFSBackup = "fs-backup" + ArtifactSQLDump = "sql-dump" + ArtifactServiceUnit = "service-unit" +) + +// Options configures one rollback. Most paths default to what the run's own +// checkpoint recorded, so an operator rolling back days later doesn't have +// to remember - and can't mistype - where the backup went. +type Options struct { + // Deployment names the service to stop and start. Controller overrides + // it outright when a caller already has one (cutover will, and the + // tests do). + Deployment service.Options + Controller service.Controller + + StopTimeout time.Duration // how long to wait for the service to actually stop; default 60s + StartTimeout time.Duration // how long to wait for it to come back; default 60s + + // DataDir is where an embedded-backend store is restored to. Required + // for rocksdb/sqlite runs. + DataDir string + // BackupDir and ManifestPath default to the fs-snapshot step's recorded + // artifact and manifest. + BackupDir string + ManifestPath string + + // SQL configures an external-database restore. OutPath defaults to the + // recorded sql-dump artifact; the connection fields must be supplied, + // since the checkpoint deliberately never stores database credentials. + SQL backup.SQLOptions + + // BinaryPath is where the preserved old binary goes back to. + BinaryPath string + // ServiceUnitPath is where a preserved systemd unit or Compose file + // goes back to. Both this and an ArtifactServiceUnit record are needed + // for that step to do anything. + ServiceUnitPath string + + AdminURL string + AdminUser string + AdminPassword string + HTTPClient *http.Client + VerifyTimeout time.Duration +} + +// Plan is what a rollback would do, resolved from the run's checkpoint +// before anything is touched. Building it can fail; executing it is what +// takes mail delivery down, so every reason to refuse is found here first. +type Plan struct { + RunID string + SourceVersion string // the version this rollback restores the instance to + Method string // "filesystem", "postgresql", or "mysql" + + Target string // what the service controller acts on + + BackupDir string + ManifestPath string + DataDir string + SQLDumpPath string + SQLDatabase string + + PreservedBinary string // preserved old binary to reinstall, "" if none was preserved + BinaryPath string + + ServiceUnitSource string // preserved unit/compose file, "" if none + ServiceUnitDest string +} + +// String renders the plan as the confirmation an operator should read +// before agreeing to it - this phase overwrites a live data directory, so +// "what exactly is about to happen" has to be answerable without reading +// the source. +func (p Plan) String() string { + var b strings.Builder + fmt.Fprintf(&b, "rollback plan for run %s:\n", p.RunID) + fmt.Fprintf(&b, " 1. stop %s\n", p.Target) + switch p.Method { + case "filesystem": + fmt.Fprintf(&b, " 2. move %s aside to %s.failed-%s (nothing is deleted)\n", p.DataDir, p.DataDir, p.RunID) + fmt.Fprintf(&b, " 3. restore %s from the verified backup at %s\n", p.DataDir, p.BackupDir) + default: + fmt.Fprintf(&b, " 2. replay the %s critical-table dump at %s into database %q\n", p.Method, p.SQLDumpPath, p.SQLDatabase) + fmt.Fprintf(&b, " (this overwrites those tables in place - unlike the filesystem path, the current contents are NOT preserved)\n") + } + if p.PreservedBinary != "" { + fmt.Fprintf(&b, " 4. reinstall %s as %s (the current binary is moved aside, not deleted)\n", p.PreservedBinary, p.BinaryPath) + } else { + fmt.Fprintf(&b, " 4. leave the binary alone - this run never preserved one\n") + } + if p.ServiceUnitSource != "" { + fmt.Fprintf(&b, " 5. restore the service definition %s to %s\n", p.ServiceUnitSource, p.ServiceUnitDest) + } else { + fmt.Fprintf(&b, " 5. leave the service definition alone - this run never preserved one\n") + } + version := p.SourceVersion + if version == "" { + version = "the version this run started from" + } + fmt.Fprintf(&b, " 6. start %s and verify it came back on %s\n", p.Target, version) + return b.String() +} + +// BuildPlan resolves what a rollback of this run would do, refusing up +// front for anything it can't undo. Every refusal here happens before the +// service is stopped, which is the whole point of separating this from Run: +// discovering "there's no backup to restore" after taking mail delivery +// down would be the worst possible time to find out. +func BuildPlan(rs *checkpoint.RunState, opts Options) (Plan, error) { + p := Plan{ + RunID: rs.RunID, SourceVersion: rs.SourceVersion, + BinaryPath: opts.BinaryPath, ServiceUnitDest: opts.ServiceUnitPath, + SQLDatabase: opts.SQL.Database, + } + + if rs.RollbackWindowClosed { + return p, fmt.Errorf( + "rollback: run %s had its rollback window closed by `confirm` - the operator declared the migration good, "+ + "and the backups this would restore from may since have been pruned. Restore manually if you're sure", rs.RunID) + } + + controller := opts.Controller + if controller == nil { + var err error + controller, err = service.New(deploymentFor(rs, opts)) + if err != nil { + return p, err + } + } + p.Target = controller.Target() + + backends := strings.ToLower(rs.Topology.StoreBackend) + switch { + case strings.Contains(backends, "rocksdb") || strings.Contains(backends, "sqlite"): + p.Method = "filesystem" + art, found := rs.Artifacts[ArtifactFSBackup] + if !found && opts.BackupDir == "" { + return p, fmt.Errorf("rollback: run %s recorded no %s artifact - there is no filesystem backup to restore, so this run cannot be rolled back by this tool", rs.RunID, ArtifactFSBackup) + } + p.BackupDir = opts.BackupDir + if p.BackupDir == "" { + p.BackupDir = art.Path + } + p.ManifestPath = opts.ManifestPath + if p.ManifestPath == "" { + p.ManifestPath = rs.Outcome(checkpoint.PhaseBackup, "fs-snapshot").Extra + } + if p.ManifestPath == "" { + return p, fmt.Errorf("rollback: run %s recorded no backup manifest path - without it the backup can't be verified before it's restored; pass one explicitly if you have it", rs.RunID) + } + if opts.DataDir == "" { + return p, fmt.Errorf("rollback: a data directory to restore into is required for a %s backend", rs.Topology.StoreBackend) + } + p.DataDir = opts.DataDir + + case strings.Contains(backends, "postgresql"), strings.Contains(backends, "mysql"): + p.Method = "postgresql" + if strings.Contains(backends, "mysql") { + p.Method = "mysql" + } + art, found := rs.Artifacts[ArtifactSQLDump] + if !found && opts.SQL.OutPath == "" { + return p, fmt.Errorf("rollback: run %s recorded no %s artifact - there is no database dump to restore", rs.RunID, ArtifactSQLDump) + } + p.SQLDumpPath = opts.SQL.OutPath + if p.SQLDumpPath == "" { + p.SQLDumpPath = art.Path + } + if opts.SQL.Database == "" || opts.SQL.User == "" { + return p, fmt.Errorf("rollback: database name and user are required to restore a %s backend - the checkpoint deliberately doesn't store database credentials", p.Method) + } + + case strings.Contains(backends, "foundationdb"): + return p, fmt.Errorf( + "rollback: run %s uses a FoundationDB backend, whose backup step only *starts* an fdbbackup job - restoring it means "+ + "`fdbrestore` against a quiesced cluster, which this tool doesn't automate. Roll back manually and don't rely on this command", rs.RunID) + + default: + return p, fmt.Errorf( + "rollback: run %s recorded no recognized store backend (topology.store_backend=%q), so there's no way to know what to restore", + rs.RunID, rs.Topology.StoreBackend) + } + + if art, found := rs.Artifacts[ArtifactOldBinary]; found { + if opts.BinaryPath == "" { + return p, fmt.Errorf("rollback: run %s preserved the old binary at %s, but no path to reinstall it to was given", rs.RunID, art.Path) + } + p.PreservedBinary = art.Path + } + if art, found := rs.Artifacts[ArtifactServiceUnit]; found && opts.ServiceUnitPath != "" { + p.ServiceUnitSource = art.Path + } + return p, nil +} + +func deploymentFor(rs *checkpoint.RunState, opts Options) service.Options { + d := opts.Deployment + if d.Kind == "" { + d.Kind = service.Kind(rs.Topology.DeploymentKind) + } + return d +} + +// Run executes ARCHITECTURE.md §4.8: stop the service, put the verified +// pre-migration state back, restart the old binary, and confirm the +// restored instance actually works rather than assuming it does. Every step +// is checkpointed under PhaseRollback, so a rollback interrupted partway - +// which is exactly when a machine is most likely to be rebooted out from +// under it - resumes where it stopped instead of restarting a destructive +// sequence from the top. +// +// Nothing from the failed attempt is deleted: the half-migrated data +// directory and the new binary are moved aside under ".failed-" +// names, so a retry after the underlying issue is fixed still has both the +// evidence and the artifacts it needs. +func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState, opts Options) (Report, error) { + var report Report + + plan, err := BuildPlan(rs, opts) + if err != nil { + report.Results = append(report.Results, CheckResult{Name: "plan", Status: StatusFail, Detail: err.Error()}) + return report, err + } + + controller := opts.Controller + if controller == nil { + controller, err = service.New(deploymentFor(rs, opts)) + if err != nil { + report.Results = append(report.Results, CheckResult{Name: "plan", Status: StatusFail, Detail: err.Error()}) + return report, err + } + } + + step := func(name string, fn func() (checkpoint.StepOutcome, error)) error { + outcome, err := store.RunStep(rs, checkpoint.PhaseRollback, name, fn) + if err != nil { + report.Results = append(report.Results, CheckResult{Name: name, Status: StatusFail, Detail: err.Error()}) + return err + } + status := Status(outcome.Verdict) + if status == "" { + status = StatusOK + } + report.Results = append(report.Results, CheckResult{Name: name, Status: status, Detail: outcome.Detail}) + return nil + } + + var manifest *backup.Manifest + + // Verify the backup before stopping anything. Discovering that the + // backup is corrupt is survivable while the failed-but-running instance + // is still up; discovering it after the data directory has been moved + // aside is not. + if err := step("verify-backup", func() (checkpoint.StepOutcome, error) { + if plan.Method != "filesystem" { + art, found := rs.Artifacts[ArtifactSQLDump] + if !found { + return checkpoint.StepOutcome{Verdict: string(StatusSkipped), Detail: fmt.Sprintf("dump at %s was supplied by hand, with no recorded checksum to check it against", plan.SQLDumpPath)}, nil + } + sum, size, err := hashFile(plan.SQLDumpPath) + if err != nil { + return checkpoint.StepOutcome{}, err + } + if sum != art.SHA256 { + return checkpoint.StepOutcome{}, fmt.Errorf( + "the dump at %s has sha256 %s but the checkpoint recorded %s when it was taken - refusing to restore a dump that changed since the backup", + plan.SQLDumpPath, sum, art.SHA256) + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("dump at %s (%d bytes) still matches the checksum recorded at backup time", plan.SQLDumpPath, size)}, nil + } + m, err := backup.ReadManifest(plan.ManifestPath) + if err != nil { + return checkpoint.StepOutcome{}, err + } + if err := backup.VerifyDataDirBackup(plan.BackupDir, m); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{ + Detail: fmt.Sprintf("re-hashed %d file(s) in %s, all still match the manifest recorded at backup time", len(m.Files), plan.BackupDir), + Extra: plan.ManifestPath, + }, nil + }); err != nil { + return report, err + } + + // A resumed run skips the step above, so the manifest is loaded here + // rather than inside it - the restore below needs it either way. + if plan.Method == "filesystem" { + manifest, err = backup.ReadManifest(plan.ManifestPath) + if err != nil { + report.Results = append(report.Results, CheckResult{Name: "restore-data", Status: StatusFail, Detail: err.Error()}) + return report, err + } + } + + stopTimeout := opts.StopTimeout + if stopTimeout <= 0 { + stopTimeout = 60 * time.Second + } + if err := step("stop-service", func() (checkpoint.StepOutcome, error) { + if err := controller.Stop(ctx); err != nil { + return checkpoint.StepOutcome{}, err + } + if err := service.WaitFor(ctx, controller, false, stopTimeout); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("%s is stopped", controller.Target())}, nil + }); err != nil { + return report, err + } + + if err := step("preserve-failed-state", func() (checkpoint.StepOutcome, error) { + if plan.Method != "filesystem" { + return checkpoint.StepOutcome{ + Verdict: string(StatusSkipped), + Detail: "an external SQL store's current contents can't be moved aside the way a data directory can - the restore below " + + "overwrites those tables in place. Take your own dump first if the failed attempt's state matters", + }, nil + } + preserved, moved, err := PreserveFailedState(plan.DataDir, rs.RunID) + if err != nil { + return checkpoint.StepOutcome{}, err + } + if !moved { + return checkpoint.StepOutcome{Detail: fmt.Sprintf("nothing to move aside (%s was already preserved, or %s doesn't exist)", preserved, plan.DataDir), Extra: preserved}, nil + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("moved the failed attempt's data directory to %s - it is not deleted", preserved), Extra: preserved}, nil + }); err != nil { + return report, err + } + + if err := step("restore-data", func() (checkpoint.StepOutcome, error) { + if plan.Method != "filesystem" { + sqlOpts := opts.SQL + sqlOpts.OutPath = plan.SQLDumpPath + restore := RunPsqlRestore + if plan.Method == "mysql" { + restore = RunMySQLRestore + } + if err := restore(ctx, sqlOpts); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("replayed %s into database %s", plan.SQLDumpPath, sqlOpts.Database)}, nil + } + if err := RestoreDataDir(plan.BackupDir, plan.DataDir, manifest); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{ + Detail: fmt.Sprintf("restored %d file(s) to %s and re-verified every one against the backup manifest", len(manifest.Files), plan.DataDir), + }, nil + }); err != nil { + return report, err + } + + if err := step("restore-binary", func() (checkpoint.StepOutcome, error) { + if plan.PreservedBinary == "" { + return checkpoint.StepOutcome{ + Verdict: string(StatusSkipped), + Detail: "this run never preserved an old binary (a dry run, or one that failed before the backup phase) - nothing to reinstall", + }, nil + } + art := rs.Artifacts[ArtifactOldBinary] + displaced, err := RestoreBinary(plan.PreservedBinary, plan.BinaryPath, art.SHA256, rs.RunID) + if err != nil { + return checkpoint.StepOutcome{}, err + } + detail := fmt.Sprintf("reinstalled the %s binary at %s", rs.SourceVersion, plan.BinaryPath) + if displaced != "" { + detail += fmt.Sprintf("; the binary that was there is preserved at %s for a retry", displaced) + } + return checkpoint.StepOutcome{Detail: detail, Extra: displaced}, nil + }); err != nil { + return report, err + } + + if err := step("restore-service-config", func() (checkpoint.StepOutcome, error) { + if plan.ServiceUnitSource == "" { + return checkpoint.StepOutcome{ + Verdict: string(StatusSkipped), + Detail: fmt.Sprintf("no %q artifact recorded for this run - nothing rewrote the service definition, so there's nothing to put back "+ + "(cutover, once it exists, is what will record one)", ArtifactServiceUnit), + }, nil + } + if err := RestoreFile(plan.ServiceUnitSource, plan.ServiceUnitDest); err != nil { + return checkpoint.StepOutcome{}, err + } + if err := controller.ReloadConfig(ctx); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("restored %s from %s and reloaded the service definition", plan.ServiceUnitDest, plan.ServiceUnitSource)}, nil + }); err != nil { + return report, err + } + + startTimeout := opts.StartTimeout + if startTimeout <= 0 { + startTimeout = 60 * time.Second + } + if err := step("start-service", func() (checkpoint.StepOutcome, error) { + if err := controller.Start(ctx); err != nil { + return checkpoint.StepOutcome{}, err + } + if err := service.WaitFor(ctx, controller, true, startTimeout); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("%s is running again", controller.Target())}, nil + }); err != nil { + return report, err + } + + // The verification results are reported individually rather than + // collapsed into the single step outcome, so an operator sees which + // check failed, not just that one did. + var verifyResults []CheckResult + if err := step("verify-rollback", func() (checkpoint.StepOutcome, error) { + results, err := Verify(ctx, VerifyOptions{ + BinaryPath: plan.BinaryPath, ExpectVersion: rs.SourceVersion, + AdminURL: opts.AdminURL, AdminUser: opts.AdminUser, AdminPassword: opts.AdminPassword, + HTTPClient: opts.HTTPClient, Snapshot: rs.PreflightSnapshot, Timeout: opts.VerifyTimeout, + }) + verifyResults = results + if err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{Detail: summarize(results)}, nil + }); err != nil { + report.Results = append(report.Results, verifyResults...) + return report, err + } + report.Results = append(report.Results, verifyResults...) + + return report, nil +} + +func summarize(results []CheckResult) string { + parts := make([]string, 0, len(results)) + for _, r := range results { + parts = append(parts, fmt.Sprintf("%s=%s", r.Name, r.Status)) + } + return strings.Join(parts, " ") +} diff --git a/internal/rollback/rollback_test.go b/internal/rollback/rollback_test.go new file mode 100644 index 0000000..16243b9 --- /dev/null +++ b/internal/rollback/rollback_test.go @@ -0,0 +1,349 @@ +package rollback + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" + "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" + "github.com/LINUXexpert-org/stalwart-migrator/internal/service" +) + +// fsRun builds a checkpoint that looks like a real embedded-backend run +// that got as far as taking (and recording) a verified filesystem backup: +// the state a rollback is actually invoked against. +func fsRun(t *testing.T) (store *checkpoint.Store, rs *checkpoint.RunState, dataDir, backupDir string) { + t.Helper() + root := t.TempDir() + dataDir = writeTree(t, filepath.Join(root, "data"), map[string]string{ + "config": "original settings", "blobs/one": "original mail", + }) + backupDir = filepath.Join(root, "backup") + + store = checkpoint.NewStore(filepath.Join(root, "runs")) + rs, err := store.Create("0.15.5", "0.16.14") + if err != nil { + t.Fatal(err) + } + rs.Topology = checkpoint.Topology{DeploymentKind: "systemd", StoreBackend: "rocksdb"} + + manifest, err := backup.CopyDataDir(dataDir, backupDir) + if err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(root, "backup.manifest.json") + if err := backup.WriteManifest(manifestPath, manifest); err != nil { + t.Fatal(err) + } + sum, err := manifest.Checksum() + if err != nil { + t.Fatal(err) + } + rs.RecordArtifact(ArtifactFSBackup, checkpoint.Artifact{Path: backupDir, SHA256: sum, SizeBytes: manifest.TotalBytes}) + if _, err := store.RunStep(rs, checkpoint.PhaseBackup, "fs-snapshot", func() (checkpoint.StepOutcome, error) { + return checkpoint.StepOutcome{Detail: "copied", Extra: manifestPath}, nil + }); err != nil { + t.Fatal(err) + } + + // Simulate the half-migrated state a failed cutover leaves behind. + if err := os.WriteFile(filepath.Join(dataDir, "config"), []byte("half-migrated settings"), 0o640); err != nil { + t.Fatal(err) + } + return store, rs, dataDir, backupDir +} + +func TestBuildPlanRefusesOnceTheRollbackWindowIsClosed(t *testing.T) { + _, rs, dataDir, _ := fsRun(t) + rs.RollbackWindowClosed = true + + _, err := BuildPlan(rs, Options{DataDir: dataDir, Controller: &fakeController{}}) + if err == nil { + t.Fatal("BuildPlan: want refusal once the operator has confirmed the migration, got nil") + } + if !strings.Contains(err.Error(), "rollback window closed") { + t.Errorf("error %q should say why it refuses", err) + } +} + +func TestBuildPlanRefusesWhatItCannotRestore(t *testing.T) { + for _, tc := range []struct { + name string + backend string + wantIn string + }{ + {"foundationdb", "foundationdb", "fdbrestore"}, + {"unrecognized", "", "no recognized store backend"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, rs, dataDir, _ := fsRun(t) + rs.Topology.StoreBackend = tc.backend + _, err := BuildPlan(rs, Options{DataDir: dataDir, Controller: &fakeController{}}) + if err == nil { + t.Fatalf("BuildPlan for backend %q: want refusal, got nil", tc.backend) + } + if !strings.Contains(err.Error(), tc.wantIn) { + t.Errorf("error %q should mention %q", err, tc.wantIn) + } + }) + } +} + +func TestBuildPlanRefusesWhenNoBackupWasEverRecorded(t *testing.T) { + _, rs, dataDir, _ := fsRun(t) + delete(rs.Artifacts, ArtifactFSBackup) + + _, err := BuildPlan(rs, Options{DataDir: dataDir, Controller: &fakeController{}}) + if err == nil { + t.Fatal("BuildPlan: want refusal when there's no backup to restore, got nil") + } + if !strings.Contains(err.Error(), "cannot be rolled back") { + t.Errorf("error %q should say the run can't be rolled back by this tool", err) + } +} + +func TestBuildPlanRefusesAnUnknownDeploymentKind(t *testing.T) { + _, rs, dataDir, _ := fsRun(t) + rs.Topology.DeploymentKind = string(service.Unknown) + + _, err := BuildPlan(rs, Options{DataDir: dataDir}) + if err == nil { + t.Fatal("BuildPlan: want refusal when it doesn't know how to stop Stalwart, got nil") + } +} + +func TestBuildPlanFillsPathsFromTheCheckpoint(t *testing.T) { + _, rs, dataDir, backupDir := fsRun(t) + rs.RecordArtifact(ArtifactOldBinary, checkpoint.Artifact{Path: "/usr/local/bin/stalwart.v0.15.5", SHA256: "abc"}) + + plan, err := BuildPlan(rs, Options{ + DataDir: dataDir, BinaryPath: "/usr/local/bin/stalwart", Controller: &fakeController{}, + }) + if err != nil { + t.Fatalf("BuildPlan: %v", err) + } + if plan.Method != "filesystem" { + t.Errorf("Method = %q, want filesystem", plan.Method) + } + if plan.BackupDir != backupDir { + t.Errorf("BackupDir = %q, want the recorded artifact %q", plan.BackupDir, backupDir) + } + if plan.ManifestPath == "" { + t.Error("ManifestPath should come from the fs-snapshot step's recorded outcome") + } + if plan.PreservedBinary != "/usr/local/bin/stalwart.v0.15.5" { + t.Errorf("PreservedBinary = %q, want the recorded artifact", plan.PreservedBinary) + } + if !strings.Contains(plan.String(), "nothing is deleted") { + t.Errorf("plan text should tell the operator nothing is deleted:\n%s", plan) + } +} + +func TestRunRestoresTheDataDirectoryWhileTheServiceIsDown(t *testing.T) { + store, rs, dataDir, _ := fsRun(t) + var configAtStop string + ctl := &fakeController{active: true} + ctl.onStop = func() { configAtStop = readFile(t, filepath.Join(dataDir, "config")) } + + report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: ctl}) + if err != nil { + t.Fatalf("Run: %v\n%s", err, report) + } + + if got, want := strings.Join(ctl.calls, ","), "stop,start"; got != want { + t.Errorf("controller calls = %q, want %q", got, want) + } + if configAtStop != "half-migrated settings" { + t.Errorf("data was already touched when the service stopped (config=%q) - the restore must happen after the stop", configAtStop) + } + if got := readFile(t, filepath.Join(dataDir, "config")); got != "original settings" { + t.Errorf("restored config = %q, want the pre-migration contents", got) + } + if got := readFile(t, filepath.Join(dataDir, "blobs/one")); got != "original mail" { + t.Errorf("restored mail = %q, want the pre-migration contents", got) + } + if got := readFile(t, filepath.Join(dataDir+".failed-"+rs.RunID, "config")); got != "half-migrated settings" { + t.Errorf("failed attempt's data = %q, want it preserved rather than deleted", got) + } + if report.Blocking() { + t.Errorf("report should be clean:\n%s", report) + } +} + +// Discovering a corrupt backup is survivable while the failed instance is +// still up, and unsurvivable once its data directory has been moved aside. +func TestRunVerifiesTheBackupBeforeStoppingAnything(t *testing.T) { + store, rs, dataDir, backupDir := fsRun(t) + if err := os.WriteFile(filepath.Join(backupDir, "blobs/one"), []byte("corrupt"), 0o640); err != nil { + t.Fatal(err) + } + ctl := &fakeController{active: true} + + report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: ctl}) + if err == nil { + t.Fatal("Run: want failure for a corrupt backup, got nil") + } + if len(ctl.calls) != 0 { + t.Errorf("controller was called %v - the service must not be stopped when the backup can't be trusted", ctl.calls) + } + if got := readFile(t, filepath.Join(dataDir, "config")); got != "half-migrated settings" { + t.Errorf("data directory was modified (%q) despite the refusal", got) + } + if !report.Blocking() { + t.Error("report should be blocking") + } +} + +func TestRunResumesWithoutRedoingCompletedSteps(t *testing.T) { + store, rs, dataDir, _ := fsRun(t) + first := &fakeController{active: true} + if _, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: first}); err != nil { + t.Fatal(err) + } + + // Re-invoking a completed rollback must be inert: stopping the service + // a second time, or re-restoring over a data directory that has been + // live since, would turn a no-op into an outage. + second := &fakeController{active: true} + reloaded, err := store.Load(rs.RunID) + if err != nil { + t.Fatal(err) + } + if _, err := Run(context.Background(), store, reloaded, Options{DataDir: dataDir, Controller: second}); err != nil { + t.Fatal(err) + } + if len(second.calls) != 0 { + t.Errorf("second invocation called the controller %v, want nothing - every step was already done", second.calls) + } +} + +func TestRunReinstallsThePreservedBinary(t *testing.T) { + store, rs, dataDir, _ := fsRun(t) + binDir := t.TempDir() + binaryPath := filepath.Join(binDir, "stalwart") + preserved := binaryPath + ".v0.15.5" + // Real scripts, not placeholder bytes: the verification step runs the + // restored binary's --version, so this also proves the instance really + // came back on the version the run started from. + if err := os.WriteFile(preserved, []byte("#!/bin/sh\necho 'stalwart 0.15.5'\n"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\necho 'stalwart 0.16.14'\n"), 0o755); err != nil { + t.Fatal(err) + } + sum, _, err := hashFile(preserved) + if err != nil { + t.Fatal(err) + } + rs.RecordArtifact(ArtifactOldBinary, checkpoint.Artifact{Path: preserved, SHA256: sum}) + + report, err := Run(context.Background(), store, rs, Options{ + DataDir: dataDir, BinaryPath: binaryPath, Controller: &fakeController{active: true}, + }) + if err != nil { + t.Fatalf("Run: %v\n%s", err, report) + } + if got := readFile(t, binaryPath); !strings.Contains(got, "0.15.5") { + t.Errorf("binary = %q, want the preserved old one reinstalled", got) + } + if got := readFile(t, binaryPath+".failed-"+rs.RunID); !strings.Contains(got, "0.16.14") { + t.Errorf("displaced binary = %q, want the new one kept for a retry", got) + } +} + +func TestRunRestoresAPreservedServiceUnit(t *testing.T) { + store, rs, dataDir, _ := fsRun(t) + unitDir := t.TempDir() + unitPath := filepath.Join(unitDir, "stalwart.service") + preservedUnit := filepath.Join(unitDir, "stalwart.service.preserved") + if err := os.WriteFile(preservedUnit, []byte("ExecStart=/usr/local/bin/stalwart\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(unitPath, []byte("ExecStart=/usr/local/bin/stalwart-0.16\n"), 0o644); err != nil { + t.Fatal(err) + } + rs.RecordArtifact(ArtifactServiceUnit, checkpoint.Artifact{Path: preservedUnit}) + + ctl := &fakeController{active: true} + report, err := Run(context.Background(), store, rs, Options{ + DataDir: dataDir, ServiceUnitPath: unitPath, Controller: ctl, + }) + if err != nil { + t.Fatalf("Run: %v\n%s", err, report) + } + if got := readFile(t, unitPath); !strings.Contains(got, "/usr/local/bin/stalwart\n") { + t.Errorf("unit = %q, want the preserved definition restored", got) + } + if got, want := strings.Join(ctl.calls, ","), "stop,reload,start"; got != want { + t.Errorf("controller calls = %q, want %q - a restored unit has to be reloaded before the start", got, want) + } +} + +// Nothing writes a service-unit artifact yet (cutover, which would rewrite +// the unit in the first place, doesn't exist). That has to read as an +// explicit skip, not a silent success. +func TestRunSkipsServiceConfigRestoreWithAnExplanation(t *testing.T) { + store, rs, dataDir, _ := fsRun(t) + report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: &fakeController{active: true}}) + if err != nil { + t.Fatal(err) + } + var found bool + for _, res := range report.Results { + if res.Name == "restore-service-config" { + found = true + if res.Status != StatusSkipped { + t.Errorf("restore-service-config status = %q, want %q", res.Status, StatusSkipped) + } + if !strings.Contains(res.Detail, "cutover") { + t.Errorf("skip detail %q should say what would record one", res.Detail) + } + } + } + if !found { + t.Errorf("no restore-service-config result in report:\n%s", report) + } +} + +func TestRunFailsWhenTheServiceWontStop(t *testing.T) { + store, rs, dataDir, _ := fsRun(t) + ctl := &fakeController{active: true, stopErr: os.ErrPermission} + + report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: ctl}) + if err == nil { + t.Fatal("Run: want failure when the service can't be stopped, got nil") + } + if got := readFile(t, filepath.Join(dataDir, "config")); got != "half-migrated settings" { + t.Errorf("data directory was touched (%q) even though the service never stopped", got) + } + if !report.Blocking() { + t.Error("report should be blocking") + } +} + +func TestRunSurfacesVerificationFailures(t *testing.T) { + store, rs, dataDir, _ := fsRun(t) + binDir := withFakeExecutable(t, "stalwart", "#!/bin/sh\necho 'stalwart 0.16.14'\n") + + report, err := Run(context.Background(), store, rs, Options{ + DataDir: dataDir, BinaryPath: filepath.Join(binDir, "stalwart"), Controller: &fakeController{active: true}, + }) + if err == nil { + t.Fatal("Run: want failure when the restored instance isn't on the original version, got nil") + } + var sawVersionFailure bool + for _, res := range report.Results { + if res.Name == "version" && res.Status == StatusFail { + sawVersionFailure = true + if !strings.Contains(res.Detail, "0.15.5") { + t.Errorf("version failure %q should name the version it expected", res.Detail) + } + } + } + if !sawVersionFailure { + t.Errorf("individual verification results should be in the report, not collapsed into one line:\n%s", report) + } +} diff --git a/internal/rollback/verify.go b/internal/rollback/verify.go new file mode 100644 index 0000000..c9c7992 --- /dev/null +++ b/internal/rollback/verify.go @@ -0,0 +1,170 @@ +package rollback + +import ( + "context" + "fmt" + "net/http" + "sort" + "strings" + "time" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" + "github.com/LINUXexpert-org/stalwart-migrator/internal/preflight" + "github.com/LINUXexpert-org/stalwart-migrator/internal/stalwartapi" +) + +// VerifyOptions configures the reduced validation suite that runs against +// the *restored* instance - ARCHITECTURE.md §4.8 step 5. It's deliberately +// smaller than §4.7's post-migration suite: the question here is "is the +// old instance actually back", not "did a migration preserve everything", +// and every check has to be one that would still pass on a healthy 0.15.5 +// install. +type VerifyOptions struct { + BinaryPath string // checked with --version; skipped if empty + ExpectVersion string // the run's recorded source version + + AdminURL string + AdminUser string + AdminPassword string + HTTPClient *http.Client + + // Snapshot is the pre-migration snapshot preflight captured. When set, + // the restored instance's account count and domains are compared + // against it - the "directory counts" half of §4.8 step 5. Per-mailbox + // message counts are deliberately not re-checked here: the restore is a + // byte-for-byte copy already verified against its manifest, so a + // per-mailbox walk would cost a lot of time on a large install to + // re-answer a question the manifest verification already answered. + Snapshot *checkpoint.PreflightSnapshot + + // Timeout bounds how long the reachability check waits for the restored + // service to answer, since it was started moments earlier. + Timeout time.Duration +} + +// Verify runs the reduced suite and returns one CheckResult per check. +// It always returns every result, even after a failure, so an operator sees +// the whole picture of a bad rollback in one pass rather than one problem +// at a time. The error is non-nil if any check failed. +func Verify(ctx context.Context, o VerifyOptions) ([]CheckResult, error) { + var results []CheckResult + fail := func(name, format string, args ...any) { + results = append(results, CheckResult{Name: name, Status: StatusFail, Detail: fmt.Sprintf(format, args...)}) + } + ok := func(name, format string, args ...any) { + results = append(results, CheckResult{Name: name, Status: StatusOK, Detail: fmt.Sprintf(format, args...)}) + } + skip := func(name, detail string) { + results = append(results, CheckResult{Name: name, Status: StatusSkipped, Detail: detail}) + } + + switch { + case o.BinaryPath == "" || o.ExpectVersion == "": + skip("version", "no binary path or recorded source version to check against") + default: + got, err := preflight.DetectVersion(ctx, o.BinaryPath) + switch { + case err != nil: + fail("version", "couldn't read the restored binary's version: %v", err) + case got != o.ExpectVersion: + fail("version", "restored binary reports %s, but this run started from %s - the rollback did not put the original binary back", got, o.ExpectVersion) + default: + ok("version", "restored binary reports %s, matching the version this run started from", got) + } + } + + if o.AdminURL == "" { + skip("reachable", "no --admin-url configured - can't confirm the restored instance answers") + skip("directory-counts", "no --admin-url configured - can't compare the restored directory against the pre-migration snapshot") + return results, resultsError(results) + } + + client := &stalwartapi.Client{ + BaseURL: o.AdminURL, Username: o.AdminUser, Password: o.AdminPassword, HTTPClient: o.HTTPClient, + } + timeout := o.Timeout + if timeout <= 0 { + timeout = 60 * time.Second + } + if err := waitForPing(ctx, client, timeout); err != nil { + fail("reachable", "restored instance never answered at %s within %s: %v", o.AdminURL, timeout, err) + skip("directory-counts", "skipped because the restored instance isn't reachable") + return results, resultsError(results) + } + ok("reachable", "restored instance answered a JMAP session request at %s", o.AdminURL) + + if o.Snapshot == nil { + skip("directory-counts", "this run has no pre-migration snapshot to compare against") + return results, resultsError(results) + } + + snap, err := client.AccountSnapshot(ctx) + if err != nil { + fail("directory-counts", "couldn't read the restored instance's directory: %v", err) + return results, resultsError(results) + } + if problems := compareDirectory(o.Snapshot, snap); len(problems) > 0 { + fail("directory-counts", "restored directory doesn't match the pre-migration snapshot: %s", strings.Join(problems, "; ")) + } else { + ok("directory-counts", "restored instance has %d account(s) and %d domain(s), matching the pre-migration snapshot", + snap.AccountCount, len(snap.Domains)) + } + return results, resultsError(results) +} + +// compareDirectory reports every way the restored directory differs from +// the pre-migration snapshot. Unlike the post-migration comparison in +// internal/validate, account names are compared exactly: the v0.16 +// migration's bare-username-to-email rewrite is precisely what a rollback +// undoes, so a restored instance that still shows rewritten names has not +// been restored. +func compareDirectory(before *checkpoint.PreflightSnapshot, after *stalwartapi.Snapshot) []string { + var problems []string + if before.AccountCount != after.AccountCount { + problems = append(problems, fmt.Sprintf("%d account(s) before, %d after", before.AccountCount, after.AccountCount)) + } + beforeDomains := append([]string(nil), before.Domains...) + afterDomains := append([]string(nil), after.Domains...) + sort.Strings(beforeDomains) + sort.Strings(afterDomains) + if strings.Join(beforeDomains, ",") != strings.Join(afterDomains, ",") { + problems = append(problems, fmt.Sprintf("domains were [%s], now [%s]", + strings.Join(beforeDomains, " "), strings.Join(afterDomains, " "))) + } + return problems +} + +// waitForPing polls until the instance accepts an authenticated session +// request or timeout elapses. The service was started seconds ago, so the +// first attempt failing is expected rather than meaningful. +func waitForPing(ctx context.Context, client *stalwartapi.Client, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + var lastErr error + for { + lastErr = client.Ping(ctx) + if lastErr == nil { + return nil + } + if !time.Now().Before(deadline) { + return lastErr + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(500 * time.Millisecond): + } + } +} + +func resultsError(results []CheckResult) error { + var failed []string + for _, r := range results { + if r.Status == StatusFail { + failed = append(failed, r.Name) + } + } + if len(failed) == 0 { + return nil + } + return fmt.Errorf("rollback verification failed: %s", strings.Join(failed, ", ")) +} diff --git a/internal/rollback/verify_test.go b/internal/rollback/verify_test.go new file mode 100644 index 0000000..2f2f29f --- /dev/null +++ b/internal/rollback/verify_test.go @@ -0,0 +1,166 @@ +package rollback + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" +) + +// restoredInstance is a fake Stalwart answering the two things the reduced +// post-rollback suite asks of it: a JMAP session document (reachability) +// and the x:Account/* management calls behind the directory comparison. +// Per-account mailbox impersonation is refused, which is deliberate - the +// reduced suite must not depend on it, since a rollback's guarantee comes +// from the verified manifest, not from re-walking every mailbox. +func restoredInstance(t *testing.T, accounts []map[string]any) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/.well-known/jmap" { + if user, _, _ := r.BasicAuth(); strings.Contains(user, "%") { + w.WriteHeader(http.StatusForbidden) // no impersonate grant + return + } + json.NewEncoder(w).Encode(map[string]any{"apiUrl": "/api"}) + return + } + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + calls := body["methodCalls"].([]any) + name := calls[0].([]any)[0].(string) + + switch name { + case "x:Account/query": + ids := make([]string, len(accounts)) + for i, a := range accounts { + ids[i] = a["id"].(string) + } + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Account/query", map[string]any{"ids": ids}, "q"}, + }}) + case "x:Account/get": + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Account/get", map[string]any{"list": accounts}, "g"}, + }}) + default: + t.Errorf("unexpected method call %s - the reduced suite should not be making it", name) + } + })) + t.Cleanup(srv.Close) + return srv +} + +func resultFor(t *testing.T, results []CheckResult, name string) CheckResult { + t.Helper() + for _, r := range results { + if r.Name == name { + return r + } + } + t.Fatalf("no %q result in %+v", name, results) + return CheckResult{} +} + +func TestVerifyPassesOnAProperlyRestoredInstance(t *testing.T) { + srv := restoredInstance(t, []map[string]any{ + {"id": "a1", "name": "alice@example.com", "domainId": "example.com"}, + {"id": "a2", "name": "bob@example.com", "domainId": "example.com"}, + }) + binDir := withFakeExecutable(t, "stalwart", "#!/bin/sh\necho 'stalwart 0.15.5'\n") + + results, err := Verify(context.Background(), VerifyOptions{ + BinaryPath: filepath.Join(binDir, "stalwart"), ExpectVersion: "0.15.5", + AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "hunter2", + Snapshot: &checkpoint.PreflightSnapshot{AccountCount: 2, Domains: []string{"example.com"}}, + }) + if err != nil { + t.Fatalf("Verify: %v (%+v)", err, results) + } + for _, name := range []string{"version", "reachable", "directory-counts"} { + if got := resultFor(t, results, name); got.Status != StatusOK { + t.Errorf("%s = %s: %s", name, got.Status, got.Detail) + } + } +} + +func TestVerifyDetectsTheWrongVersionStillInstalled(t *testing.T) { + binDir := withFakeExecutable(t, "stalwart", "#!/bin/sh\necho 'stalwart 0.16.14'\n") + + results, err := Verify(context.Background(), VerifyOptions{ + BinaryPath: filepath.Join(binDir, "stalwart"), ExpectVersion: "0.15.5", + }) + if err == nil { + t.Fatal("Verify: want error when the new binary is still installed, got nil") + } + res := resultFor(t, results, "version") + if res.Status != StatusFail { + t.Errorf("version = %s, want fail", res.Status) + } + if !strings.Contains(res.Detail, "did not put the original binary back") { + t.Errorf("detail %q should say plainly what went wrong", res.Detail) + } +} + +func TestVerifyDetectsADirectoryThatDoesNotMatchTheSnapshot(t *testing.T) { + srv := restoredInstance(t, []map[string]any{ + {"id": "a1", "name": "alice@example.com", "domainId": "example.com"}, + }) + + results, err := Verify(context.Background(), VerifyOptions{ + AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "hunter2", + Snapshot: &checkpoint.PreflightSnapshot{AccountCount: 2, Domains: []string{"example.com", "example.org"}}, + }) + if err == nil { + t.Fatal("Verify: want error when the restored directory is smaller than the snapshot, got nil") + } + res := resultFor(t, results, "directory-counts") + if res.Status != StatusFail { + t.Fatalf("directory-counts = %s, want fail", res.Status) + } + for _, want := range []string{"2 account(s) before, 1 after", "example.org"} { + if !strings.Contains(res.Detail, want) { + t.Errorf("detail %q should include %q", res.Detail, want) + } + } +} + +func TestVerifyReportsAnUnreachableInstance(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer srv.Close() + + results, err := Verify(context.Background(), VerifyOptions{ + AdminURL: srv.URL, AdminUser: "admin", Timeout: 300 * time.Millisecond, + Snapshot: &checkpoint.PreflightSnapshot{AccountCount: 1}, + }) + if err == nil { + t.Fatal("Verify: want error when the restored instance never answers, got nil") + } + if got := resultFor(t, results, "reachable"); got.Status != StatusFail { + t.Errorf("reachable = %s, want fail", got.Status) + } + // Reporting "directory matches" against an instance that never answered + // would be worse than reporting nothing. + if got := resultFor(t, results, "directory-counts"); got.Status != StatusSkipped { + t.Errorf("directory-counts = %s, want skip when the instance is unreachable", got.Status) + } +} + +func TestVerifySkipsRatherThanInventsWhatItCannotCheck(t *testing.T) { + results, err := Verify(context.Background(), VerifyOptions{}) + if err != nil { + t.Fatalf("Verify with nothing to check should not fail: %v", err) + } + for _, name := range []string{"version", "reachable", "directory-counts"} { + if got := resultFor(t, results, name); got.Status != StatusSkipped { + t.Errorf("%s = %s, want skip", name, got.Status) + } + } +} diff --git a/internal/service/doc.go b/internal/service/doc.go new file mode 100644 index 0000000..d43b79b --- /dev/null +++ b/internal/service/doc.go @@ -0,0 +1,3 @@ +// Package service starts and stops the Stalwart service itself, whether it's run as a systemd unit or a Docker container. +// See ARCHITECTURE.md §4.5 and §4.8 for the design. +package service diff --git a/internal/service/exectest_test.go b/internal/service/exectest_test.go new file mode 100644 index 0000000..8307585 --- /dev/null +++ b/internal/service/exectest_test.go @@ -0,0 +1,45 @@ +package service + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +// withFakeExecutable puts a fake executable named `name` at the front of +// PATH for the duration of the test, so code that shells out to a +// real-world tool (systemctl, docker) can be exercised without stopping +// anything real. t.Setenv restores PATH automatically and marks the test +// non-parallel. +func withFakeExecutable(t *testing.T, name, script string) (dir string) { + t.Helper() + dir = t.TempDir() + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + return dir +} + +func argsFile(t *testing.T, dir string) string { + t.Helper() + return filepath.Join(dir, "invoked-args.log") +} + +func readArgsFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return "" + } + if err != nil { + t.Fatal(err) + } + return string(data) +} + +func fakeScriptLoggingArgs(logPath string, extraBody string) string { + return fmt.Sprintf("#!/bin/sh\necho \"$@\" >> %q\n%s\n", logPath, extraBody) +} diff --git a/internal/service/service.go b/internal/service/service.go new file mode 100644 index 0000000..c344391 --- /dev/null +++ b/internal/service/service.go @@ -0,0 +1,192 @@ +package service + +import ( + "context" + "fmt" + "os/exec" + "strings" + "time" +) + +// Kind is how a Stalwart instance is run, and therefore how it has to be +// stopped and started. Preflight detects it (preflight.DetectDeploymentKind +// is an alias for this type's detector-side constants) and records it in +// the checkpoint's Topology, so a rollback days later controls the same +// thing the original run observed rather than re-guessing. +type Kind string + +const ( + Systemd Kind = "systemd" + Docker Kind = "docker" + Unknown Kind = "unknown" +) + +// Options names the thing to control. Only the field matching Kind is used. +type Options struct { + Kind Kind + UnitName string // systemd; defaults to "stalwart" + ContainerName string // docker; defaults to "stalwart" +} + +// Controller stops and starts one Stalwart deployment. It is deliberately +// the only thing in this tool that shells out to systemctl or docker, for +// the same reason stalwartapi is the only thing that speaks JMAP: the +// commands that can take mail delivery down belong in one auditable place, +// not scattered across the phases that happen to need them. +type Controller interface { + // Stop stops the service and returns once the command reports success. + // It does not wait for the process to actually be gone - use WaitFor + // for that, since both systemd and docker can report success while the + // unit is still shutting down. + Stop(ctx context.Context) error + // Start starts the service. + Start(ctx context.Context) error + // Active reports whether the service is currently running (or still + // transitioning into or out of running). An error means the state + // couldn't be determined at all - which is different from, and must + // never be silently collapsed into, "not running". + Active(ctx context.Context) (bool, error) + // ReloadConfig re-reads unit/service definitions after one has been + // rewritten on disk. It's a no-op for deployments that don't have such + // a step, so callers never need to branch on Kind. + ReloadConfig(ctx context.Context) error + // Target describes what this controller acts on, for operator-facing + // messages ("stopped systemd unit stalwart"). + Target() string +} + +// New returns a Controller for the given deployment. It refuses an Unknown +// (or unrecognized) kind rather than guessing: picking the wrong mechanism +// here means a rollback that reports "service stopped" while the old +// instance is still running and holding the data directory open, which is +// exactly the kind of quiet wrongness this tool exists to avoid. +func New(o Options) (Controller, error) { + switch o.Kind { + case Systemd: + unit := o.UnitName + if unit == "" { + unit = "stalwart" + } + return &systemdController{unit: unit}, nil + case Docker: + name := o.ContainerName + if name == "" { + name = "stalwart" + } + return &dockerController{container: name}, nil + case Unknown, "": + return nil, fmt.Errorf("service: deployment kind is unknown - this tool won't guess how to stop Stalwart; re-run preflight, or name the systemd unit or docker container explicitly") + default: + return nil, fmt.Errorf("service: unsupported deployment kind %q", o.Kind) + } +} + +// WaitFor polls c.Active until it reports want, or timeout elapses. Both +// systemctl and docker return as soon as the *request* to stop succeeded, +// so without this a caller would move on to overwriting the data directory +// while the old process still had it open. +func WaitFor(ctx context.Context, c Controller, want bool, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + var lastErr error + for { + active, err := c.Active(ctx) + if err != nil { + lastErr = err + } else if active == want { + return nil + } + if !time.Now().Before(deadline) { + break + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(250 * time.Millisecond): + } + } + state := "stopped" + if want { + state = "running" + } + if lastErr != nil { + return fmt.Errorf("service: %s was still not %s after %s, and its state couldn't be read: %w", c.Target(), state, timeout, lastErr) + } + return fmt.Errorf("service: %s was still not %s after %s", c.Target(), state, timeout) +} + +type systemdController struct{ unit string } + +func (s *systemdController) Target() string { return "systemd unit " + s.unit } + +func (s *systemdController) Stop(ctx context.Context) error { return s.run(ctx, "stop") } +func (s *systemdController) Start(ctx context.Context) error { return s.run(ctx, "start") } + +func (s *systemdController) ReloadConfig(ctx context.Context) error { + return runCommand(ctx, "systemctl", "daemon-reload") +} + +func (s *systemdController) run(ctx context.Context, verb string) error { + return runCommand(ctx, "systemctl", verb, s.unit) +} + +// Active maps `systemctl is-active` output rather than its exit status: +// the command exits non-zero for every not-active state, so treating a +// non-zero exit as a failure to read the state would make "inactive" - +// the answer we most want - look like an error. +func (s *systemdController) Active(ctx context.Context) (bool, error) { + out, err := exec.CommandContext(ctx, "systemctl", "is-active", s.unit).Output() + state := strings.TrimSpace(string(out)) + switch state { + case "active", "activating", "reloading", "deactivating": + // deactivating counts as active on purpose: it means the old + // process is still there, which is precisely what a caller waiting + // for a clean stop must not mistake for "gone". + return true, nil + case "inactive", "failed": + return false, nil + } + if err != nil { + return false, fmt.Errorf("service: systemctl is-active %s: %w (output: %q)", s.unit, err, state) + } + return false, fmt.Errorf("service: systemctl is-active %s returned unrecognized state %q", s.unit, state) +} + +type dockerController struct{ container string } + +func (d *dockerController) Target() string { return "docker container " + d.container } + +func (d *dockerController) Stop(ctx context.Context) error { + return runCommand(ctx, "docker", "stop", d.container) +} + +func (d *dockerController) Start(ctx context.Context) error { + return runCommand(ctx, "docker", "start", d.container) +} + +// ReloadConfig is a no-op: a container has no equivalent of daemon-reload - +// a changed Compose file takes effect when the container is recreated, and +// recreating containers is beyond what this controller does. +func (d *dockerController) ReloadConfig(context.Context) error { return nil } + +func (d *dockerController) Active(ctx context.Context) (bool, error) { + out, err := exec.CommandContext(ctx, "docker", "inspect", "-f", "{{.State.Running}}", d.container).Output() + state := strings.TrimSpace(string(out)) + switch state { + case "true": + return true, nil + case "false": + return false, nil + } + if err != nil { + return false, fmt.Errorf("service: docker inspect %s: %w (output: %q)", d.container, err, state) + } + return false, fmt.Errorf("service: docker inspect %s returned unrecognized state %q", d.container, state) +} + +func runCommand(ctx context.Context, name string, args ...string) error { + out, err := exec.CommandContext(ctx, name, args...).CombinedOutput() + if err != nil { + return fmt.Errorf("service: %s %s: %w (output: %s)", name, strings.Join(args, " "), err, strings.TrimSpace(string(out))) + } + return nil +} diff --git a/internal/service/service_test.go b/internal/service/service_test.go new file mode 100644 index 0000000..c59e675 --- /dev/null +++ b/internal/service/service_test.go @@ -0,0 +1,227 @@ +package service + +import ( + "context" + "fmt" + "strings" + "testing" + "time" +) + +func TestNewRefusesUnknownKind(t *testing.T) { + for _, kind := range []Kind{Unknown, "", "kubernetes"} { + if _, err := New(Options{Kind: kind}); err == nil { + t.Errorf("New(%q): want error, got nil - guessing how to stop Stalwart is exactly what this must not do", kind) + } + } +} + +func TestNewDefaultsTargetNames(t *testing.T) { + systemd, err := New(Options{Kind: Systemd}) + if err != nil { + t.Fatal(err) + } + if got, want := systemd.Target(), "systemd unit stalwart"; got != want { + t.Errorf("Target() = %q, want %q", got, want) + } + docker, err := New(Options{Kind: Docker}) + if err != nil { + t.Fatal(err) + } + if got, want := docker.Target(), "docker container stalwart"; got != want { + t.Errorf("Target() = %q, want %q", got, want) + } +} + +func TestSystemdStopStartReloadInvocations(t *testing.T) { + dir := t.TempDir() + log := argsFile(t, dir) + withFakeExecutable(t, "systemctl", fakeScriptLoggingArgs(log, "exit 0")) + + c, err := New(Options{Kind: Systemd, UnitName: "stalwart-mail"}) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + if err := c.Stop(ctx); err != nil { + t.Fatal(err) + } + if err := c.ReloadConfig(ctx); err != nil { + t.Fatal(err) + } + if err := c.Start(ctx); err != nil { + t.Fatal(err) + } + + want := "stop stalwart-mail\ndaemon-reload\nstart stalwart-mail\n" + if got := readArgsFile(t, log); got != want { + t.Errorf("systemctl invocations:\ngot: %q\nwant: %q", got, want) + } +} + +func TestSystemdStopReportsCommandFailure(t *testing.T) { + withFakeExecutable(t, "systemctl", "#!/bin/sh\necho 'Failed to stop stalwart.service: Access denied' >&2\nexit 1\n") + c, err := New(Options{Kind: Systemd}) + if err != nil { + t.Fatal(err) + } + err = c.Stop(context.Background()) + if err == nil { + t.Fatal("Stop: want error when systemctl fails, got nil") + } + if !strings.Contains(err.Error(), "Access denied") { + t.Errorf("Stop error %q does not carry systemctl's own output, which is the only clue an operator gets", err) + } +} + +// systemctl exits non-zero for every non-active state, so Active has to +// read its output rather than its exit status - otherwise "inactive", the +// answer a rollback most needs, would look like a failure to read state. +func TestSystemdActiveReadsOutputNotExitStatus(t *testing.T) { + for _, tc := range []struct { + state string + exitCode int + want bool + }{ + {"active", 0, true}, + {"activating", 3, true}, + {"reloading", 3, true}, + {"deactivating", 3, true}, // still holding the data directory open + {"inactive", 3, false}, + {"failed", 3, false}, + } { + t.Run(tc.state, func(t *testing.T) { + withFakeExecutable(t, "systemctl", fmt.Sprintf("#!/bin/sh\necho %s\nexit %d\n", tc.state, tc.exitCode)) + c, err := New(Options{Kind: Systemd}) + if err != nil { + t.Fatal(err) + } + active, err := c.Active(context.Background()) + if err != nil { + t.Fatalf("Active() for state %q: unexpected error %v", tc.state, err) + } + if active != tc.want { + t.Errorf("Active() for state %q = %v, want %v", tc.state, active, tc.want) + } + }) + } +} + +func TestSystemdActiveErrorsOnUnrecognizedState(t *testing.T) { + withFakeExecutable(t, "systemctl", "#!/bin/sh\necho 'command not found'\nexit 127\n") + c, err := New(Options{Kind: Systemd}) + if err != nil { + t.Fatal(err) + } + if _, err := c.Active(context.Background()); err == nil { + t.Error("Active: want error for unreadable state, got nil - an unreadable state must never collapse into 'not running'") + } +} + +func TestDockerStopStartInvocations(t *testing.T) { + dir := t.TempDir() + log := argsFile(t, dir) + withFakeExecutable(t, "docker", fakeScriptLoggingArgs(log, "exit 0")) + + c, err := New(Options{Kind: Docker, ContainerName: "mail"}) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + if err := c.Stop(ctx); err != nil { + t.Fatal(err) + } + if err := c.Start(ctx); err != nil { + t.Fatal(err) + } + if err := c.ReloadConfig(ctx); err != nil { + t.Fatalf("ReloadConfig should be a no-op for docker: %v", err) + } + + want := "stop mail\nstart mail\n" + if got := readArgsFile(t, log); got != want { + t.Errorf("docker invocations:\ngot: %q\nwant: %q", got, want) + } +} + +func TestDockerActive(t *testing.T) { + for _, tc := range []struct { + out string + want bool + }{{"true", true}, {"false", false}} { + t.Run(tc.out, func(t *testing.T) { + withFakeExecutable(t, "docker", fmt.Sprintf("#!/bin/sh\necho %s\n", tc.out)) + c, err := New(Options{Kind: Docker}) + if err != nil { + t.Fatal(err) + } + active, err := c.Active(context.Background()) + if err != nil { + t.Fatal(err) + } + if active != tc.want { + t.Errorf("Active() = %v, want %v", active, tc.want) + } + }) + } +} + +func TestDockerActiveErrorsWhenContainerMissing(t *testing.T) { + withFakeExecutable(t, "docker", "#!/bin/sh\necho 'Error: No such object: stalwart' >&2\nexit 1\n") + c, err := New(Options{Kind: Docker}) + if err != nil { + t.Fatal(err) + } + if _, err := c.Active(context.Background()); err == nil { + t.Error("Active: want error when the container doesn't exist, got nil") + } +} + +// WaitFor exists because systemctl and docker both return as soon as the +// *request* succeeded: this proves it keeps polling past a still-running +// state rather than accepting the first answer. +func TestWaitForPollsUntilStateChanges(t *testing.T) { + dir := t.TempDir() + counter := dir + "/calls" + withFakeExecutable(t, "systemctl", fmt.Sprintf( + "#!/bin/sh\nn=$(cat %[1]q 2>/dev/null || echo 0)\nn=$((n+1))\necho $n > %[1]q\n"+ + "if [ $n -lt 3 ]; then echo deactivating; exit 3; fi\necho inactive; exit 3\n", counter)) + + c, err := New(Options{Kind: Systemd}) + if err != nil { + t.Fatal(err) + } + if err := WaitFor(context.Background(), c, false, 5*time.Second); err != nil { + t.Fatalf("WaitFor: %v", err) + } +} + +func TestWaitForTimesOutWhileStillActive(t *testing.T) { + withFakeExecutable(t, "systemctl", "#!/bin/sh\necho active\n") + c, err := New(Options{Kind: Systemd}) + if err != nil { + t.Fatal(err) + } + err = WaitFor(context.Background(), c, false, 300*time.Millisecond) + if err == nil { + t.Fatal("WaitFor: want timeout error while the unit is still active, got nil") + } + if !strings.Contains(err.Error(), "not stopped") { + t.Errorf("WaitFor error %q should say what it was waiting for", err) + } +} + +func TestWaitForSurfacesLastStateReadError(t *testing.T) { + withFakeExecutable(t, "systemctl", "#!/bin/sh\necho 'no such unit' >&2\nexit 4\n") + c, err := New(Options{Kind: Systemd}) + if err != nil { + t.Fatal(err) + } + err = WaitFor(context.Background(), c, false, 300*time.Millisecond) + if err == nil { + t.Fatal("WaitFor: want error when the state can't be read at all, got nil") + } + if !strings.Contains(err.Error(), "state couldn't be read") { + t.Errorf("WaitFor error %q should distinguish 'never reached the state' from 'never could tell'", err) + } +}