amadan

carlos / platform Public

host: a torn region HA marker stays legacy; the floor decision waits for the apply lock (Opus merge review M1-1)

Main writes update-begun.json through a .tmp. A crash in that write, a
malformed marker, or a concurrent run's in-flight .tmp made every later
apply and recover exit fatally on region HA boxes. Only a marker naming a
floor transaction (id) or a floor artifact now needs a policy, and Run
reads the guard directory only once it holds the lock.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 7800180330c3f0c22067c36095c5f6be60eb0d35 parent d905565
3 files changed, +76 −23
  • internal/host/guardparticipation_test.go +37 −2
  • internal/host/guardtransaction.go +29 −17
  • internal/host/systemapply.go +10 −4

CI passed — run details

diff --git a/internal/host/guardparticipation_test.go b/internal/host/guardparticipation_test.go
index 5307ca2ac..58a66bb32 100644
--- a/internal/host/guardparticipation_test.go
+++ b/internal/host/guardparticipation_test.go
@@ -158,7 +158,7 @@ func TestSystemGuardUnpublishedKeepsRegionHAParticipation(t *testing.T) {
}
func TestSystemGuardHookRecognizesOnlyLegacyRegionState(t *testing.T) {
- for _, state := range []string{"absent", "empty", "region-marker", "floor-marker", "malformed-marker", "checker", "identities", "activation", "completed", "policy", "symlink"} {
+ for _, state := range []string{"absent", "empty", "region-marker", "floor-marker", "malformed-marker", "tmp-only", "marker+tmp", "floor-marker+tmp", "checker", "identities", "activation", "completed", "policy", "symlink"} {
t.Run(state, func(t *testing.T) {
f := newApplyFixture(t, "stable", "v1")
t.Setenv("CARLOS_RELEASE_PUBKEY", base64.StdEncoding.EncodeToString(f.pub))
@@ -181,6 +181,16 @@ func TestSystemGuardHookRecognizesOnlyLegacyRegionState(t *testing.T) {
name, body = updateBegunFile, `{"id":"activation"}`
case "malformed-marker":
name, body = updateBegunFile, `{`
+ case "tmp-only":
+ name, body = updateBegunFile+".tmp", `{"tar`
+ case "marker+tmp", "floor-marker+tmp":
+ name, body = updateBegunFile, `{"target":"v2"}`
+ if state == "floor-marker+tmp" {
+ body = `{"id":"activation"}`
+ }
+ if err := os.WriteFile(filepath.Join(dir, updateBegunFile+".tmp"), []byte(`{"tar`), 0644); err != nil {
+ t.Fatal(err)
+ }
case "checker":
name = "carlos"
case "identities":
@@ -196,7 +206,9 @@ func TestSystemGuardHookRecognizesOnlyLegacyRegionState(t *testing.T) {
}
}
err := installFloorHook(f.a)
- legacy := state == "absent" || state == "empty" || state == "region-marker"
+ // A crash inside main's durable marker write leaves the .tmp or a
+ // torn marker; neither is floor state (Opus merge review M1-1).
+ legacy := state == "absent" || state == "empty" || state == "region-marker" || state == "malformed-marker" || state == "tmp-only" || state == "marker+tmp"
if legacy {
if err != nil || f.a.guarded() {
t.Fatalf("legacy state armed floor: %v", err)
@@ -213,3 +225,26 @@ func TestSystemGuardHookRecognizesOnlyLegacyRegionState(t *testing.T) {
})
}
}
+
+// The floor decision is read under the apply lock: a locked-out run never
+// looks at a guard directory another run may be writing (Opus merge M1-1).
+func TestSystemFloorHookRunsOnlyUnderTheApplyLock(t *testing.T) {
+ f := newApplyFixture(t, "stable", "v1")
+ f.promote("v2", []byte("x"), false)
+ called := 0
+ f.a.floorHook = func(*systemApplier) error { called++; return nil }
+ held, err := os.OpenFile(f.a.lockPath(), os.O_CREATE|os.O_RDWR, 0o600)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := flockExclusiveNonblock(held); err != nil {
+ t.Fatal(err)
+ }
+ if err := f.a.Run(t.Context(), false); err != nil || called != 0 {
+ t.Fatalf("locked-out run: err=%v hook calls=%d", err, called)
+ }
+ held.Close()
+ if err := f.a.Run(t.Context(), false); err != nil || called != 1 {
+ t.Fatalf("lock-holding run: err=%v hook calls=%d", err, called)
+ }
+}
diff --git a/internal/host/guardtransaction.go b/internal/host/guardtransaction.go
index 0af20c11b..26c888cb4 100644
--- a/internal/host/guardtransaction.go
+++ b/internal/host/guardtransaction.go
@@ -261,25 +261,13 @@ func installFloorHook(a *systemApplier) error {
if err != nil {
return err
}
- // Main's participation marker predates floor arming and leaves an empty
- // directory after completion. Only those two shapes are legacy state.
- if len(entries) == 0 {
+ // Main's participation marker predates floor arming, is written through
+ // update-begun.json.tmp, and leaves an empty directory after completion.
+ // A torn or leftover write of it is still legacy state; only a marker
+ // naming a floor transaction ("id") or a floor artifact needs a policy.
+ if legacyGuardEntries(dir, entries) {
return nil
}
- if len(entries) == 1 && entries[0].Name() == updateBegunFile && entries[0].Type().IsRegular() {
- raw, err := os.ReadFile(updateBegunPath(a.optDir))
- if err != nil {
- return err
- }
- var marker struct {
- updateBegun
- ID string `json:"id"`
- }
- if json.Unmarshal(raw, &marker) == nil && marker.ID == "" && marker.Target != "" {
- return nil
- }
- return hostfloor.ErrFloor
- }
d, err := floorDisk(a.optDir)
if err != nil {
return err
@@ -348,3 +336,27 @@ func (a *systemApplier) prepareGuard(ctx context.Context, target *release.System
txRaw, _ := json.Marshal(guardTransaction{raw, id, actor, enrollment.Enrollment})
return hostfloor.Durable(filepath.Join(a.floor.Dir, "activation.json"), txRaw, 0644)
}
+
+// legacyGuardEntries reports whether the guard directory holds nothing but
+// main's region participation marker, its temporary file, or both, with no
+// floor transaction ID in the marker.
+func legacyGuardEntries(dir string, entries []os.DirEntry) bool {
+ for _, e := range entries {
+ if !e.Type().IsRegular() || (e.Name() != updateBegunFile && e.Name() != updateBegunFile+".tmp") {
+ return false
+ }
+ if e.Name() == updateBegunFile {
+ raw, err := os.ReadFile(filepath.Join(dir, e.Name()))
+ if err != nil {
+ return false
+ }
+ var marker map[string]json.RawMessage
+ if json.Unmarshal(raw, &marker) == nil {
+ if _, ok := marker["id"]; ok {
+ return false
+ }
+ }
+ }
+ }
+ return true
+}
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go
index 1598f0434..adfda8305 100644
--- a/internal/host/systemapply.go
+++ b/internal/host/systemapply.go
@@ -111,9 +111,7 @@ func RunSystemApply(args []string) {
platform: runtime.GOOS + "-" + runtime.GOARCH, gateWait: systemGateWait, soak: systemGateSoak,
regionHA: regionHAEnabled(os.Getenv),
}
- if err := installFloorHook(a); err != nil {
- log.Fatal(err)
- }
+ a.floorHook = installFloorHook // decided under the apply lock, in Run
if err := a.Run(ctx, *recover); err != nil {
log.Fatalf("%v", err)
}
@@ -205,7 +203,10 @@ func productionSysOps() sysOps {
}
type systemApplier struct {
- floor *hostfloor.Disk
+ floor *hostfloor.Disk
+ // floorHook, when set, decides legacy or floor-guarded state once Run
+ // holds the apply lock, so a concurrent run's marker write is never read.
+ floorHook func(*systemApplier) error
guardOpen func(string) (*os.File, error)
ops sysOps
// openStore is called once, by verifyPointer, and only on a run that
@@ -269,6 +270,11 @@ func (a *systemApplier) Run(ctx context.Context, recover bool) (retErr error) {
a.ops.logf("system apply: another apply is running; leaving it to finish")
return nil
}
+ if a.floorHook != nil {
+ if err := a.floorHook(a); err != nil {
+ return err
+ }
+ }
// Rejected floor recovery must retain staging and the journal.
if err := a.checkGuardRecovery(); err != nil {
return err