diff --git a/tools/thothctl/internal/pi/update.go b/tools/thothctl/internal/pi/update.go index 4ce191a7..50b3826d 100644 --- a/tools/thothctl/internal/pi/update.go +++ b/tools/thothctl/internal/pi/update.go @@ -114,11 +114,7 @@ func updateWithHooks(ctx context.Context, runner Runner, request Request, hooks } if clearErr := setMaintenance(context.Background(), runner, false); clearErr != nil { result = Result{Phase: PhaseFailed, StatePath: request.StatePath} - if retErr == nil { - retErr = errors.New("maintenance admission gate could not be cleared: recovery required") - } else { - retErr = fmt.Errorf("%w; maintenance admission gate could not be cleared: recovery required", retErr) - } + retErr = errors.Join(retErr, fmt.Errorf("maintenance admission gate could not be cleared: %w", clearErr)) } }() @@ -297,11 +293,7 @@ func rollbackWithHooks(ctx context.Context, runner Runner, statePath string, con } if clearErr := setMaintenance(context.Background(), runner, false); clearErr != nil { result = Result{Phase: PhaseFailed, StatePath: statePath} - if retErr == nil { - retErr = errors.New("maintenance admission gate could not be cleared: recovery required") - } else { - retErr = fmt.Errorf("%w; maintenance admission gate could not be cleared: recovery required", retErr) - } + retErr = errors.Join(retErr, fmt.Errorf("maintenance admission gate could not be cleared: %w", clearErr)) } }() if maintenanceErr == nil { @@ -481,7 +473,9 @@ func setMaintenance(ctx context.Context, runner Runner, enabled bool) error { // reading the durable gate state before deciding that operator recovery is required. observed, statusErr := MaintenanceStatus(ctx, runner) if statusErr == nil && observed.Active == enabled && observed.Admissions == 0 { - if observed.RecoveryRequired { + // curl exit 22 means the server explicitly rejected the POST. A matching marker after + // that rejection selects the safest state, but cannot prove the failed write was durable. + if observed.RecoveryRequired || result.ExitCode == 22 { return recoveryRequired("maintenance durability was explicitly not acknowledged", err) } return nil diff --git a/tools/thothctl/internal/pi/update_test.go b/tools/thothctl/internal/pi/update_test.go index 3f3391ce..e21dc94a 100644 --- a/tools/thothctl/internal/pi/update_test.go +++ b/tools/thothctl/internal/pi/update_test.go @@ -209,6 +209,62 @@ func TestMaintenanceReconciliationDoesNotMaskExplicitDurabilityFailure(t *testin } } +func TestMaintenanceReconciliationRequiresDurabilityProofAfterExplicitPOSTFailure(t *testing.T) { + fake := newFakeRunner() + fake.fail = "maintenance-activate-durability-without-status-flag" + + err := setMaintenance(context.Background(), fake, true) + + var recovery *RecoveryRequiredError + if err == nil || !errors.As(err, &recovery) || !recovery.RecoveryRequired() { + t.Fatalf("maintenance error = %v; want typed recovery-required result", err) + } + if !fake.maintenance { + t.Fatal("safe marker state was not retained after activation durability failure") + } +} + +func TestSuccessfulLifecycleCommandsPreserveTypedRecoveryErrorFromMaintenanceCleanup(t *testing.T) { + for _, operation := range []string{"update", "rollback"} { + t.Run(operation, func(t *testing.T) { + fake := newFakeRunner() + statePath := filepath.Join(t.TempDir(), "state.json") + if operation == "rollback" { + if _, err := Update(context.Background(), fake, Request{ + StatePath: statePath, + Version: "0.81.0", + Source: BuildSource, + Confirm: true, + }); err != nil { + t.Fatal(err) + } + } + fake.failDeactivationDurability = true + + var result Result + var err error + if operation == "update" { + result, err = Update(context.Background(), fake, Request{ + StatePath: statePath, + Version: "0.81.0", + Source: BuildSource, + Confirm: true, + }) + } else { + result, err = Rollback(context.Background(), fake, statePath, true) + } + + var recovery *RecoveryRequiredError + if err == nil || !errors.As(err, &recovery) || !recovery.RecoveryRequired() { + t.Fatalf("%s error = %v; want typed recovery-required result", operation, err) + } + if result.Phase != PhaseFailed { + t.Fatalf("%s phase = %q, want %q", operation, result.Phase, PhaseFailed) + } + }) + } +} + func TestCompensationReactivatesMaintenanceAndRescansBeforeRollback(t *testing.T) { fake := newFakeRunner() fake.fail = "version" @@ -742,6 +798,8 @@ type fakeRunner struct { execFailuresWhileStopped int maintenanceHelperImages []string maintenanceClearImages []string + failDeactivationDurability bool + deactivationFailed bool restoredProofComplete bool candidateVersion string candidateExpectedVersion string @@ -822,7 +880,7 @@ func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose return compose.Result{Stdout: `[{"Type":"volume","Name":"settings","Source":"settings","Destination":"/data/settings","RW":true},{"Type":"volume","Name":"pi-state","Source":"pi-state","Destination":"/home/thoth/.pi","RW":true},{"Type":"volume","Name":"sessions","Source":"sessions","Destination":"/data/sessions","RW":true},{"Type":"volume","Name":"workspace-registry","Source":"workspace-registry","Destination":"/data/workspace-registry","RW":true}]`}, nil case strings.Contains(call, "/internal/maintenance/activate"): f.maintenance = true - if f.fail == "maintenance-activate-durability" { + if f.fail == "maintenance-activate-durability" || f.fail == "maintenance-activate-durability-without-status-flag" { return compose.Result{ExitCode: 22}, errors.New("maintenance activation durability was not acknowledged") } if f.lostMaintenanceResponse == "activate" { @@ -836,6 +894,10 @@ func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose } f.maintenance = false f.maintenanceClearImages = append(f.maintenanceClearImages, f.currentImage) + if f.failDeactivationDurability { + f.deactivationFailed = true + return compose.Result{ExitCode: 22}, errors.New("maintenance deactivation durability was not acknowledged") + } if f.lostMaintenanceResponse == "deactivate" { f.lostMaintenanceResponse = "" return compose.Result{ExitCode: 52}, errors.New("lost deactivation response") @@ -845,6 +907,9 @@ func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose if f.fail == "maintenance-activate-durability" { return compose.Result{Stdout: fmt.Sprintf(`{"active":%t,"admissions":0,"recoveryRequired":true}`, f.maintenance)}, nil } + if f.deactivationFailed { + return compose.Result{Stdout: fmt.Sprintf(`{"active":%t,"admissions":0,"recoveryRequired":true}`, f.maintenance)}, nil + } return compose.Result{Stdout: fmt.Sprintf(`{"active":%t,"admissions":0}`, f.maintenance)}, nil case strings.Contains(call, "/sessions?scope=all"): if f.sessionsWire != "" {