diff --git a/cmd/mesh-controller/module_health.go b/cmd/mesh-controller/module_health.go index 1975c6b7..39ba6f01 100644 --- a/cmd/mesh-controller/module_health.go +++ b/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 { diff --git a/cmd/mesh-controller/urgent_unhealthy_test.go b/cmd/mesh-controller/urgent_unhealthy_test.go new file mode 100644 index 00000000..661ff392 --- /dev/null +++ b/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) + } +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 21ec6363..9fc35664 100644 --- a/internal/catalogue/manifest.go +++ b/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 `.` // (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) diff --git a/internal/catalogue/unhealthy.go b/internal/catalogue/unhealthy.go new file mode 100644 index 00000000..b6e55837 --- /dev/null +++ b/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...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 +} diff --git a/internal/catalogue/unhealthy_test.go b/internal/catalogue/unhealthy_test.go new file mode 100644 index 00000000..c6c9b888 --- /dev/null +++ b/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) + } +}