From 39a0fdbd0023819121a4e425acc0bc6de018d651 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 18 Aug 2026 02:01:42 +0200 Subject: [PATCH] fix(auth): harden backup restore lifecycle cleanup --- tools/tht/internal/backup/create.go | 79 ++++- tools/tht/internal/backup/create_test.go | 204 ++++++++++++ tools/tht/internal/backup/restore.go | 130 ++++++-- tools/tht/internal/backup/restore_host.go | 22 +- tools/tht/internal/backup/restore_test.go | 379 +++++++++++++++++++++- 5 files changed, 764 insertions(+), 50 deletions(-) diff --git a/tools/tht/internal/backup/create.go b/tools/tht/internal/backup/create.go index 4015bdd0..400d2d0a 100644 --- a/tools/tht/internal/backup/create.go +++ b/tools/tht/internal/backup/create.go @@ -56,6 +56,10 @@ const ( helperImage = "busybox:1.36.1" drainPollInterval = time.Second maxDrainPolls = 300 + // Cleanup must survive a caller timeout or a lost Docker response, but it must not run + // indefinitely after the command has returned. Archive recovery can require volume work, so + // keep this deliberately longer than an individual health check. + cleanupOperationTimeout = 5 * time.Minute ) // CreateRequest controls one explicit backup request. @@ -124,9 +128,9 @@ func createWithDependencies(ctx context.Context, installation config.Installatio return Result{}, err } defer func() { - if releaseErr := lock.Release(); releaseErr != nil && resultErr == nil { + if releaseErr := lock.Release(); releaseErr != nil { result = Result{} - resultErr = releaseErr + resultErr = errors.Join(resultErr, fmt.Errorf("release backup lifecycle lock: %w", releaseErr)) } }() @@ -187,35 +191,57 @@ func createWithDependencies(ctx context.Context, installation config.Installatio if err != nil { return Result{}, err } - maintenanceActive := false - stopped := false + // Record attempted mutations before invoking Docker. Docker can apply a mutation and then + // lose its response, so a successful return is not evidence that compensation is unnecessary. + maintenanceAttempted := false + stopAttempted := false defer func() { - if stopped { - if startErr := composeStartAndVerify(ctx, installation, dependencies.runner); startErr != nil && resultErr == nil { - result = Result{} - resultErr = startErr + var cleanupErr error + restartCompleted := true + if wasRunning && stopAttempted { + startErr, started := retryBoundedCleanup(func(cleanupContext context.Context) error { + return composeStartAndVerify(cleanupContext, installation, dependencies.runner) + }) + if startErr != nil { + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("backup maintenance cleanup restart: %w", startErr)) + } + if started { + stopAttempted = false + } else { + restartCompleted = false } } - if maintenanceActive { - if deactivateErr := maintenance(ctx, installation, dependencies.runner, false); deactivateErr != nil && resultErr == nil { - result = Result{} - resultErr = deactivateErr + // Do not reopen admissions while the installation is known to be stopped. If a retry could + // not establish a running core, retain the durable barrier and report every cleanup error. + if maintenanceAttempted && restartCompleted { + deactivateErr, deactivated := retryBoundedCleanup(func(cleanupContext context.Context) error { + return maintenance(cleanupContext, installation, dependencies.runner, false) + }) + if deactivateErr != nil { + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("backup maintenance cleanup: %w", deactivateErr)) } + if deactivated { + maintenanceAttempted = false + } + } + if cleanupErr != nil { + result = Result{} + resultErr = errors.Join(resultErr, cleanupErr) } }() if wasRunning { + maintenanceAttempted = true if err := maintenance(ctx, installation, dependencies.runner, true); err != nil { return Result{}, err } - maintenanceActive = true if err := waitForNoActiveSessions(ctx, installation, dependencies.runner, request.Drain, dependencies.sleep); err != nil { return Result{}, err } + stopAttempted = true if err := runCompose(ctx, installation, dependencies.runner, "stop"); err != nil { return Result{}, err } - stopped = true } manifest := Manifest{ @@ -236,11 +262,11 @@ func createWithDependencies(ctx context.Context, installation config.Installatio if err := composeStartAndVerify(ctx, installation, dependencies.runner); err != nil { return Result{}, err } - stopped = false + stopAttempted = false if err := maintenance(ctx, installation, dependencies.runner, false); err != nil { return Result{}, err } - maintenanceActive = false + maintenanceAttempted = false } result = Result{Path: output} if manifest.IncludesSecrets { @@ -689,6 +715,27 @@ func composeStartAndVerify(ctx context.Context, installation config.Installation return service.WaitForHealthy(ctx, installation, runner) } +func boundedCleanupContext() (context.Context, context.CancelFunc) { + return context.WithTimeout(context.Background(), cleanupOperationTimeout) +} + +// retryBoundedCleanup retries an idempotent compensating mutation once. It preserves a lost +// response as part of the returned error while reporting whether the retry established the final +// state needed by the next cleanup action. +func retryBoundedCleanup(operation func(context.Context) error) (error, bool) { + run := func() error { + cleanupContext, cancel := boundedCleanupContext() + defer cancel() + return operation(cleanupContext) + } + firstErr := run() + if firstErr == nil { + return nil, true + } + retryErr := run() + return errors.Join(firstErr, retryErr), retryErr == nil +} + func runCompose(ctx context.Context, installation config.Installation, runner archiveRunner, command ...string) error { result, err := runner.Run(ctx, installation.ComposeArgs(command...), nil) if err != nil { diff --git a/tools/tht/internal/backup/create_test.go b/tools/tht/internal/backup/create_test.go index 1364c7fa..b4c21c39 100644 --- a/tools/tht/internal/backup/create_test.go +++ b/tools/tht/internal/backup/create_test.go @@ -557,6 +557,136 @@ func TestCreateRefusesMutableNonRunningServiceStates(t *testing.T) { } } +func TestCreateCleansMutationsWhenDockerLosesTheResponse(t *testing.T) { + activationResponseLost := errors.New("activation response lost") + stopResponseLost := errors.New("stop response lost") + for _, scenario := range []struct { + name string + failure *commandFailure + wantErr error + cancelCaller bool + wantStopCount int + wantStartCount int + wantCleanupCalls int + }{ + { + name: "maintenance activation", + wantErr: activationResponseLost, + failure: &commandFailure{ + match: func(command string) bool { return strings.Contains(command, " maintenance-activate") }, + err: activationResponseLost, + remaining: 1, + }, + wantCleanupCalls: 1, + }, + { + name: "stop response loss", + wantErr: stopResponseLost, + wantStopCount: 1, + wantStartCount: 1, + failure: &commandFailure{ + match: func(command string) bool { return strings.HasSuffix(command, " stop") }, + err: stopResponseLost, + effect: func() { + // A mutating command may have completed before its response was lost. + }, + remaining: 1, + }, + wantCleanupCalls: 2, + }, + { + name: "caller cancellation after stop", + wantErr: context.Canceled, + cancelCaller: true, + wantStopCount: 1, + wantStartCount: 1, + failure: &commandFailure{ + match: func(command string) bool { return strings.HasSuffix(command, " stop") }, + err: context.Canceled, + remaining: 1, + }, + wantCleanupCalls: 2, + }, + } { + scenario := scenario + t.Run(scenario.name, func(t *testing.T) { + fixture := newBackupFixture(t, "local") + backing := newBackupRunner(fixture.installation, true) + caller, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + failure := *scenario.failure + originalEffect := failure.effect + failure.effect = func() { + if strings.Contains(scenario.name, "activation") { + backing.maintenance = true + } else { + backing.stopCount++ + backing.running, backing.coreRunning = false, false + } + if originalEffect != nil { + originalEffect() + } + if scenario.cancelCaller { + cancel() + } + } + runner := &commandFailureRunner{fakeBackupRunner: backing, failures: []*commandFailure{&failure}} + + _, err := createWithDependencies(caller, fixture.installation, CreateRequest{Output: filepath.Join(t.TempDir(), "backup.zip")}, testDependencies(t, runner)) + if !errors.Is(err, scenario.wantErr) { + t.Fatalf("Create() error = %v, want %v", err, scenario.wantErr) + } + if backing.running != true || backing.coreRunning != true || backing.maintenance != false { + t.Fatalf("cleanup state = running:%t core:%t maintenance:%t, want running and admitted", backing.running, backing.coreRunning, backing.maintenance) + } + if backing.stopCount != scenario.wantStopCount || backing.startCount != scenario.wantStartCount { + t.Fatalf("lifecycle commands = stop:%d start:%d, want stop:%d start:%d", backing.stopCount, backing.startCount, scenario.wantStopCount, scenario.wantStartCount) + } + if len(runner.cleanupCommandContexts) != scenario.wantCleanupCalls { + t.Fatalf("cleanup command contexts = %d, want %d", len(runner.cleanupCommandContexts), scenario.wantCleanupCalls) + } + assertIndependentBoundedCleanupContexts(t, runner.cleanupCommandContexts) + }) + } +} + +func TestCreateJoinsPrimaryAndCleanupFailuresAfterPartialRestart(t *testing.T) { + fixture := newBackupFixture(t, "local") + backing := newBackupRunner(fixture.installation, true) + startResponseLost := errors.New("start response lost") + deactivationResponseLost := errors.New("deactivation response lost") + runner := &commandFailureRunner{ + fakeBackupRunner: backing, + failures: []*commandFailure{ + { + match: func(command string) bool { return strings.HasSuffix(command, " start") }, + err: startResponseLost, + effect: func() { + backing.startCount++ + backing.running, backing.coreRunning = true, true + }, + remaining: 1, + }, + { + match: func(command string) bool { return strings.Contains(command, " maintenance-deactivate") }, + err: deactivationResponseLost, + effect: func() { + backing.maintenance = false + }, + remaining: 1, + }, + }, + } + + _, err := createWithDependencies(context.Background(), fixture.installation, CreateRequest{Output: filepath.Join(t.TempDir(), "backup.zip")}, testDependencies(t, runner)) + if !errors.Is(err, startResponseLost) || !errors.Is(err, deactivationResponseLost) { + t.Fatalf("Create() error = %v, want joined start and deactivation failures", err) + } + if backing.startCount != 2 || !backing.running || backing.maintenance { + t.Fatalf("partial-success cleanup state = starts:%d running:%t maintenance:%t", backing.startCount, backing.running, backing.maintenance) + } +} + type backupFixture struct { root string installationID string @@ -656,6 +786,80 @@ type fakeBackupRunner struct { serviceStates map[string]string } +type commandFailure struct { + match func(string) bool + err error + effect func() + skip int + remaining int +} + +type commandFailureRunner struct { + *fakeBackupRunner + failures []*commandFailure + cleanupCommandContexts []cleanupContextObservation + streamFailure func(context.Context) error +} + +type cleanupContextObservation struct { + err error + deadline time.Time + hasDeadline bool +} + +func observeCleanupContext(ctx context.Context) cleanupContextObservation { + deadline, hasDeadline := ctx.Deadline() + return cleanupContextObservation{err: ctx.Err(), deadline: deadline, hasDeadline: hasDeadline} +} + +func (r *commandFailureRunner) Run(ctx context.Context, args []string, stdin io.Reader) (compose.Result, error) { + command := strings.Join(args, " ") + if strings.HasSuffix(command, " start") || strings.Contains(command, " maintenance-deactivate") { + r.cleanupCommandContexts = append(r.cleanupCommandContexts, observeCleanupContext(ctx)) + } + for _, failure := range r.failures { + if failure.remaining != 0 && failure.match(command) { + if failure.skip > 0 { + failure.skip-- + continue + } + if failure.remaining > 0 { + failure.remaining-- + } + if failure.effect != nil { + failure.effect() + } + return compose.Result{}, failure.err + } + } + return r.fakeBackupRunner.Run(ctx, args, stdin) +} + +func (r *commandFailureRunner) Stream(ctx context.Context, args []string, stdin io.Reader, stdout io.Writer) (compose.Result, error) { + if r.streamFailure != nil { + return compose.Result{}, r.streamFailure(ctx) + } + return r.fakeBackupRunner.Stream(ctx, args, stdin, stdout) +} + +func assertIndependentBoundedCleanupContexts(t *testing.T, contexts []cleanupContextObservation) { + t.Helper() + if len(contexts) == 0 { + t.Fatal("expected cleanup commands") + } + for _, cleanupContext := range contexts { + if cleanupContext.err != nil { + t.Fatalf("cleanup used a cancelled context: %v", cleanupContext.err) + } + if !cleanupContext.hasDeadline { + t.Fatal("cleanup context has no deadline") + } + if remaining := time.Until(cleanupContext.deadline); remaining <= 0 || remaining > 10*time.Minute { + t.Fatalf("unexpected cleanup deadline remaining: %s", remaining) + } + } +} + func newBackupRunner(installation config.Installation, running bool) *fakeBackupRunner { return &fakeBackupRunner{installation: installation, running: running, coreRunning: running} } diff --git a/tools/tht/internal/backup/restore.go b/tools/tht/internal/backup/restore.go index e8de5d86..8c9aa4d2 100644 --- a/tools/tht/internal/backup/restore.go +++ b/tools/tht/internal/backup/restore.go @@ -47,6 +47,26 @@ type restoreDependencies struct { verify map[string]restoreVerify } +// restoreTransactionState is the restore admission state machine. The durable maintenance +// barrier is released only after the target archive has been verified, or after a separately +// verified checkpoint recovery. Every mutable Docker command is tracked before it is invoked. +type restoreTransactionState struct { + wasRunning bool + maintenanceAttempted bool + stopAttempted bool + mutated bool + verified bool + recovered bool +} + +func (state restoreTransactionState) recoveryRequired(resultErr error) bool { + return resultErr != nil && state.mutated && !state.verified +} + +func (state restoreTransactionState) mayDeactivateMaintenance() bool { + return !state.mutated || state.verified || state.recovered +} + // Restore runs the host transaction through the same concrete Docker/filesystem boundaries used // by backup creation. The injectable core below exists only to make every failure boundary // deterministic in tests. @@ -94,8 +114,9 @@ func restoreWithDependencies(ctx context.Context, installation config.Installati return result, err } defer func() { - if releaseErr := lock.Release(); releaseErr != nil && resultErr == nil { - resultErr = releaseErr + if releaseErr := lock.Release(); releaseErr != nil { + result = RestoreResult{} + resultErr = errors.Join(resultErr, fmt.Errorf("release restore lifecycle lock: %w", releaseErr)) } }() @@ -103,35 +124,63 @@ func restoreWithDependencies(ctx context.Context, installation config.Installati if err != nil { return result, err } - mutated := false - maintenanceAttempted := false - stopAttempted := false + state := restoreTransactionState{wasRunning: wasRunning} defer func() { - if resultErr != nil && mutated { - if recoveryErr := deps.recover(context.Background(), installation, recovery, wasRunning); recoveryErr != nil { + if state.recoveryRequired(resultErr) { + recoveryContext, cancel := boundedCleanupContext() + recoveryErr := deps.recover(recoveryContext, installation, recovery, state.wasRunning) + cancel() + if recoveryErr != nil { resultErr = errors.Join(resultErr, fmt.Errorf("restore recovery checkpoint: %w", recoveryErr)) + if recoveryReachedVerifiedState(recoveryErr) { + state.recovered = true + state.stopAttempted = false + } } else { - stopAttempted = false + state.recovered = true + state.stopAttempted = false } } - if maintenanceAttempted { - cleanupContext := context.Background() - if wasRunning && stopAttempted { - if startErr := composeStartAndVerify(cleanupContext, installation, deps.runner); startErr != nil { - resultErr = errors.Join(resultErr, fmt.Errorf("restore maintenance cleanup restart: %w", startErr)) - } else { - stopAttempted = false - } + if !state.maintenanceAttempted { + return + } + var cleanupErr error + restartCompleted := true + if state.wasRunning && state.stopAttempted && state.mayDeactivateMaintenance() { + startErr, started := retryBoundedCleanup(func(cleanupContext context.Context) error { + return composeStartAndVerify(cleanupContext, installation, deps.runner) + }) + if startErr != nil { + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("restore maintenance cleanup restart: %w", startErr)) } - if deactivateErr := maintenance(cleanupContext, installation, deps.runner, false); deactivateErr != nil { - resultErr = errors.Join(resultErr, fmt.Errorf("restore maintenance cleanup: %w", deactivateErr)) + if started { + state.stopAttempted = false + } else { + restartCompleted = false } } + // A failed checkpoint recovery deliberately leaves admissions blocked. Starting or + // deactivating at that point would expose an unverified, possibly partial restore. + if state.mayDeactivateMaintenance() && restartCompleted { + deactivateErr, deactivated := retryBoundedCleanup(func(cleanupContext context.Context) error { + return maintenance(cleanupContext, installation, deps.runner, false) + }) + if deactivateErr != nil { + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("restore maintenance cleanup: %w", deactivateErr)) + } + if deactivated { + state.maintenanceAttempted = false + } + } + if cleanupErr != nil { + result = RestoreResult{} + resultErr = errors.Join(resultErr, cleanupErr) + } }() - if wasRunning { + if state.wasRunning { // The activation command may take effect even when its response is lost. Track the attempt, - // not merely a successful return, so every subsequent path removes the admissions barrier. - maintenanceAttempted = true + // not merely a successful return, so every subsequent path compensates from durable state. + state.maintenanceAttempted = true if err := maintenance(ctx, installation, deps.runner, true); err != nil { return result, err } @@ -140,40 +189,61 @@ func restoreWithDependencies(ctx context.Context, installation config.Installati } // Compose may stop the core and then lose its response. Cleanup must therefore restart after // any stop attempt, including a command that returns an error. - stopAttempted = true + state.stopAttempted = true if err := runCompose(ctx, installation, deps.runner, "stop"); err != nil { return result, err } } - mutated = true + state.mutated = true if err := restoreVerifiedEntries(ctx, installation, preflight, archive, deps.restoreFile, deps.restoreVolume); err != nil { return result, err } if err := deps.resetAuthenticationState(ctx, installation, deps.runner); err != nil { return result, fmt.Errorf("reset authentication state: %w", err) } - if wasRunning { + if state.wasRunning { if err := composeStartAndVerify(ctx, installation, deps.runner); err != nil { return result, err } - stopAttempted = false + state.stopAttempted = false result.Restarted = true + } + if err := verifyRestoreTransaction(ctx, installation, deps); err != nil { + return result, err + } + state.verified = true + result.Verified = true + if state.wasRunning { if err := maintenance(ctx, installation, deps.runner, false); err != nil { return result, err } - maintenanceAttempted = false + state.maintenanceAttempted = false } + return result, nil +} + +func verifyRestoreTransaction(ctx context.Context, installation config.Installation, deps restoreDependencies) error { for _, name := range []string{"health", "doctor", "pi", "workspace"} { check := deps.verify[name] if check == nil { - return result, fmt.Errorf("restore verification %q is unavailable", name) + return fmt.Errorf("restore verification %q is unavailable", name) } if err := check(ctx, installation, deps.runner); err != nil { - return result, fmt.Errorf("restore verification %s: %w", name, err) + return fmt.Errorf("restore verification %s: %w", name, err) } } - result.Verified = true - return result, nil + return nil +} + +type verifiedRecoveryError struct{ err error } + +func (err *verifiedRecoveryError) Error() string { return err.err.Error() } + +func (err *verifiedRecoveryError) Unwrap() error { return err.err } + +func recoveryReachedVerifiedState(err error) bool { + var verifiedErr *verifiedRecoveryError + return errors.As(err, &verifiedErr) } func restoreVerifiedEntries( diff --git a/tools/tht/internal/backup/restore_host.go b/tools/tht/internal/backup/restore_host.go index e66e0dce..8519886d 100644 --- a/tools/tht/internal/backup/restore_host.go +++ b/tools/tht/internal/backup/restore_host.go @@ -94,6 +94,11 @@ func recoverRestoreTransaction(ctx context.Context, installation config.Installa var resultErr error if err := runCompose(ctx, installation, deps.runner, "stop"); err != nil { resultErr = errors.Join(resultErr, err) + // The first stop may already have taken effect before Docker lost its response. Retry the + // idempotent command so no recovery mutation starts while the candidate core is running. + if retryErr := runCompose(ctx, installation, deps.runner, "stop"); retryErr != nil { + return errors.Join(resultErr, retryErr) + } } archive, err := recovery.RevalidateArchive() if err != nil { @@ -110,10 +115,23 @@ func recoverRestoreTransaction(ctx context.Context, installation config.Installa } if wasRunning { if err := composeStartAndVerify(ctx, installation, deps.runner); err != nil { - return errors.Join(resultErr, err) + resultErr = errors.Join(resultErr, err) + // As with stop, a start can succeed while the client loses its response. A successful + // retry includes its own health check before recovery verification proceeds. + if retryErr := composeStartAndVerify(ctx, installation, deps.runner); retryErr != nil { + return errors.Join(resultErr, retryErr) + } } } - return resultErr + if err := verifyRestoreTransaction(ctx, installation, deps); err != nil { + return errors.Join(resultErr, err) + } + if resultErr != nil { + // A prior response was lost, but the checkpoint has been restored and fully verified. The + // caller may safely remove maintenance while still returning every observed error. + return &verifiedRecoveryError{err: resultErr} + } + return nil } func restoreCheckpointPath(installation config.Installation, now time.Time) (string, error) { diff --git a/tools/tht/internal/backup/restore_test.go b/tools/tht/internal/backup/restore_test.go index 60d8a2b0..7181c8d4 100644 --- a/tools/tht/internal/backup/restore_test.go +++ b/tools/tht/internal/backup/restore_test.go @@ -450,6 +450,378 @@ func TestRestoreVerificationFailureRecoversPreviouslyRunningTarget(t *testing.T) } } +func TestRestoreKeepsAdmissionBarrierActiveUntilVerificationCommits(t *testing.T) { + installation := preflightTestInstallation(t) + archive := restoreArchive(t) + runner := newBackupRunner(installation, true) + deps := restoreTestDependencies(t, runner) + verificationEntered := make(chan struct{}) + allowVerification := make(chan struct{}) + deps.verify["health"] = func(context.Context, config.Installation, archiveRunner) error { + close(verificationEntered) + <-allowVerification + if !runner.maintenance { + return errors.New("maintenance barrier was removed before verification committed") + } + return nil + } + + type restoreOutcome struct { + result RestoreResult + err error + } + done := make(chan restoreOutcome, 1) + go func() { + result, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: archive, Confirm: true}, deps) + done <- restoreOutcome{result: result, err: err} + }() + select { + case <-verificationEntered: + case <-time.After(2 * time.Second): + t.Fatal("restore did not reach post-start verification") + } + + // A newly admitted operation observes the same durable barrier as the backend gate. No + // operation may enter after restore mutation and before the verification transaction commits. + admissionAttempt := make(chan bool, 1) + go func() { + admissionAttempt <- !runner.maintenance + }() + admissionAllowed := <-admissionAttempt + close(allowVerification) + outcome := <-done + if admissionAllowed { + t.Fatal("a new operation could enter while restore verification was still in progress") + } + if outcome.err != nil || !outcome.result.Verified { + t.Fatalf("restore result = %#v, %v; want successful verified restore", outcome.result, outcome.err) + } + if runner.maintenance { + t.Fatal("maintenance barrier remained active after successful verification") + } +} + +func TestRestoreRecoversBehindBarrierForEveryVerificationFailure(t *testing.T) { + for _, verification := range []string{"health", "doctor", "pi", "workspace"} { + verification := verification + t.Run(verification, func(t *testing.T) { + installation := preflightTestInstallation(t) + runner := newBackupRunner(installation, true) + deps := restoreTestDependencies(t, runner) + verificationErr := fmt.Errorf("%s verification failed", verification) + deps.verify[verification] = func(context.Context, config.Installation, archiveRunner) error { + return verificationErr + } + var recoveryBarrierActive bool + var recoveryContext cleanupContextObservation + deps.recover = func(ctx context.Context, _ config.Installation, _ PreflightResult, _ bool) error { + recoveryContext = observeCleanupContext(ctx) + recoveryBarrierActive = runner.maintenance + runner.running, runner.coreRunning = true, true + return nil + } + + _, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps) + if !errors.Is(err, verificationErr) { + t.Fatalf("Restore() error = %v, want %v", err, verificationErr) + } + if !recoveryBarrierActive { + t.Fatal("recovery started after the admission barrier had been removed") + } + assertIndependentBoundedCleanupContexts(t, []cleanupContextObservation{recoveryContext}) + if runner.maintenance || !runner.running { + t.Fatalf("recovery cleanup state = maintenance:%t running:%t", runner.maintenance, runner.running) + } + }) + } +} + +func TestRestoreDoesNotRollbackAfterFinalDeactivationResponseLoss(t *testing.T) { + installation := preflightTestInstallation(t) + backing := newBackupRunner(installation, true) + deactivationResponseLost := errors.New("deactivation response lost") + runner := &commandFailureRunner{ + fakeBackupRunner: backing, + failures: []*commandFailure{{ + match: func(command string) bool { return strings.Contains(command, " maintenance-deactivate") }, + err: deactivationResponseLost, + effect: func() { + // The barrier was removed, but Docker lost the response to the operator command. + backing.maintenance = false + }, + remaining: 1, + }}, + } + deps := restoreTestDependencies(t, runner) + recoveryCalls := 0 + deps.recover = func(context.Context, config.Installation, PreflightResult, bool) error { + recoveryCalls++ + return nil + } + + _, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps) + if !errors.Is(err, deactivationResponseLost) { + t.Fatalf("Restore() error = %v, want %v", err, deactivationResponseLost) + } + if recoveryCalls != 0 { + t.Fatalf("successful verification triggered an unnecessary rollback: %d recoveries", recoveryCalls) + } + if backing.maintenance || !backing.running { + t.Fatalf("final deactivation cleanup state = maintenance:%t running:%t", backing.maintenance, backing.running) + } +} + +func TestRestoreUsesBoundedIndependentCleanupContextsAfterCanceledStopResponse(t *testing.T) { + installation := preflightTestInstallation(t) + backing := newBackupRunner(installation, true) + caller, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + runner := &commandFailureRunner{ + fakeBackupRunner: backing, + failures: []*commandFailure{{ + match: func(command string) bool { return strings.HasSuffix(command, " stop") }, + err: context.Canceled, + effect: func() { + backing.stopCount++ + backing.running, backing.coreRunning = false, false + cancel() + }, + remaining: 1, + }}, + } + + _, err := restoreWithDependencies(caller, installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, restoreTestDependencies(t, runner)) + if !errors.Is(err, context.Canceled) { + t.Fatalf("Restore() error = %v, want context cancellation", err) + } + if backing.maintenance || !backing.running || backing.startCount != 1 { + t.Fatalf("cancelled cleanup state = maintenance:%t running:%t starts:%d", backing.maintenance, backing.running, backing.startCount) + } + assertIndependentBoundedCleanupContexts(t, runner.cleanupCommandContexts) +} + +func TestRestoreUsesBoundedRecoveryContextAfterPostMutationCancellation(t *testing.T) { + installation := preflightTestInstallation(t) + runner := newBackupRunner(installation, true) + caller, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + deps := restoreTestDependencies(t, runner) + deps.restoreFile = func(context.Context, config.Installation, ArchiveEntryMetadata, io.Reader) error { + cancel() + return context.Canceled + } + var recoveryContext cleanupContextObservation + var recoveryBarrierActive bool + deps.recover = func(ctx context.Context, _ config.Installation, _ PreflightResult, _ bool) error { + recoveryContext = observeCleanupContext(ctx) + recoveryBarrierActive = runner.maintenance + runner.running, runner.coreRunning = true, true + return nil + } + + _, err := restoreWithDependencies(caller, installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps) + if !errors.Is(err, context.Canceled) { + t.Fatalf("Restore() error = %v, want cancelled post-mutation restore", err) + } + if !recoveryBarrierActive { + t.Fatal("post-mutation recovery started after the admission barrier was removed") + } + assertIndependentBoundedCleanupContexts(t, []cleanupContextObservation{recoveryContext}) + if runner.maintenance || !runner.running { + t.Fatalf("post-mutation cancellation cleanup state = maintenance:%t running:%t", runner.maintenance, runner.running) + } +} + +func TestRestoreJoinsPrimaryRestartAndDeactivationFailures(t *testing.T) { + installation := preflightTestInstallation(t) + backing := newBackupRunner(installation, true) + stopResponseLost := errors.New("stop response lost") + startResponseLost := errors.New("restart response lost") + deactivationResponseLost := errors.New("deactivation response lost") + runner := &commandFailureRunner{ + fakeBackupRunner: backing, + failures: []*commandFailure{ + { + match: func(command string) bool { return strings.HasSuffix(command, " stop") }, + err: stopResponseLost, + effect: func() { + backing.stopCount++ + backing.running, backing.coreRunning = false, false + }, + remaining: 1, + }, + { + match: func(command string) bool { return strings.HasSuffix(command, " start") }, + err: startResponseLost, + effect: func() { + backing.startCount++ + backing.running, backing.coreRunning = true, true + }, + remaining: 1, + }, + { + match: func(command string) bool { return strings.Contains(command, " maintenance-deactivate") }, + err: deactivationResponseLost, + effect: func() { + backing.maintenance = false + }, + remaining: 1, + }, + }, + } + + _, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, restoreTestDependencies(t, runner)) + if !errors.Is(err, stopResponseLost) || !errors.Is(err, startResponseLost) || !errors.Is(err, deactivationResponseLost) { + t.Fatalf("Restore() error = %v, want joined stop/restart/deactivation failures", err) + } + if backing.maintenance || !backing.running { + t.Fatalf("response-loss cleanup state = maintenance:%t running:%t", backing.maintenance, backing.running) + } +} + +func TestRecoverRestoreTransactionVerifiesRecoveredStateBeforeReturning(t *testing.T) { + installation := preflightTestInstallation(t) + recovery, err := Preflight(context.Background(), installation, PreflightRequest{Archive: restoreArchive(t), Confirm: true, AllowExternalSecrets: true}, permissivePreflightDependencies()) + if err != nil { + t.Fatal(err) + } + defer recovery.CloseArchive() + runner := newBackupRunner(installation, true) + deps := restoreTestDependencies(t, runner) + var checks []string + for _, name := range []string{"health", "doctor", "pi", "workspace"} { + name := name + deps.verify[name] = func(context.Context, config.Installation, archiveRunner) error { + checks = append(checks, name) + return nil + } + } + + if err := recoverRestoreTransaction(context.Background(), installation, recovery, true, deps); err != nil { + t.Fatal(err) + } + if got, want := checks, []string{"health", "doctor", "pi", "workspace"}; !equalStrings(got, want) { + t.Fatalf("recovery checks = %v, want %v", got, want) + } +} + +func TestRecoverRestoreTransactionFailsClosedForEveryVerification(t *testing.T) { + for _, verification := range []string{"health", "doctor", "pi", "workspace"} { + verification := verification + t.Run(verification, func(t *testing.T) { + installation := preflightTestInstallation(t) + recovery, err := Preflight(context.Background(), installation, PreflightRequest{Archive: restoreArchive(t), Confirm: true, AllowExternalSecrets: true}, permissivePreflightDependencies()) + if err != nil { + t.Fatal(err) + } + defer recovery.CloseArchive() + runner := newBackupRunner(installation, true) + runner.maintenance = true + deps := restoreTestDependencies(t, runner) + verificationErr := fmt.Errorf("recovery %s verification failed", verification) + deps.verify[verification] = func(context.Context, config.Installation, archiveRunner) error { + if !runner.maintenance { + t.Fatal("recovery verification ran after the maintenance barrier was removed") + } + return verificationErr + } + + err = recoverRestoreTransaction(context.Background(), installation, recovery, true, deps) + if !errors.Is(err, verificationErr) { + t.Fatalf("recoverRestoreTransaction() error = %v, want %v", err, verificationErr) + } + if !runner.maintenance { + t.Fatal("recovery removed the maintenance barrier after a failed verification") + } + }) + } +} + +func TestRestoreReleasesBarrierOnlyAfterVerifiedRecoveryFromLostResponse(t *testing.T) { + for _, scenario := range []struct { + name string + failure commandFailure + }{ + { + name: "recovery stop", + failure: commandFailure{ + match: func(command string) bool { return strings.HasSuffix(command, " stop") }, + err: errors.New("recovery stop response lost"), + effect: func() { + // The stop took effect before Docker lost the response. + }, + skip: 1, + remaining: 1, + }, + }, + { + name: "recovery start", + failure: commandFailure{ + match: func(command string) bool { return strings.HasSuffix(command, " start") }, + err: errors.New("recovery start response lost"), + effect: func() { + // The start took effect before Docker lost the response. + }, + remaining: 1, + }, + }, + } { + scenario := scenario + t.Run(scenario.name, func(t *testing.T) { + installation := preflightTestInstallation(t) + archive := restoreArchive(t) + recovery, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true, AllowExternalSecrets: true}, permissivePreflightDependencies()) + if err != nil { + t.Fatal(err) + } + backing := newBackupRunner(installation, true) + failure := scenario.failure + originalEffect := failure.effect + failure.effect = func() { + if strings.Contains(scenario.name, "stop") { + backing.stopCount++ + backing.running, backing.coreRunning = false, false + } else { + backing.startCount++ + backing.running, backing.coreRunning = true, true + } + if originalEffect != nil { + originalEffect() + } + } + runner := &commandFailureRunner{fakeBackupRunner: backing, failures: []*commandFailure{&failure}} + deps := restoreTestDependencies(t, runner) + deps.checkpoint = func(context.Context, config.Installation, CreateRequest) (Result, error) { + return Result{Path: "/tmp/recovery.zip"}, nil + } + deps.prepareRecovery = func(context.Context, config.Installation, string) (PreflightResult, error) { + return recovery, nil + } + restoreCalls := 0 + mutationErr := errors.New("target mutation failed") + deps.restoreFile = func(context.Context, config.Installation, ArchiveEntryMetadata, io.Reader) error { + restoreCalls++ + if restoreCalls == 1 { + return mutationErr + } + return nil + } + deps.recover = func(ctx context.Context, target config.Installation, checkpoint PreflightResult, wasRunning bool) error { + return recoverRestoreTransaction(ctx, target, checkpoint, wasRunning, deps) + } + + _, err = restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: archive, Confirm: true}, deps) + if !errors.Is(err, mutationErr) || !errors.Is(err, failure.err) { + t.Fatalf("Restore() error = %v, want joined mutation and recovery response-loss errors", err) + } + if backing.maintenance || !backing.running { + t.Fatalf("verified recovery state = maintenance:%t running:%t", backing.maintenance, backing.running) + } + assertIndependentBoundedCleanupContexts(t, runner.cleanupCommandContexts) + }) + } +} + func TestRestoreRefusesActiveSessionsWithoutDrain(t *testing.T) { installation := preflightTestInstallation(t) archive := restoreArchive(t) @@ -543,8 +915,11 @@ func TestRestoreCleansMaintenanceAfterMutationAndRollbackFailures(t *testing.T) if !errors.Is(err, mutationErr) || (test.recoveryErr != nil && !errors.Is(err, test.recoveryErr)) { t.Fatalf("restore error = %v, want mutation and rollback failures", err) } - if backing.maintenance || !backing.running { - t.Fatalf("failure cleanup state: maintenance=%t running=%t", backing.maintenance, backing.running) + if test.recoveryErr == nil && (backing.maintenance || !backing.running) { + t.Fatalf("successful recovery cleanup state: maintenance=%t running=%t", backing.maintenance, backing.running) + } + if test.recoveryErr != nil && !backing.maintenance { + t.Fatal("failed recovery removed the maintenance barrier before a verified rollback") } }) }