From 09eeea17e5d6355c37a9588cd868a2ee22bbe486 Mon Sep 17 00:00:00 2001 From: mptyl Date: Wed, 5 Aug 2026 00:10:13 +0200 Subject: [PATCH] fix: harden maintenance failure reconciliation --- tools/thothctl/internal/pi/update.go | 21 +++++++------- tools/thothctl/internal/pi/update_test.go | 35 +++++++++++++++++++---- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/tools/thothctl/internal/pi/update.go b/tools/thothctl/internal/pi/update.go index 50b3826d..19edffb8 100644 --- a/tools/thothctl/internal/pi/update.go +++ b/tools/thothctl/internal/pi/update.go @@ -469,23 +469,24 @@ func setMaintenance(ctx context.Context, runner Runner, enabled bool) error { if err == nil && valid && status.Active == enabled && status.Admissions == 0 && !status.RecoveryRequired { return nil } - // A core recreate or transport interruption may lose only the response. Resolve ambiguity by - // reading the durable gate state before deciding that operator recovery is required. + // Status identifies the safest immediate state after an ambiguous response. It cannot + // acknowledge durability for an operation whose command returned an error. observed, statusErr := MaintenanceStatus(ctx, runner) - if statusErr == nil && observed.Active == enabled && observed.Admissions == 0 { - // 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 - } if valid && status.RecoveryRequired { return recoveryRequired("maintenance durability was explicitly not acknowledged", err) } + if statusErr == nil && observed.RecoveryRequired { + return recoveryRequired("maintenance durability was explicitly not acknowledged", err) + } if err != nil { + if statusErr == nil && observed.Active == enabled && observed.Admissions == 0 { + return recoveryRequired("maintenance durability was not acknowledged after a failed command", err) + } return commandError("maintenance admission gate", result, err) } + if statusErr == nil && observed.Active == enabled && observed.Admissions == 0 { + return nil + } return errors.New("maintenance admission gate did not acknowledge a quiescent state") } diff --git a/tools/thothctl/internal/pi/update_test.go b/tools/thothctl/internal/pi/update_test.go index e21dc94a..111487c8 100644 --- a/tools/thothctl/internal/pi/update_test.go +++ b/tools/thothctl/internal/pi/update_test.go @@ -176,13 +176,26 @@ func TestDigestPinnedConfiguredImageIsNeverUsedAsARollbackTagTarget(t *testing.T } } -func TestMaintenanceLostResponsesAreResolvedByStatusAndEveryRecreateStartsGated(t *testing.T) { +func TestMaintenanceTransportLossAfterBackendRestartRequiresRecovery(t *testing.T) { for _, lost := range []string{"activate", "deactivate"} { t.Run(lost, func(t *testing.T) { fake := newFakeRunner() fake.lostMaintenanceResponse = lost - if _, err := Update(context.Background(), fake, Request{StatePath: filepath.Join(t.TempDir(), "state.json"), Version: "0.81.0", Source: BuildSource, Confirm: true}); err != nil { - t.Fatal(err) + fake.restartBackendOnLoss = true + + _, err := Update(context.Background(), fake, Request{ + StatePath: filepath.Join(t.TempDir(), "state.json"), + Version: "0.81.0", + Source: BuildSource, + Confirm: true, + }) + + var recovery *RecoveryRequiredError + if err == nil || !errors.As(err, &recovery) || !recovery.RecoveryRequired() { + t.Fatalf("Update() error = %v; want typed recovery-required result", err) + } + if fake.backendRestarts != 1 { + t.Fatalf("backend restarts = %d, want 1", fake.backendRestarts) } assertCalled(t, fake.calls, "/internal/maintenance/status") for index, active := range fake.maintenanceAtRecreate { @@ -254,6 +267,9 @@ func TestSuccessfulLifecycleCommandsPreserveTypedRecoveryErrorFromMaintenanceCle result, err = Rollback(context.Background(), fake, statePath, true) } + if !fake.maintenance { + t.Fatalf("%s did not retain the restored maintenance marker", operation) + } var recovery *RecoveryRequiredError if err == nil || !errors.As(err, &recovery) || !recovery.RecoveryRequired() { t.Fatalf("%s error = %v; want typed recovery-required result", operation, err) @@ -791,6 +807,8 @@ type fakeRunner struct { maintenance bool maintenanceAtRecreate []bool lostMaintenanceResponse string + restartBackendOnLoss bool + backendRestarts int dropMaintenanceAfterCandidate bool modelsWire string rollbackPrepared bool @@ -885,6 +903,9 @@ func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose } if f.lostMaintenanceResponse == "activate" { f.lostMaintenanceResponse = "" + if f.restartBackendOnLoss { + f.backendRestarts++ + } return compose.Result{ExitCode: 52}, errors.New("lost activation response") } return compose.Result{Stdout: `{"active":true,"admissions":0}`}, nil @@ -892,14 +913,18 @@ func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose if f.fail == "maintenance-clear" { return compose.Result{ExitCode: 53}, errors.New("maintenance clear failure") } - f.maintenance = false - f.maintenanceClearImages = append(f.maintenanceClearImages, f.currentImage) if f.failDeactivationDurability { f.deactivationFailed = true + f.maintenance = true return compose.Result{ExitCode: 22}, errors.New("maintenance deactivation durability was not acknowledged") } + f.maintenance = false + f.maintenanceClearImages = append(f.maintenanceClearImages, f.currentImage) if f.lostMaintenanceResponse == "deactivate" { f.lostMaintenanceResponse = "" + if f.restartBackendOnLoss { + f.backendRestarts++ + } return compose.Result{ExitCode: 52}, errors.New("lost deactivation response") } return compose.Result{Stdout: `{"active":false,"admissions":0}`}, nil