From 6ca6d5b35272b97c769ee877576bf32ff0b4e051 Mon Sep 17 00:00:00 2001 From: dtoro Date: Tue, 28 Jul 2026 21:33:48 +0200 Subject: [PATCH] fix(scheduler): derive entity health from all its checks, not the last one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit host:strong logged 226 health.changed events in one hour, oscillating down/healthy while the host was fine throughout. host:hubris did it 126 times. runCheck wrote entity_status.health on every check completion, so an entity's health was simply whichever of its checks finished most recently. A host with six checks reported whichever facet happened to be sampled last, and one failing probe alternating with five passing ones flapped forever. resolveSignal forced "healthy" too, a second path by which one passing probe erased another probe's genuine failure. On this fleet the trigger is a known false positive: the scheduler's network vantage point cannot ICMP host:strong, so its ping check fails while every ssh-script check succeeds. Under last-writer-wins that single probe declared the whole host down, twice a minute. Each check now records its own verdict (check_defs.last_health, migration 027) and the entity's health is the worst across its enabled checks. A failing probe now degrades the entity honestly and *stably*, without erasing what the other five report, and health.changed fires only when that aggregate actually moves. Checks that have never run are ignored rather than counted as unknown, so adding a check cannot drag a known-good entity down before it has a verdict. Also declares service:oikos in the seed. The previous commit re-pointed the mcp ingress at it, but the entity only ever existed in the production database — so a fresh seed (a new install, or a DR restore) failed on an unresolvable edge. Caught by seeding an empty database rather than a copy of prod, which is the only way that class of bug shows up. Co-Authored-By: Claude --- internal/db/queries/operations.sql | 16 ++++- internal/db/sqlcgen/models.go | 16 +++++ internal/db/sqlcgen/operations.sql.go | 36 ++++++++-- internal/scheduler/scheduler.go | 89 +++++++++++++++++-------- migrations/027_check_last_health.up.sql | 28 ++++++++ seeds/inventory.yaml | 10 +++ 6 files changed, 161 insertions(+), 34 deletions(-) create mode 100644 migrations/027_check_last_health.up.sql diff --git a/internal/db/queries/operations.sql b/internal/db/queries/operations.sql index 0172b30..0742d55 100644 --- a/internal/db/queries/operations.sql +++ b/internal/db/queries/operations.sql @@ -68,7 +68,21 @@ WHERE cd.enabled = true OR cd.last_run_at <= now() - make_interval(secs => cd.interval_s)); -- name: MarkCheckRun :exec -UPDATE check_defs SET last_run_at = now() WHERE entity_id = $1; +UPDATE check_defs SET last_run_at = now(), last_health = $2 WHERE entity_id = $1; + +-- name: WorstHealthForTarget :one +-- An entity is as healthy as its unhealthiest check. Checks that have not run +-- yet (last_health IS NULL) are ignored rather than counted as unknown, so a +-- newly added check does not drag a known-good entity down before it has +-- produced a verdict. +SELECT COALESCE( + (SELECT last_health FROM check_defs + WHERE enabled AND target_id = $1 AND last_health IS NOT NULL + ORDER BY CASE last_health + WHEN 'down' THEN 0 WHEN 'degraded' THEN 1 WHEN 'stale' THEN 2 + WHEN 'unknown' THEN 3 ELSE 4 END + LIMIT 1), + 'unknown')::text AS health; -- name: GetCheckDef :one SELECT * FROM check_defs WHERE entity_id = $1; diff --git a/internal/db/sqlcgen/models.go b/internal/db/sqlcgen/models.go index e584131..7295791 100644 --- a/internal/db/sqlcgen/models.go +++ b/internal/db/sqlcgen/models.go @@ -114,6 +114,8 @@ type CheckDef struct { UpdatedAt time.Time // When this check last executed. NULL = never, due immediately. Compared against interval_s to decide due-ness. LastRunAt *time.Time + // This check's own most recent verdict (healthy/degraded/down/unknown). entity_status.health is the worst of these across the target's enabled checks. + LastHealth *string } type Classification struct { @@ -255,6 +257,20 @@ type KnowledgeEntity struct { UpdatedAt time.Time ContentHash *string Search interface{} + EditedBy string + DeletedAt *time.Time +} + +type KnowledgeRevision struct { + ID int64 + EntityID uuid.UUID + Title string + Content string + Source *string + Tags []string + EditedBy string + VersionAt time.Time + RevisedAt time.Time } type Ledger struct { diff --git a/internal/db/sqlcgen/operations.sql.go b/internal/db/sqlcgen/operations.sql.go index a0e52d3..4424565 100644 --- a/internal/db/sqlcgen/operations.sql.go +++ b/internal/db/sqlcgen/operations.sql.go @@ -51,7 +51,7 @@ func (q *Queries) GetAutonomySetting(ctx context.Context, key string) (string, e } const getCheckDef = `-- name: GetCheckDef :one -SELECT entity_id, target_id, target_type, kind, config, interval_s, timeout_s, zone, enabled, updated_at, last_run_at FROM check_defs WHERE entity_id = $1 +SELECT entity_id, target_id, target_type, kind, config, interval_s, timeout_s, zone, enabled, updated_at, last_run_at, last_health FROM check_defs WHERE entity_id = $1 ` func (q *Queries) GetCheckDef(ctx context.Context, entityID uuid.UUID) (CheckDef, error) { @@ -69,6 +69,7 @@ func (q *Queries) GetCheckDef(ctx context.Context, entityID uuid.UUID) (CheckDef &i.Enabled, &i.UpdatedAt, &i.LastRunAt, + &i.LastHealth, ) return i, err } @@ -1133,11 +1134,16 @@ func (q *Queries) ListSkills(ctx context.Context, status *string) ([]Skill, erro } const markCheckRun = `-- name: MarkCheckRun :exec -UPDATE check_defs SET last_run_at = now() WHERE entity_id = $1 +UPDATE check_defs SET last_run_at = now(), last_health = $2 WHERE entity_id = $1 ` -func (q *Queries) MarkCheckRun(ctx context.Context, entityID uuid.UUID) error { - _, err := q.db.Exec(ctx, markCheckRun, entityID) +type MarkCheckRunParams struct { + EntityID uuid.UUID + LastHealth *string +} + +func (q *Queries) MarkCheckRun(ctx context.Context, arg MarkCheckRunParams) error { + _, err := q.db.Exec(ctx, markCheckRun, arg.EntityID, arg.LastHealth) return err } @@ -1462,3 +1468,25 @@ func (q *Queries) UpsertSignal(ctx context.Context, arg UpsertSignalParams) (Sig ) return i, err } + +const worstHealthForTarget = `-- name: WorstHealthForTarget :one +SELECT COALESCE( + (SELECT last_health FROM check_defs + WHERE enabled AND target_id = $1 AND last_health IS NOT NULL + ORDER BY CASE last_health + WHEN 'down' THEN 0 WHEN 'degraded' THEN 1 WHEN 'stale' THEN 2 + WHEN 'unknown' THEN 3 ELSE 4 END + LIMIT 1), + 'unknown')::text AS health +` + +// An entity is as healthy as its unhealthiest check. Checks that have not run +// yet (last_health IS NULL) are ignored rather than counted as unknown, so a +// newly added check does not drag a known-good entity down before it has +// produced a verdict. +func (q *Queries) WorstHealthForTarget(ctx context.Context, targetID *uuid.UUID) (string, error) { + row := q.db.QueryRow(ctx, worstHealthForTarget, targetID) + var health string + err := row.Scan(&health) + return health, err +} diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index c8868c3..79a523b 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -115,8 +115,16 @@ func runCheck(ctx context.Context, pool *db.Pool, cd sqlcgen.ListEnabledCheckDef // Stamp the run before processing the result: due-ness must advance even // when a check fails, or a permanently failing check would be re-run on - // every pass instead of at its declared interval. - if err := q.MarkCheckRun(ctx, cd.EntityID); err != nil { + // every pass instead of at its declared interval. last_health records THIS + // check's own verdict, which is what makes the aggregation below possible. + checkHealth := result.health + if checkHealth == "" { + checkHealth = "healthy" + } + if err := q.MarkCheckRun(ctx, sqlcgen.MarkCheckRunParams{ + EntityID: cd.EntityID, + LastHealth: &checkHealth, + }); err != nil { slog.Error("scheduler: mark check run", "entity", cd.EntitySlug, "error", err) } @@ -150,16 +158,12 @@ func runCheck(ctx context.Context, pool *db.Pool, cd sqlcgen.ListEnabledCheckDef if result.signalKind == "" || result.health == "healthy" { resolveSignal(ctx, pool, cd.EntityID, targetID, cd.EntitySlug) - _ = q.UpsertEntityStatus(ctx, sqlcgen.UpsertEntityStatusParams{ - EntityID: targetID, - Health: "healthy", - LastCheckAt: &[]time.Time{time.Now()}[0], - Details: []byte(`{}`), - }) - if prevHealth != "" && prevHealth != "healthy" { - emitSchedulerEvent(ctx, pool, "health.changed", targetID, "info", - map[string]any{"slug": cd.EntitySlug, "from": prevHealth, "to": "healthy"}) - } + // NOT unconditionally "healthy": this check passing says nothing about + // the entity's other checks. Writing healthy here is what let one + // passing probe erase a genuine failure reported by another — and, + // alternating with a failing probe, produced 226 health flips an hour + // on a host that was fine throughout. + applyAggregateHealth(ctx, pool, q, targetID, cd.EntitySlug, prevHealth) return } @@ -180,22 +184,52 @@ func runCheck(ctx context.Context, pool *db.Pool, cd sqlcgen.ListEnabledCheckDef return } - _ = q.UpsertEntityStatus(ctx, sqlcgen.UpsertEntityStatusParams{ - EntityID: targetID, - Health: result.health, - LastCheckAt: &[]time.Time{time.Now()}[0], - Details: []byte(`{}`), - }) _ = sig if prevHealth == "" || prevHealth == "healthy" { emitSchedulerEvent(ctx, pool, "signal.raised", targetID, severity, map[string]any{"slug": cd.EntitySlug, "kind": result.signalKind, "evidence": result.evidence}) } - if prevHealth != result.health { - emitSchedulerEvent(ctx, pool, "health.changed", targetID, severity, - map[string]any{"slug": cd.EntitySlug, "from": prevHealth, "to": result.health}) + applyAggregateHealth(ctx, pool, q, targetID, cd.EntitySlug, prevHealth) +} + +// applyAggregateHealth sets the target's health to the worst verdict across +// all of its enabled checks, and emits health.changed only when that aggregate +// actually moves. +// +// Health is a property of the entity, but each check only ever observes one +// facet of it — reachability, disk, a systemd unit. Letting whichever check +// finished last overwrite the entity's health meant a host with six checks +// reported whichever facet was sampled most recently, so one failing probe and +// five passing ones oscillated forever instead of settling on "degraded". +func applyAggregateHealth(ctx context.Context, pool *db.Pool, q *sqlcgen.Queries, + targetID uuid.UUID, checkSlug, prevHealth string) { + + health, err := q.WorstHealthForTarget(ctx, &targetID) + if err != nil { + slog.Error("scheduler: aggregate health", "entity", checkSlug, "error", err) + return } + + _ = q.UpsertEntityStatus(ctx, sqlcgen.UpsertEntityStatusParams{ + EntityID: targetID, + Health: health, + LastCheckAt: &[]time.Time{time.Now()}[0], + Details: []byte(`{}`), + }) + + if prevHealth == health { + return + } + severity := "info" + switch health { + case "down": + severity = "critical" + case "degraded", "stale": + severity = "warning" + } + emitSchedulerEvent(ctx, pool, "health.changed", targetID, severity, + map[string]any{"slug": checkSlug, "from": prevHealth, "to": health}) } // currentHealth reads the last recorded health for an entity, or "" if none. @@ -217,7 +251,6 @@ func emitSchedulerEvent(ctx context.Context, pool *db.Pool, eventType string, en // checkID matches how signals are keyed (UpsertSignal uses the check's own // entity id); targetID is the observed entity whose status this affects. func resolveSignal(ctx context.Context, pool *db.Pool, checkID, targetID uuid.UUID, slug string) { - q := sqlcgen.New(pool) // Check if there's an open signal on this entity tag, err := pool.Exec(ctx, `UPDATE signals SET state = 'resolved', updated_at = now() WHERE entity_id = $1 AND state = 'raised'`, checkID) @@ -227,14 +260,12 @@ func resolveSignal(ctx context.Context, pool *db.Pool, checkID, targetID uuid.UU if tag.RowsAffected() > 0 { emitSchedulerEvent(ctx, pool, "signal.resolved", targetID, "info", map[string]any{"slug": slug}) + slog.Info("scheduler: signal resolved", "entity", slug) } - _ = q.UpsertEntityStatus(ctx, sqlcgen.UpsertEntityStatusParams{ - EntityID: targetID, - Health: "healthy", - LastCheckAt: &[]time.Time{time.Now()}[0], - Details: []byte(`{}`), - }) - slog.Info("scheduler: signal resolved", "entity", slug) + // Deliberately does NOT write health. Resolving THIS check's signal says + // nothing about the target's other checks; the caller re-derives health + // from all of them. Forcing "healthy" here was a second path by which one + // passing probe erased another probe's genuine failure. } // checkResult bundles the outcome of a single check execution. diff --git a/migrations/027_check_last_health.up.sql b/migrations/027_check_last_health.up.sql new file mode 100644 index 0000000..7bb374b --- /dev/null +++ b/migrations/027_check_last_health.up.sql @@ -0,0 +1,28 @@ +-- 027_check_last_health.up.sql +-- Aggregate an entity's health across its checks instead of last-writer-wins. +-- +-- runCheck wrote entity_status.health on every check completion, so an +-- entity's health was simply whichever of its checks finished most recently. +-- host:hubris has 6 checks, host:strong 6 — one failing probe alternating with +-- five passing ones produced a permanent flap: 226 health.changed events for +-- host:strong in a single hour, oscillating down/healthy, while the host was +-- fine the whole time. +-- +-- On this fleet the trigger is a known false positive: the scheduler's network +-- vantage point cannot ICMP host:strong, so its ping check fails while every +-- ssh-script check succeeds. Under last-writer-wins that one probe was enough +-- to declare the whole host down, twice a minute. +-- +-- Storing each check's own verdict lets entity health be derived as the worst +-- current result across that entity's enabled checks — so a single failing +-- probe degrades the entity honestly without erasing what the other five say, +-- and a passing probe cannot mask a genuine failure elsewhere. + +ALTER TABLE check_defs ADD COLUMN IF NOT EXISTS last_health TEXT; + +COMMENT ON COLUMN check_defs.last_health IS 'This check''s own most recent verdict (healthy/degraded/down/unknown). entity_status.health is the worst of these across the target''s enabled checks.'; + +-- The aggregation reads every enabled check for one target on each completion. +CREATE INDEX IF NOT EXISTS idx_check_defs_target_health + ON check_defs (target_id) + WHERE enabled AND target_id IS NOT NULL; diff --git a/seeds/inventory.yaml b/seeds/inventory.yaml index fb97788..3198468 100644 --- a/seeds/inventory.yaml +++ b/seeds/inventory.yaml @@ -270,6 +270,15 @@ entities: attributes: {url: "https://teddy.hubris.network", doc_page: knowledge/wiki/containers/131-teddycloud.md, risk_notes: "no forward-auth gate — reachable by anyone on LAN/mesh"}} + # The Go control plane itself: api/scheduler/notifier/web on the mac-mini, + # and what mcp.hubris.network fronts since the cutover. It existed in the + # database (created outside the seed) but was never declared here, so a + # fresh seed could not resolve the routes-to edge below. + - {slug: "service:oikos", type: service, name: oikos, + attributes: {url: "https://oikos.hubris.network", + host: "ws:mac-mini", + ports: {api: 8090, web: 8091, nomos_gateway: 8092}, + note: "homelab automation platform — api/scheduler/notifier/web on mac-mini docker compose (project name oikos)"}} - {slug: "service:homelab-mcp", type: service, name: homelab_mcp, attributes: {port: 9810, systemd_unit: homelab-mcp, endpoint: "https://mcp.hubris.network/mcp", @@ -451,6 +460,7 @@ relationships: # fronts the Go api. Nomos recorded this correctly on 2026-07-12; the seed # was the stale one, and re-asserting the old edge alongside it is what made # ingress:mcp a cardinality violation. + - {source: "ws:mac-mini", target: "service:oikos", type: provides} - {source: "ingress:mcp.hubris.network", target: "service:oikos", type: routes-to} - {source: "ingress:secrets.hubris.network", target: "service:secrets-issuance", type: routes-to} - {source: "ingress:house.hubris.network", target: "service:house", type: routes-to}