fix: preserve Pi recovery error semantics

This commit is contained in:
2026-08-04 23:59:01 +02:00
parent 935bb1db0e
commit 70dabcc192
2 changed files with 71 additions and 12 deletions
+5 -11
View File
@@ -114,11 +114,7 @@ func updateWithHooks(ctx context.Context, runner Runner, request Request, hooks
} }
if clearErr := setMaintenance(context.Background(), runner, false); clearErr != nil { if clearErr := setMaintenance(context.Background(), runner, false); clearErr != nil {
result = Result{Phase: PhaseFailed, StatePath: request.StatePath} result = Result{Phase: PhaseFailed, StatePath: request.StatePath}
if retErr == nil { retErr = errors.Join(retErr, fmt.Errorf("maintenance admission gate could not be cleared: %w", clearErr))
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)
}
} }
}() }()
@@ -297,11 +293,7 @@ func rollbackWithHooks(ctx context.Context, runner Runner, statePath string, con
} }
if clearErr := setMaintenance(context.Background(), runner, false); clearErr != nil { if clearErr := setMaintenance(context.Background(), runner, false); clearErr != nil {
result = Result{Phase: PhaseFailed, StatePath: statePath} result = Result{Phase: PhaseFailed, StatePath: statePath}
if retErr == nil { retErr = errors.Join(retErr, fmt.Errorf("maintenance admission gate could not be cleared: %w", clearErr))
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)
}
} }
}() }()
if maintenanceErr == nil { 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. // reading the durable gate state before deciding that operator recovery is required.
observed, statusErr := MaintenanceStatus(ctx, runner) observed, statusErr := MaintenanceStatus(ctx, runner)
if statusErr == nil && observed.Active == enabled && observed.Admissions == 0 { 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 recoveryRequired("maintenance durability was explicitly not acknowledged", err)
} }
return nil return nil
+66 -1
View File
@@ -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) { func TestCompensationReactivatesMaintenanceAndRescansBeforeRollback(t *testing.T) {
fake := newFakeRunner() fake := newFakeRunner()
fake.fail = "version" fake.fail = "version"
@@ -742,6 +798,8 @@ type fakeRunner struct {
execFailuresWhileStopped int execFailuresWhileStopped int
maintenanceHelperImages []string maintenanceHelperImages []string
maintenanceClearImages []string maintenanceClearImages []string
failDeactivationDurability bool
deactivationFailed bool
restoredProofComplete bool restoredProofComplete bool
candidateVersion string candidateVersion string
candidateExpectedVersion 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 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"): case strings.Contains(call, "/internal/maintenance/activate"):
f.maintenance = true 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") return compose.Result{ExitCode: 22}, errors.New("maintenance activation durability was not acknowledged")
} }
if f.lostMaintenanceResponse == "activate" { if f.lostMaintenanceResponse == "activate" {
@@ -836,6 +894,10 @@ func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose
} }
f.maintenance = false f.maintenance = false
f.maintenanceClearImages = append(f.maintenanceClearImages, f.currentImage) 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" { if f.lostMaintenanceResponse == "deactivate" {
f.lostMaintenanceResponse = "" f.lostMaintenanceResponse = ""
return compose.Result{ExitCode: 52}, errors.New("lost deactivation response") 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" { if f.fail == "maintenance-activate-durability" {
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,"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 return compose.Result{Stdout: fmt.Sprintf(`{"active":%t,"admissions":0}`, f.maintenance)}, nil
case strings.Contains(call, "/sessions?scope=all"): case strings.Contains(call, "/sessions?scope=all"):
if f.sessionsWire != "" { if f.sessionsWire != "" {