From d5cf3b42fec6f5bce99dc6dd37754cee2ce96246 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 03:05:34 +0200 Subject: [PATCH] Say a waiting provider in the operator's words, and which state it is when a consumer is held (issue 405 review) --- cmd/mesh-controller/module_health.go | 17 ++++--- cmd/mesh-controller/waiting_provider_test.go | 49 +++++++++++++++++++- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/cmd/mesh-controller/module_health.go b/cmd/mesh-controller/module_health.go index 3d273781..2a65f06a 100644 --- a/cmd/mesh-controller/module_health.go +++ b/cmd/mesh-controller/module_health.go @@ -239,9 +239,14 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi } o := moduleUnhealthyObservation(m, node, unhealthy[m]) if hold != nil { - if p, held := hold.heldUnder(node, m, unhealthy[m]); held { - // Held under the provider's condition: nothing of its own, and the provider's says it waits. - heldOn[o.Key()] = p.Module + " on " + p.Node + if by, held := hold.heldWith(node, m, unhealthy[m]); held { + // Held under the provider's condition: nothing of its own, and the provider's says it waits. The + // clearing line says which the provider is: unhealthy, or waiting for the operator (issue 405). + p := by.provider + heldOn[o.Key()] = p.Module + " on " + p.Node + ", which is unhealthy" + if len(by.waits) > 0 { + heldOn[o.Key()] = p.Module + " on " + p.Node + ", which waits for you" + } providers[p] = true continue } @@ -249,8 +254,8 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi // A provider that waits for the operator for a part this finding cannot be matched to does not hold it // (novox/hq issue 405): raised as its own, and saying the provider waits, so neither is hidden. if p, waiting := hold.waitingUncovered(node, m, unhealthy[m]); waiting { - o.Said += fmt.Sprintf("; its provider %s on %s waits for the operator, for a part not known to be "+ - "the one %s needs, so this is said as its own", p.Module, p.Node, m) + o.Said += fmt.Sprintf("; %s on %s waits for you, but not for anything %s is known to need, so %s's "+ + "fault is said on its own", p.Module, p.Node, m, m) } } seen[o.Key()] = true @@ -282,7 +287,7 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi why = fmt.Sprintf("%s says %s no longer waits for the operator", node, module) } if on, held := heldOn[key]; held { - why = fmt.Sprintf("what %s finds on %s waits on %s, which is unhealthy or waits for the operator: held under its condition", module, node, on) + why = fmt.Sprintf("what %s finds on %s waits on %s: held under its condition", module, node, on) } // **A condition that became the other kind** is not "working again" (issue 318 review): its clearing // line says what it became. diff --git a/cmd/mesh-controller/waiting_provider_test.go b/cmd/mesh-controller/waiting_provider_test.go index 30d0357b..7ab5e780 100644 --- a/cmd/mesh-controller/waiting_provider_test.go +++ b/cmd/mesh-controller/waiting_provider_test.go @@ -216,7 +216,7 @@ func TestAConsumerOfAnotherPartIsRaisedOnItsOwn(t *testing.T) { if consumer == nil || conditionOf(open, needsOperatorKey("db", "anchor")) == nil { t.Fatalf("raised %v; want shop's own unhealthy beside db's needs-operator", openKeysOf(open)) } - if said := consumer.Evidence[0].Said; !strings.Contains(said, "its provider db on anchor waits for the operator") { + if said := consumer.Evidence[0].Said; !strings.Contains(said, "db on anchor waits for you, but not for anything shop is known to need, so shop's fault is said on its own") { t.Fatalf("the consumer's condition does not say its provider waits: %s", said) } } @@ -326,3 +326,50 @@ func openKeysOf(cs []conditions.Condition) []string { } return out } + +// A consumer raised on its own, whose provider then waits for the operator for the part it needs, is cleared saying +// which the provider is: it waits for you, not that it is unhealthy (novox/hq issue 405 review). +func TestAConsumerHeldOnceItsProviderWaitsSaysWhichItIs(t *testing.T) { + k, _ := withConditionsInMemory(t) + ctx := t.Context() + start := time.Now().Add(-time.Second) + healthy := waitingDatabase() + healthy.State, healthy.Waits = link.StateHealthy, nil + healths := shopFailingBeside(healthy) + was := readHoldingFor + t.Cleanup(func() { readHoldingFor = was }) + readHoldingFor = func(_ context.Context, _ *inventory.Inventory, open []conditions.Condition) (*holding, error) { + return holdingOf(open, healths, shopOnTheDatabase), nil + } + shop := map[string][]inventory.ResourceHealth{"shop": {failingConsumer("shop")}} + for look := 1; look <= 2; look++ { + if err := judgeModuleHealth(ctx, nil, k, "laptop", shop, map[string]int{"shop": look}, time.Now()); err != nil { + t.Fatal(err) + } + } + if open, _ := k.Open(ctx); conditionOf(open, moduleUnhealthyKey("shop", "laptop")) == nil { + t.Fatalf("shop beside a healthy provider is not raised: %v", openKeysOf(open)) + } + healths = shopFailingBeside(waitingDatabase()) + if err := judgeModuleHealth(ctx, nil, k, "laptop", shop, map[string]int{"shop": 3}, time.Now()); err != nil { + t.Fatal(err) + } + var why string + for deadline := time.Now().Add(2 * time.Second); why == "" && time.Now().Before(deadline); { + events, err := k.HistorySince(ctx, start) + if err != nil { + t.Fatal(err) + } + for _, e := range events { + if e.Key == moduleUnhealthyKey("shop", "laptop") && e.Change == conditions.ChangeCleared { + why = e.Why + } + } + if why == "" { + time.Sleep(10 * time.Millisecond) + } + } + if !strings.Contains(why, "waits on db on anchor, which waits for you") { + t.Fatalf("shop's clearing says %q; want it to say db on anchor waits for you", why) + } +}