From d7a880a608789cd0f8c6022f3bdddd9af751c814 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Fri, 16 Oct 2026 16:42:39 +0000 Subject: [PATCH] Say once after a restart that nothing waits, so an old retirement condition clears (issue 937) The controller clears a provider's retire-waiting condition only on its settled word, and a provider says settled only for a set its own memory held. A provider restarted after the set stopped waiting (here: the delivery of the grant-file fix) never says it, so the condition kept asking the operator to approve retiring what nobody waits on. The first pass of a process that finds every consumer asked now says settled once, with no set; the controller clears both conditions on it. --- .../cmd/keycloak-provider/retirement.go | 11 +++++ .../cmd/keycloak-provider/retirement_test.go | 48 +++++++++++++++++++ .../cmd/keycloak-provider/standing_test.go | 12 +++-- .../cmd/postgres-provider/retirement.go | 11 +++++ .../cmd/postgres-provider/retirement_test.go | 48 +++++++++++++++++++ .../cmd/postgres-provider/standing_test.go | 12 +++-- 6 files changed, 136 insertions(+), 6 deletions(-) diff --git a/modules/keycloak/cmd/keycloak-provider/retirement.go b/modules/keycloak/cmd/keycloak-provider/retirement.go index 3c3f578b..f48afde8 100644 --- a/modules/keycloak/cmd/keycloak-provider/retirement.go +++ b/modules/keycloak/cmd/keycloak-provider/retirement.go @@ -105,6 +105,9 @@ type retirement struct { lastWant map[string]bool seeded bool seedSaid string + // noneSaid is whether this process has said that nothing waits: an earlier process may have left a set + // waiting that this one never knew (issue 937). + noneSaid bool } type waitingSet struct { @@ -257,7 +260,15 @@ func (h *Harness) settle(why string) { set = h.r.waiting.set case h.r.rejected != nil: set = h.r.rejected.set + case h.r.noneSaid: + return default: + // A restart forgets a set an earlier process waited on, and so would never say it ended: the + // controller's condition would ask a person to approve what nobody waits on any more (issue 937). + // The first time this process finds every consumer held asked for, it says so once, with no set. + h.r.noneSaid = true + h.say("nothing waits for a person: %s", why) + h.announce(EventRetirement, h.retirementBody(ChangeSettled, []map[string]any{}, map[string]any{"why": why})) return } h.r.waiting, h.r.rejected = nil, nil diff --git a/modules/keycloak/cmd/keycloak-provider/retirement_test.go b/modules/keycloak/cmd/keycloak-provider/retirement_test.go index cad4850f..407d4c2f 100644 --- a/modules/keycloak/cmd/keycloak-provider/retirement_test.go +++ b/modules/keycloak/cmd/keycloak-provider/retirement_test.go @@ -121,6 +121,11 @@ func retirements(said []announced) []string { return out } +func noneWaits(a announced) bool { + c, _ := a.body["consumers"].([]map[string]any) + return a.body["change"] == ChangeSettled && len(c) == 0 +} + func lastRetirement(t *testing.T, said []announced) map[string]any { t.Helper() for i := len(said) - 1; i >= 0; i-- { @@ -521,3 +526,46 @@ func TestAFailingConsumerRetiredIsSaidRecovered(t *testing.T) { t.Fatalf("%v", *said) } } + +// everyWord is a world that keeps every word the provider announces. +func everyWord(t *testing.T) (*world, *[]announced) { + w := newWorld(t) + var said []announced + w.h.Announce = func(e string, b map[string]any) { said = append(said, announced{e, b}) } + w.h.Node = "anchor" + return w, &said +} + +// A restarted provider that finds every consumer it holds asked for says once that nothing waits, so the controller +// clears a waiting condition an earlier process raised and this one never knew (issue 937); it says it no more. +func TestARestartedProviderSaysOnceThatNothingWaits(t *testing.T) { + w, said := everyWord(t) + holding(w, 2) + w.pass() + w.pass() + var words []announced + for _, a := range *said { + if a.event == EventRetirement { + words = append(words, a) + } + } + if len(words) != 1 || !noneWaits(words[0]) || words[0].body["provider-node"] != w.h.Node { + t.Fatalf("said %+v, want one settled with no set", words) + } +} + +// A provider whose own set waits does not say that nothing waits. +func TestAProviderThatWaitsDoesNotSayNothingWaits(t *testing.T) { + w, said := everyWord(t) + w.a.inv.Active = []string{"c1", "c2", "c3", "c4", "c5"} + w.give(map[string]any{"as": "c1", "node": "anchor"}) + w.settle() + for _, a := range *said { + if a.event == EventRetirement && noneWaits(a) { + t.Fatalf("said nothing waits while %v waited", w.h.r.waiting) + } + } + if w.h.r.waiting == nil { + t.Fatal("nothing waited: the test does not hold the case") + } +} diff --git a/modules/keycloak/cmd/keycloak-provider/standing_test.go b/modules/keycloak/cmd/keycloak-provider/standing_test.go index 28d50b40..870bb95a 100644 --- a/modules/keycloak/cmd/keycloak-provider/standing_test.go +++ b/modules/keycloak/cmd/keycloak-provider/standing_test.go @@ -21,8 +21,10 @@ func standingWorld(t *testing.T) (*world, *[]announced) { w := newWorld(t) var said []announced w.h.Announce = func(e string, b map[string]any) { - // The first success since start is its own test's; every other test reads past it. - if b["why"] != "first-success" { + // The first success since start, and the first word that nothing waits (issue 937), are their own + // tests'; every other test reads past them. + if c, _ := b["consumers"].([]map[string]any); b["why"] != "first-success" && + !(e == EventRetirement && b["change"] == ChangeSettled && len(c) == 0) { said = append(said, announced{e, b}) } } @@ -178,7 +180,11 @@ func (classing) Class(error) string { return "its-own" } func TestTheFirstSuccessSinceStartIsAnnouncedOnce(t *testing.T) { w := newWorld(t) var said []announced - w.h.Announce = func(e string, b map[string]any) { said = append(said, announced{e, b}) } + w.h.Announce = func(e string, b map[string]any) { + if e != EventRetirement { + said = append(said, announced{e, b}) + } + } w.give(map[string]any{"as": "a"}) w.passes(3 * time.Minute) if events(said) != EventRecovered || said[0].body["why"] != "first-success" || said[0].body["consumer"] != "a" { diff --git a/modules/postgres/cmd/postgres-provider/retirement.go b/modules/postgres/cmd/postgres-provider/retirement.go index 3c3f578b..f48afde8 100644 --- a/modules/postgres/cmd/postgres-provider/retirement.go +++ b/modules/postgres/cmd/postgres-provider/retirement.go @@ -105,6 +105,9 @@ type retirement struct { lastWant map[string]bool seeded bool seedSaid string + // noneSaid is whether this process has said that nothing waits: an earlier process may have left a set + // waiting that this one never knew (issue 937). + noneSaid bool } type waitingSet struct { @@ -257,7 +260,15 @@ func (h *Harness) settle(why string) { set = h.r.waiting.set case h.r.rejected != nil: set = h.r.rejected.set + case h.r.noneSaid: + return default: + // A restart forgets a set an earlier process waited on, and so would never say it ended: the + // controller's condition would ask a person to approve what nobody waits on any more (issue 937). + // The first time this process finds every consumer held asked for, it says so once, with no set. + h.r.noneSaid = true + h.say("nothing waits for a person: %s", why) + h.announce(EventRetirement, h.retirementBody(ChangeSettled, []map[string]any{}, map[string]any{"why": why})) return } h.r.waiting, h.r.rejected = nil, nil diff --git a/modules/postgres/cmd/postgres-provider/retirement_test.go b/modules/postgres/cmd/postgres-provider/retirement_test.go index cad4850f..407d4c2f 100644 --- a/modules/postgres/cmd/postgres-provider/retirement_test.go +++ b/modules/postgres/cmd/postgres-provider/retirement_test.go @@ -121,6 +121,11 @@ func retirements(said []announced) []string { return out } +func noneWaits(a announced) bool { + c, _ := a.body["consumers"].([]map[string]any) + return a.body["change"] == ChangeSettled && len(c) == 0 +} + func lastRetirement(t *testing.T, said []announced) map[string]any { t.Helper() for i := len(said) - 1; i >= 0; i-- { @@ -521,3 +526,46 @@ func TestAFailingConsumerRetiredIsSaidRecovered(t *testing.T) { t.Fatalf("%v", *said) } } + +// everyWord is a world that keeps every word the provider announces. +func everyWord(t *testing.T) (*world, *[]announced) { + w := newWorld(t) + var said []announced + w.h.Announce = func(e string, b map[string]any) { said = append(said, announced{e, b}) } + w.h.Node = "anchor" + return w, &said +} + +// A restarted provider that finds every consumer it holds asked for says once that nothing waits, so the controller +// clears a waiting condition an earlier process raised and this one never knew (issue 937); it says it no more. +func TestARestartedProviderSaysOnceThatNothingWaits(t *testing.T) { + w, said := everyWord(t) + holding(w, 2) + w.pass() + w.pass() + var words []announced + for _, a := range *said { + if a.event == EventRetirement { + words = append(words, a) + } + } + if len(words) != 1 || !noneWaits(words[0]) || words[0].body["provider-node"] != w.h.Node { + t.Fatalf("said %+v, want one settled with no set", words) + } +} + +// A provider whose own set waits does not say that nothing waits. +func TestAProviderThatWaitsDoesNotSayNothingWaits(t *testing.T) { + w, said := everyWord(t) + w.a.inv.Active = []string{"c1", "c2", "c3", "c4", "c5"} + w.give(map[string]any{"as": "c1", "node": "anchor"}) + w.settle() + for _, a := range *said { + if a.event == EventRetirement && noneWaits(a) { + t.Fatalf("said nothing waits while %v waited", w.h.r.waiting) + } + } + if w.h.r.waiting == nil { + t.Fatal("nothing waited: the test does not hold the case") + } +} diff --git a/modules/postgres/cmd/postgres-provider/standing_test.go b/modules/postgres/cmd/postgres-provider/standing_test.go index f286f286..a6b416f4 100644 --- a/modules/postgres/cmd/postgres-provider/standing_test.go +++ b/modules/postgres/cmd/postgres-provider/standing_test.go @@ -21,8 +21,10 @@ func standingWorld(t *testing.T) (*world, *[]announced) { w := newWorld(t) var said []announced w.h.Announce = func(e string, b map[string]any) { - // The first success since start is its own test's; every other test reads past it. - if b["why"] != "first-success" { + // The first success since start, and the first word that nothing waits (issue 937), are their own + // tests'; every other test reads past them. + if c, _ := b["consumers"].([]map[string]any); b["why"] != "first-success" && + !(e == EventRetirement && b["change"] == ChangeSettled && len(c) == 0) { said = append(said, announced{e, b}) } } @@ -178,7 +180,11 @@ func (classing) Class(error) string { return "its-own" } func TestTheFirstSuccessSinceStartIsAnnouncedOnce(t *testing.T) { w := newWorld(t) var said []announced - w.h.Announce = func(e string, b map[string]any) { said = append(said, announced{e, b}) } + w.h.Announce = func(e string, b map[string]any) { + if e != EventRetirement { + said = append(said, announced{e, b}) + } + } w.give(map[string]any{"as": "a"}) w.passes(3 * time.Minute) if events(said) != EventRecovered || said[0].body["why"] != "first-success" || said[0].body["consumer"] != "a" { -- 2.54.0