Merge pull request 'A provider whose wait fails its check lists who waits on it (issue 450)' (#221) from fix/450-a-provider-whose-wait-fails-lists-its-waiters into main

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",
+		shelf: map[string]catalogue.Manifest{"db": {Module: "db", Version: "1", OwnSecrets: dbSecrets,
-			Provides: []catalogue.Offer{{Name: "postgres-database", Scope: catalogue.ScopeMesh}}}}}
+			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