Check a provider's waits before saying who waits on it (issue 450)

A provider whose wait fails the check raises unhealthy, but the holding read
its stored statement with the wait unchecked: sayWaiters built the
needs-operator key, found it not open and listed no held consumer. The
holding now reads each statement as judged (ADR 0283 decision 3), in
sayWaiters and in the provider's state its consumers are held by.

cmd/mesh-controller/failed_wait_provider_test.go

@@ -0,0 +1,66 @@
+package main
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/conditions"
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// A provider whose wait fails the controller's check lists who waits on it (novox/hq issue 450).
+//
+// A wait that does not check out is judged unhealthy (ADR 0283 decision 3), so the provider raises its unhealthy
+// condition and its consumers are held under it (ADR 0240 rule 5). What is stored is what the machine said, the
+// wait unchecked: read as said, the provider seems to wait for the operator, and the held consumers were never
+// listed on the condition that is open.
+
+// refusedWait is a wait for a secret the database's manifest does not declare: it fails the check.
+var refusedWait = inventory.Wait{Part: "postgres-database", Secret: "certificate", What: "the database's certificate"}
+
+// judgedRefused is the provider's resource as the controller judges it: unhealthy, saying why.
+func judgedRefused() inventory.ResourceHealth {
+	r := waitingDatabaseFor(refusedWait)
+	r.State, r.Waits = link.StateUnhealthy, nil
+	r.Reason = "says it waits for the secret certificate, which db does not declare"
+	return r
+}
+
+// The consumer's statement arrives after the provider's, so it is the consumer's judging (sayWaiters) that must list
+// it at the provider's unhealthy condition, made urgent by who waits on it.
+func TestAProviderWhoseWaitFailedItsCheckListsWhoWaitsOnIt(t *testing.T) {
+	k, _ := withConditionsInMemory(t)
+	stored := waitingDatabaseFor(refusedWait)
+	open := judgeBoth(t, k, []inventory.ResourceHealth{judgedRefused()},
+		map[string]inventory.NodeHealth{"anchor": {Node: "anchor", Resources: []inventory.ResourceHealth{stored}}},
+		shopFailingBeside(stored))
+	provider := conditionOf(open, moduleUnhealthyKey("db", "anchor"))
+	if len(open) != 1 || provider == nil {
+		t.Fatalf("a provider whose wait failed its check and a consumer of it raised %v; want the provider's "+
+			"unhealthy alone", openKeysOf(open))
+	}
+	if said := provider.Evidence[0].Said; !strings.Contains(said, "shop on laptop") {
+		t.Fatalf("the provider's unhealthy condition does not list shop on laptop as waiting on it: %s", said)
+	}
+	if provider.Severity != conditions.Urgent {
+		t.Fatalf("the provider's unhealthy condition is %s with a consumer waiting on it; want urgent", provider.Severity)
+	}
+}
+
+// A provider that waits for a secret already given is unhealthy to its consumers too, before its own condition opens:
+// they are held under it as under any unhealthy provider, never as waiting for the operator, and its condition, when
+// open, is said as unhealthy with who waits on it.
+func TestAProviderWaitingForASecretAlreadyGivenIsUnhealthyToItsConsumers(t *testing.T) {
+	given := holdingOf(nil, shopFailingBeside(waitingDatabase()), shopOnTheDatabase)
+	given.waits.given["db@anchor"] = map[string]time.Time{"licence": time.Now().Add(-time.Hour)}
+	by, held := given.heldWith("laptop", "shop", []inventory.ResourceHealth{failingConsumer("shop")})
+	if !held || by.provider != theDatabase || len(by.waits) > 0 {
+		t.Fatalf("shop under a database waiting for a licence already given: held %v under %v with waits %v; want "+
+			"held under db on anchor as unhealthy, no waits", held, by.provider, by.waits)
+	}
+	if _, uncovered := given.waitingUncovered("laptop", "shop", []inventory.ResourceHealth{failingConsumer("shop")}); uncovered {
+		t.Fatalf("a provider whose wait failed its check is said as waiting for another part")
+	}
+}

cmd/mesh-controller/module_health.go

