amadan
host: the prepared build re-checks the install's authority before every bucket write (Task 6 fix round 1, finding 1)
installAuthority re-reads the claim and holds it to this install (the op at phase local with this box executing, the holder at the captured stamp); installAuthorityStore runs it immediately before every write, retries included, as region HA's authorityStore does, and never deletes. A stale build whose authority moved writes nothing more and never wins the create-only manifest. Regression: authority moves to another executor between the first object upload and the next write; the stale writer leaves one object and no manifest, the step is not journalled and the record does not move; once authority returns the installer completes, reusing that object (one version per object). Also the local_ header's step (6), which still said the manifest was skipped until C8 existed (Minor).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+154
−3
internal/host/movesinstall.go+67 −3internal/host/movesland_test.go+87 −0
diff --git a/internal/host/movesinstall.go b/internal/host/movesinstall.go| index b835be64a..6a72ed6ce 100644 |
| --- a/internal/host/movesinstall.go |
| +++ b/internal/host/movesinstall.go |
| @@ -237,6 +237,62 @@ func newInstallRun(s release.Store, w *movesWorker, rec provision.Record, src in |
| func (ir *installRun) cfg() moveCfg { return ir.w.cfg } |
| +// installAuthority is the install's authority re-check for the prepared |
| +// manifest's bucket writes (fix round 1, finding 1; region HA's |
| +// authorityStore pattern): a fresh read of the claim, the install's own |
| +// claim check at phase (the op at that phase with this box executing) and |
| +// the holder still at the captured stamp. It runs immediately before every |
| +// write attempt, retries included, so a delayed build whose executor, |
| +// phase or stamp moved writes nothing more. |
| +func (ir *installRun) installAuthority(ctx context.Context, captured hostguard.Captured, phase string) func() error { |
| + check := ir.src.ClaimCheck(phase) |
| + return func() error { |
| + if err := ctx.Err(); err != nil { |
| + return err |
| + } |
| + obs, err := hostclaim.Read(ctx, ir.s, ir.rec.Host) |
| + if err != nil { |
| + return err |
| + } |
| + if obs.Absent || obs.Claim == nil { |
| + return fmt.Errorf("%w: %s has no claim", errLandingChanged, ir.rec.Host) |
| + } |
| + if err := check(obs.Claim); err != nil { |
| + return err |
| + } |
| + if obs.Claim.Holder == nil || obs.Claim.Holder.Stamp != captured.Stamp { |
| + return fmt.Errorf("%w: the claim's holder is not at the installation stamp %d", errLandingChanged, captured.Stamp) |
| + } |
| + return nil |
| + } |
| +} |
| + |
| +// installAuthorityStore is the versioned store the prepared build writes |
| +// through: every write re-checks the install's authority first. Reads are |
| +// the store's own; the build never deletes. |
| +type installAuthorityStore struct { |
| + lease.VersionedStore |
| + check func() error |
| +} |
| + |
| +func (s installAuthorityStore) PutIfAbsent(ctx context.Context, bucket, key string, body []byte) (string, error) { |
| + if err := s.check(); err != nil { |
| + return "", err |
| + } |
| + return s.VersionedStore.PutIfAbsent(ctx, bucket, key, body) |
| +} |
| + |
| +func (s installAuthorityStore) CopyVersion(ctx context.Context, sb, sk, sv, db, dk string, createOnly bool) (string, string, error) { |
| + if err := s.check(); err != nil { |
| + return "", "", err |
| + } |
| + return s.VersionedStore.CopyVersion(ctx, sb, sk, sv, db, dk, createOnly) |
| +} |
| + |
| +func (installAuthorityStore) DeleteVersion(context.Context, string, string, string) error { |
| + return fmt.Errorf("%w: the prepared build never deletes", errLandingChanged) |
| +} |
| + |
| // filesManifestBuilt reports whether the journal records the prepared |
| // files manifest as built: the step done with a binding, not the "skipped" |
| // value journals written before C8 existed, which a replay builds (ruling |
| @@ -648,8 +704,13 @@ func (ir *installRun) published(ctx context.Context) (bool, error) { |
| // finding 1; a retry clears only its staging); (4) the new paths are |
| // registered under the holder's stamp; (5) the timers table is the sealed |
| // snapshot with its registrations, the carried columns are set and the |
| -// restore-verification entry removed; (6) the prepared files manifest, |
| -// skipped with a log line until C8 exists; (7) the files directory is synced |
| +// restore-verification entry removed; (6) the prepared files manifest (C8, |
| +// Task 6): the placed tree, <host>.env and the wake state as create-only |
| +// objects and one create-only sequence-0 manifest under files/g-<gen>/ in |
| +// the location, each write re-checking the install's authority, the |
| +// binding journalled with the manifest's digest and exact version, and a |
| +// manifest already there that is not this install's refused |
| +// (RollbackManifestOccupied); (7) the files directory is synced |
| // and .files-gen written last, so its presence vouches for every step |
| // before it. |
| // |
| @@ -809,7 +870,10 @@ func (ir *installRun) local_(ctx context.Context) (bool, error) { |
| if err != nil { |
| return false, err |
| } |
| - pm, err := buildPreparedManifest(ctx, vs, cfg.OwnBucket, holder.Location, gen, id, captured.Stamp, dir, envPath, hostfiles.WakeStateOf(host, rows, src.NextDue, src.Replication)) |
| + // Every write re-checks the authority first (installAuthority): a |
| + // stale build never wins the create-only manifest. |
| + authority := installAuthorityStore{VersionedStore: vs, check: ir.installAuthority(ctx, captured, phase)} |
| + pm, err := buildPreparedManifest(ctx, authority, cfg.OwnBucket, holder.Location, gen, id, captured.Stamp, dir, envPath, hostfiles.WakeStateOf(host, rows, src.NextDue, src.Replication)) |
| if errors.Is(err, hostfiles.ErrManifestExists) { |
| log.Printf("moves: %s: the prepared files manifest of %s is not this install's: %v", host, id, err) |
| return ir.refuse(ctx, refuseManifestOccupied) |
diff --git a/internal/host/movesland_test.go b/internal/host/movesland_test.go| index 51fede800..ece16725a 100644 |
| --- a/internal/host/movesland_test.go |
| +++ b/internal/host/movesland_test.go |
| @@ -11,6 +11,7 @@ import ( |
| "log/slog" |
| "os" |
| "path/filepath" |
| + "slices" |
| "sort" |
| "strings" |
| "sync" |
| @@ -1449,6 +1450,92 @@ func TestLocalRefusesAForeignPreparedManifest(t *testing.T) { |
| } |
| } |
| +// moveAuthority hands the landing's op to exec in the claim, as a |
| +// replacement or a return of the executor would. |
| +func moveAuthority(t *testing.T, h *landingHarness, exec string) { |
| + t.Helper() |
| + obs, err := hostclaim.Read(context.Background(), h.s, p1Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := hostclaim.Update(context.Background(), h.s, obs, func(c *hostclaim.Claim) error { |
| + c.Op.Exec = exec |
| + if !slices.Contains(c.Op.Participants, exec) { |
| + c.Op.Participants = append(c.Op.Participants, exec) |
| + } |
| + return nil |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +// The prepared build re-checks the installation's authority (the op at |
| +// phase local with this box executing, the holder at the captured stamp) |
| +// immediately before every bucket write, retries included (fix round 1, |
| +// finding 1; region HA's authorityStore). Authority moves to another |
| +// executor between the first object upload and the next write: the stale |
| +// writer writes nothing more, no manifest exists, and the step is not |
| +// journalled. Once authority returns, the installer completes, reusing |
| +// the one object the stale writer left. |
| +func TestPreparedBuildRechecksAuthorityBeforeEveryWrite(t *testing.T) { |
| + h := newLandingHarness(t) |
| + h.flip(t) |
| + c := h.claim(t) |
| + prefix := movesFilesPrefix(c.Holder.Location, p1ID) |
| + moved := false |
| + h.vs.deny = func(op, bucket, key string) error { |
| + if op == "put-if-absent" && strings.HasPrefix(key, prefix+"objects/") && !moved { |
| + moved = true |
| + moveAuthority(t, h, "other-box") |
| + } |
| + return nil |
| + } |
| + h.land(t) |
| + if !moved { |
| + t.Fatal("the build never uploaded an object") |
| + } |
| + objects := 0 |
| + for k := range h.f.buckets["data-uk"] { |
| + if strings.HasPrefix(k, prefix+"objects/") { |
| + objects++ |
| + } |
| + if k == hostfiles.ManifestKey(prefix) { |
| + t.Fatal("the stale writer published the manifest") |
| + } |
| + } |
| + if objects != 1 { |
| + t.Fatalf("%d objects after authority moved, want the one uploaded before it", objects) |
| + } |
| + if _, ok := h.landJournal(t).Steps[landStepFilesManifest]; ok { |
| + t.Fatal("the step was journalled without the manifest") |
| + } |
| + if m := h.read(t, p1Host).Move; m.LandingPhase != provision.LandLocal || m.Reason != "" { |
| + t.Fatalf("the record moved: %+v", m) |
| + } |
| + moveAuthority(t, h, "uk-box") |
| + h.vs.deny = nil |
| + h.freshTarget() |
| + h.land(t) |
| + h.installed(t) |
| + pm, m, _ := landedPreparedManifest(t, h) |
| + if pm.Op != p1ID || m.Seq != 0 { |
| + t.Fatalf("the authorized installer did not complete: %+v", pm) |
| + } |
| + vs, err := h.vs.ListVersions(context.Background(), "data-uk", prefix+"objects/") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + seen := map[string]int{} |
| + for _, v := range vs { |
| + seen[v.Key]++ |
| + } |
| + for k, n := range seen { |
| + if n != 1 { |
| + t.Fatalf("%s has %d versions: the replay did not reuse it", k, n) |
| + } |
| + } |
| +} |
| + |
| // Nothing is written under the location's files prefix before the |
| // files_manifest step: the manifest follows the placed files, the env |
| // file, the timers and the columns (the step order of local, §7.6). |