lease: reclaim a lock this process abandoned, and stop denying DeleteObject
ips.bdn.oncarlos.com answered "instance is waking elsewhere; retry shortly" twice this evening, each time for the full fifteen-minute lease TTL, each time recovering by itself the moment the lock expired. The cause was the store rollout's own IAM. host_store carries an explicit Deny so a careless store prefix can never hand a box CreateBucket or DeleteBucket on the deployment it runs from — good instinct, but the Deny also named s3:DeleteObject, and releasing an instance's single-writer lease IS a DeleteObject on instances/<host>/lock.json. Applied at 23:26 UTC; the first stranded lock followed at 23:35. Every hibernation after that left its lock standing, litestream's L0 retention stopped reclaiming, the registry-snapshot prune stopped pruning, and the housekeeper lease stopped being handed back. The agent was locked out by its own stale lease, and the log line said so plainly: LEASE HELD by "ip-172-31-13-235:1735186" — the pid of the very process reading it. Two changes, because the policy was the bug but the fragility was ours. The Deny keeps the bucket actions and drops the object one; deleting an object in these buckets is not something a box might do by accident, it is how the platform works. The overlap the Deny was really written for is now refused at plan time by a precondition on the store prefix, which is what the original commit message said it wanted anyway — a prefix that cannot collide beats one that gets caught. And a lease whose lock names THIS process, that no renewer here is holding, is now reclaimed on the next acquire instead of waited out. Three conditions gate it: our own owner stamp, absent from the package's live set, and an If-Match delete on the ETag just read — so a real holder that renews, a peer that takes over, and a genuine double-acquire bug all still lose. Any doubt returns "not ours" and the caller 503s exactly as before. LeaseTTL's comment promised that "every graceful path releases the lease explicitly and is unaffected"; this is what makes that true even when the release fails. A failed release is also its own typed error now, joined rather than masked by whatever else went wrong in the same teardown, and the activator turns it into a member-visible platform error. It stayed invisible for fifteen minutes tonight because it only ever reached the box journal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0193MziBtjrcbbv64zoExL7W
7 files changed,
+422
−17
infra/modules/region-host/main.tf+34 −6internal/activator/activator.go+16 −0internal/lease/checkpoint_test.go+28 −3internal/lease/health.go+12 −2internal/lease/lease.go+52 −6internal/lease/reclaim.go+166 −0internal/lease/reclaim_test.go+114 −0
CI passed — run details
diff --git a/infra/modules/region-host/main.tf b/infra/modules/region-host/main.tf| index 14b7724a6..57295d504 100644 |
| --- a/infra/modules/region-host/main.tf |
| +++ b/infra/modules/region-host/main.tf |
| @@ -622,17 +622,29 @@ resource "aws_iam_role_policy" "host_store" { |
| }, |
| # A deployment whose store prefix also matches its OWN buckets — the |
| # obvious "carlos-" does exactly that against carlos-<deployment>-<acct> |
| - # — would otherwise hand the box DeleteBucket on the control plane it |
| - # runs from. An explicit Deny beats every Allow in IAM, so this holds |
| - # whatever prefix an operator chooses, including a careless one. |
| + # — would otherwise hand the box CreateBucket/DeleteBucket on the control |
| + # plane it runs from. An explicit Deny beats every Allow in IAM, so this |
| + # holds whatever prefix an operator chooses, including a careless one. |
| + # |
| + # BUCKET actions only. s3:DeleteObject was here too until 2026-09-10 and |
| + # it took the fleet down: deleting an object in the data bucket is not |
| + # something the box might do by accident, it is how the platform WORKS. |
| + # Releasing an instance's single-writer lease is a DeleteObject on |
| + # instances/<host>/lock.json, so with it denied every hibernation left |
| + # its lock standing for the full 15-minute lease TTL and the host 503'd |
| + # "instance is waking elsewhere" until the lock expired — the agent |
| + # locked out by its own stale lease. litestream's L0 retention, the |
| + # registry-snapshot prune, the residence-move mirror and the housekeeper |
| + # lease all delete in these buckets too, and all of them broke the same |
| + # way. The overlap this Deny exists to catch is handled properly by the |
| + # precondition below, which refuses a colliding prefix at plan time |
| + # rather than half-breaking the box at apply time. |
| { |
| Effect = "Deny" |
| - Action = ["s3:DeleteBucket", "s3:DeleteObject", "s3:CreateBucket"] |
| + Action = ["s3:DeleteBucket", "s3:CreateBucket"] |
| Resource = [ |
| var.deployment_bucket_arn, |
| - "${var.deployment_bucket_arn}/*", |
| var.data_bucket_arn, |
| - "${var.data_bucket_arn}/*", |
| ] |
| }, |
| # The two calls that could escalate, each refused unless the user being |
| @@ -668,4 +680,20 @@ resource "aws_iam_role_policy" "host_store" { |
| }, |
| ] |
| }) |
| + |
| + # The overlap the Deny above used to paper over, refused at plan time |
| + # instead. `arn:aws:s3:::name` → `name`, and a store prefix that any of this |
| + # deployment's own bucket names begins with would put tenant-store Allows on |
| + # the platform's own storage. Catching it here is what lets the Deny stay |
| + # narrow: an operator sees "pick a different prefix" in the plan rather than |
| + # a fleet that half-works after the apply. |
| + lifecycle { |
| + precondition { |
| + condition = !anytrue([ |
| + for arn in [var.deployment_bucket_arn, var.data_bucket_arn] : |
| + startswith(replace(arn, "arn:aws:s3:::", ""), var.store_bucket_prefix) |
| + ]) |
| + error_message = "store_bucket_prefix (${var.store_bucket_prefix}) also matches this deployment's own buckets (${var.deployment_bucket_arn}, ${var.data_bucket_arn}). Pick a prefix that cannot collide, e.g. \"carlos-store-\"." |
| + } |
| + } |
| } |
diff --git a/internal/activator/activator.go b/internal/activator/activator.go| index fac275fa8..19caf68f9 100644 |
| --- a/internal/activator/activator.go |
| +++ b/internal/activator/activator.go |
| @@ -55,6 +55,10 @@ import ( |
| "time" |
| "github.com/carlosframework/platform/internal/edge" |
| + // Aliased: `lease` is a local variable all through doWake (the Lease seam |
| + // below is this package's own interface), and a package of the same name |
| + // would read as that variable to anyone skimming. |
| + leasepkg "github.com/carlosframework/platform/internal/lease" |
| "github.com/carlosframework/platform/internal/registry" |
| ) |
| @@ -1035,6 +1039,18 @@ func (a *Service) stop(e *actEntry) { |
| outcome.replicaClosed = repErr == nil |
| if err := repErr; err != nil { |
| log.Printf("activator: final checkpoint %s: %v", e.host, err) |
| + // A release that failed is not the same class of problem as a sync |
| + // that failed, and it is the one with a member-visible consequence: |
| + // the lock stands in S3 until the TTL runs out. The next wake now |
| + // reclaims it (internal/lease's reclaimOwn), so this is no longer an |
| + // outage — but it is still a fault, and one that stayed invisible for |
| + // as long as it only ever reached the box journal (2026-09-10: |
| + // DeleteObject denied fleet-wide, found by a member reporting 503s). |
| + var relErr *leasepkg.ReleaseError |
| + if errors.As(err, &relErr) { |
| + emitEvent(a.Logs, e.route, "error", |
| + "could not release this instance's single-writer lease at sleep; the platform will reclaim it on the next wake: "+relErr.Err.Error()) |
| + } |
| } |
| a.mu.Lock() |
| outcome.uncertain = !outcome.complete() |
diff --git a/internal/lease/checkpoint_test.go b/internal/lease/checkpoint_test.go| index 2727cfda4..e5828a98c 100644 |
| --- a/internal/lease/checkpoint_test.go |
| +++ b/internal/lease/checkpoint_test.go |
| @@ -29,6 +29,7 @@ type lockStore struct { |
| body []byte |
| etag string |
| puts, deletes int |
| + denyDelete bool |
| beforePut func(context.Context, *s3.PutObjectInput) error |
| } |
| @@ -60,9 +61,30 @@ func (s *lockStore) PutObject(ctx context.Context, p *s3.PutObjectInput, _ ...fu |
| s.body = raw |
| return &s3.PutObjectOutput{ETag: aws.String(s.etag)}, nil |
| } |
| +func (s *lockStore) setDenyDelete(v bool) { s.mu.Lock(); s.denyDelete = v; s.mu.Unlock() } |
| +func (s *lockStore) setOwner(t *testing.T, owner string) { |
| + t.Helper() |
| + s.mu.Lock() |
| + defer s.mu.Unlock() |
| + var rec map[string]any |
| + if e := json.Unmarshal(s.body, &rec); e != nil { |
| + t.Fatalf("stored lock is not JSON: %v", e) |
| + } |
| + rec["owner"] = owner |
| + raw, e := json.Marshal(rec) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + s.body = raw |
| +} |
| func (s *lockStore) DeleteObject(ctx context.Context, p *s3.DeleteObjectInput, _ ...func(*s3.Options)) (*s3.DeleteObjectOutput, error) { |
| s.mu.Lock() |
| defer s.mu.Unlock() |
| + // The flagship failure of 2026-09-10 in one field: the store answers, the |
| + // object is there, the caller simply may not delete it. |
| + if s.denyDelete { |
| + return nil, &smithy.GenericAPIError{Code: "AccessDenied"} |
| + } |
| if p.IfMatch == nil || aws.ToString(p.IfMatch) != s.etag { |
| return nil, &smithy.GenericAPIError{Code: "PreconditionFailed"} |
| } |
| @@ -107,8 +129,11 @@ func (s *lockStore) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| var api *smithy.GenericAPIError |
| if errors.As(err, &api) { |
| code := 412 |
| - if api.Code == "NoSuchKey" { |
| + switch api.Code { |
| + case "NoSuchKey": |
| code = 404 |
| + case "AccessDenied": |
| + code = 403 |
| } |
| w.Header().Set("Content-Type", "application/xml") |
| w.WriteHeader(code) |
| @@ -402,7 +427,7 @@ func TestLateRenewalCannotResurrect(t *testing.T) { |
| if e != nil { |
| t.Fatal(e) |
| } |
| - rn := startRenewer(leaser, lock, started, 30*time.Millisecond) |
| + rn := startRenewer(leaser, lock, started, 30*time.Millisecond, nil) |
| await(t, entered) |
| await(t, rn.ctx.Done()) |
| if !errors.Is(rn.Err(), ErrLost) { |
| @@ -547,7 +572,7 @@ func TestRenewDeadlineStartsBeforeRemoteRequest(t *testing.T) { |
| if e != nil { |
| t.Fatal(e) |
| } |
| - rn := startRenewer(leaser, lock, started, 100*time.Millisecond) |
| + rn := startRenewer(leaser, lock, started, 100*time.Millisecond, nil) |
| defer func() { leaser.ReleaseLease(context.Background(), rn.stop()) }() |
| initial, e := rn.SafeUntil() |
| if e != nil { |
diff --git a/internal/lease/health.go b/internal/lease/health.go| index 69814f15c..ceab11132 100644 |
| --- a/internal/lease/health.go |
| +++ b/internal/lease/health.go |
| @@ -41,11 +41,18 @@ type renewer struct { |
| watchDone chan struct{} |
| changed chan struct{} |
| lost chan struct{} |
| + // onStop retires this lease's entry from the package's live set, once. |
| + // stop() is the single point every terminal path goes through — Held. |
| + // Release, Replicator.Close, Replicator.Abandon — so hanging the |
| + // bookkeeping here is what keeps the set honest without four call sites |
| + // each able to forget. |
| + onStop sync.Once |
| + retire func() |
| } |
| -func startRenewer(leaser *lss3.Leaser, lease *litestream.Lease, started time.Time, margin time.Duration) *renewer { |
| +func startRenewer(leaser *lss3.Leaser, lease *litestream.Lease, started time.Time, margin time.Duration, retire func()) *renewer { |
| ctx, cancel := context.WithCancel(context.Background()) |
| - rn := &renewer{ctx: ctx, leaser: leaser, lease: lease, until: started.Add(leaser.TTL - margin), margin: margin, cancel: cancel, done: make(chan struct{}), watchDone: make(chan struct{}), changed: make(chan struct{}, 1), lost: make(chan struct{})} |
| + rn := &renewer{ctx: ctx, leaser: leaser, lease: lease, until: started.Add(leaser.TTL - margin), margin: margin, cancel: cancel, done: make(chan struct{}), watchDone: make(chan struct{}), changed: make(chan struct{}, 1), lost: make(chan struct{}), retire: retire} |
| go rn.watch(ctx) |
| go rn.renew(ctx) |
| return rn |
| @@ -157,6 +164,9 @@ func (rn *renewer) renew(ctx context.Context) { |
| } |
| func (rn *renewer) stop() *litestream.Lease { |
| + if rn.retire != nil { |
| + rn.onStop.Do(rn.retire) |
| + } |
| rn.mu.Lock() |
| rn.checkLocked() |
| rn.stopped = true |
diff --git a/internal/lease/lease.go b/internal/lease/lease.go| index 8755c2ed3..8ae46409d 100644 |
| --- a/internal/lease/lease.go |
| +++ b/internal/lease/lease.go |
| @@ -85,6 +85,18 @@ type HeldError struct { |
| ExpiresAt time.Time |
| } |
| +// ReleaseError reports that giving the lease back failed. It matters on its |
| +// own, apart from whatever else went wrong in the same teardown: the lock |
| +// stays in S3 until the TTL expires, so this host cannot be woken for up to |
| +// LeaseTTL. reclaimOwn makes the next wake recover on its own, but a release |
| +// that keeps failing is an IAM or store fault that wants a human — the |
| +// activator turns this into a member-visible error rather than a journal line |
| +// nobody reads. |
| +type ReleaseError struct{ Err error } |
| + |
| +func (e *ReleaseError) Error() string { return "release lease: " + e.Err.Error() } |
| +func (e *ReleaseError) Unwrap() error { return e.Err } |
| + |
| func (e *HeldError) Error() string { |
| if e.Owner != "" { |
| return fmt.Sprintf("lease already held by %s until %s", e.Owner, e.ExpiresAt.Format(time.RFC3339)) |
| @@ -142,6 +154,14 @@ func acquireTTL(ctx context.Context, bucket, prefix, region string, ttl time.Dur |
| leaser.Bucket = bucket |
| leaser.Path = prefix // lock lives at <prefix>/lock.json, colocated with the replica |
| leaser.TTL = ttl |
| + // Explicit, not NewLeaser's own derivation of the same string: reclaimOwn |
| + // compares a contested lock's owner against Owner() to decide whether it |
| + // is ours to break, and the two must be one definition. |
| + leaser.Owner = Owner() |
| + lockKey := prefix + "/lock.json" |
| + if prefix == "" { |
| + lockKey = "lock.json" |
| + } |
| started := time.Now() |
| lease, err := leaser.AcquireLease(ctx) |
| @@ -150,11 +170,29 @@ func acquireTTL(ctx context.Context, bucket, prefix, region string, ttl time.Dur |
| // re-surfaced as *HeldError so callers stay litestream-free. |
| var lexists *litestream.LeaseExistsError |
| if errors.As(err, &lexists) { |
| - return nil, fmt.Errorf("acquire lease: %w", &HeldError{Owner: lexists.Owner, ExpiresAt: lexists.ExpiresAt}) |
| + // ...unless "someone else" is us. A lock carrying this process's |
| + // own stamp that no renewer here is holding is rubbish left by a |
| + // release that failed (see reclaimOwn), and waiting out fifteen |
| + // minutes of TTL for it means fifteen minutes of 503s for nothing. |
| + // Break it and try once more; anything doubtful leaves the lock |
| + // alone and falls through to the ordinary contested answer. |
| + if lexists.Owner == Owner() { |
| + if took, _ := reclaimOwn(ctx, client, bucket, lockKey); took { |
| + started = time.Now() |
| + lease, err = leaser.AcquireLease(ctx) |
| + } |
| + } |
| + } |
| + if err != nil { |
| + if errors.As(err, &lexists) { |
| + return nil, fmt.Errorf("acquire lease: %w", &HeldError{Owner: lexists.Owner, ExpiresAt: lexists.ExpiresAt}) |
| + } |
| + return nil, fmt.Errorf("acquire lease: %w", err) |
| } |
| - return nil, fmt.Errorf("acquire lease: %w", err) |
| } |
| - rn := startRenewer(leaser, lease, started, marginForTTL(ttl)) |
| + key := liveKey(bucket, lockKey) |
| + liveAdd(key) |
| + rn := startRenewer(leaser, lease, started, marginForTTL(ttl), func() { liveDrop(key) }) |
| if err := rn.Err(); err != nil { |
| cur := rn.stop() |
| relCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| @@ -238,7 +276,10 @@ func (h *Held) Release(ctx context.Context) error { |
| if lease == nil { |
| return nil |
| } |
| - return h.leaser.ReleaseLease(ctx, lease) |
| + if err := h.leaser.ReleaseLease(ctx, lease); err != nil { |
| + return &ReleaseError{Err: err} |
| + } |
| + return nil |
| } |
| // Replicate trades the held lease for running replication of dbPath to the |
| @@ -386,8 +427,13 @@ func (r *Replicator) Close(ctx context.Context) error { |
| // one small S3 DELETE; five seconds is generous. |
| relCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| defer cancel() |
| - if e := r.leaser.ReleaseLease(relCtx, lease); e != nil && err == nil { |
| - err = e |
| + // Joined, never "only if the checkpoint succeeded". A release failure |
| + // is the one that costs a quarter of an hour of 503s on the next wake |
| + // (see reclaimOwn), so it must never be masked by a sync error that |
| + // happened to arrive first — the activator keys a member-visible |
| + // error off *ReleaseError. |
| + if e := r.leaser.ReleaseLease(relCtx, lease); e != nil { |
| + err = errors.Join(err, &ReleaseError{Err: e}) |
| } |
| } |
| return err |
diff --git a/internal/lease/reclaim.go b/internal/lease/reclaim.go| new file mode 100644 |
| index 000000000..1d459f42b |
| --- /dev/null |
| +++ b/internal/lease/reclaim.go |
| @@ -0,0 +1,166 @@ |
| +package lease |
| + |
| +import ( |
| + "context" |
| + "encoding/json" |
| + "errors" |
| + "fmt" |
| + "io" |
| + "os" |
| + "strings" |
| + "sync" |
| + |
| + "github.com/aws/aws-sdk-go-v2/aws" |
| + awss3 "github.com/aws/aws-sdk-go-v2/service/s3" |
| + "github.com/aws/smithy-go" |
| +) |
| + |
| +// owner is this process's lease stamp, computed once. It is byte-identical to |
| +// what litestream's NewLeaser derives (hostname:pid, or pid-N when the |
| +// hostname is unavailable) — acquireTTL assigns it explicitly rather than |
| +// letting the two derivations drift, because reclaimOwn's whole safety |
| +// argument is that this string identifies THIS process and no other. |
| +var owner = sync.OnceValue(func() string { |
| + h, _ := os.Hostname() |
| + if h == "" { |
| + return fmt.Sprintf("pid-%d", os.Getpid()) |
| + } |
| + return fmt.Sprintf("%s:%d", h, os.Getpid()) |
| +}) |
| + |
| +// Owner reports the lease stamp this process writes into lock.json. Exported |
| +// for the activator, which compares it against a contested lease's holder to |
| +// tell "someone else has it" from "we left our own behind". |
| +func Owner() string { return owner() } |
| + |
| +// live is the set of lease prefixes this process is CURRENTLY holding — keyed |
| +// bucket/lockKey, one entry per live renewer. It is the guard that makes |
| +// reclaiming our own lock safe: a lock stamped with our owner and absent from |
| +// this set is one no goroutine here is renewing, i.e. rubbish left by a |
| +// release that failed. A lock stamped with our owner that IS in this set means |
| +// a genuine double-acquire bug in the caller, and it must stay loud. |
| +var live struct { |
| + mu sync.Mutex |
| + m map[string]int |
| +} |
| + |
| +func liveKey(bucket, lockKey string) string { return bucket + "/" + lockKey } |
| + |
| +func liveAdd(key string) { |
| + live.mu.Lock() |
| + defer live.mu.Unlock() |
| + if live.m == nil { |
| + live.m = map[string]int{} |
| + } |
| + live.m[key]++ |
| +} |
| + |
| +func liveDrop(key string) { |
| + live.mu.Lock() |
| + defer live.mu.Unlock() |
| + if live.m[key] <= 1 { |
| + delete(live.m, key) |
| + return |
| + } |
| + live.m[key]-- |
| +} |
| + |
| +func liveHas(key string) bool { |
| + live.mu.Lock() |
| + defer live.mu.Unlock() |
| + return live.m[key] > 0 |
| +} |
| + |
| +// reclaimOwn deletes a lock.json this very process left behind, so the caller |
| +// can acquire immediately instead of waiting out the TTL. It reports whether |
| +// it removed anything. |
| +// |
| +// Why this exists (2026-09-10, flagship). LeaseTTL's comment promises that |
| +// "every graceful path (sleep, deploy, shutdown) releases the lease explicitly |
| +// and is unaffected" — but a release is one S3 DeleteObject, and an IAM change |
| +// that denied DeleteObject on the data bucket made every one of them fail. |
| +// Nothing retried and nothing alerted: each hibernation left its lock standing |
| +// and the host answered "instance is waking elsewhere" for the full fifteen |
| +// minutes, blocked by a lease its own process had abandoned. The IAM policy |
| +// was the bug; needing a correct IAM policy for a host to be reachable at all |
| +// is the fragility. This makes that whole class self-healing — a failed |
| +// release costs one extra round trip on the next wake instead of a quarter of |
| +// an hour of downtime. |
| +// |
| +// The three conditions together are what make it safe: |
| +// |
| +// - the lock names US as owner (hostname:pid, unique to this process), so no |
| +// other box and no other incarnation can be the one being broken; |
| +// - the prefix is not in `live`, so no goroutine here is renewing it; |
| +// - the DELETE carries If-Match on the ETag we just read, so a holder that |
| +// renews or a peer that takes over between the read and the delete wins |
| +// the race and we fall back to the ordinary contested-lease answer. |
| +// |
| +// Any doubt at all returns (false, nil): the caller then 503s exactly as it |
| +// did before, which is the behaviour this is trying to improve on, never one |
| +// worse than it. |
| +func reclaimOwn(ctx context.Context, client *awss3.Client, bucket, lockKey string) (bool, error) { |
| + if liveHas(liveKey(bucket, lockKey)) { |
| + return false, nil |
| + } |
| + out, err := client.GetObject(ctx, &awss3.GetObjectInput{Bucket: &bucket, Key: &lockKey}) |
| + if err != nil { |
| + if isNotFound(err) { |
| + return false, nil // already gone; the retried acquire will win it |
| + } |
| + return false, fmt.Errorf("read lease %s: %w", lockKey, err) |
| + } |
| + body, err := io.ReadAll(io.LimitReader(out.Body, 64<<10)) |
| + out.Body.Close() |
| + if err != nil { |
| + return false, fmt.Errorf("read lease %s: %w", lockKey, err) |
| + } |
| + var rec struct { |
| + Owner string `json:"owner"` |
| + } |
| + if err := json.Unmarshal(body, &rec); err != nil { |
| + // Not something we wrote, or not JSON at all — leave it alone. |
| + return false, nil |
| + } |
| + if rec.Owner == "" || rec.Owner != Owner() { |
| + return false, nil |
| + } |
| + etag := aws.ToString(out.ETag) |
| + if etag == "" { |
| + // No ETag to condition on means no safe delete. The TTL is slower but |
| + // it is never wrong. |
| + return false, nil |
| + } |
| + _, err = client.DeleteObject(ctx, &awss3.DeleteObjectInput{ |
| + Bucket: &bucket, |
| + Key: &lockKey, |
| + IfMatch: &etag, |
| + }) |
| + if err != nil { |
| + // PreconditionFailed: somebody rewrote the lock under us and it is |
| + // legitimately theirs now. Anything else (a store with no If-Match on |
| + // DELETE, DeleteObject still denied) is equally a reason to stand |
| + // down rather than force it. |
| + return false, nil |
| + } |
| + return true, nil |
| +} |
| + |
| +// isNotFound covers both shapes S3 returns for a missing key: the typed |
| +// NoSuchKey on GetObject and the bare 404 an S3-compatible may answer with. |
| +func isNotFound(err error) bool { |
| + if err == nil { |
| + return false |
| + } |
| + if errors.Is(err, os.ErrNotExist) { |
| + return true |
| + } |
| + var ae smithy.APIError |
| + if errors.As(err, &ae) { |
| + switch ae.ErrorCode() { |
| + case "NoSuchKey", "NotFound", "404": |
| + return true |
| + } |
| + } |
| + return strings.Contains(err.Error(), "StatusCode: 404") |
| +} |
diff --git a/internal/lease/reclaim_test.go b/internal/lease/reclaim_test.go| new file mode 100644 |
| index 000000000..418bf0583 |
| --- /dev/null |
| +++ b/internal/lease/reclaim_test.go |
| @@ -0,0 +1,114 @@ |
| +// Reclaim tests. The scenario they pin came off flagship on 2026-09-10: an |
| +// IAM policy denied s3:DeleteObject on the data bucket, so every lease release |
| +// failed, and every hibernating instance then answered "instance is waking |
| +// elsewhere" for the full 15-minute TTL — locked out by a lock its own process |
| +// had written and walked away from. The IAM policy was fixed; these tests are |
| +// the part that makes the platform survive the next one. |
| +package lease |
| + |
| +import ( |
| + "context" |
| + "errors" |
| + "net/http/httptest" |
| + "testing" |
| + "time" |
| +) |
| + |
| +// leaseServer points the package at a fake S3 for the duration of a test and |
| +// returns the backend behind it. |
| +func leaseServer(t *testing.T) *lockStore { |
| + t.Helper() |
| + backend := &lockStore{} |
| + srv := httptest.NewServer(backend) |
| + t.Cleanup(srv.Close) |
| + fakeCreds(t) |
| + t.Setenv("CARLOS_S3_ENDPOINT", srv.URL) |
| + return backend |
| +} |
| + |
| +// abandonViaFailedRelease acquires a lease and then releases it with the |
| +// store refusing DELETE, which is exactly how the lock gets stranded in |
| +// production: the handle is gone here, the object is still there. |
| +func abandonViaFailedRelease(t *testing.T, backend *lockStore, prefix string) { |
| + t.Helper() |
| + h, err := acquireTTL(context.Background(), "test-bucket", prefix, "test-region", time.Minute) |
| + if err != nil { |
| + t.Fatalf("first acquire: %v", err) |
| + } |
| + backend.setDenyDelete(true) |
| + err = h.Release(context.Background()) |
| + if err == nil { |
| + t.Fatal("release succeeded against a store that refuses DELETE — the fake is wrong") |
| + } |
| + var relErr *ReleaseError |
| + if !errors.As(err, &relErr) { |
| + t.Fatalf("release error is %T (%v), want a *ReleaseError so the activator can tell this apart from a failed sync", err, err) |
| + } |
| + backend.setDenyDelete(false) |
| +} |
| + |
| +func TestAcquireReclaimsALockThisProcessAbandoned(t *testing.T) { |
| + backend := leaseServer(t) |
| + abandonViaFailedRelease(t, backend, "one.example") |
| + |
| + h, err := acquireTTL(context.Background(), "test-bucket", "one.example", "test-region", time.Minute) |
| + if err != nil { |
| + t.Fatalf("second acquire: %v — a lock stamped with this process's own owner, that nothing here is renewing, must be reclaimed rather than waited out for the TTL", err) |
| + } |
| + t.Cleanup(func() { _ = h.Release(context.Background()) }) |
| +} |
| + |
| +func TestAcquireLeavesAnotherHoldersLockAlone(t *testing.T) { |
| + backend := leaseServer(t) |
| + abandonViaFailedRelease(t, backend, "one.example") |
| + // Same stranded lock, but it belongs to a different box now. |
| + backend.setOwner(t, "some-other-box:999") |
| + _, deletesBefore := backend.counts() |
| + |
| + _, err := acquireTTL(context.Background(), "test-bucket", "one.example", "test-region", time.Minute) |
| + var held *HeldError |
| + if !errors.As(err, &held) { |
| + t.Fatalf("acquire returned %v, want a *HeldError — a lease belonging to somebody else is never ours to break", err) |
| + } |
| + if held.Owner != "some-other-box:999" { |
| + t.Fatalf("HeldError owner = %q, want the other box", held.Owner) |
| + } |
| + if _, deletesAfter := backend.counts(); deletesAfter != deletesBefore { |
| + t.Fatalf("deletes went %d → %d: another holder's lock was removed", deletesBefore, deletesAfter) |
| + } |
| +} |
| + |
| +func TestAcquireWillNotReclaimALeaseItIsStillHolding(t *testing.T) { |
| + backend := leaseServer(t) |
| + h, err := acquireTTL(context.Background(), "test-bucket", "one.example", "test-region", time.Minute) |
| + if err != nil { |
| + t.Fatalf("first acquire: %v", err) |
| + } |
| + t.Cleanup(func() { _ = h.Release(context.Background()) }) |
| + _, deletesBefore := backend.counts() |
| + |
| + // A second acquire of a prefix this process is actively renewing is a |
| + // double-acquire bug in the caller. The owner matches, but the prefix is |
| + // live here, so it must stay loud rather than quietly stealing the lock |
| + // out from under the goroutine using it. |
| + _, err = acquireTTL(context.Background(), "test-bucket", "one.example", "test-region", time.Minute) |
| + var held *HeldError |
| + if !errors.As(err, &held) { |
| + t.Fatalf("second acquire returned %v, want a *HeldError — a live lease must never be reclaimed", err) |
| + } |
| + if _, deletesAfter := backend.counts(); deletesAfter != deletesBefore { |
| + t.Fatalf("deletes went %d → %d: a lease this process is still renewing was broken", deletesBefore, deletesAfter) |
| + } |
| + if err := h.Err(); err != nil { |
| + t.Fatalf("the original handle lost its lease to the second acquire: %v", err) |
| + } |
| +} |
| + |
| +func TestOwnerIsStableAndProcessScoped(t *testing.T) { |
| + if Owner() == "" { |
| + t.Fatal("Owner() is empty — reclaim's whole safety argument rests on this identifying THIS process") |
| + } |
| + if Owner() != Owner() { |
| + t.Fatal("Owner() is not stable across calls") |
| + } |
| +} |