amadan
hostbox: the quiet window gates reconcile's execution paths but not the hold-only write, and Install's check, decision and path act on one observer answer
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+93
−33
internal/hostbox/reconcile.go+43 −20internal/hostbox/reconcile_relocation_test.go+50 −13
diff --git a/internal/hostbox/reconcile.go b/internal/hostbox/reconcile.go| index 2b308aa01..766e2cb9a 100644 |
| --- a/internal/hostbox/reconcile.go |
| +++ b/internal/hostbox/reconcile.go |
| @@ -100,7 +100,23 @@ func (a activationReader) Read(ctx context.Context, host string) (hostclaim.Obse |
| } |
| return obs, nil |
| } |
| + |
| +// decided is Install's one row decision for a relocating claim and the |
| +// path it chose: holdOnly is a kept row's hold write, with no execution. |
| +type decided struct { |
| + row RowDecision |
| + holdOnly bool |
| +} |
| + |
| func (r Reconciler) check(ctx context.Context, obs hostclaim.Observed, selected RecordSelection) error { |
| + return r.checkOn(ctx, obs, selected, nil) |
| +} |
| + |
| +// checkOn is check on the row decision dec when the caller already made it |
| +// (Install), or on its own fresh one (dec nil). The hold-only write is not |
| +// execution, so the region's quiet window does not gate it; every other |
| +// path, the activation reader's and inventory's included, is gated. |
| +func (r Reconciler) checkOn(ctx context.Context, obs hostclaim.Observed, selected RecordSelection, dec *decided) error { |
| if r.Claims == nil || r.CheckRecord == nil || r.CheckLocal == nil || r.Now == nil || r.Box == "" || r.Region == "" || selected.ETag == "" || obs.Absent || obs.Claim == nil || obs.Claim.Holder == nil { |
| return ErrUnsupported |
| } |
| @@ -108,14 +124,20 @@ func (r Reconciler) check(ctx context.Context, obs hostclaim.Observed, selected |
| if obs.Host != selected.Host || h.Key != selected.Key || h.Incarnation != selected.Incarnation || h.Family != selected.Family || h.Stamp == 0 { |
| return ErrChanged |
| } |
| + holdOnly := false |
| if !r.relocates(obs.Claim) { |
| if err := Serving(obs.Claim, Selection{Box: r.Box}); err != nil { |
| return err |
| } |
| } else { |
| - d, _, err := r.rowDecision(ctx, obs) |
| - if err != nil { |
| - return err |
| + d := RowDecision{} |
| + if dec != nil { |
| + d, holdOnly = dec.row, dec.holdOnly |
| + } else { |
| + var err error |
| + if d, _, err = r.rowDecision(ctx, obs); err != nil { |
| + return err |
| + } |
| } |
| switch d.Case { |
| case RowReady, RowHeld: |
| @@ -123,7 +145,7 @@ func (r Reconciler) check(ctx context.Context, obs hostclaim.Observed, selected |
| return ErrChanged |
| } |
| } |
| - if until, ok := obs.Claim.QuietAfter[r.Region]; ok && !r.Now().After(until) { |
| + if until, ok := obs.Claim.QuietAfter[r.Region]; ok && !holdOnly && !r.Now().After(until) { |
| return ErrChanged |
| } |
| if err := r.CheckRecord(ctx, selected); err != nil { |
| @@ -157,27 +179,28 @@ func (r Reconciler) Install(ctx context.Context, selected RecordSelection, write |
| return err |
| } |
| } |
| - if err := r.check(ctx, obs, selected); err != nil { |
| - return err |
| - } |
| - if selected.DryRun { |
| - return nil |
| - } |
| + var dec *decided |
| if r.relocates(obs.Claim) { |
| - // One answer from the observer, for the decision and the path, |
| - // asked only when the decision depends on it. |
| + // One answer from the observer, for the check, the decision and |
| + // the path, asked only when the decision depends on it. |
| d, closed, err := r.rowDecision(ctx, obs) |
| if err != nil { |
| return err |
| } |
| - if d.Case == RowHeld && (obs.Claim.Op.Exec != r.Box || closed) { |
| - // No installation of ours to activate, or one whose admission |
| - // fence is closed (the source in releasing, still op.exec, |
| - // which With refuses): a kept row's hold only, under the |
| - // flock. A target before installing has no row and mints none |
| - // (ruling 17). |
| - return r.holdOnly(ctx, selected, d) |
| - } |
| + // No installation of ours to activate, or one whose admission |
| + // fence is closed (the source in releasing, still op.exec, which |
| + // With refuses): a kept row's hold only, under the flock. A |
| + // target before installing has no row and mints none (ruling 17). |
| + dec = &decided{row: d, holdOnly: d.Case == RowHeld && (obs.Claim.Op.Exec != r.Box || closed)} |
| + } |
| + if err := r.checkOn(ctx, obs, selected, dec); err != nil { |
| + return err |
| + } |
| + if selected.DryRun { |
| + return nil |
| + } |
| + if dec != nil && dec.holdOnly { |
| + return r.holdOnly(ctx, selected, dec.row) |
| } |
| // Activate's participant callback is not an admission validator for executors. |
| // The reader validates EVERY fresh read while Activate holds hostguard. |
diff --git a/internal/hostbox/reconcile_relocation_test.go b/internal/hostbox/reconcile_relocation_test.go| index 7c8e36d6c..048122928 100644 |
| --- a/internal/hostbox/reconcile_relocation_test.go |
| +++ b/internal/hostbox/reconcile_relocation_test.go |
| @@ -224,21 +224,22 @@ func TestReconcileWithAFenceObserverLeavesMovesToMain(t *testing.T) { |
| } |
| } |
| -// Install asks the fence observer once for its own decision: the hold-only |
| -// path acts on the answer the decision was made from. The source in |
| -// releasing is the one row case that depends on the observer, so it is |
| -// asked here three times (check, Install, holdOnly), as before the final |
| -// review's fix; a box whose case does not depend on it is never asked |
| -// (TestReconcileRelocationTargetWithNoLocalRecordMintsItsRow). |
| +// Install asks the fence observer once for its own decision, and its |
| +// check and its path both act on that answer: the hold-only path acts on |
| +// the answer the decision was made from. The source in releasing is the |
| +// one row case that depends on the observer. Before the final review's |
| +// fixes it was asked three times here (check, Install, holdOnly); now |
| +// Install's one answer serves its check too, so it is asked twice |
| +// (Install, holdOnly under the flock). A box whose case does not depend on |
| +// it is never asked (TestReconcileRelocationTargetWithNoLocalRecordMintsItsRow). |
| func TestReconcileInstallAsksTheFenceObserverOnce(t *testing.T) { |
| r, selected, holds, _ := relocationReconciler(t) |
| relocate(t, r, selected.Host, hostclaim.PhaseReleasing) |
| ctx := context.Background() |
| - // The first answer (check's) is open, Install's own is closed, and |
| - // every later one open: acting on one answer, Install takes the hold |
| - // path, whose decision under the flock then differs, so nothing is |
| - // written and nothing held. |
| - answers := []bool{false, true} |
| + // Install's answer is closed, and every later one open: acting on one |
| + // answer, Install takes the hold path, whose decision under the flock |
| + // then differs, so nothing is written and nothing held. |
| + answers := []bool{true} |
| calls := 0 |
| r.FenceClosed = func(context.Context, hostclaim.Observed) (bool, error) { |
| calls++ |
| @@ -251,8 +252,44 @@ func TestReconcileInstallAsksTheFenceObserverOnce(t *testing.T) { |
| if err := r.Install(ctx, selected, func() error { writes++; return nil }); !errors.Is(err, ErrChanged) { |
| t.Fatalf("a changed answer under the flock: %v", err) |
| } |
| - if calls != 3 || writes != 0 || len(*holds) != 0 { |
| - t.Fatalf("observer calls %d (check, Install, holdOnly) writes %d holds %v", calls, writes, *holds) |
| + if calls != 2 || writes != 0 || len(*holds) != 0 { |
| + t.Fatalf("observer calls %d (Install, holdOnly) writes %d holds %v", calls, writes, *holds) |
| + } |
| +} |
| + |
| +// Final review: the hold-only write is not execution, so the region's |
| +// quiet window does not gate it. The activation path still waits for it. |
| +func TestReconcileQuietWindowGatesExecutionNotTheHold(t *testing.T) { |
| + r, selected, holds, guard := relocationReconciler(t) |
| + obs := relocate(t, r, selected.Host, hostclaim.PhaseReleasing) |
| + ctx := context.Background() |
| + obs, err := hostclaim.Update(ctx, r.Claims, obs, func(c *hostclaim.Claim) error { |
| + c.QuietAfter = map[string]time.Time{"ie": r.Now().Add(time.Minute)} |
| + return nil |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + writes := 0 |
| + // The source before its fence closes takes the activation path: quiet. |
| + if err := r.Install(ctx, selected, func() error { writes++; return nil }); !errors.Is(err, ErrChanged) || writes != 0 || len(*holds) != 0 { |
| + t.Fatalf("activation in the quiet window: %v writes %d holds %v", err, writes, *holds) |
| + } |
| + captured, err := hostguard.Capture(guard.DataDir, selected.Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + target := hostguard.FenceTarget{FenceID: "fence-1", Host: selected.Host, Incarnation: captured.Incarnation, Stamp: captured.Stamp, Session: obs.Claim.Holder.Session, |
| + Op: "rel-1", Phase: hostclaim.PhaseReleasing, Box: "box", Enrollment: "enrollment-1", BootID: "boot", ControllerID: "ctl", ClaimETag: obs.ETag, DirectoryRevision: "4"} |
| + if _, err := guard.BeginFence(ctx, target); err != nil { |
| + t.Fatal(err) |
| + } |
| + // The fenced source's row is held at once, inside the quiet window. |
| + if err := r.Install(ctx, selected, func() error { writes++; return nil }); err != nil { |
| + t.Fatalf("the fenced source's hold in the quiet window: %v", err) |
| + } |
| + if writes != 0 || len(*holds) != 1 || (*holds)[0] != "rel-1" { |
| + t.Fatalf("writes %d holds %v", writes, *holds) |
| } |
| } |