amadan
fix wave 1: a seed whose termination is unknown keeps the lease (failed discharge after an unproved success, unreadable obligation)
Also narrows item 1's unproven-sidecar rule to a failed stop that left the process running, the flag-off main process's own rule (TestMaintenanceFenceChecksEveryStopOutcome/sidecar-alive).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+128
−13
internal/activator/activator.go+18 −8internal/activator/backend_exec.go+28 −5internal/activator/containment_test.go+82 −0
diff --git a/internal/activator/activator.go b/internal/activator/activator.go| index b7d07bc47..7cae2b2a1 100644 |
| --- a/internal/activator/activator.go |
| +++ b/internal/activator/activator.go |
| @@ -742,11 +742,11 @@ func stopDiscardedSidecar(sc Proc) { |
| // stopReturnedSidecar stops a sidecar a flag-off restart brought back for |
| // an entry that is stopping (Task 8, F6 plan 5): synchronously, bounded by |
| // sidecarStopTimeout, so the start's join closes only after it. A sidecar |
| -// whose stop did not prove its exit is not forgotten (fix wave 1, item 1): |
| -// it is kept on the entry, and the stop keeps its lease until it has exited |
| -// (sidecarsExited). The stop's host proof follows, and with a delegated |
| -// cgroup that proof also reaches whatever this stop left. Never called with |
| -// a.mu held. |
| +// whose stop did not prove its exit (sidecarStopUnproven) is not forgotten |
| +// (fix wave 1, item 1): it is kept on the entry, and the stop keeps its |
| +// lease until it has exited (sidecarsExited). The stop's host proof |
| +// follows, and with a delegated cgroup that proof also reaches whatever |
| +// this stop left. Never called with a.mu held. |
| func (a *Service) stopReturnedSidecar(e *actEntry, sc Proc) { |
| if sc == nil { |
| return |
| @@ -754,15 +754,25 @@ func (a *Service) stopReturnedSidecar(e *actEntry, sc Proc) { |
| ctx, cancel := context.WithTimeout(context.Background(), sidecarStopTimeout) |
| err := sc.Stop(ctx) |
| cancel() |
| - if sc.Exited() { |
| + if !sidecarStopUnproven(sc, err) { |
| return |
| } |
| - log.Printf("activator: %s: the sidecar a restart brought back for a stopping entry has not exited (%v); keeping the lease until it has", e.host, err) |
| + log.Printf("activator: %s: the sidecar a restart brought back for a stopping entry is not proven gone (%v); keeping the lease until it has exited", e.host, err) |
| a.mu.Lock() |
| e.unprovenSidecars = append(e.unprovenSidecars, sc) |
| a.mu.Unlock() |
| } |
| +// sidecarStopUnproven reports whether a flag-off sidecar stop left its |
| +// termination unproven: the stop failed (a nested unit's ErrStillRunning, |
| +// a signal that could not be sent) and the process has not exited. A stop |
| +// that returned nil is the stop's own proof, as it is for the main |
| +// process's flag-off stop: every real Proc returns nil only once its |
| +// process is gone. |
| +func sidecarStopUnproven(sc Proc, err error) bool { |
| + return err != nil && !sc.Exited() |
| +} |
| + |
| // sidecarsExited reports whether every sidecar a flag-off stop kept |
| // unproven (actEntry.unprovenSidecars) has exited: the sidecar's own stop |
| // proof, which is the only one a nested row has. With restop it first |
| @@ -3686,7 +3696,7 @@ func (a *Service) stop(e *actEntry) { |
| log.Printf("activator: stop sidecar %s: %v", e.host, err) |
| } |
| scCancel() |
| - if !a.gens && !e.sidecar.Exited() { |
| + if !a.gens && sidecarStopUnproven(e.sidecar, err) { |
| // Flag off, its exit is kept to be proven before the lease goes |
| // (fix wave 1, item 1): a nested row has no host-cgroup proof to |
| // reach it. |
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go| index b95208e75..4a6df2a32 100644 |
| --- a/internal/activator/backend_exec.go |
| +++ b/internal/activator/backend_exec.go |
| @@ -1100,12 +1100,32 @@ func (b *ExecBackend) seedChild(ctx context.Context, rt registry.Route, exe stri |
| // we started is ours to remove, and under the lease, with the seed's |
| // processes proven gone above, nothing else can have written it. A |
| // discharge that fails keeps the obligation and fails the wake while |
| - // nothing serves; the next wake proves and removes again. |
| + // nothing serves; the next wake proves and removes again. A successful |
| + // flag-off seed's failed discharge first proves its processes gone, since |
| + // no instance will (ErrSeedUnproven otherwise). |
| if derr := dischargeSeedObligation(dbPath, failure != nil && seedOwnsDB); derr != nil { |
| + unprovedSuccess := spawned && failure == nil && child == "" // the proof above was skipped |
| if failure == nil { |
| - return fmt.Errorf("seed for %s: %w", host, derr) |
| + failure = fmt.Errorf("seed for %s: %w", host, derr) |
| + } else { |
| + failure = fmt.Errorf("%w; and %w", failure, derr) |
| + } |
| + if unprovedSuccess { |
| + // A successful flag-off seed has no proof of its own: its helper |
| + // is left for the instance's terminal stop to prove. With the |
| + // discharge failed the wake fails, so no instance will run to |
| + // prove it, and releasing the lease over it would be the hazard |
| + // itself (fix wave 1, item 2): the proof is tried now. Unproven, |
| + // the seed is ErrSeedUnproven and the lease is left to its TTL. |
| + pctx, pcancel := context.WithTimeout(context.WithoutCancel(ctx), actStopTimeout) |
| + perr := b.seedDescendantsGone(pctx, rt, child) |
| + pcancel() |
| + if perr != nil { |
| + log.Printf("activator: %s: the seed's obligation could not be discharged and its processes are not proven gone; %s stays", host, dbPath+seedUnprovenSuffix) |
| + return fmt.Errorf("%w: %w: the seed's processes are not proven gone: %v", failure, ErrSeedUnproven, perr) |
| + } |
| } |
| - return fmt.Errorf("%w; and %w", failure, derr) |
| + return failure |
| } |
| if failure == nil { |
| if err != nil { |
| @@ -1285,8 +1305,11 @@ func (b *ExecBackend) settleSeedObligation(ctx context.Context, rt registry.Rout |
| return nil |
| } |
| if err != nil { |
| - // "Couldn't tell" is never "no obligation". |
| - return fmt.Errorf("%s: read the seed obligation %s: %w", host, dbPath+seedUnprovenSuffix, err) |
| + // "Couldn't tell" is never "no obligation", and never "proven |
| + // gone" either (fix wave 1, item 2): ErrSeedUnproven leaves the |
| + // lease to its TTL. A nested wake runs no host proof before this, |
| + // so nothing else says the seed's processes are gone. |
| + return fmt.Errorf("%s: %w: read the seed obligation %s: %w", host, ErrSeedUnproven, dbPath+seedUnprovenSuffix, err) |
| } |
| if err := b.seedObligationProof(ctx, rt, o); err != nil { |
| return fmt.Errorf("%s: %w: an earlier seed's processes are not proven gone, so its database is not used: %v", host, ErrSeedUnproven, err) |
diff --git a/internal/activator/containment_test.go b/internal/activator/containment_test.go| index a6680b4e4..e0aac8bb1 100644 |
| --- a/internal/activator/containment_test.go |
| +++ b/internal/activator/containment_test.go |
| @@ -685,3 +685,85 @@ func TestAFlagOnFailedSeedProvesItsRoleChildBeforeRemovingTheDB(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// Fix wave 1, item 2(a) (Astra final review 1, finding 2): a successful |
| +// flag-off seed has no proof of its own (a helper it left stays in the host |
| +// cgroup), so when its obligation cannot be discharged the wake fails with |
| +// that helper's termination unknown. The proof is tried then: here the |
| +// host cgroup still holds a process, so the seed answers ErrSeedUnproven and |
| +// the lease is left to its TTL. The seed below swaps its obligation for a |
| +// non-empty directory, which the discharge cannot remove. |
| +func TestAFlagOffSeedWhoseDischargeFailsIsUnprovenWhileItsHelperRemains(t *testing.T) { |
| + const host = "discharge-fails.example.com" |
| + b, _ := delegatedBackend(t, host) |
| + stageSeedScript(t, b, host, "v1", fmt.Sprintf( |
| + "rm -f \"$3%[1]s\" && mkdir -p \"$3%[1]s/x\"\nprintf seeded > \"$3\"\nexit 0\n", seedUnprovenSuffix)) |
| + var asked []string |
| + b.cgroupKill = func(_ context.Context, _ registry.Route, child string) error { |
| + asked = append(asked, child) |
| + return errors.New("the host cgroup still holds a process") |
| + } |
| + |
| + err := b.Seed(context.Background(), registry.Route{Host: host, Kind: "instance", Seed: `["seed"]`}) |
| + if !errors.Is(err, ErrSeedUnproven) { |
| + t.Fatalf("Seed = %v, want ErrSeedUnproven: the discharge failed and the seed's helper is not proven gone", err) |
| + } |
| + if len(asked) != 1 || asked[0] != "" { |
| + t.Fatalf("proof asked of %q, want the host cgroup once", asked) |
| + } |
| +} |
| + |
| +// Fix wave 1, item 2(a), the other side: the discharge fails but the proof, |
| +// tried then, holds. Nothing of the seed is left, so the failure is an |
| +// ordinary one and the lease may go; the obligation stays for the next |
| +// wake. |
| +func TestAFlagOffSeedWhoseDischargeFailsAfterAProofIsAnOrdinaryFailure(t *testing.T) { |
| + const host = "discharge-fails-proven.example.com" |
| + b, _ := delegatedBackend(t, host) |
| + stageSeedScript(t, b, host, "v1", fmt.Sprintf( |
| + "rm -f \"$3%[1]s\" && mkdir -p \"$3%[1]s/x\"\nprintf seeded > \"$3\"\nexit 0\n", seedUnprovenSuffix)) |
| + b.cgroupKill = func(context.Context, registry.Route, string) error { return nil } |
| + |
| + err := b.Seed(context.Background(), registry.Route{Host: host, Kind: "instance", Seed: `["seed"]`}) |
| + if err == nil || errors.Is(err, ErrSeedUnproven) { |
| + t.Fatalf("Seed = %v, want an ordinary failure: the proof held", err) |
| + } |
| + if _, err := os.Stat(filepath.Join(b.DataDir, host+".db"+seedUnprovenSuffix)); err != nil { |
| + t.Fatalf("the obligation is gone (%v); it must stay for the next wake", err) |
| + } |
| +} |
| + |
| +// Fix wave 1, item 2(b): an obligation that is there but cannot be read |
| +// says nothing of whether its seed's processes are gone, so Restore refuses |
| +// with ErrSeedUnproven (the lease is left to its TTL), never an ordinary |
| +// error that releases it. The host cgroup here still holds a process. |
| +func TestAnUnreadableObligationIsUnproven(t *testing.T) { |
| + if os.Geteuid() == 0 { |
| + t.Skip("root reads a mode-000 file") |
| + } |
| + const host = "unreadable-obligation.example.com" |
| + b, _ := delegatedBackend(t, host) |
| + b.cgroupKill = func(context.Context, registry.Route, string) error { |
| + return errors.New("the host cgroup still holds a process") |
| + } |
| + db := filepath.Join(b.DataDir, host+".db") |
| + if err := os.MkdirAll(filepath.Dir(db), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(db, []byte("half"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + ob := db + seedUnprovenSuffix |
| + if err := os.WriteFile(ob, []byte(seedObligationHeader+"\n"+seedObligationOwned+"\n"+seedObligationFlat+"\n"), 0o000); err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { _ = os.Chmod(ob, 0o600) }) |
| + |
| + fresh, err := b.Restore(context.Background(), registry.Route{Host: host, Kind: "instance", Seed: `["seed"]`}) |
| + if !errors.Is(err, ErrSeedUnproven) || fresh { |
| + t.Fatalf("Restore = %v, %v; want a refusal with ErrSeedUnproven", fresh, err) |
| + } |
| + if got, _ := os.ReadFile(db); string(got) != "half" { |
| + t.Fatalf("database = %q after a refused wake, want it untouched", got) |
| + } |
| +} |