Keep a waiting provider's hold within ADR 0283 (issue 405 review)

A consumer held under a provider that only waits for the operator now
passes its gate as a wait for a person, carrying the wait, so no walk
waits on the operator's secret. Only consumers of the waiting part are
held; one that cannot be matched is raised on its own and says its
provider waits. needs-operator stays a warning while consumers wait.

cmd/mesh-controller/provider_hold.go

@@ -3,7 +3,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"slices"
 	"sort"
 	"strings"
 
@@ -116,45 +115,137 @@ func (h *holding) lookUpProvider(machine, consumer, provision string) (catalogue
 	return catalogue.Chosen{}, false
 }
 
-// unhealthy says a provider is not healthy on the record: its unhealthy or needs-operator condition is open, or its
-// machine's newest statement says a resource of it is unhealthy or waiting. **A provider waiting for the operator
-// holds its consumers too** (novox/hq issue 405): its secret or setting not given, what its consumers find is no more
-// theirs than when it is broken, and ADR 0240 rule 5 says it once, at the provider — here under its needs-operator
-// condition, which names the act.
-func (h *holding) unhealthy(p catalogue.Chosen) bool {
-	keys := []string{moduleUnhealthyKey(p.Module, p.Node), needsOperatorKey(p.Module, p.Node)}
+// providerState is how a provider stands on the record: unhealthy when its unhealthy condition is open or its
+// machine's newest statement says a resource of it is unhealthy; else waiting for the operator when that statement
+// says a resource of it waits, with the waits it names, or its needs-operator condition is open (waits then
+// unknown); else healthy.
+func (h *holding) providerState(p catalogue.Chosen) (unhealthy, waiting bool, waits []inventory.Wait) {
 	for _, c := range h.open {
-		if slices.Contains(keys, c.Key) {
-			return true
+		if c.Key == moduleUnhealthyKey(p.Module, p.Node) {
+			return true, false, nil
 		}
 	}
 	for _, r := range h.healths[p.Node].Resources {
-		if r.Module == p.Module && (r.State == link.StateUnhealthy || r.State == link.StateWaiting) {
-			return true
+		if r.Module != p.Module {
+			continue
+		}
+		switch r.State {
+		case link.StateUnhealthy:
+			return true, false, nil
+		case link.StateWaiting:
+			waiting = true
+			waits = append(waits, r.Waits...)
 		}
 	}
-	return false
+	if !waiting {
+		for _, c := range h.open {
+			waiting = waiting || c.Key == needsOperatorKey(p.Module, p.Node)
+		}
+	}
+	return false, waiting, waits
+}
+
+// manifestOf is a module's manifest from the catalogue, read once; false when it cannot be read.
+func (h *holding) manifestOf(module string) (catalogue.Manifest, bool) {
+	if h.shelf == nil && h.inv != nil {
+		shelf, err := h.inv.Catalogue(h.ctx)
+		if err != nil {
+			return catalogue.Manifest{}, false
+		}
+		h.shelf = shelf
+	}
+	m, ok := h.shelf[module]
+	return m, ok
+}
+
+// covering is the waits of a provider that cover a provision it gives (novox/hq issue 405): a module that waits
+// says nothing else of it is wrong and names each part that waits (ADR 0283 decision 1), so only a consumer of
+// the waiting part waits on it. A wait covers a provision when its part is that provision, or the secret it waits
+// for is the provision's shared credential (ADR 0158). Nothing when no wait can be matched: a consumer failing
+// then is not held, since holding it would hide a fault that may be its own.
+func (h *holding) covering(p catalogue.Chosen, provision string, waits []inventory.Wait) []inventory.Wait {
+	credential := ""
+	if m, ok := h.manifestOf(p.Module); ok {
+		credential, _ = m.SharedCredentialOf(provision)
+	}
+	var out []inventory.Wait
+	for _, w := range waits {
+		if w.Part == provision || (w.Secret != "" && w.Secret == credential) {
+			out = append(out, w)
+		}
+	}
+	return out
+}
+
+// heldUnderProvider is the provider a consumer's findings are held under, and — when that provider only waits for the
+// operator, for the part the consumer needs — the waits that hold it.
+type heldUnderProvider struct {
+	provider catalogue.Chosen
+	// waits is set when every finding held waits on a provider that only waits for the operator: the consumer's
+	// gate then reads as ADR 0254's waits for a person, a pass with the wait carried (ADR 0283 decision 4).
+	waits []inventory.Wait
 }
 
 // heldUnder is the provider a consumer's unhealthy resources wait on: when every one of them is a finding
-// of a check naming a provision whose provider for this consumer is unhealthy on the record. False when any
-// is the consumer's own.
+// of a check naming a provision whose provider for this consumer is unhealthy on the record, or waits for the
+// operator for the part that gives it (novox/hq issue 405). False when any is the consumer's own.
 func (h *holding) heldUnder(machine, module string, rs []inventory.ResourceHealth) (catalogue.Chosen, bool) {
-	var on catalogue.Chosen
+	by, held := h.heldWith(machine, module, rs)
+	return by.provider, held
+}
+
+func (h *holding) heldWith(machine, module string, rs []inventory.ResourceHealth) (heldUnderProvider, bool) {
+	var on heldUnderProvider
+	onlyWaits := true
 	for _, r := range rs {
 		if r.State != link.StateUnhealthy {
 			continue
 		}
 		if !heldFinding(r) {
-			return catalogue.Chosen{}, false
+			return heldUnderProvider{}, false
 		}
 		p, ok := h.providerFor(machine, module, r.Needs)
-		if !ok || (p.Node == machine && p.Module == module) || !h.unhealthy(p) {
-			return catalogue.Chosen{}, false
+		if !ok || (p.Node == machine && p.Module == module) {
+			return heldUnderProvider{}, false
 		}
-		on = p
+		unhealthy, waiting, waits := h.providerState(p)
+		switch {
+		case unhealthy:
+			onlyWaits = false
+		case waiting:
+			covered := h.covering(p, r.Needs, waits)
+			if len(covered) == 0 {
+				return heldUnderProvider{}, false
+			}
+			on.waits = append(on.waits, covered...)
+		default:
+			return heldUnderProvider{}, false
+		}
+		on.provider = p
 	}
-	return on, on.Module != ""
+	if !onlyWaits {
+		on.waits = nil
+	}
+	return on, on.provider.Module != ""
+}
+
+// waitingUncovered is a provider of a consumer's failing finding that waits for the operator for a part the
+// finding cannot be matched to (novox/hq issue 405): the consumer is raised on its own, and its condition says
+// the provider waits, so neither is hidden.
+func (h *holding) waitingUncovered(machine, module string, rs []inventory.ResourceHealth) (catalogue.Chosen, bool) {
+	for _, r := range rs {
+		if r.State != link.StateUnhealthy || !heldFinding(r) {
+			continue
+		}
+		p, ok := h.providerFor(machine, module, r.Needs)
+		if !ok || (p.Node == machine && p.Module == module) {
+			continue
+		}
+		if unhealthy, waiting, waits := h.providerState(p); !unhealthy && waiting && len(h.covering(p, r.Needs, waits)) == 0 {
+			return p, true
+		}
+	}
+	return catalogue.Chosen{}, false
 }
 
 // waitersOn is every consumer held under a provider, as "<module> on <machine>", sorted.
@@ -178,8 +269,8 @@ func (h *holding) waitersOn(p catalogue.Chosen) []string {
 }
 
 // heldModules is, for one machine's statement, each module whose finding is held, with the provider.
-func (h *holding) heldModules(machine string) map[string]catalogue.Chosen {
-	out := map[string]catalogue.Chosen{}
+func (h *holding) heldModules(machine string) map[string]heldUnderProvider {
+	out := map[string]heldUnderProvider{}
 	byModule := map[string][]inventory.ResourceHealth{}
 	for _, r := range h.healths[machine].Resources {
 		if r.Module != "" && r.State == link.StateUnhealthy {
@@ -187,7 +278,7 @@ func (h *holding) heldModules(machine string) map[string]catalogue.Chosen {
 		}
 	}
 	for module, rs := range byModule {
-		if on, held := h.heldUnder(machine, module, rs); held {
+		if on, held := h.heldWith(machine, module, rs); held {
 			out[module] = on
 		}
 	}