@@ -326,8 +326,10 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi
 // sayWaiters observes a provider's open condition again, with who waits on it, from its machine's newest
 // statement. Nothing when its condition is not open: it is raised by its own statements, on its own looks.
 func sayWaiters(ctx context.Context, k *conditions.Keeper, hold *holding, p catalogue.Chosen, now time.Time) error {
+	// Its statement as it was judged, its waits checked (novox/hq issue 450): a wait that failed the check is
+	// unhealthy, so the condition built here is the one its own statement raised, and who waits on it is listed.
 	var rs []inventory.ResourceHealth
-	for _, r := range hold.healths[p.Node].Resources {
+	for _, r := range hold.checked(p.Node) {
 		if r.Module == p.Module && (r.State == link.StateUnhealthy || r.State == link.StateWaiting) {
 			rs = append(rs, r)
 		}

cmd/mesh-controller/provider_hold.go

@@ -37,6 +37,32 @@ type holding struct {
 	shelf   map[string]catalogue.Manifest
 	// providers memoises providerFor by machine, consumer and provision.
 	providers map[string]providerLookup
+	// waits is what the waits of a statement are checked against, read as they are asked for (novox/hq issue 450).
+	waits operatorWaitFacts
+}
+
+// checked is a machine's newest statement as the controller judges it: each wait checked (ADR 0283 decision 3), so
+// a wait that does not check out reads as unhealthy, as it did when the statement was judged (novox/hq issue 450).
+// What is stored is what the machine said, the waits unchecked; read as stored, a provider whose wait failed its
+// check seems to wait for the operator while its unhealthy condition is open.
+func (h *holding) checked(machine string) []inventory.ResourceHealth {
+	rs := h.healths[machine].Resources
+	mods := waitingModules(rs)
+	if len(mods) == 0 {
+		return rs
+	}
+	if h.waits.manifests == nil {
+		h.waits.manifests = map[string]catalogue.Manifest{}
+	}
+	for _, module := range mods {
+		if _, has := h.waits.manifests[module]; !has {
+			if m, ok := h.manifestOf(module); ok {
+				h.waits.manifests[module] = m
+			}
+		}
+	}
+	readWaitFacts(h.ctx, h.inv, machine, mods, nil, &h.waits)
+	return checkWaiting(machine, rs, h.waits)
 }
 
 type providerLookup struct {
@@ -125,7 +151,7 @@ func (h *holding) providerState(p catalogue.Chosen) (unhealthy, waiting bool, wa
 			return true, false, nil
 		}
 	}
-	for _, r := range h.healths[p.Node].Resources {
+	for _, r := range h.checked(p.Node) {
 		if r.Module != p.Module {
 			continue
 		}

cmd/mesh-controller/waiting_provider_test.go

@@ -41,12 +41,21 @@ func failingConsumer(module string) inventory.ResourceHealth {
 		State: link.StateUnhealthy, Reason: "http /health on web: answered 500", Check: "http", Needs: "postgres-database"}
 }
 
+// dbSecrets is what the database's waits name: own secrets issued outside the mesh, so a wait for one checks out
+// while nobody gave it (ADR 0283 decision 3, novox/hq issue 450).
+var dbSecrets = catalogue.OwnSecrets{
+	"licence":    {Path: "/s/licence", IssuedBy: catalogue.IssuedOutside},
+	"backup-key": {Path: "/s/backup-key", IssuedBy: catalogue.IssuedOutside},
+}
+
 // holdingOf is one reading of the record without a store: the newest statements, the open conditions, the
-// catalogue, and each consumer's provider already looked up, as providerFor memoises it.
+// catalogue, what was given on the provider's machine (nothing), and each consumer's provider already looked up,
+// as providerFor memoises it.
 func holdingOf(open []conditions.Condition, healths map[string]inventory.NodeHealth, bound map[[3]string]catalogue.Chosen) *holding {
 	h := &holding{ctx: context.Background(), healths: healths, open: open, providers: map[string]providerLookup{},
-		shelf: map[string]catalogue.Manifest{"db": {Module: "db", Version: "1",
-			Provides: []catalogue.Offer{{Name: "postgres-database", Scope: catalogue.ScopeMesh}}}}}
+		shelf: map[string]catalogue.Manifest{"db": {Module: "db", Version: "1", OwnSecrets: dbSecrets,
+			Provides: []catalogue.Offer{{Name: "postgres-database", Scope: catalogue.ScopeMesh}}}},
+		waits: operatorWaitFacts{given: map[string]map[string]time.Time{"db@anchor": {}}}}
 	for k, p := range bound {
 		h.providers[k[0]+"\x00"+k[1]+"\x00"+k[2]] = providerLookup{p, true}
 	}
@@ -75,7 +84,7 @@ func TestAProviderWaitingForTheOperatorHoldsOnlyTheConsumersOfTheWaitingPart(t *
 	healthyDB.State, healthyDB.Waits = link.StateHealthy, nil
 	byCredential := holdingOf(nil, shopFailingBeside(waitingDatabaseFor(inventory.Wait{Part: "the server",
 		Secret: "licence", What: "the licence key"})), shopOnTheDatabase)
-	byCredential.shelf["db"] = catalogue.Manifest{Module: "db", Version: "1", Provides: []catalogue.Offer{{
+	byCredential.shelf["db"] = catalogue.Manifest{Module: "db", Version: "1", OwnSecrets: dbSecrets, Provides: []catalogue.Offer{{
 		Name: "postgres-database", Credential: &catalogue.OfferCredential{Own: "licence"}}}}
 	for _, c := range []struct {
 		name      string