diff --git a/modules/keycloak/README.md b/modules/keycloak/README.md index a400077..e786fc0 100644 --- a/modules/keycloak/README.md +++ b/modules/keycloak/README.md @@ -47,8 +47,8 @@ every 15 minutes while the failure lasts. `provisioner.recovered` follows the fi A consumer the mesh no longer asks for is **retired**, not deleted (novox/hq ADR 0230): its client is disabled — Keycloak refuses its authorization and token requests — and marked with `mesh.retired` and `mesh.retired-why`; its secret, redirects and mappers are kept, and asked for again it is enabled as it -was. Retiring waits five passes, and for a person's `retire approve` when it is more than three clients -or more than half of those held. Only `cleanup delete` removes a client, and only a disabled one the +was. Retiring waits five passes and ten minutes, and for a person's `retire approve` when it is more +than three clients or more than half of those held. Only `cleanup delete` removes a client, and only a disabled one the mesh made. The tools `provisioner_retirement`, `provisioner_retire_approve`, `provisioner_retire_reject` and `provisioner_delete` are what the controller's `retire` and `cleanup` verbs ask. `MESH_KEYCLOAK_LIVE_URL` and `MESH_KEYCLOAK_LIVE_PASSWORD` run `live_retire_test.go` against a throwaway diff --git a/modules/keycloak/cmd/keycloak-provider/harness.go b/modules/keycloak/cmd/keycloak-provider/harness.go index 6fb7bff..46d013d 100644 --- a/modules/keycloak/cmd/keycloak-provider/harness.go +++ b/modules/keycloak/cmd/keycloak-provider/harness.go @@ -21,7 +21,8 @@ package main // (novox/hq ADR 0230, the operator's model of 2026-10-06).** Retiring disables its access — reversibly — // and marks its login and data "to delete" with when and why; nothing is deleted. Asked for again, the // ordinary create re-enables it as it was. It is retired only once the same set has gone unasked in -// StablePasses consecutive passes, and a set larger than the bound waits for a person. retirement.go. +// StablePasses consecutive passes and for StableFor, and a set larger than the bound waits for a +// person. retirement.go. // // Carried, identical, by every Go provider until the Go SDK has the loop: postgres and keycloak. // Each module's `harness_same_test.go` fails when its copy and the other's differ (this file and @@ -92,11 +93,12 @@ type Harness struct { // missed the first hears the next, and a standing nobody repeats can be told from one that holds. FailingAfter time.Duration SayAgainEvery time.Duration - // StablePasses (5) is how many consecutive passes must see the same set no longer asked for before - // it is retired. RetireAtOnce (3) and RetireFraction (0.5) bound what is retired without a person: + // StablePasses (5) is how many consecutive passes must see the same set no longer asked for, and + // StableFor (10m) how long it must have held since the first of them, before it is retired. RetireAtOnce (3) and RetireFraction (0.5) bound what is retired without a person: // more consumers than RetireAtOnce, or — where more than one is held — a larger share of those held // than RetireFraction, waits for `retire approve` (novox/hq ADR 0230). StablePasses int + StableFor time.Duration RetireAtOnce int RetireFraction float64 @@ -202,6 +204,9 @@ func (h *Harness) init() { if h.StablePasses == 0 { h.StablePasses = 5 } + if h.StableFor == 0 { + h.StableFor = StableFor + } if h.RetireAtOnce == 0 { h.RetireAtOnce = 3 } diff --git a/modules/keycloak/cmd/keycloak-provider/harness_test.go b/modules/keycloak/cmd/keycloak-provider/harness_test.go index 964e536..2112371 100644 --- a/modules/keycloak/cmd/keycloak-provider/harness_test.go +++ b/modules/keycloak/cmd/keycloak-provider/harness_test.go @@ -66,13 +66,13 @@ func TestAConsumerNoLongerAskedForIsRetiredOnceStable(t *testing.T) { w.give(map[string]any{"as": "a"}, map[string]any{"as": "b"}) w.h.Reconcile(ctx) w.give(map[string]any{"as": "a"}) - w.passes(25 * time.Second) // five passes + w.settle() // five passes and ten minutes if strings.Join(w.a.removed, ",") != "b" { t.Fatal(w.a.removed) } // Only a file that says nobody asks retires the last one. w.give() - w.passes(25 * time.Second) + w.settle() if strings.Join(w.a.removed, ",") != "b,a" { t.Fatal(w.a.removed) } diff --git a/modules/keycloak/cmd/keycloak-provider/retirement.go b/modules/keycloak/cmd/keycloak-provider/retirement.go index 3268591..f15f181 100644 --- a/modules/keycloak/cmd/keycloak-provider/retirement.go +++ b/modules/keycloak/cmd/keycloak-provider/retirement.go @@ -12,10 +12,12 @@ package main // 3. DELETED, only through `cleanup delete`, a person's act, which this provider executes because it // owns its backend. // -// **Stable removals.** A consumer is retired only after the same set of consumers has gone unasked in -// StablePasses (5) consecutive passes that read the file. A pass that could not read it is not a -// result and starts the count again; so does a different set. At a pass every five seconds that is -// twenty seconds after the first pass that saw the set. Additions and changes are never delayed. +// **Stable removals.** A consumer is retired only once the same set of consumers has gone unasked BOTH +// in StablePasses (5) consecutive passes that read the file AND for at least StableFor (10 minutes) +// since the first pass that saw it. Five passes alone are twenty-five seconds — shorter than a +// controller restart, a store reconnecting or a file half written. A pass that could not read the file +// is not a result and starts both again; so does a different set. Additions and changes are never +// delayed. // // **Too many is a person.** A stable set of more than RetireAtOnce (3), or — where more than one is // held — of more than RetireFraction (half) of those held, retires nothing: it WAITS, said in the @@ -62,6 +64,10 @@ const ( ChangeAdopted = "adopted" // found disabled without the mark, now retired on record ) +// StableFor is the least time the same unasked set must hold before it is retired (novox/hq ADR 0230): +// longer than a controller restart, a store reconnecting or a file half written. +const StableFor = 10 * time.Minute + // KindConsumer is a retired consumer. A backend may list other things set aside for deletion under a // word of its own (postgres: a database renamed aside). const KindConsumer = "consumer" @@ -139,7 +145,7 @@ func (h *Harness) seed(ctx context.Context, want map[string]bool) { // No hash: asked for again, it is applied again, which is harmless and marks it. h.applied[as] = appliedEntry{} h.say("%s: held by the backend and no longer asked for; retired once that holds for %d passes "+ - "(novox/hq ADR 0230)", as, h.StablePasses) + "and %s (novox/hq ADR 0230)", as, h.StablePasses, h.StableFor) } } now := h.Now() @@ -190,20 +196,20 @@ func (h *Harness) retireUnasked(ctx context.Context, want map[string]bool) { h.settle("the set the mesh no longer asks for changed") } h.r.key, h.r.count, h.r.firstSeen = key, 1, now - h.say("no longer asked for: %s; retired once the same set holds for %d passes", strings.Join(unasked, ", "), - h.StablePasses) + h.say("no longer asked for: %s; retired once the same set holds for %d passes and %s", + strings.Join(unasked, ", "), h.StablePasses, h.StableFor) default: h.r.count++ } - if h.r.rejected != nil || h.r.count < h.StablePasses { + if h.r.rejected != nil || h.r.count < h.StablePasses || now.Sub(h.r.firstSeen) < h.StableFor { return } if h.overTheBound(len(unasked), len(h.applied)) { h.wait(unasked) return } - why := fmt.Sprintf("the mesh stopped asking for it: the same result in %d consecutive passes from %s", - h.StablePasses, stamp(h.r.firstSeen)) + why := fmt.Sprintf("the mesh stopped asking for it: the same result in %d consecutive passes over %s, from %s", + h.r.count, now.Sub(h.r.firstSeen).Round(time.Second), stamp(h.r.firstSeen)) h.retire(ctx, unasked, why, nil) } @@ -416,7 +422,7 @@ func (h *Harness) Retirement(ctx context.Context) (map[string]any, error) { "retired_at": stampOr(r.RetiredAt), "why": r.Why, "size_bytes": r.SizeBytes, "kind": kindOf(r)}) } out := map[string]any{"resource": h.Resource, "node": h.Node, "held": orEmptyList(held), - "stable_passes": h.StablePasses, "bound": h.bound(len(h.applied)), "waiting": nil, "rejected": nil, + "stable_passes": h.StablePasses, "stable_for_seconds": int(h.StableFor.Seconds()), "bound": h.bound(len(h.applied)), "waiting": nil, "rejected": nil, "retired": retired} if w := h.r.waiting; w != nil { out["waiting"] = map[string]any{"consumers": h.named(w.set), "since": stamp(w.since), "held": w.held} diff --git a/modules/keycloak/cmd/keycloak-provider/retirement_test.go b/modules/keycloak/cmd/keycloak-provider/retirement_test.go index 3894d47..cad4850 100644 --- a/modules/keycloak/cmd/keycloak-provider/retirement_test.go +++ b/modules/keycloak/cmd/keycloak-provider/retirement_test.go @@ -140,6 +140,9 @@ func namesOf(body map[string]any) string { return strings.Join(out, ",") } +// settle passes every five seconds until the same answer has held for StableFor, and one pass more. +func (w *world) settle() { w.passes(StableFor + 5*time.Second) } + func TestATransientEmptyListForFourPassesRetiresNothing(t *testing.T) { w, said := standingWorld(t) given := holding(w, 1) @@ -152,33 +155,55 @@ func TestATransientEmptyListForFourPassesRetiresNothing(t *testing.T) { if len(w.a.removed) != 0 || len(retirements(*said)) != 0 { t.Fatalf("four passes retired %v and said %v", w.a.removed, retirements(*said)) } - // Gone again: counted from one, not from where the transient left off. - w.give() - for i := 0; i < 4; i++ { - w.pass() - } - if len(w.a.removed) != 0 { - t.Fatalf("retired after four passes: %v", w.a.removed) - } - w.pass() - if strings.Join(w.a.removed, ",") != "c1" { - t.Fatalf("the fifth pass did not retire: %v", w.a.removed) - } } -func TestFiveStablePassesRetireAndAskedAgainReEnables(t *testing.T) { +// Five identical passes are twenty-five seconds — shorter than a controller restart. Both must hold: +// five passes AND ten minutes of the same answer (novox/hq ADR 0230). +func TestFivePassesInTwentyFiveSecondsRetireNothingTenMinutesDo(t *testing.T) { w, said := standingWorld(t) - given := holding(w, 3) - w.give(given[:2]...) + holding(w, 1) + w.give() for i := 0; i < 5; i++ { w.pass() } + if len(w.a.removed) != 0 || len(retirements(*said)) != 0 { + t.Fatalf("five passes in 25 s retired %v", w.a.removed) + } + w.passes(StableFor - 30*time.Second) + if len(w.a.removed) != 0 { + t.Fatalf("retired before the set held %s: %v", StableFor, w.a.removed) + } + w.passes(time.Minute) + if strings.Join(w.a.removed, ",") != "c1" { + t.Fatalf("the same set held %s and was not retired: %v", StableFor, w.a.removed) + } + // Ten minutes in one slow pass are not five passes. + w2 := newWorld(t) + holding(w2, 1) + w2.give() + w2.pass() + w2.now = w2.now.Add(StableFor) + w2.pass() + if len(w2.a.removed) != 0 { + t.Fatalf("two passes ten minutes apart retired %v", w2.a.removed) + } + w2.passes(15 * time.Second) + if len(w2.a.removed) != 1 { + t.Fatalf("five passes over ten minutes did not retire: %v", w2.a.removed) + } +} + +func TestAStableSetIsRetiredAndAskedAgainReEnables(t *testing.T) { + w, said := standingWorld(t) + given := holding(w, 3) + w.give(given[:2]...) + w.settle() if strings.Join(w.a.removed, ",") != "c3" { t.Fatalf("retired %v", w.a.removed) } body := lastRetirement(t, *said) if body["change"] != ChangeRetired || namesOf(body) != "c3" || body["held"] != 3 || body["provider-node"] != "anchor" || - !strings.Contains(body["why"].(string), "5 consecutive passes") { + !strings.Contains(body["why"].(string), "consecutive passes over 10m") { t.Fatalf("%v", body) } if len(w.a.inv.Retired) != 1 || w.a.inv.Retired[0].Consumer != "c3" { @@ -203,21 +228,17 @@ func TestAnUnreadablePassStartsTheCountAgain(t *testing.T) { w := newWorld(t) holding(w, 1) w.give() - for i := 0; i < 3; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) os.WriteFile(w.receives, []byte("{"), 0o600) w.pass() w.give() - for i := 0; i < 4; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) if len(w.a.removed) != 0 { t.Fatalf("an unreadable pass counted: %v", w.a.removed) } - w.pass() + w.passes(2 * time.Minute) if len(w.a.removed) != 1 { - t.Fatalf("not retired after five readable passes: %v", w.a.removed) + t.Fatalf("not retired after ten minutes of readable passes: %v", w.a.removed) } } @@ -225,17 +246,13 @@ func TestADifferentSetStartsTheCountAgain(t *testing.T) { w := newWorld(t) given := holding(w, 5) w.give(given[:4]...) - for i := 0; i < 3; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) w.give(given[:3]...) - for i := 0; i < 4; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) if len(w.a.removed) != 0 { t.Fatalf("a changed set kept its count: %v", w.a.removed) } - w.pass() + w.passes(2 * time.Minute) if strings.Join(w.a.removed, ",") != "c4,c5" { t.Fatalf("%v", w.a.removed) } @@ -260,7 +277,7 @@ func TestTooManyWaitsForAPersonAndApproveRetiresExactlyThatSet(t *testing.T) { w, said := standingWorld(t) holding(w, 4) w.give() - w.passes(10 * time.Minute) + w.passes(15 * time.Minute) if len(w.a.removed) != 0 { t.Fatalf("four of four were retired without a person: %v", w.a.removed) } @@ -275,7 +292,7 @@ func TestTooManyWaitsForAPersonAndApproveRetiresExactlyThatSet(t *testing.T) { t.Fatal("the wait was not said") } // Said again every quarter of an hour while it waits. - w.passes(6 * time.Minute) + w.passes(11 * time.Minute) if got := strings.Join(retirements(*said), ","); got != "waiting,waiting" { t.Fatalf("%q", got) } @@ -309,7 +326,7 @@ func TestRejectedIsKeptAndNotAskedAgainUntilTheSetChanges(t *testing.T) { w, said := standingWorld(t) given := holding(w, 4) w.give() - w.passes(time.Minute) + w.settle() if _, err := w.h.Reject([]string{"c1"}, "no", "operator", ""); err == nil { t.Fatal("rejected a set that is not the one waiting") } @@ -331,7 +348,11 @@ func TestRejectedIsKeptAndNotAskedAgainUntilTheSetChanges(t *testing.T) { // One of them asked for again: a different answer, so the rejection is settled and counting // starts over — three of four is over the bound again, and waits again. w.give(given[0]) - w.passes(30 * time.Second) + w.pass() + if got := strings.Join(retirements(*said), ","); got != "waiting,rejected,settled" { + t.Fatalf("%q", got) + } + w.settle() if got := strings.Join(retirements(*said), ","); got != "waiting,rejected,settled,waiting" { t.Fatalf("%q", got) } @@ -339,7 +360,7 @@ func TestRejectedIsKeptAndNotAskedAgainUntilTheSetChanges(t *testing.T) { w2, _ := standingWorld(t) holding(w2, 4) w2.give() - w2.passes(time.Minute) + w2.settle() w2.h.Reject([]string{"c1", "c2", "c3", "c4"}, "wait", "operator", "") if done, err := w2.h.Approve(ctx, []string{"c1", "c2", "c3", "c4"}, "now", "operator", ""); err != nil || len(done) != 4 { t.Fatal(done, err) @@ -350,7 +371,7 @@ func TestAWaitingSetAskedForAgainSettles(t *testing.T) { w, said := standingWorld(t) given := holding(w, 4) w.give() - w.passes(time.Minute) + w.settle() w.give(given...) w.pass() if got := strings.Join(retirements(*said), ","); got != "waiting,settled" || len(w.a.removed) != 0 { @@ -365,7 +386,7 @@ func TestDeleteRemovesOnlyThatRetiredConsumer(t *testing.T) { w, said := standingWorld(t) given := holding(w, 5) w.give(given[:3]...) - w.passes(25 * time.Second) + w.settle() if strings.Join(w.a.removed, ",") != "c4,c5" { t.Fatalf("%v", w.a.removed) } @@ -426,9 +447,7 @@ func TestARestartRetiresWhatTheBackendHoldsUnaskedAndAdoptsWhatItFindsDisabled(t if strings.Join(w.a.removed, ",") != "locked-before" { t.Fatalf("adopting did not mark it: %v", w.a.removed) } - for i := 0; i < 5; i++ { - w.pass() - } + w.settle() if strings.Join(w.a.removed, ",") != "locked-before,orphan" { t.Fatalf("an orphan the backend holds was not retired: %v", w.a.removed) } @@ -444,7 +463,7 @@ func TestABackendThatCannotBeListedIsSaidAndAskedAgain(t *testing.T) { w.a.invErr = nil w.a.inv = Inventory{Active: []string{"c1", "orphan"}} w.pass() - w.passes(25 * time.Second) + w.settle() if strings.Join(w.a.removed, ",") != "orphan" { t.Fatalf("%v", w.a.removed) } @@ -454,7 +473,7 @@ func TestTheToolsAnswer(t *testing.T) { w, _ := standingWorld(t) holding(w, 4) w.give() - w.passes(time.Minute) + w.settle() tools := map[string]func(map[string]any) (any, error){} for _, tool := range RetirementTools(w.h) { tools[tool.Name] = tool.Run @@ -491,7 +510,7 @@ func TestAFailingConsumerRetiredIsSaidRecovered(t *testing.T) { w.a.failing = errors.New("boom") w.passes(7 * time.Minute) w.give(given[0]) - w.passes(25 * time.Second) + w.settle() recovered := false for _, a := range *said { if a.event == EventRecovered && a.body["consumer"] == "c2" && a.body["why"] == "retired" { diff --git a/modules/postgres/README.md b/modules/postgres/README.md index 290beb5..ff38e6b 100644 --- a/modules/postgres/README.md +++ b/modules/postgres/README.md @@ -39,8 +39,8 @@ for, so one removed by hand is installed again. ## What is never done - **No database is dropped by the mesh on its own.** A consumer the mesh no longer asks for is - *retired* (novox/hq ADR 0230), once the same result holds for five passes and, if it is more than - three consumers or more than half of those held, once a person approved: its login is set `NOLOGIN`, + *retired* (novox/hq ADR 0230), once the same result holds for five passes and ten minutes and, if + it is more than three consumers or more than half of those held, once a person approved: its login is set `NOLOGIN`, its sessions are ended, its role's comment marks it retired with when and why, and its database stays exactly as it was under its own name — still backed up. A consumer asked for again is enabled at once with the same database. Only `cleanup delete`, a person's act through the controller, drops it. diff --git a/modules/postgres/cmd/postgres-provider/harness.go b/modules/postgres/cmd/postgres-provider/harness.go index 6fb7bff..46d013d 100644 --- a/modules/postgres/cmd/postgres-provider/harness.go +++ b/modules/postgres/cmd/postgres-provider/harness.go @@ -21,7 +21,8 @@ package main // (novox/hq ADR 0230, the operator's model of 2026-10-06).** Retiring disables its access — reversibly — // and marks its login and data "to delete" with when and why; nothing is deleted. Asked for again, the // ordinary create re-enables it as it was. It is retired only once the same set has gone unasked in -// StablePasses consecutive passes, and a set larger than the bound waits for a person. retirement.go. +// StablePasses consecutive passes and for StableFor, and a set larger than the bound waits for a +// person. retirement.go. // // Carried, identical, by every Go provider until the Go SDK has the loop: postgres and keycloak. // Each module's `harness_same_test.go` fails when its copy and the other's differ (this file and @@ -92,11 +93,12 @@ type Harness struct { // missed the first hears the next, and a standing nobody repeats can be told from one that holds. FailingAfter time.Duration SayAgainEvery time.Duration - // StablePasses (5) is how many consecutive passes must see the same set no longer asked for before - // it is retired. RetireAtOnce (3) and RetireFraction (0.5) bound what is retired without a person: + // StablePasses (5) is how many consecutive passes must see the same set no longer asked for, and + // StableFor (10m) how long it must have held since the first of them, before it is retired. RetireAtOnce (3) and RetireFraction (0.5) bound what is retired without a person: // more consumers than RetireAtOnce, or — where more than one is held — a larger share of those held // than RetireFraction, waits for `retire approve` (novox/hq ADR 0230). StablePasses int + StableFor time.Duration RetireAtOnce int RetireFraction float64 @@ -202,6 +204,9 @@ func (h *Harness) init() { if h.StablePasses == 0 { h.StablePasses = 5 } + if h.StableFor == 0 { + h.StableFor = StableFor + } if h.RetireAtOnce == 0 { h.RetireAtOnce = 3 } diff --git a/modules/postgres/cmd/postgres-provider/harness_test.go b/modules/postgres/cmd/postgres-provider/harness_test.go index 3783362..7e4733b 100644 --- a/modules/postgres/cmd/postgres-provider/harness_test.go +++ b/modules/postgres/cmd/postgres-provider/harness_test.go @@ -78,13 +78,13 @@ func TestAConsumerNoLongerAskedForIsRetiredOnceStable(t *testing.T) { w.give(map[string]any{"as": "a"}, map[string]any{"as": "b"}) w.h.Reconcile(ctx) w.give(map[string]any{"as": "a"}) - w.passes(25 * time.Second) // five passes + w.settle() // five passes and ten minutes if strings.Join(w.a.removed, ",") != "b" { t.Fatal(w.a.removed) } // Only a file that says nobody asks retires the last one. w.give() - w.passes(25 * time.Second) + w.settle() if strings.Join(w.a.removed, ",") != "b,a" { t.Fatal(w.a.removed) } diff --git a/modules/postgres/cmd/postgres-provider/retirement.go b/modules/postgres/cmd/postgres-provider/retirement.go index 3268591..f15f181 100644 --- a/modules/postgres/cmd/postgres-provider/retirement.go +++ b/modules/postgres/cmd/postgres-provider/retirement.go @@ -12,10 +12,12 @@ package main // 3. DELETED, only through `cleanup delete`, a person's act, which this provider executes because it // owns its backend. // -// **Stable removals.** A consumer is retired only after the same set of consumers has gone unasked in -// StablePasses (5) consecutive passes that read the file. A pass that could not read it is not a -// result and starts the count again; so does a different set. At a pass every five seconds that is -// twenty seconds after the first pass that saw the set. Additions and changes are never delayed. +// **Stable removals.** A consumer is retired only once the same set of consumers has gone unasked BOTH +// in StablePasses (5) consecutive passes that read the file AND for at least StableFor (10 minutes) +// since the first pass that saw it. Five passes alone are twenty-five seconds — shorter than a +// controller restart, a store reconnecting or a file half written. A pass that could not read the file +// is not a result and starts both again; so does a different set. Additions and changes are never +// delayed. // // **Too many is a person.** A stable set of more than RetireAtOnce (3), or — where more than one is // held — of more than RetireFraction (half) of those held, retires nothing: it WAITS, said in the @@ -62,6 +64,10 @@ const ( ChangeAdopted = "adopted" // found disabled without the mark, now retired on record ) +// StableFor is the least time the same unasked set must hold before it is retired (novox/hq ADR 0230): +// longer than a controller restart, a store reconnecting or a file half written. +const StableFor = 10 * time.Minute + // KindConsumer is a retired consumer. A backend may list other things set aside for deletion under a // word of its own (postgres: a database renamed aside). const KindConsumer = "consumer" @@ -139,7 +145,7 @@ func (h *Harness) seed(ctx context.Context, want map[string]bool) { // No hash: asked for again, it is applied again, which is harmless and marks it. h.applied[as] = appliedEntry{} h.say("%s: held by the backend and no longer asked for; retired once that holds for %d passes "+ - "(novox/hq ADR 0230)", as, h.StablePasses) + "and %s (novox/hq ADR 0230)", as, h.StablePasses, h.StableFor) } } now := h.Now() @@ -190,20 +196,20 @@ func (h *Harness) retireUnasked(ctx context.Context, want map[string]bool) { h.settle("the set the mesh no longer asks for changed") } h.r.key, h.r.count, h.r.firstSeen = key, 1, now - h.say("no longer asked for: %s; retired once the same set holds for %d passes", strings.Join(unasked, ", "), - h.StablePasses) + h.say("no longer asked for: %s; retired once the same set holds for %d passes and %s", + strings.Join(unasked, ", "), h.StablePasses, h.StableFor) default: h.r.count++ } - if h.r.rejected != nil || h.r.count < h.StablePasses { + if h.r.rejected != nil || h.r.count < h.StablePasses || now.Sub(h.r.firstSeen) < h.StableFor { return } if h.overTheBound(len(unasked), len(h.applied)) { h.wait(unasked) return } - why := fmt.Sprintf("the mesh stopped asking for it: the same result in %d consecutive passes from %s", - h.StablePasses, stamp(h.r.firstSeen)) + why := fmt.Sprintf("the mesh stopped asking for it: the same result in %d consecutive passes over %s, from %s", + h.r.count, now.Sub(h.r.firstSeen).Round(time.Second), stamp(h.r.firstSeen)) h.retire(ctx, unasked, why, nil) } @@ -416,7 +422,7 @@ func (h *Harness) Retirement(ctx context.Context) (map[string]any, error) { "retired_at": stampOr(r.RetiredAt), "why": r.Why, "size_bytes": r.SizeBytes, "kind": kindOf(r)}) } out := map[string]any{"resource": h.Resource, "node": h.Node, "held": orEmptyList(held), - "stable_passes": h.StablePasses, "bound": h.bound(len(h.applied)), "waiting": nil, "rejected": nil, + "stable_passes": h.StablePasses, "stable_for_seconds": int(h.StableFor.Seconds()), "bound": h.bound(len(h.applied)), "waiting": nil, "rejected": nil, "retired": retired} if w := h.r.waiting; w != nil { out["waiting"] = map[string]any{"consumers": h.named(w.set), "since": stamp(w.since), "held": w.held} diff --git a/modules/postgres/cmd/postgres-provider/retirement_test.go b/modules/postgres/cmd/postgres-provider/retirement_test.go index 3894d47..cad4850 100644 --- a/modules/postgres/cmd/postgres-provider/retirement_test.go +++ b/modules/postgres/cmd/postgres-provider/retirement_test.go @@ -140,6 +140,9 @@ func namesOf(body map[string]any) string { return strings.Join(out, ",") } +// settle passes every five seconds until the same answer has held for StableFor, and one pass more. +func (w *world) settle() { w.passes(StableFor + 5*time.Second) } + func TestATransientEmptyListForFourPassesRetiresNothing(t *testing.T) { w, said := standingWorld(t) given := holding(w, 1) @@ -152,33 +155,55 @@ func TestATransientEmptyListForFourPassesRetiresNothing(t *testing.T) { if len(w.a.removed) != 0 || len(retirements(*said)) != 0 { t.Fatalf("four passes retired %v and said %v", w.a.removed, retirements(*said)) } - // Gone again: counted from one, not from where the transient left off. - w.give() - for i := 0; i < 4; i++ { - w.pass() - } - if len(w.a.removed) != 0 { - t.Fatalf("retired after four passes: %v", w.a.removed) - } - w.pass() - if strings.Join(w.a.removed, ",") != "c1" { - t.Fatalf("the fifth pass did not retire: %v", w.a.removed) - } } -func TestFiveStablePassesRetireAndAskedAgainReEnables(t *testing.T) { +// Five identical passes are twenty-five seconds — shorter than a controller restart. Both must hold: +// five passes AND ten minutes of the same answer (novox/hq ADR 0230). +func TestFivePassesInTwentyFiveSecondsRetireNothingTenMinutesDo(t *testing.T) { w, said := standingWorld(t) - given := holding(w, 3) - w.give(given[:2]...) + holding(w, 1) + w.give() for i := 0; i < 5; i++ { w.pass() } + if len(w.a.removed) != 0 || len(retirements(*said)) != 0 { + t.Fatalf("five passes in 25 s retired %v", w.a.removed) + } + w.passes(StableFor - 30*time.Second) + if len(w.a.removed) != 0 { + t.Fatalf("retired before the set held %s: %v", StableFor, w.a.removed) + } + w.passes(time.Minute) + if strings.Join(w.a.removed, ",") != "c1" { + t.Fatalf("the same set held %s and was not retired: %v", StableFor, w.a.removed) + } + // Ten minutes in one slow pass are not five passes. + w2 := newWorld(t) + holding(w2, 1) + w2.give() + w2.pass() + w2.now = w2.now.Add(StableFor) + w2.pass() + if len(w2.a.removed) != 0 { + t.Fatalf("two passes ten minutes apart retired %v", w2.a.removed) + } + w2.passes(15 * time.Second) + if len(w2.a.removed) != 1 { + t.Fatalf("five passes over ten minutes did not retire: %v", w2.a.removed) + } +} + +func TestAStableSetIsRetiredAndAskedAgainReEnables(t *testing.T) { + w, said := standingWorld(t) + given := holding(w, 3) + w.give(given[:2]...) + w.settle() if strings.Join(w.a.removed, ",") != "c3" { t.Fatalf("retired %v", w.a.removed) } body := lastRetirement(t, *said) if body["change"] != ChangeRetired || namesOf(body) != "c3" || body["held"] != 3 || body["provider-node"] != "anchor" || - !strings.Contains(body["why"].(string), "5 consecutive passes") { + !strings.Contains(body["why"].(string), "consecutive passes over 10m") { t.Fatalf("%v", body) } if len(w.a.inv.Retired) != 1 || w.a.inv.Retired[0].Consumer != "c3" { @@ -203,21 +228,17 @@ func TestAnUnreadablePassStartsTheCountAgain(t *testing.T) { w := newWorld(t) holding(w, 1) w.give() - for i := 0; i < 3; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) os.WriteFile(w.receives, []byte("{"), 0o600) w.pass() w.give() - for i := 0; i < 4; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) if len(w.a.removed) != 0 { t.Fatalf("an unreadable pass counted: %v", w.a.removed) } - w.pass() + w.passes(2 * time.Minute) if len(w.a.removed) != 1 { - t.Fatalf("not retired after five readable passes: %v", w.a.removed) + t.Fatalf("not retired after ten minutes of readable passes: %v", w.a.removed) } } @@ -225,17 +246,13 @@ func TestADifferentSetStartsTheCountAgain(t *testing.T) { w := newWorld(t) given := holding(w, 5) w.give(given[:4]...) - for i := 0; i < 3; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) w.give(given[:3]...) - for i := 0; i < 4; i++ { - w.pass() - } + w.passes(StableFor - time.Minute) if len(w.a.removed) != 0 { t.Fatalf("a changed set kept its count: %v", w.a.removed) } - w.pass() + w.passes(2 * time.Minute) if strings.Join(w.a.removed, ",") != "c4,c5" { t.Fatalf("%v", w.a.removed) } @@ -260,7 +277,7 @@ func TestTooManyWaitsForAPersonAndApproveRetiresExactlyThatSet(t *testing.T) { w, said := standingWorld(t) holding(w, 4) w.give() - w.passes(10 * time.Minute) + w.passes(15 * time.Minute) if len(w.a.removed) != 0 { t.Fatalf("four of four were retired without a person: %v", w.a.removed) } @@ -275,7 +292,7 @@ func TestTooManyWaitsForAPersonAndApproveRetiresExactlyThatSet(t *testing.T) { t.Fatal("the wait was not said") } // Said again every quarter of an hour while it waits. - w.passes(6 * time.Minute) + w.passes(11 * time.Minute) if got := strings.Join(retirements(*said), ","); got != "waiting,waiting" { t.Fatalf("%q", got) } @@ -309,7 +326,7 @@ func TestRejectedIsKeptAndNotAskedAgainUntilTheSetChanges(t *testing.T) { w, said := standingWorld(t) given := holding(w, 4) w.give() - w.passes(time.Minute) + w.settle() if _, err := w.h.Reject([]string{"c1"}, "no", "operator", ""); err == nil { t.Fatal("rejected a set that is not the one waiting") } @@ -331,7 +348,11 @@ func TestRejectedIsKeptAndNotAskedAgainUntilTheSetChanges(t *testing.T) { // One of them asked for again: a different answer, so the rejection is settled and counting // starts over — three of four is over the bound again, and waits again. w.give(given[0]) - w.passes(30 * time.Second) + w.pass() + if got := strings.Join(retirements(*said), ","); got != "waiting,rejected,settled" { + t.Fatalf("%q", got) + } + w.settle() if got := strings.Join(retirements(*said), ","); got != "waiting,rejected,settled,waiting" { t.Fatalf("%q", got) } @@ -339,7 +360,7 @@ func TestRejectedIsKeptAndNotAskedAgainUntilTheSetChanges(t *testing.T) { w2, _ := standingWorld(t) holding(w2, 4) w2.give() - w2.passes(time.Minute) + w2.settle() w2.h.Reject([]string{"c1", "c2", "c3", "c4"}, "wait", "operator", "") if done, err := w2.h.Approve(ctx, []string{"c1", "c2", "c3", "c4"}, "now", "operator", ""); err != nil || len(done) != 4 { t.Fatal(done, err) @@ -350,7 +371,7 @@ func TestAWaitingSetAskedForAgainSettles(t *testing.T) { w, said := standingWorld(t) given := holding(w, 4) w.give() - w.passes(time.Minute) + w.settle() w.give(given...) w.pass() if got := strings.Join(retirements(*said), ","); got != "waiting,settled" || len(w.a.removed) != 0 { @@ -365,7 +386,7 @@ func TestDeleteRemovesOnlyThatRetiredConsumer(t *testing.T) { w, said := standingWorld(t) given := holding(w, 5) w.give(given[:3]...) - w.passes(25 * time.Second) + w.settle() if strings.Join(w.a.removed, ",") != "c4,c5" { t.Fatalf("%v", w.a.removed) } @@ -426,9 +447,7 @@ func TestARestartRetiresWhatTheBackendHoldsUnaskedAndAdoptsWhatItFindsDisabled(t if strings.Join(w.a.removed, ",") != "locked-before" { t.Fatalf("adopting did not mark it: %v", w.a.removed) } - for i := 0; i < 5; i++ { - w.pass() - } + w.settle() if strings.Join(w.a.removed, ",") != "locked-before,orphan" { t.Fatalf("an orphan the backend holds was not retired: %v", w.a.removed) } @@ -444,7 +463,7 @@ func TestABackendThatCannotBeListedIsSaidAndAskedAgain(t *testing.T) { w.a.invErr = nil w.a.inv = Inventory{Active: []string{"c1", "orphan"}} w.pass() - w.passes(25 * time.Second) + w.settle() if strings.Join(w.a.removed, ",") != "orphan" { t.Fatalf("%v", w.a.removed) } @@ -454,7 +473,7 @@ func TestTheToolsAnswer(t *testing.T) { w, _ := standingWorld(t) holding(w, 4) w.give() - w.passes(time.Minute) + w.settle() tools := map[string]func(map[string]any) (any, error){} for _, tool := range RetirementTools(w.h) { tools[tool.Name] = tool.Run @@ -491,7 +510,7 @@ func TestAFailingConsumerRetiredIsSaidRecovered(t *testing.T) { w.a.failing = errors.New("boom") w.passes(7 * time.Minute) w.give(given[0]) - w.passes(25 * time.Second) + w.settle() recovered := false for _, a := range *said { if a.event == EventRecovered && a.body["consumer"] == "c2" && a.body["why"] == "retired" {