Merge pull request 'A module may declare its unhealthy urgent, with why (hq ADR 0355, issue 691)' (#330) from adr-355-urgent-unhealthy into main

cmd/mesh-controller/module_health.go

@@ -240,6 +240,7 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi
 			continue
 		}
 		o := moduleUnhealthyObservation(m, node, unhealthy[m])
+		declaredSeverity(&o, hold, m)
 		if hold != nil {
 			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
@@ -347,6 +348,7 @@ func sayWaiters(ctx context.Context, k *conditions.Keeper, hold *holding, p cata
 	} else {
 		_, rest := besideAWait(p.Module, rs)
 		o = moduleUnhealthyObservation(p.Module, p.Node, rest)
+		declaredSeverity(&o, hold, p.Module)
 		if said, waits := personWait(p.Module, p.Node, rest); waits {
 			o = reloginObservation(p.Module, p.Node, said, operatorOn(ctx, hold.inv, p.Node), rest)
 		}
@@ -481,6 +483,19 @@ func sayWaitingOn(o *conditions.Observation, waiters []string) {
 	o.Explanation += fmt.Sprintf(" %d module(s) that depend on it wait for it.", len(waiters))
 }
 
+// declaredSeverity raises a module's unhealthy condition urgent when its manifest declares `unhealthy` urgent
+// (novox/hq ADR 0355 §1). Only that condition changes (ADR 0355 §4): a wait for a new login, a module that needs
+// the operator and a directory used as found keep their severities, and without the field, or without a manifest
+// to read it from, it stays the warning it was.
+func declaredSeverity(o *conditions.Observation, hold *holding, module string) {
+	if o.Kind != kindModuleUnhealthy || hold == nil {
+		return
+	}
+	if m, ok := hold.manifestOf(module); ok && m.UnhealthyIsUrgent() {
+		o.Severity = conditions.Urgent
+	}
+}
+
 // moduleUnhealthyObservation is a module unhealthy on a machine, in words: the summary names the module,
 // the machine and what is wrong with each resource; the detail — targets, streaks, since — is evidence.
 func moduleUnhealthyObservation(module, node string, rs []inventory.ResourceHealth) conditions.Observation {

cmd/mesh-controller/urgent_unhealthy_test.go

@@ -0,0 +1,91 @@
+package main
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/conditions"
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// A module may declare its unhealthy urgent (novox/hq ADR 0355): its module-unhealthy condition is then raised
+// urgent; without the field it stays a warning; a wait for a new login stays a warning whatever the module says.
+
+// judgedTwice judges one machine's statement twice, over a catalogue in which mail declares its unhealthy
+// urgent and notes says nothing, and returns what is open.
+func judgedTwice(t *testing.T, unhealthy map[string][]inventory.ResourceHealth) []conditions.Condition {
+	t.Helper()
+	ctx := t.Context()
+	k, _ := withConditionsInMemory(t)
+	was := readHoldingFor
+	t.Cleanup(func() { readHoldingFor = was })
+	readHoldingFor = func(_ context.Context, _ *inventory.Inventory, open []conditions.Condition) (*holding, error) {
+		return &holding{ctx: context.Background(), open: open, providers: map[string]providerLookup{},
+			shelf: map[string]catalogue.Manifest{
+				"mail": {Module: "mail", Version: "1", Unhealthy: &catalogue.UnhealthyPolicy{
+					Severity: catalogue.UnhealthyUrgent, Why: "mail is refused or stuck for everyone with a mailbox"}},
+				"notes": {Module: "notes", Version: "1"},
+			}}, nil
+	}
+	streaks := map[string]int{}
+	for look := 1; look <= 2; look++ {
+		for m := range unhealthy {
+			streaks[m] = look
+		}
+		if err := judgeModuleHealth(ctx, nil, k, "anchor", unhealthy, streaks, time.Now()); err != nil {
+			t.Fatal(err)
+		}
+	}
+	open, err := k.Open(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return open
+}
+
+func containerDown(module string) inventory.ResourceHealth {
+	return inventory.ResourceHealth{Module: module, Resource: module + ".web", Kind: "container", Target: module,
+		State: link.StateUnhealthy, Reason: "down"}
+}
+
+func TestAModuleThatDeclaresItsUnhealthyUrgentIsRaisedUrgent(t *testing.T) {
+	open := judgedTwice(t, map[string][]inventory.ResourceHealth{"mail": {containerDown("mail")}})
+	c := conditionOf(open, "module.mail.anchor.unhealthy")
+	if c == nil {
+		t.Fatalf("mail's unhealthy condition was not raised: %+v", open)
+	}
+	if c.Severity != conditions.Urgent {
+		t.Errorf("mail declares its unhealthy urgent, and it was raised %s", c.Severity)
+	}
+}
+
+func TestAModuleThatSaysNothingOfItsUnhealthyStaysAWarning(t *testing.T) {
+	open := judgedTwice(t, map[string][]inventory.ResourceHealth{"notes": {containerDown("notes")}})
+	c := conditionOf(open, "module.notes.anchor.unhealthy")
+	if c == nil {
+		t.Fatalf("notes' unhealthy condition was not raised: %+v", open)
+	}
+	if c.Severity != conditions.Warning {
+		t.Errorf("notes says nothing of its unhealthy, and it was raised %s", c.Severity)
+	}
+}
+
+// Only the module-unhealthy condition changes (ADR 0355 §4): a wait for a new login (ADR 0254) of a module that
+// declares its unhealthy urgent stays a warning.
+func TestAWaitForANewLoginStaysAWarningWhenTheModuleDeclaresItsUnhealthyUrgent(t *testing.T) {
+	open := judgedTwice(t, map[string][]inventory.ResourceHealth{"mail": {relogin("mail", "operator"),
+		userUnit("mail", "operator")}})
+	c := conditionOf(open, "module.mail.anchor.relogin-needed")
+	if c == nil {
+		t.Fatalf("mail's relogin-needed condition was not raised: %+v", open)
+	}
+	if c.Severity != conditions.Warning {
+		t.Errorf("a wait for a new login was raised %s; want a warning", c.Severity)
+	}
+	if conditionOf(open, "module.mail.anchor.unhealthy") != nil {
+		t.Errorf("a wait for a new login raised mail's unhealthy condition beside it: %+v", open)
+	}
+}

internal/catalogue/manifest.go

@@ -552,6 +552,10 @@ type Manifest struct {
 	// why. A person's choice through the `upgrade` verb stands over it; the bus records whatever it says.
 	Upgrade *UpgradePolicy `json:"upgrade,omitempty"`
 
+	// Unhealthy is how loud this module's unhealthy condition is (novox/hq ADR 0355): a warning when unsaid;
+	// `urgent`, with why, for a module whose failure costs people something they rely on.
+	Unhealthy *UnhealthyPolicy `json:"unhealthy,omitempty"`
+
 	// Reads are other modules' state this module reads and watches, each `<module>.<name>`
 	// (novox/hq ADR 0201). Read-only: only the owner's instances write.
 	Reads []string `json:"reads,omitempty"`
@@ -2326,6 +2330,7 @@ func ParseManifest(raw []byte) (Manifest, error) {
 	problems = append(problems, m.placementProblems()...)
 	problems = append(problems, m.dataProblems()...)
 	problems = append(problems, m.upgradeProblems()...)
+	problems = append(problems, m.unhealthyProblems()...)
 
 	for i, r := range m.Resources {
 		id, _ := r["id"].(string)

internal/catalogue/unhealthy.go

@@ -0,0 +1,55 @@
+package catalogue
+
+import (
+	"fmt"
+	"strings"
+)
+
+// How loud a module's unhealthy condition is (novox/hq ADR 0355, extending ADR 0240).
+//
+// **A module may declare its unhealthy urgent.** A module whose failure costs people something they rely on
+// (mail refused or stuck for everyone with a mailbox) says so with `unhealthy`, and its condition
+// `module.<module>.<machine>.unhealthy` is raised urgent instead of a warning. **Urgent needs a why**, as an
+// `upgrade` policy other than roll does: a person reading the catalogue sees what is lost while it is broken.
+
+// The severities a module may declare for its unhealthy condition.
+const (
+	// UnhealthyUrgent raises the module's unhealthy condition urgent, on every machine it runs on.
+	UnhealthyUrgent = "urgent"
+	// UnhealthyWarning is what a module's unhealthy condition is without the field, said out loud.
+	UnhealthyWarning = "warning"
+)
+
+// UnhealthyPolicy is what a module says about how loud its unhealthy condition is: `unhealthy` in its manifest
+// (novox/hq ADR 0355).
+type UnhealthyPolicy struct {
+	Severity string `json:"severity"`
+	// Why names what people lose while the module is broken. It is required: a module that says nothing of
+	// why it is loud is refused.
+	Why string `json:"why,omitempty"`
+}
+
+// unhealthyProblems refuses an `unhealthy` without a why, and any severity but urgent or warning (novox/hq ADR
+// 0355 §2).
+func (m Manifest) unhealthyProblems() []string {
+	if m.Unhealthy == nil {
+		return nil
+	}
+	var problems []string
+	switch m.Unhealthy.Severity {
+	case UnhealthyUrgent, UnhealthyWarning:
+	default:
+		problems = append(problems, fmt.Sprintf("unhealthy is %q or %q, not %q", UnhealthyUrgent, UnhealthyWarning,
+			m.Unhealthy.Severity))
+	}
+	if strings.TrimSpace(m.Unhealthy.Why) == "" {
+		problems = append(problems, "unhealthy says why: a module that declares how loud its failure is names "+
+			"what people lose while it is broken")
+	}
+	return problems
+}
+
+// UnhealthyIsUrgent is whether the module declares its unhealthy condition urgent (novox/hq ADR 0355 §1).
+func (m Manifest) UnhealthyIsUrgent() bool {
+	return m.Unhealthy != nil && m.Unhealthy.Severity == UnhealthyUrgent
+}

internal/catalogue/unhealthy_test.go

@@ -0,0 +1,53 @@
+package catalogue
+
+import (
+	"strings"
+	"testing"
+)
+
+// A module may declare its unhealthy urgent, with why; the module check refuses it without why and with any
+// severity but urgent or warning (novox/hq ADR 0355).
+
+func TestAnUnhealthyDeclaredUrgentIsUrgent(t *testing.T) {
+	m, err := ParseManifest([]byte(`{"module":"mail","version":"1",` +
+		`"unhealthy":{"severity":"urgent","why":"mail is refused or stuck for everyone with a mailbox"}}`))
+	if err != nil {
+		t.Fatalf("a manifest declaring its unhealthy urgent with why was refused: %v", err)
+	}
+	if !m.UnhealthyIsUrgent() {
+		t.Errorf("a manifest declaring its unhealthy urgent reads as not urgent: %+v", m.Unhealthy)
+	}
+}
+
+func TestAManifestWithoutUnhealthyIsNotUrgent(t *testing.T) {
+	m, err := ParseManifest([]byte(`{"module":"notes","version":"1"}`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if m.UnhealthyIsUrgent() {
+		t.Errorf("a manifest saying nothing of its unhealthy reads as urgent")
+	}
+	warning := Manifest{Module: "notes", Unhealthy: &UnhealthyPolicy{Severity: UnhealthyWarning, Why: "notes wait"}}
+	if warning.UnhealthyIsUrgent() || len(warning.unhealthyProblems()) != 0 {
+		t.Errorf("a warning said with why reads as urgent or is refused: %v", warning.unhealthyProblems())
+	}
+}
+
+func TestAnUnhealthyWithoutWhyIsRefused(t *testing.T) {
+	for _, why := range []string{"", "   "} {
+		if problems := (Manifest{Module: "mail", Unhealthy: &UnhealthyPolicy{Severity: UnhealthyUrgent, Why: why}}).unhealthyProblems(); len(problems) == 0 {
+			t.Errorf("urgent with why %q was accepted", why)
+		}
+	}
+	if _, err := ParseManifest([]byte(`{"module":"mail","version":"1","unhealthy":{"severity":"urgent"}}`)); err == nil ||
+		!strings.Contains(err.Error(), "unhealthy says why") {
+		t.Errorf("a manifest declaring its unhealthy urgent without why parsed: %v", err)
+	}
+}
+
+func TestAnUnhealthySeverityOtherThanUrgentOrWarningIsRefused(t *testing.T) {
+	if _, err := ParseManifest([]byte(`{"module":"mail","version":"1","unhealthy":{"severity":"loud","why":"mail stops"}}`)); err == nil ||
+		!strings.Contains(err.Error(), `not "loud"`) {
+		t.Errorf("a manifest declaring its unhealthy loud parsed: %v", err)
+	}
+}