diff --git a/CLAUDE.md b/CLAUDE.md index 3b0e7f4..8449527 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,9 +91,33 @@ recorded: `8.8.8.8:53`. Bypasses DNS; proves raw link. - **`domain`** — HTTP GET to `generate_204`-style endpoints. Exercises the DNS path too. -The verdict is deliberately conservative: **down ⇔ (all `ip` targets fail) OR (all -`domain` targets fail)**. A single failed target is never a trigger — that guards -against rebooting the router because one remote server hiccuped. +`domain.Verdict` grades one round into three states (issue #6): **`down` ⇔ all `ip` +targets fail** (the only verdict a reboot may follow); **`degraded` ⇔ all `domain` +targets fail while an `ip` target still answers** — the DNS/HTTPS path is broken, the +raw link is not; everything else is `up`. A single failed target within a group is never +a trigger, and an empty group is "not applicable" rather than vacuously failed — except +that with no `ip` targets configured at all, the `domain` group becomes the only evidence +there is and a full failure of it counts as `down`. + +The split is measured, not theoretical: of 29 "down" verdicts recorded in 21 days, 28 had +both `ip` targets connecting in 19-1118 ms and ICMP answering 4/4 while only the two +HTTPS probes timed out — they share one DNS path, in production through the very router +being rebooted. Four reboots in seven hours on 2026-07-30 did not stop them. + +Two further guards sit between a `down` verdict and a reboot, both in +`services/orchestrator.go`: + +- **In-run confirmation** — `confirmDown` re-probes `CHECK_CONFIRMATIONS` times, + `CHECK_CONFIRM_DELAY` apart (persisted under `PhaseConfirm`); one healthy sample ends + the run as `unconfirmed`, correcting the `internet_ok` it wrote from the first sample. +- **Cross-run corroboration** — `outageSustained` requires the previous + `REBOOT_MIN_STREAK - 1` runs to have seen the outage too, via + `RunRepository.RecentRuns`. A run that falls short ends as `unsustained`. Fail-closed: + an unreadable history withholds the reboot rather than granting it. + +Neither guard sends a Telegram message — silence is the point; the `runs` row and a log +line carry the record. `degraded` and cooldown-skip notices are edge-triggered +(`shouldAnnounce`), so a state that lasts hours is announced once, not once per run. ### Data model @@ -186,7 +210,8 @@ prepended. `README.md` holds the authoritative table. Required: `LABEL`, `TELEGRAMBOT_DSN`, `ROUTER_USER`, `ROUTER_PASS`. Optional (with defaults): `ROUTER_ADDR`, `ROUTER_KIND`, -`DB_PATH`, `CHECK_IPS`, `CHECK_DOMAINS`, `CHECK_TIMEOUT`, `RECOVERY_INTERVAL`, +`DB_PATH`, `CHECK_IPS`, `CHECK_DOMAINS`, `CHECK_TIMEOUT`, `CHECK_CONFIRMATIONS`, +`CHECK_CONFIRM_DELAY`, `REBOOT_MIN_STREAK`, `RECOVERY_INTERVAL`, `RECOVERY_TIMEOUT`, `REBOOT_COOLDOWN`, `RETENTION_DAYS`, `REBOOT_ENABLED` (monitor-only when `false`: check/record/notify but never reboot), `METRICS_ENABLED`, `METRICS_TEMPERATURE`, `METRICS_FANS`, `METRICS_CPU`, `METRICS_MEMORY`, `METRICS_DISK`, @@ -220,6 +245,7 @@ response (`writeError500`). The contract has these surfaces: | 3 | reboot request failed (login / `reboot.cgi`) | | 4 | reboot done, internet not restored within the timeout | | 5 | reboot skipped due to cooldown | + | 6 | a failure was observed but deliberately not acted on (`degraded`, `unsustained`) | - **Daemon exit codes** — its own disjoint contract: `0` clean shutdown (SIGINT/SIGTERM, drained), `1` configuration error (missing `DAEMON_TOKEN`, unparseable value), `2` the diff --git a/README.md b/README.md index a4e9a50..aacecc4 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,9 @@ API and status dashboard over the LAN. - checks connectivity against independent IP and domain targets - soft mode (default) reboots only when the uplink is actually down; hard mode (`--hard-reboot`) reboots unconditionally +- a down verdict must survive re-probing within the run and be corroborated by the previous + run before anything reboots; a failure of the domain targets alone, while raw IP + connectivity holds, is reported as degraded and never reboots - monitor-only mode (`YARDDOG_REBOOT_ENABLED=false`): watch and record, never reboot - records every run, check and recovery phase in SQLite, with optional per-run host-telemetry and ping metrics diff --git a/cmd/yarddog/main.go b/cmd/yarddog/main.go index e591d9a..3ff8af8 100644 --- a/cmd/yarddog/main.go +++ b/cmd/yarddog/main.go @@ -128,13 +128,16 @@ func run() int { } settings := services.Settings{ - Label: cfg.Label, - RebootCooldown: cfg.RebootCooldown, - RecoveryInterval: cfg.RecoveryInterval, - RecoveryTimeout: cfg.RecoveryTimeout, - RebootEnabled: cfg.RebootEnabled, - MetricsEnabled: cfg.MetricsEnabled, - PingEnabled: len(cfg.PingHosts) > 0, + Label: cfg.Label, + RebootCooldown: cfg.RebootCooldown, + RecoveryInterval: cfg.RecoveryInterval, + RecoveryTimeout: cfg.RecoveryTimeout, + CheckConfirmations: cfg.CheckConfirmations, + CheckConfirmDelay: cfg.CheckConfirmDelay, + RebootMinStreak: cfg.RebootMinStreak, + RebootEnabled: cfg.RebootEnabled, + MetricsEnabled: cfg.MetricsEnabled, + PingEnabled: len(cfg.PingHosts) > 0, } code := services.Execute(ctx, settings, st, st, st, chk, rb, nt, mc, pc, clk, mode) diff --git a/domain/check.go b/domain/check.go index e38ce0e..238aed7 100644 --- a/domain/check.go +++ b/domain/check.go @@ -10,9 +10,31 @@ const ( CheckKindGateway = "gateway" ) -// Verdict applies the conservative down rule (design §4.2): the internet is down when -// every ip target fails OR every domain target fails. A single failed target within a -// group never trips it; an empty group is "not applicable", never vacuously down. +// StateUp, StateDegraded, and StateDown are the tri-state verdict Verdict +// produces (issue #6). The two failure states are deliberately not the same +// event: StateDown means the raw link is gone and a router reboot is on the +// table, while StateDegraded means only the name-resolution/HTTPS path broke +// while raw connectivity provably held — recorded and announced, never +// rebooted. +const ( + StateUp = "up" + StateDegraded = "degraded" + StateDown = "down" +) + +// Verdict grades one round of probes into StateUp, StateDegraded, or StateDown +// (issue #6, superseding design §4.2's single "all ip OR all domain failed" +// rule). Only a fully failed ip group — raw TCP dials to IP literals, which +// need no DNS — is StateDown. A fully failed domain group while any ip target +// still connects is StateDegraded: both domain probes are HTTPS GETs sharing +// one DNS path (in production, through the very router this watchdog reboots), +// so they fail together as a matter of course, and 28 of 29 historical "down" +// verdicts were exactly that with the ip group answering in 19-1118ms. +// +// When no ip targets are configured at all, the domain group is the only +// evidence there is and a fully failed one is StateDown rather than a verdict +// nothing could ever act on. A single failed target within a group never trips +// anything; an empty group is "not applicable", never vacuously failed. func Verdict(targets []TargetResult) Result { var ip, dom []TargetResult for _, t := range targets { @@ -23,16 +45,35 @@ func Verdict(targets []TargetResult) Result { dom = append(dom, t) } } - return Result{Down: allFailed(ip) || allFailed(dom), Targets: targets} + + state := StateUp + switch { + case allFailed(ip): + state = StateDown + case allFailed(dom) && len(ip) == 0: + state = StateDown + case allFailed(dom): + state = StateDegraded + } + + return Result{State: state, Targets: targets} } // Result is the outcome of one connectivity check: every probed target's -// individual result plus the aggregate quorum verdict (design §4.2). +// individual result plus the aggregate verdict (design §4.2, issue #6). type Result struct { - Down bool + State string Targets []TargetResult } +// IsDegraded reports whether the application-layer path is broken while raw +// connectivity holds — worth recording and announcing, never worth a reboot. +func (r Result) IsDegraded() bool { return r.State == StateDegraded } + +// IsDown reports whether the raw link is gone, the only verdict a reboot may +// ever follow from. +func (r Result) IsDown() bool { return r.State == StateDown } + // TargetResult is one probed target's outcome, shaped to map directly onto a // checks row (design §9) once the caller supplies RunID and Phase. type TargetResult struct { diff --git a/domain/check_test.go b/domain/check_test.go index db9763f..9776e7d 100644 --- a/domain/check_test.go +++ b/domain/check_test.go @@ -5,80 +5,106 @@ import "testing" func TestVerdict(t *testing.T) { t.Parallel() - t.Run("one down target among several is not a down verdict", func(t *testing.T) { - t.Parallel() - - targets := []TargetResult{ - {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: true}, - {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: false}, - {Target: "https://example.com/generate_204", Kind: CheckKindDomain, OK: true}, - } - - got := Verdict(targets) - - if got.Down { - t.Fatalf("Down = true, want false (only one ip target failed): %+v", got.Targets) - } - }) - - t.Run("all ip targets down is a down verdict regardless of domains", func(t *testing.T) { - t.Parallel() - - targets := []TargetResult{ - {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: false}, - {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: false}, - {Target: "https://example.com/generate_204", Kind: CheckKindDomain, OK: true}, - } - - got := Verdict(targets) - - if !got.Down { - t.Fatalf("Down = false, want true (all ip targets failed): %+v", got.Targets) - } - }) - - t.Run("ip up but all domains down is a down verdict", func(t *testing.T) { - t.Parallel() - - targets := []TargetResult{ - {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: true}, - {Target: "https://a.example/", Kind: CheckKindDomain, OK: false}, - {Target: "https://b.example/", Kind: CheckKindDomain, OK: false}, - } - - got := Verdict(targets) - - if !got.Down { - t.Fatalf("Down = false, want true (all domain targets failed): %+v", got.Targets) - } - }) - - t.Run("at least one ip and one domain up is an up verdict", func(t *testing.T) { - t.Parallel() - - targets := []TargetResult{ - {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: true}, - {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: false}, - {Target: "https://a.example/", Kind: CheckKindDomain, OK: true}, - {Target: "https://b.example/", Kind: CheckKindDomain, OK: false}, - } - - got := Verdict(targets) - - if got.Down { - t.Fatalf("Down = true, want false: %+v", got.Targets) - } - }) - - t.Run("empty group is not applicable, never vacuously down", func(t *testing.T) { - t.Parallel() - - got := Verdict(nil) - - if got.Down { - t.Fatal("Down = true, want false: an empty target list must never be a vacuous down verdict") - } - }) + cases := []struct { + name string + targets []TargetResult + want string + }{ + { + name: "one down target among several is not a failure verdict", + targets: []TargetResult{ + {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: true}, + {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: false}, + {Target: "https://example.com/generate_204", Kind: CheckKindDomain, OK: true}, + }, + want: StateUp, + }, + { + name: "all ip targets down is a down verdict regardless of domains", + targets: []TargetResult{ + {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: false}, + {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: false}, + {Target: "https://example.com/generate_204", Kind: CheckKindDomain, OK: true}, + }, + want: StateDown, + }, + { + name: "every target down is a down verdict", + targets: []TargetResult{ + {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: false}, + {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: false}, + {Target: "https://a.example/", Kind: CheckKindDomain, OK: false}, + }, + want: StateDown, + }, + { + name: "ip up but all domains down is degraded, not down (issue #6)", + targets: []TargetResult{ + {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: true}, + {Target: "https://a.example/", Kind: CheckKindDomain, OK: false}, + {Target: "https://b.example/", Kind: CheckKindDomain, OK: false}, + }, + want: StateDegraded, + }, + { + name: "one ip up among failures still keeps a domain wipeout at degraded", + targets: []TargetResult{ + {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: false}, + {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: true}, + {Target: "https://a.example/", Kind: CheckKindDomain, OK: false}, + {Target: "https://b.example/", Kind: CheckKindDomain, OK: false}, + }, + want: StateDegraded, + }, + { + name: "with no ip targets configured, all domains down is the only evidence there is", + targets: []TargetResult{ + {Target: "https://a.example/", Kind: CheckKindDomain, OK: false}, + {Target: "https://b.example/", Kind: CheckKindDomain, OK: false}, + }, + want: StateDown, + }, + { + name: "at least one ip and one domain up is an up verdict", + targets: []TargetResult{ + {Target: "1.1.1.1:443", Kind: CheckKindIP, OK: true}, + {Target: "8.8.8.8:53", Kind: CheckKindIP, OK: false}, + {Target: "https://a.example/", Kind: CheckKindDomain, OK: true}, + {Target: "https://b.example/", Kind: CheckKindDomain, OK: false}, + }, + want: StateUp, + }, + { + name: "empty group is not applicable, never vacuously down", + targets: nil, + want: StateUp, + }, + { + name: "a gateway probe alone decides nothing", + targets: []TargetResult{ + {Target: "192.168.1.1:80", Kind: CheckKindGateway, OK: false}, + }, + want: StateUp, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + got := Verdict(c.targets) + + if got.State != c.want { + t.Fatalf("State = %q, want %q: %+v", got.State, c.want, got.Targets) + } + if got.IsDown() != (c.want == StateDown) { + t.Fatalf("IsDown() = %v, want %v", got.IsDown(), c.want == StateDown) + } + if got.IsDegraded() != (c.want == StateDegraded) { + t.Fatalf("IsDegraded() = %v, want %v", got.IsDegraded(), c.want == StateDegraded) + } + }) + } t.Run("targets slice is carried through unmodified", func(t *testing.T) { t.Parallel() diff --git a/domain/run.go b/domain/run.go index 8d9ec3f..13f2a80 100644 --- a/domain/run.go +++ b/domain/run.go @@ -20,22 +20,36 @@ const ( ActionSkippedDisabled = "skipped_disabled" ) -// PhaseInitial and PhaseRecovery are the values allowed in checks.phase -// (design §9). +// PhaseInitial, PhaseConfirm, and PhaseRecovery are the values allowed in +// checks.phase (design §9; PhaseConfirm from issue #6's re-check of a down +// verdict). const ( PhaseInitial = "initial" + PhaseConfirm = "confirm" PhaseRecovery = "recovery" ) // OutcomeOK, OutcomeRebootFailed, OutcomeTimeout, OutcomeSkipped, and // OutcomeRebootDisabled are the values written to runs.outcome (design §9; // plans/003-host-telemetry.md for OutcomeRebootDisabled). +// +// OutcomeDegraded, OutcomeUnconfirmed, and OutcomeUnsustained (issue #6) name +// the three ways a run can see a failure and still, correctly, not reboot: +// the failure was application-layer only (StateDegraded), a down verdict did +// not survive re-probing within the run, or a confirmed outage was not yet +// corroborated by the preceding runs. They are distinct values because an +// operator reads them differently — degraded means "something really is +// broken, just not the link", the other two mean "nothing was broken long +// enough to act on". const ( OutcomeOK = "ok" OutcomeRebootFailed = "reboot_failed" OutcomeTimeout = "timeout" OutcomeSkipped = "skipped" OutcomeRebootDisabled = "reboot_disabled" + OutcomeDegraded = "degraded" + OutcomeUnconfirmed = "unconfirmed" + OutcomeUnsustained = "unsustained" ) // Run is one row of the runs table (design §9), as returned by @@ -43,7 +57,10 @@ const ( // lexicographic and chronological order coincide) rather than an // autoincrement integer. InternetOK is nil for a hard-mode run, which // performs no initial check; every *At field is nil until its phase -// transition has happened. +// transition has happened. Since issue #6 InternetOK tracks StateDown +// specifically — a degraded run (raw link up, domain probes failing) records +// true and says so through its outcome — so it reads as "was the link itself +// gone", the one question the reboot decision turns on. type Run struct { ID string StartedAt time.Time @@ -72,11 +89,24 @@ type Check struct { Error string } +// RunSummary is the slice of a past run the reboot guards read back (issue +// #6): whether that run saw the link as down, and how it ended. Deliberately +// narrower than Run — the guards corroborate a present outage against recent +// history and must not grow a dependency on every column of it. +type RunSummary struct { + InternetOK *bool + Outcome string +} + // RunUpdate carries the phase timestamps, action, outcome, and error to // apply to a runs row. Every field is a pointer so RunRepository.UpdateRun // can tell "leave unchanged" (nil) apart from "set this value". Its SQL // rendering is a persistence detail that stays in infrastructure, not here. +// InternetOK exists so a run whose down verdict is later cleared by +// re-probing (issue #6) can correct the column it wrote from its first, +// mistaken sample. type RunUpdate struct { + InternetOK *bool Action *string RebootStartedAt *time.Time RouterDownAt *time.Time diff --git a/domain/watchdog.go b/domain/watchdog.go index b1b81de..ebdd266 100644 --- a/domain/watchdog.go +++ b/domain/watchdog.go @@ -12,6 +12,11 @@ import ( // ExitConfigError and ExitLockHeld are produced by main itself before Execute // is ever called (a bad .env, or a lock already held) and are declared here // only so every code in the design's table has one canonical home. +// ExitNoAction (issue #6) reports the state the original table had no honest +// code for: the run observed a real failure — a degraded application path, or +// an outage that did not survive the confirmation guards — and deliberately +// took no action. It is not ExitOK, which claims nothing was wrong, and not +// ExitSkippedCooldown, which means a reboot was due and suppressed. const ( ExitOK = 0 ExitConfigError = 1 @@ -19,6 +24,7 @@ const ( ExitRebootFailed = 3 ExitRecoveryTimeout = 4 ExitSkippedCooldown = 5 + ExitNoAction = 6 ) // ReasonNoInternet and ReasonScheduledHardReboot are the two reasons the diff --git a/gateway/httpapi/handlers_test.go b/gateway/httpapi/handlers_test.go index 498f4de..53d7b8e 100644 --- a/gateway/httpapi/handlers_test.go +++ b/gateway/httpapi/handlers_test.go @@ -516,15 +516,20 @@ func TestServer_handleOverview(t *testing.T) { srv := newTestServer(&fakeRepo{}, "tok") - rec := doRequest(t, srv, http.MethodGet, "/api/v1/overview?since=2026-07-01T00:00:00Z&bucket=2h", "tok") + // relative to now, not a literal date: QueryService floors ?since= at + // now-31d, so a hardcoded timestamp forwards intact only until the + // wall clock walks past it. + since := time.Now().UTC().Add(-24 * time.Hour).Truncate(time.Second).Format(time.RFC3339) + + rec := doRequest(t, srv, http.MethodGet, "/api/v1/overview?since="+since+"&bucket=2h", "tok") if rec.Code != http.StatusOK { t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) } var body dto.OverviewResponse decodeJSON(t, rec, &body) - if body.Window.Since != "2026-07-01T00:00:00Z" { - t.Fatalf("Window.Since = %q, want the forwarded since", body.Window.Since) + if body.Window.Since != since { + t.Fatalf("Window.Since = %q, want the forwarded since %q", body.Window.Since, since) } if body.Window.Bucket != "2h0m0s" { t.Fatalf("Window.Bucket = %q, want %q", body.Window.Bucket, "2h0m0s") diff --git a/infrastructure/check.go b/infrastructure/check.go index b6bc5b5..bb88902 100644 --- a/infrastructure/check.go +++ b/infrastructure/check.go @@ -2,9 +2,11 @@ package infrastructure import ( "context" + "crypto/tls" "fmt" "net" "net/http" + "net/http/httptrace" "net/url" "sync" "time" @@ -93,7 +95,11 @@ func (c *Checker) Gateway(ctx context.Context) domain.TargetResult { // probeHTTP GETs target within c.timeout and accepts any 2xx status (204 for // generate_204, 200 for cdn-cgi/trace) as success; anything else — including // a redirect, which CheckRedirect stops the client from following — is a -// failure with the response status recorded in Error. +// failure with the response status recorded in Error. A failure is tagged with +// the request stage it stalled in (issue #6): every recorded domain failure to +// date reads "context deadline exceeded", which does not say whether name +// resolution, the TCP connect, or the TLS handshake was what hung — and those +// have different causes and different fixes. func (c *Checker) probeHTTP(ctx context.Context, target string) domain.TargetResult { start := time.Now() @@ -102,10 +108,13 @@ func (c *Checker) probeHTTP(ctx context.Context, target string) domain.TargetRes return domain.TargetResult{Target: target, Kind: domain.CheckKindDomain, OK: false, Latency: time.Since(start), Error: err.Error()} } + stage := newStageTracker() + req = req.WithContext(httptrace.WithClientTrace(req.Context(), stage.trace())) + resp, err := c.httpClient.Do(req) latency := time.Since(start) if err != nil { - return domain.TargetResult{Target: target, Kind: domain.CheckKindDomain, OK: false, Latency: latency, Error: err.Error()} + return domain.TargetResult{Target: target, Kind: domain.CheckKindDomain, OK: false, Latency: latency, Error: stage.annotate(err)} } defer resp.Body.Close() @@ -161,3 +170,78 @@ func hostPortFromURL(rawURL string) (string, error) { return net.JoinHostPort(u.Hostname(), port), nil } + +// stageDNS, stageConnect, stageTLS, stageRequest, stageResponse, and +// stageBody name the phases of one HTTP probe request, in the order a request +// passes through them. +const ( + stageDNS = "dns" + stageConnect = "connect" + stageTLS = "tls" + stageRequest = "request" + stageResponse = "response" + stageBody = "body" +) + +// newStageTracker returns a tracker parked in stageDNS, the first phase a +// fresh request enters. A request served from the connection pool skips +// straight past it via GotConn. +func newStageTracker() *stageTracker { + return &stageTracker{stage: stageDNS} +} + +// stageTracker records how far one HTTP probe request got before it failed +// (issue #6). "context deadline exceeded" alone cannot distinguish a stalled +// resolver from a blackholed TCP connect or a hung TLS handshake, and those +// are different faults with different remedies — the tracker turns the +// recorded error into evidence. Its callbacks fire on whichever goroutine the +// transport is using (parallel dials during Happy Eyeballs included), hence +// the mutex. +type stageTracker struct { + mu sync.Mutex + stage string +} + +// annotate tags err with the stage the request was in when it failed. +func (s *stageTracker) annotate(err error) string { + s.mu.Lock() + defer s.mu.Unlock() + return fmt.Sprintf("%s (stage=%s)", err, s.stage) +} + +// set advances the tracker to stage. +func (s *stageTracker) set(stage string) { + s.mu.Lock() + defer s.mu.Unlock() + s.stage = stage +} + +// trace builds the ClientTrace that drives the tracker. Each hook records the +// phase the request is entering, so a request that dies mid-phase leaves that +// phase behind as its epitaph; the *Done hooks that carry an error leave the +// failed phase in place rather than advancing past it. +func (s *stageTracker) trace() *httptrace.ClientTrace { + return &httptrace.ClientTrace{ + DNSStart: func(httptrace.DNSStartInfo) { s.set(stageDNS) }, + DNSDone: func(info httptrace.DNSDoneInfo) { + if info.Err == nil { + s.set(stageConnect) + } + }, + ConnectStart: func(string, string) { s.set(stageConnect) }, + ConnectDone: func(_, _ string, err error) { + if err == nil { + s.set(stageTLS) + } + }, + TLSHandshakeStart: func() { s.set(stageTLS) }, + TLSHandshakeDone: func(_ tls.ConnectionState, err error) { + if err == nil { + s.set(stageRequest) + } + }, + GotConn: func(httptrace.GotConnInfo) { s.set(stageRequest) }, + WroteRequest: func(httptrace.WroteRequestInfo) { s.set(stageResponse) }, + GotFirstResponseByte: func() { s.set(stageBody) }, + } +} diff --git a/infrastructure/check_test.go b/infrastructure/check_test.go index 150ccc2..cd35fd6 100644 --- a/infrastructure/check_test.go +++ b/infrastructure/check_test.go @@ -5,6 +5,7 @@ import ( "net" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -72,8 +73,8 @@ func TestChecker_Check(t *testing.T) { c := newTestChecker(t, []string{up, down}, []string{domainUp}, time.Second) result := c.Check(t.Context()) - if result.Down { - t.Fatalf("Down = true, want false (only one ip target failed): %+v", result.Targets) + if result.State != domain.StateUp { + t.Fatalf("State = %q, want %q (only one ip target failed): %+v", result.State, domain.StateUp, result.Targets) } }) @@ -87,12 +88,12 @@ func TestChecker_Check(t *testing.T) { c := newTestChecker(t, []string{down1, down2}, []string{domainUp}, time.Second) result := c.Check(t.Context()) - if !result.Down { - t.Fatalf("Down = false, want true (all ip targets failed): %+v", result.Targets) + if !result.IsDown() { + t.Fatalf("State = %q, want %q (all ip targets failed): %+v", result.State, domain.StateDown, result.Targets) } }) - t.Run("ip up but all domains down is a down verdict", func(t *testing.T) { + t.Run("ip up but all domains down is degraded, not down", func(t *testing.T) { t.Parallel() up := newReachableListener(t) @@ -102,8 +103,8 @@ func TestChecker_Check(t *testing.T) { c := newTestChecker(t, []string{up}, []string{domainDown1, domainDown2}, time.Second) result := c.Check(t.Context()) - if !result.Down { - t.Fatalf("Down = false, want true (all domain targets failed): %+v", result.Targets) + if !result.IsDegraded() { + t.Fatalf("State = %q, want %q (all domain targets failed while an ip target answered): %+v", result.State, domain.StateDegraded, result.Targets) } }) @@ -118,8 +119,8 @@ func TestChecker_Check(t *testing.T) { c := newTestChecker(t, []string{up, down}, []string{domainUp, domainDown}, time.Second) result := c.Check(t.Context()) - if result.Down { - t.Fatalf("Down = true, want false: %+v", result.Targets) + if result.State != domain.StateUp { + t.Fatalf("State = %q, want %q: %+v", result.State, domain.StateUp, result.Targets) } }) @@ -209,6 +210,60 @@ func TestChecker_Check(t *testing.T) { } }) + t.Run("a failed domain probe records the request stage it stalled in", func(t *testing.T) { + t.Parallel() + + const timeout = 50 * time.Millisecond + hung := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case <-r.Context().Done(): + case <-time.After(5 * time.Second): + w.WriteHeader(http.StatusNoContent) + } + })) + t.Cleanup(hung.Close) + + cases := []struct { + name string + target string + wantStage string + }{ + { + // RFC 2606 reserves .invalid, so this name resolves nowhere. + name: "unresolvable host stalls in dns", + target: "http://yarddog-issue-6.invalid/", + wantStage: "stage=" + stageDNS, + }, + { + name: "refused connection stalls in connect", + target: "http://" + newClosedAddr(t) + "/", + wantStage: "stage=" + stageConnect, + }, + { + name: "a server that never answers stalls awaiting the response", + target: hung.URL, + wantStage: "stage=" + stageResponse, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + chk := newTestChecker(t, nil, []string{c.target}, timeout) + result := chk.Check(t.Context()) + + got := result.Targets[0] + if got.OK { + t.Fatalf("OK = true, want false: %+v", got) + } + if !strings.Contains(got.Error, c.wantStage) { + t.Fatalf("Error = %q, want it to carry %q", got.Error, c.wantStage) + } + }) + } + }) + t.Run("an already-expired context fails the probe immediately", func(t *testing.T) { t.Parallel() diff --git a/infrastructure/config.go b/infrastructure/config.go index 9e477ac..2228533 100644 --- a/infrastructure/config.go +++ b/infrastructure/config.go @@ -60,6 +60,7 @@ func LoadConfig(envPath string) (*Config, error) { {"RECOVERY_INTERVAL", &cfg.RecoveryInterval, defaultRecoveryInterval}, {"RECOVERY_TIMEOUT", &cfg.RecoveryTimeout, defaultRecoveryTimeout}, {"REBOOT_COOLDOWN", &cfg.RebootCooldown, defaultRebootCooldown}, + {"CHECK_CONFIRM_DELAY", &cfg.CheckConfirmDelay, defaultCheckConfirmDelay}, } for _, d := range durations { raw := getOrDefault(get, d.key, d.def) @@ -69,6 +70,24 @@ func LoadConfig(envPath string) (*Config, error) { } *d.dst = parsed } + cfg.CheckConfirmDelay = clampCheckConfirmDelay(cfg.CheckConfirmDelay) + + // CheckConfirmations and RebootMinStreak are the two issue #6 guards + // between a down verdict and a reboot; both are sample counts, and both + // clamp rather than fail startup, matching PING_COUNT's philosophy. + rawConfirmations := getOrDefault(get, "CHECK_CONFIRMATIONS", defaultCheckConfirmations) + confirmations, err := strconv.Atoi(rawConfirmations) + if err != nil { + return nil, fmt.Errorf("invalid %s %q: %w", envPrefix+"CHECK_CONFIRMATIONS", rawConfirmations, err) + } + cfg.CheckConfirmations = clampCheckConfirmations(confirmations) + + rawMinStreak := getOrDefault(get, "REBOOT_MIN_STREAK", defaultRebootMinStreak) + minStreak, err := strconv.Atoi(rawMinStreak) + if err != nil { + return nil, fmt.Errorf("invalid %s %q: %w", envPrefix+"REBOOT_MIN_STREAK", rawMinStreak, err) + } + cfg.RebootMinStreak = clampRebootMinStreak(minStreak) // RetentionDays now governs the *archive* tier only (issue #4): a run // stays hot for HotWindowDays regardless of this value, and @@ -165,6 +184,13 @@ type Config struct { PingHosts []string PingCount int PingTimeout time.Duration + + // CheckConfirmations, CheckConfirmDelay, and RebootMinStreak are issue + // #6's false-positive guards; see services.Settings for what each one + // buys. + CheckConfirmations int + CheckConfirmDelay time.Duration + RebootMinStreak int } const ( @@ -183,6 +209,17 @@ const ( defaultPingCount = "5" defaultPingTimeout = "4s" + // defaultCheckConfirmations and defaultCheckConfirmDelay re-probe a down + // verdict twice, 20s apart, so a reboot decision rests on ~45s of evidence + // instead of one 5s sample and still lands well inside a 5-minute cron + // slot. defaultRebootMinStreak then requires the previous run to have seen + // the outage too (issue #6): in 21 days of recorded history exactly one + // down verdict had a down predecessor, and 11 reboots fired on isolated + // samples with the ip targets answering normally throughout. + defaultCheckConfirmations = "2" + defaultCheckConfirmDelay = "20s" + defaultRebootMinStreak = "2" + // minHotWindowDays floors HOT_WINDOW_DAYS (issue #4): there is no // "disable roll-over" mode — unbounded hot growth is the bug this knob // fixes — so the floor is 1, not 0. maxHotWindowDays and maxRetentionDays @@ -214,6 +251,28 @@ const ( // every run, making the operator's setting a no-op. minPingTimeout = 1 * time.Second maxPingTimeout = 10 * time.Second + + // minCheckConfirmations bounds CHECK_CONFIRMATIONS at 0 — "act on the + // first sample", the pre-issue-#6 behaviour, which stays available to an + // operator who wants it — and the ceiling keeps the whole confirmation + // window (attempts x delay, plus each attempt's own CHECK_TIMEOUT) inside + // a single cron slot, so a run can never still be confirming when the next + // one starts and hits the flock. + minCheckConfirmations = 0 + maxCheckConfirmations = 5 + + // minCheckConfirmDelay and maxCheckConfirmDelay bound the gap between + // re-probes: below a second the "re-check" would sample the same stalled + // moment it just failed on, and above a minute the guard would outlast the + // cron cadence it is nested inside. + minCheckConfirmDelay = 1 * time.Second + maxCheckConfirmDelay = 60 * time.Second + + // minRebootMinStreak bounds REBOOT_MIN_STREAK at 1 ("this run alone is + // enough", no cross-run corroboration). The ceiling stops a typo from + // demanding a streak so long the watchdog would never reboot at all. + minRebootMinStreak = 1 + maxRebootMinStreak = 10 ) // clampHotWindowDays floors HOT_WINDOW_DAYS at minHotWindowDays (1) rather @@ -244,6 +303,42 @@ func clampRetentionDays(n int) int { return n } +// clampCheckConfirmations bounds CHECK_CONFIRMATIONS to [minCheckConfirmations, +// maxCheckConfirmations], clamping rather than refusing to start (issue #6). +func clampCheckConfirmations(n int) int { + if n < minCheckConfirmations { + return minCheckConfirmations + } + if n > maxCheckConfirmations { + return maxCheckConfirmations + } + return n +} + +// clampCheckConfirmDelay bounds CHECK_CONFIRM_DELAY to [minCheckConfirmDelay, +// maxCheckConfirmDelay] (issue #6). +func clampCheckConfirmDelay(d time.Duration) time.Duration { + if d < minCheckConfirmDelay { + return minCheckConfirmDelay + } + if d > maxCheckConfirmDelay { + return maxCheckConfirmDelay + } + return d +} + +// clampRebootMinStreak bounds REBOOT_MIN_STREAK to [minRebootMinStreak, +// maxRebootMinStreak] (issue #6). +func clampRebootMinStreak(n int) int { + if n < minRebootMinStreak { + return minRebootMinStreak + } + if n > maxRebootMinStreak { + return maxRebootMinStreak + } + return n +} + // clampPingCount bounds PING_COUNT to [minPingCount, maxPingCount], silently // correcting an out-of-range operator value rather than failing startup over // it (matching QueryService.clampLimit's "clamp, don't error" philosophy for diff --git a/infrastructure/config_test.go b/infrastructure/config_test.go index 8828501..1bb064f 100644 --- a/infrastructure/config_test.go +++ b/infrastructure/config_test.go @@ -396,6 +396,92 @@ func TestLoadConfig(t *testing.T) { } }) + t.Run("issue #6 guards default to two re-checks 20s apart and a two-run streak", func(t *testing.T) { + t.Parallel() + + path := writeConfigFixture(t, requiredOnlyEnv()) + + cfg, err := LoadConfig(path) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if cfg.CheckConfirmations != 2 { + t.Errorf("CheckConfirmations = %d, want 2", cfg.CheckConfirmations) + } + if cfg.CheckConfirmDelay != 20*time.Second { + t.Errorf("CheckConfirmDelay = %v, want 20s", cfg.CheckConfirmDelay) + } + if cfg.RebootMinStreak != 2 { + t.Errorf("RebootMinStreak = %d, want 2", cfg.RebootMinStreak) + } + }) + + t.Run("issue #6 guard values are clamped, never fatal", func(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + env map[string]string + wantConfirmations int + wantConfirmDelay time.Duration + wantRebootMinStrek int + }{ + { + name: "below the floor", + env: map[string]string{"CHECK_CONFIRMATIONS": "-1", "CHECK_CONFIRM_DELAY": "10ms", "REBOOT_MIN_STREAK": "0"}, + wantConfirmations: minCheckConfirmations, + wantConfirmDelay: minCheckConfirmDelay, + wantRebootMinStrek: minRebootMinStreak, + }, + { + name: "above the ceiling", + env: map[string]string{"CHECK_CONFIRMATIONS": "99", "CHECK_CONFIRM_DELAY": "10m", "REBOOT_MIN_STREAK": "99"}, + wantConfirmations: maxCheckConfirmations, + wantConfirmDelay: maxCheckConfirmDelay, + wantRebootMinStrek: maxRebootMinStreak, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + env := requiredOnlyEnv() + for k, v := range c.env { + env[k] = v + } + path := writeConfigFixture(t, env) + + cfg, err := LoadConfig(path) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if cfg.CheckConfirmations != c.wantConfirmations { + t.Errorf("CheckConfirmations = %d, want %d", cfg.CheckConfirmations, c.wantConfirmations) + } + if cfg.CheckConfirmDelay != c.wantConfirmDelay { + t.Errorf("CheckConfirmDelay = %v, want %v", cfg.CheckConfirmDelay, c.wantConfirmDelay) + } + if cfg.RebootMinStreak != c.wantRebootMinStrek { + t.Errorf("RebootMinStreak = %d, want %d", cfg.RebootMinStreak, c.wantRebootMinStrek) + } + }) + } + }) + + t.Run("non-integer CHECK_CONFIRMATIONS returns an error", func(t *testing.T) { + t.Parallel() + + env := requiredOnlyEnv() + env["CHECK_CONFIRMATIONS"] = "twice" + path := writeConfigFixture(t, env) + + _, err := LoadConfig(path) + if err == nil { + t.Fatal("LoadConfig() error = nil, want error for non-integer CHECK_CONFIRMATIONS") + } + }) + t.Run("PING_COUNT below the floor is clamped to 4", func(t *testing.T) { t.Parallel() diff --git a/infrastructure/store.go b/infrastructure/store.go index 5c311f5..3efa9a5 100644 --- a/infrastructure/store.go +++ b/infrastructure/store.go @@ -989,6 +989,49 @@ func (s *Store) PruneArchive(ctx context.Context, now time.Time, retentionDays i return pruned, nil } +// RecentRuns returns the hot runs immediately preceding beforeID, newest +// first, at most limit of them (issue #6). Ordering and the "preceding" +// predicate both ride on the UUIDv7 primary key (issue #4), which is +// time-ordered, so no secondary index on started_at is needed and the caller's +// own row is excluded by a plain id comparison. It reads the hot table only: +// the guards corroborate against the last few minutes, never against history +// old enough to have rolled into the archive. +func (s *Store) RecentRuns(ctx context.Context, beforeID string, limit int) ([]domain.RunSummary, error) { + rows, err := s.db.QueryContext(ctx, + fmt.Sprintf(`SELECT %s, %s FROM %s WHERE %s < ? ORDER BY %s DESC LIMIT ?`, + colRunsInternetOK, colRunsOutcome, tableRuns, colRunsID, colRunsID), + beforeID, limit, + ) + if err != nil { + return nil, fmt.Errorf("recent runs before %s: %w", beforeID, err) + } + defer rows.Close() + + var out []domain.RunSummary + for rows.Next() { + var ( + internetOK sql.NullBool + outcome sql.NullString + ) + if err := rows.Scan(&internetOK, &outcome); err != nil { + return nil, fmt.Errorf("recent runs before %s: %w", beforeID, err) + } + + var summary domain.RunSummary + if internetOK.Valid { + ok := internetOK.Bool + summary.InternetOK = &ok + } + summary.Outcome = outcome.String + out = append(out, summary) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("recent runs before %s: %w", beforeID, err) + } + + return out, nil +} + // RolloverToArchive moves every run (and all its children) with // started_at older than now - hotWindowDays days from the hot tables into // their *_archive twins, in one transaction (issue #4): this run-boundary @@ -1888,6 +1931,9 @@ func runUpdateAssignments(u domain.RunUpdate) ([]string, []any) { args = append(args, v) } + if u.InternetOK != nil { + add(colRunsInternetOK, boolToInt(*u.InternetOK)) + } if u.Action != nil { add(colRunsAction, *u.Action) } diff --git a/infrastructure/store_test.go b/infrastructure/store_test.go index 529b88a..802706e 100644 --- a/infrastructure/store_test.go +++ b/infrastructure/store_test.go @@ -1674,6 +1674,107 @@ func TestStore_ListRuns(t *testing.T) { }) } +func TestStore_RecentRuns(t *testing.T) { + t.Run("empty store returns an empty slice, not an error", func(t *testing.T) { + t.Parallel() + + s := newTestStore(t) + + got, err := s.RecentRuns(t.Context(), "ffffffff-ffff-ffff-ffff-ffffffffffff", 3) + if err != nil { + t.Fatalf("RecentRuns() error = %v", err) + } + if len(got) != 0 { + t.Fatalf("RecentRuns() = %d rows, want 0 on an empty store", len(got)) + } + }) + + t.Run("returns the runs before the given id, newest first, honouring limit", func(t *testing.T) { + t.Parallel() + + s := newTestStore(t) + down, up := false, true + states := []*bool{&up, &down, &down, &up, nil} + + var ids []string + for _, state := range states { + id, err := s.InsertRun(t.Context(), time.Now(), domain.ModeSoft, state) + if err != nil { + t.Fatalf("InsertRun() error = %v", err) + } + ids = append(ids, id) + } + + // asking from the newest run's own id must never hand it back its + // own row: the guards ask "what did the runs before me see?". + got, err := s.RecentRuns(t.Context(), ids[4], 2) + if err != nil { + t.Fatalf("RecentRuns() error = %v", err) + } + if len(got) != 2 { + t.Fatalf("RecentRuns() = %d rows, want 2 (limit honoured)", len(got)) + } + if got[0].InternetOK == nil || !*got[0].InternetOK { + t.Fatalf("RecentRuns()[0].InternetOK = %v, want true (the run immediately before)", got[0].InternetOK) + } + if got[1].InternetOK == nil || *got[1].InternetOK { + t.Fatalf("RecentRuns()[1].InternetOK = %v, want false", got[1].InternetOK) + } + }) + + t.Run("a hard run's NULL internet_ok reads back as nil, not false", func(t *testing.T) { + t.Parallel() + + s := newTestStore(t) + + if _, err := s.InsertRun(t.Context(), time.Now(), domain.ModeHard, nil); err != nil { + t.Fatalf("InsertRun() error = %v", err) + } + newest, err := s.InsertRun(t.Context(), time.Now(), domain.ModeSoft, nil) + if err != nil { + t.Fatalf("InsertRun() error = %v", err) + } + + got, err := s.RecentRuns(t.Context(), newest, 1) + if err != nil { + t.Fatalf("RecentRuns() error = %v", err) + } + if len(got) != 1 { + t.Fatalf("RecentRuns() = %d rows, want 1", len(got)) + } + if got[0].InternetOK != nil { + t.Fatalf("InternetOK = %v, want nil (a hard run never probed)", *got[0].InternetOK) + } + }) + + t.Run("carries the outcome the run ended on", func(t *testing.T) { + t.Parallel() + + s := newTestStore(t) + + first, err := s.InsertRun(t.Context(), time.Now(), domain.ModeSoft, nil) + if err != nil { + t.Fatalf("InsertRun() error = %v", err) + } + outcome := domain.OutcomeDegraded + if err := s.UpdateRun(t.Context(), first, domain.RunUpdate{Outcome: &outcome}); err != nil { + t.Fatalf("UpdateRun() error = %v", err) + } + newest, err := s.InsertRun(t.Context(), time.Now(), domain.ModeSoft, nil) + if err != nil { + t.Fatalf("InsertRun() error = %v", err) + } + + got, err := s.RecentRuns(t.Context(), newest, 1) + if err != nil { + t.Fatalf("RecentRuns() error = %v", err) + } + if len(got) != 1 || got[0].Outcome != domain.OutcomeDegraded { + t.Fatalf("RecentRuns() = %+v, want one row carrying outcome %q", got, domain.OutcomeDegraded) + } + }) +} + func TestStore_ListUnsentOutboxMessages(t *testing.T) { t.Run("orders oldest first and excludes sent rows", func(t *testing.T) { t.Parallel() @@ -2625,6 +2726,30 @@ func TestStore_RunByID(t *testing.T) { } func TestStore_UpdateRun(t *testing.T) { + t.Run("corrects internet_ok written from a mistaken first sample", func(t *testing.T) { + t.Parallel() + + s := newTestStore(t) + down := false + id, err := s.InsertRun(t.Context(), time.Now(), domain.ModeSoft, &down) + if err != nil { + t.Fatalf("InsertRun() error = %v", err) + } + + up := true + if err := s.UpdateRun(t.Context(), id, domain.RunUpdate{InternetOK: &up}); err != nil { + t.Fatalf("UpdateRun() error = %v", err) + } + + run, err := s.GetRun(t.Context(), id) + if err != nil { + t.Fatalf("GetRun() error = %v", err) + } + if run.InternetOK == nil || !*run.InternetOK { + t.Fatalf("InternetOK = %v, want true (issue #6: the re-check cleared the down verdict)", run.InternetOK) + } + }) + t.Run("applies only the non-nil fields", func(t *testing.T) { t.Parallel() diff --git a/services/fakes_test.go b/services/fakes_test.go index 2fa86b8..b96b199 100644 --- a/services/fakes_test.go +++ b/services/fakes_test.go @@ -115,11 +115,15 @@ func newFakeRunRepo() *fakeRunRepo { // real against SQLite in infrastructure/store_test.go. type fakeRunRepo struct { runs map[string]*domain.Run + checks []domain.Check nextID int64 lastRebootStartedAt time.Time lastRebootOK bool lastRebootErr error + + recentRuns []domain.RunSummary + recentRunsErr error } func (f *fakeRunRepo) GetLastRebootStartedAt(context.Context) (time.Time, bool, error) { @@ -129,10 +133,22 @@ func (f *fakeRunRepo) GetLastRebootStartedAt(context.Context) (time.Time, bool, return f.lastRebootStartedAt, f.lastRebootOK, nil } -func (f *fakeRunRepo) InsertCheck(_ context.Context, _ domain.Check) error { +func (f *fakeRunRepo) InsertCheck(_ context.Context, c domain.Check) error { + f.checks = append(f.checks, c) return nil } +// phases returns the phase of every persisted check in insert order, so a +// test can assert what a run actually recorded (issue #6: the confirmation +// re-probes must land in the record as domain.PhaseConfirm, not vanish). +func (f *fakeRunRepo) phases() []string { + out := make([]string, 0, len(f.checks)) + for _, c := range f.checks { + out = append(out, c.Phase) + } + return out +} + func (f *fakeRunRepo) InsertRun(_ context.Context, startedAt time.Time, mode string, internetOK *bool) (string, error) { f.nextID++ id := strconv.FormatInt(f.nextID, 10) @@ -140,6 +156,21 @@ func (f *fakeRunRepo) InsertRun(_ context.Context, startedAt time.Time, mode str return id, nil } +// RecentRuns serves the canned recentRuns slice (newest first), truncated to +// limit, rather than deriving history from inserted rows — the same "dumb and +// settable" stance as GetLastRebootStartedAt, so a guard test scripts exactly +// the history it wants to exercise. The real id-ordered query is exercised +// against SQLite in infrastructure/store_test.go. +func (f *fakeRunRepo) RecentRuns(_ context.Context, _ string, limit int) ([]domain.RunSummary, error) { + if f.recentRunsErr != nil { + return nil, f.recentRunsErr + } + if limit < len(f.recentRuns) { + return f.recentRuns[:limit], nil + } + return f.recentRuns, nil +} + // run returns a copy of the in-memory row id for readback assertions, // failing the test if it was never inserted. func (f *fakeRunRepo) run(t *testing.T, id string) domain.Run { @@ -158,6 +189,9 @@ func (f *fakeRunRepo) UpdateRun(_ context.Context, id string, u domain.RunUpdate return fmt.Errorf("fakeRunRepo: no run %s", id) } + if u.InternetOK != nil { + run.InternetOK = u.InternetOK + } if u.Action != nil { run.Action = *u.Action } diff --git a/services/orchestrator.go b/services/orchestrator.go index e2b0d26..0148118 100644 --- a/services/orchestrator.go +++ b/services/orchestrator.go @@ -155,6 +155,79 @@ func (r *runner) collectTelemetry(ctx context.Context, runID string) { wg.Wait() } +// confirmDown re-probes a down verdict CHECK_CONFIRMATIONS more times, +// CHECK_CONFIRM_DELAY apart, and returns the last result (issue #6): a single +// five-second sample must never cost the whole LAN a router reboot. It returns +// early on the first sample that is no longer down, and every attempt is +// persisted under domain.PhaseConfirm so the record shows what the decision +// was actually made on. The waiting goes through clock.After, so tests advance +// virtual time instead of sleeping. When the re-check clears the verdict it +// also corrects internet_ok, which InsertRun wrote from the mistaken first +// sample. +func (r *runner) confirmDown(ctx context.Context, runID string, initial domain.Result) domain.Result { + result := initial + for i := 0; i < r.settings.CheckConfirmations; i++ { + <-r.clock.After(r.settings.CheckConfirmDelay) + + result = r.checker.Check(ctx) + r.persistChecks(ctx, runID, domain.PhaseConfirm, result.Targets) + if !result.IsDown() { + break + } + } + + if !result.IsDown() { + internetOK := true + if err := r.repo.UpdateRun(ctx, runID, domain.RunUpdate{InternetOK: &internetOK}); err != nil { + r.logf("update run %s: %v", runID, err) + } + r.logf("run %s: down verdict not confirmed on re-check (state now %s): not rebooting", runID, result.State) + } + + return result +} + +// finishDegraded records a run whose domain (HTTPS) targets all failed while +// raw ip connectivity held (issue #6). The router is never rebooted for this: +// on 2026-07-30 four reboots in seven hours did not stop the same domain +// probes from stalling. The operator is told once per episode — shouldAnnounce +// suppresses the repeat while the state persists — because the fault is real +// (name resolution or the HTTPS path is broken for everything on the LAN) even +// though rebooting is not the answer to it. +func (r *runner) finishDegraded(ctx context.Context, runID string) int { + if r.shouldAnnounce(ctx, runID, domain.OutcomeDegraded) { + r.notify(ctx, "domain checks failing while raw connectivity holds (no reboot)") + } + if err := r.notifier.Flush(ctx); err != nil { + r.logf("flush outbox: %v", err) + } + + outcome := domain.OutcomeDegraded + finishedAt := r.clock.Now() + if err := r.repo.UpdateRun(ctx, runID, domain.RunUpdate{Outcome: &outcome, FinishedAt: &finishedAt}); err != nil { + r.logf("update run %s: %v", runID, err) + } + return domain.ExitNoAction +} + +// finishNoOutage closes a run that reached no reboot: outcome ok (the internet +// was up), unconfirmed (a down verdict the in-run re-check cleared), or +// unsustained (a confirmed outage the preceding runs did not corroborate). The +// last two send no Telegram message by design — silence is the point of the +// guards, and the run row plus the log line carry the record instead (issue +// #6). +func (r *runner) finishNoOutage(ctx context.Context, runID, outcome string, exitCode int) int { + if outcome == domain.OutcomeUnsustained { + r.logf("run %s: outage not corroborated by the previous %d run(s): not rebooting", runID, r.settings.RebootMinStreak-1) + } + + finishedAt := r.clock.Now() + if err := r.repo.UpdateRun(ctx, runID, domain.RunUpdate{Outcome: &outcome, FinishedAt: &finishedAt}); err != nil { + r.logf("update run %s: %v", runID, err) + } + return exitCode +} + // finishRebootDisabled records a run that would otherwise have rebooted but // REBOOT_ENABLED is off (plans/003-host-telemetry.md: monitor-only mode). // Exit stays domain.ExitOK: the reboot path was never entered, so none of @@ -288,6 +361,37 @@ func (r *runner) notify(ctx context.Context, text string) { } } +// outageSustained reports whether the REBOOT_MIN_STREAK-1 runs before runID +// also saw the internet as down (issue #6). One five-minute cron sample is not +// an outage: in 21 days of recorded history exactly one down verdict had a +// down predecessor, while 11 reboots fired on isolated samples. A run with no +// verdict at all (a hard-mode run, internet_ok NULL) does not corroborate — it +// never probed — and neither does a history too short to fill the streak. A +// failed query is fail-closed for the same reason as the cooldown guard: an +// unreadable history must not fall through to an unconditional reboot. +func (r *runner) outageSustained(ctx context.Context, runID string) bool { + need := r.settings.RebootMinStreak - 1 + if need <= 0 { + return true + } + + previous, err := r.repo.RecentRuns(ctx, runID, need) + if err != nil { + r.logf("recent runs: %v", err) + return false + } + if len(previous) < need { + return false + } + + for _, p := range previous { + if p.InternetOK == nil || *p.InternetOK { + return false + } + } + return true +} + // persistCheck writes one probed target as a checks row (design §9). func (r *runner) persistCheck(ctx context.Context, runID, phase string, tr domain.TargetResult) { latencyMS := tr.Latency.Milliseconds() @@ -361,7 +465,11 @@ func (r *runner) recoveryLoop(ctx context.Context, runID string, rebootStartedAt inet := r.checker.Check(ctx) r.persistChecks(ctx, runID, domain.PhaseRecovery, inet.Targets) - if !inet.Down { + // anything short of a down verdict ends the loop: a degraded state + // (issue #6) means the link the reboot was meant to restore is back, + // and waiting out the full RECOVERY_TIMEOUT for DNS to also recover + // would only hold the process open for a fault a reboot cannot fix. + if !inet.IsDown() { return r.finishRecoverySuccess(ctx, runID, now) } if now.Sub(rebootStartedAt) >= r.settings.RecoveryTimeout { @@ -380,11 +488,34 @@ func (r *runner) run(ctx context.Context, mode string) int { return r.softFlow(ctx, startedAt) } +// shouldAnnounce reports whether a repeating state is worth another Telegram +// message: true only when the run before runID did not already end in the same +// outcome (issue #6). A cron collector runs every few minutes, so a state that +// lasts an hour would otherwise send a message per run — the storm visible in +// issue #6's screenshot, where five identical cooldown notices arrived between +// 01:50 and 03:05. A failed history query announces: a duplicate message costs +// the operator far less than a silent outage. +func (r *runner) shouldAnnounce(ctx context.Context, runID, outcome string) bool { + previous, err := r.repo.RecentRuns(ctx, runID, 1) + if err != nil { + r.logf("recent runs: %v", err) + return true + } + if len(previous) == 0 { + return true + } + return previous[0].Outcome != outcome +} + // skipCooldown records a reboot skipped because the last one is still within // REBOOT_COOLDOWN (design §6) — rebooting through a provider-side outage -// would otherwise cycle the router every run for no benefit. +// would otherwise cycle the router every run for no benefit. The notice is +// edge-triggered (shouldAnnounce): the skip repeats every run for as long as +// the cooldown lasts, but it is only news the first time. func (r *runner) skipCooldown(ctx context.Context, runID string, age time.Duration) int { - r.notify(ctx, fmt.Sprintf("no internet, skipping reboot (cooldown: last reboot %s ago)", humanDuration(age))) + if r.shouldAnnounce(ctx, runID, domain.OutcomeSkipped) { + r.notify(ctx, fmt.Sprintf("no internet, skipping reboot (cooldown: last reboot %s ago)", humanDuration(age))) + } action := domain.ActionSkippedCooldown outcome := domain.OutcomeSkipped @@ -416,13 +547,16 @@ func (r *runner) skipCooldownUnknown(ctx context.Context, runID string, queryErr } // softFlow probes the internet first (design §6): up means nothing to do; -// down checks REBOOT_ENABLED before ever consulting the cooldown query, so a -// monitor-only host never touches the router (plans/003-host-telemetry.md). +// down goes through the issue #6 guards — re-probe within the run, then +// require the preceding runs to agree — before REBOOT_ENABLED and the +// cooldown query are ever consulted, so a monitor-only host never touches the +// router (plans/003-host-telemetry.md) and a five-second blip never reboots +// anything. func (r *runner) softFlow(ctx context.Context, startedAt time.Time) int { result := r.checker.Check(ctx) - internetOK := !result.Down + initialOK := !result.IsDown() - runID, err := r.repo.InsertRun(ctx, startedAt, domain.ModeSoft, &internetOK) + runID, err := r.repo.InsertRun(ctx, startedAt, domain.ModeSoft, &initialOK) if err != nil { r.logf("insert run: %v", err) return domain.ExitConfigError @@ -430,19 +564,27 @@ func (r *runner) softFlow(ctx context.Context, startedAt time.Time) int { r.persistChecks(ctx, runID, domain.PhaseInitial, result.Targets) r.collectTelemetry(ctx, runID) - if internetOK { - outcome := domain.OutcomeOK - finishedAt := r.clock.Now() - if err := r.repo.UpdateRun(ctx, runID, domain.RunUpdate{Outcome: &outcome, FinishedAt: &finishedAt}); err != nil { - r.logf("update run %s: %v", runID, err) - } - return domain.ExitOK + if result.IsDown() { + result = r.confirmDown(ctx, runID, result) + } + + switch { + case result.IsDegraded(): + return r.finishDegraded(ctx, runID) + case !result.IsDown() && !initialOK: + return r.finishNoOutage(ctx, runID, domain.OutcomeUnconfirmed, domain.ExitOK) + case !result.IsDown(): + return r.finishNoOutage(ctx, runID, domain.OutcomeOK, domain.ExitOK) } if !r.settings.RebootEnabled { return r.finishRebootDisabled(ctx, runID, "no internet, reboot disabled (monitor-only)") } + if !r.outageSustained(ctx, runID) { + return r.finishNoOutage(ctx, runID, domain.OutcomeUnsustained, domain.ExitNoAction) + } + lastRebootStartedAt, ok, err := r.repo.GetLastRebootStartedAt(ctx) if err != nil { r.logf("get last reboot started at: %v", err) diff --git a/services/orchestrator_test.go b/services/orchestrator_test.go index 56e8fab..1dae3c1 100644 --- a/services/orchestrator_test.go +++ b/services/orchestrator_test.go @@ -16,7 +16,7 @@ func TestExecute(t *testing.T) { t.Parallel() env := newTestEnv(t) - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true, Latency: 5 * time.Millisecond}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true, Latency: 5 * time.Millisecond}}}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -60,7 +60,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.settings.RebootCooldown = 2 * time.Hour - env.chk.checkResults = []domain.Result{{Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "dial timeout"}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "dial timeout"}}}} env.repo.lastRebootStartedAt = env.clk.now.Add(-40 * time.Minute) env.repo.lastRebootOK = true @@ -100,7 +100,7 @@ func TestExecute(t *testing.T) { t.Parallel() env := newTestEnv(t) - env.chk.checkResults = []domain.Result{{Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "dial timeout"}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "dial timeout"}}}} // fakeRunRepo.lastRebootErr stands in for any transient store error // that makes the cooldown state genuinely unknown (e.g. a corrupted // reboot_started_at column) — the real SQL failure mode is covered @@ -142,8 +142,8 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.chk.checkResults = []domain.Result{ - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "down"}}}, - {Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "down"}}}, + {State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, } env.chk.gatewayResults = []domain.TargetResult{{Target: "192.168.1.1:80", Kind: domain.CheckKindGateway, OK: true}} @@ -173,7 +173,7 @@ func TestExecute(t *testing.T) { t.Parallel() env := newTestEnv(t) - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} env.chk.gatewayResults = []domain.TargetResult{{Target: "192.168.1.1:80", Kind: domain.CheckKindGateway, OK: true}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeHard) @@ -203,11 +203,11 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.chk.checkResults = []domain.Result{ - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // initial soft check - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 1 - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 2 - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 3 - {Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, // tick 4 + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // initial soft check + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 1 + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 2 + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 3 + {State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, // tick 4 } env.chk.gatewayResults = []domain.TargetResult{ {Target: "gw", Kind: domain.CheckKindGateway, OK: false}, // tick 1: went down @@ -261,9 +261,9 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.chk.checkResults = []domain.Result{ - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // initial - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 1 - {Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, // tick 2 + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // initial + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // tick 1 + {State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, // tick 2 } // gateway never observed down: the router cycled between polls. env.chk.gatewayResults = []domain.TargetResult{{Target: "gw", Kind: domain.CheckKindGateway, OK: true}} @@ -306,7 +306,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.settings.RecoveryInterval = time.Minute env.settings.RecoveryTimeout = 15 * time.Minute - env.chk.checkResults = []domain.Result{{Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} env.chk.gatewayResults = []domain.TargetResult{{Target: "gw", Kind: domain.CheckKindGateway, OK: false}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -334,7 +334,7 @@ func TestExecute(t *testing.T) { t.Parallel() env := newTestEnv(t) - env.chk.checkResults = []domain.Result{{Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} env.rb.err = errRebootFailed code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -366,7 +366,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.settings.RebootEnabled = false - env.chk.checkResults = []domain.Result{{Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "dial timeout"}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false, Error: "dial timeout"}}}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -435,7 +435,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.settings.RebootEnabled = false - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -461,8 +461,8 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.settings.MetricsEnabled = false env.chk.checkResults = []domain.Result{ - {Down: true, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, - {Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, + {State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, } env.chk.gatewayResults = []domain.TargetResult{{Target: "gw", Kind: domain.CheckKindGateway, OK: true}} @@ -484,7 +484,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.mr.err = errors.New("disk full") - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -505,7 +505,7 @@ func TestExecute(t *testing.T) { t.Parallel() env := newTestEnv(t) - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} // Built directly rather than through Execute (which has no way to // inject a shorter-than-production metricsTimeout): r.run is the @@ -557,7 +557,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.pc.results = []domain.PingResult{{Host: "1.1.1.1", Sent: 5, Received: 5, AvgMS: 12.5, OK: true}} - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -577,7 +577,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.pc.results = []domain.PingResult{{Host: "1.1.1.1", Sent: 5, Received: 5, AvgMS: 12.5, OK: true}} - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} env.chk.gatewayResults = []domain.TargetResult{{Target: "gw", Kind: domain.CheckKindGateway, OK: true}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeHard) @@ -599,7 +599,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.settings.PingEnabled = false - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -619,7 +619,7 @@ func TestExecute(t *testing.T) { env := newTestEnv(t) env.pr.err = errors.New("disk full") - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) @@ -640,7 +640,7 @@ func TestExecute(t *testing.T) { t.Parallel() env := newTestEnv(t) - env.chk.checkResults = []domain.Result{{Down: false, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} + env.chk.checkResults = []domain.Result{{State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}} // Built directly rather than through Execute (which has no way to // inject a shorter-than-production pingTimeout): r.run is the same @@ -683,6 +683,278 @@ func TestExecute(t *testing.T) { t.Fatalf("ping saves = %+v, want none (the collector never returned within the timeout)", env.pr.calls) } }) + + t.Run("soft, down verdict cleared by the re-check: no reboot, recorded as unconfirmed", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.settings.CheckConfirmations = 2 + env.settings.CheckConfirmDelay = 20 * time.Second + env.chk.checkResults = []domain.Result{ + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // initial + {State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, // re-check clears it + } + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitOK { + t.Fatalf("Execute() = %d, want ExitOK (the outage was noise)", code) + } + if env.rb.calls != 0 { + t.Fatalf("rebooter.Reboot calls = %d, want 0 (a single sample must never reboot)", env.rb.calls) + } + if len(env.nt.messages) != 0 { + t.Fatalf("messages sent = %v, want none (silence is the point of the guard)", env.nt.messages) + } + + run := env.repo.run(t, "1") + if run.Outcome != domain.OutcomeUnconfirmed { + t.Fatalf("Outcome = %q, want %q", run.Outcome, domain.OutcomeUnconfirmed) + } + if run.InternetOK == nil || !*run.InternetOK { + t.Fatalf("InternetOK = %v, want true (the initial sample's verdict was corrected)", run.InternetOK) + } + if got, want := env.repo.phases(), []string{domain.PhaseInitial, domain.PhaseConfirm}; !equalStrings(got, want) { + t.Fatalf("check phases = %v, want %v", got, want) + } + }) + + t.Run("soft, down verdict survives the re-check: reboots on the confirmed outage", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.settings.CheckConfirmations = 2 + env.settings.CheckConfirmDelay = 20 * time.Second + env.chk.checkResults = []domain.Result{ + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // initial + both re-checks + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, + {State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, // recovery tick + } + env.chk.gatewayResults = []domain.TargetResult{{Target: "192.168.1.1:80", Kind: domain.CheckKindGateway, OK: true}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitOK { + t.Fatalf("Execute() = %d, want ExitOK (reboot completed, internet restored)", code) + } + if env.rb.calls != 1 { + t.Fatalf("rebooter.Reboot calls = %d, want 1 (three samples agreed)", env.rb.calls) + } + + run := env.repo.run(t, "1") + if run.InternetOK == nil || *run.InternetOK { + t.Fatalf("InternetOK = %v, want false (the outage was real)", run.InternetOK) + } + if got, want := env.repo.phases()[:3], []string{domain.PhaseInitial, domain.PhaseConfirm, domain.PhaseConfirm}; !equalStrings(got, want) { + t.Fatalf("check phases = %v, want %v", got, want) + } + }) + + t.Run("soft, all domain targets down while raw ip holds: degraded, never rebooted", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.settings.CheckConfirmations = 2 + env.settings.CheckConfirmDelay = 20 * time.Second + env.chk.checkResults = []domain.Result{{State: domain.StateDegraded, Targets: []domain.TargetResult{ + {Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true, Latency: 24 * time.Millisecond}, + {Target: "https://a.example/", Kind: domain.CheckKindDomain, OK: false, Error: "context deadline exceeded (stage=dns)"}, + }}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitNoAction { + t.Fatalf("Execute() = %d, want ExitNoAction", code) + } + if env.rb.calls != 0 { + t.Fatalf("rebooter.Reboot calls = %d, want 0 (a reboot does not fix a broken DNS path)", env.rb.calls) + } + if env.chk.checkCalls != 1 { + t.Fatalf("checker.Check calls = %d, want 1 (degraded is not a down verdict to re-confirm)", env.chk.checkCalls) + } + if len(env.nt.messages) != 1 || !strings.Contains(env.nt.messages[0], "domain checks failing") { + t.Fatalf("messages = %v, want one degraded notice", env.nt.messages) + } + + run := env.repo.run(t, "1") + if run.Outcome != domain.OutcomeDegraded { + t.Fatalf("Outcome = %q, want %q", run.Outcome, domain.OutcomeDegraded) + } + if run.Action != domain.ActionNone { + t.Fatalf("Action = %q, want %q", run.Action, domain.ActionNone) + } + }) + + t.Run("soft, degraded again right after a degraded run: no repeat message", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.repo.recentRuns = []domain.RunSummary{{Outcome: domain.OutcomeDegraded}} + env.chk.checkResults = []domain.Result{{State: domain.StateDegraded, Targets: []domain.TargetResult{ + {Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}, + {Target: "https://a.example/", Kind: domain.CheckKindDomain, OK: false}, + }}} + + Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if len(env.nt.messages) != 0 { + t.Fatalf("messages = %v, want none (the previous run already announced this state)", env.nt.messages) + } + }) + + t.Run("soft, confirmed outage the previous run did not see: unsustained, no reboot", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.settings.RebootMinStreak = 2 + healthy := true + env.repo.recentRuns = []domain.RunSummary{{InternetOK: &healthy, Outcome: domain.OutcomeOK}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitNoAction { + t.Fatalf("Execute() = %d, want ExitNoAction", code) + } + if env.rb.calls != 0 { + t.Fatalf("rebooter.Reboot calls = %d, want 0 (one five-minute sample is not an outage)", env.rb.calls) + } + if len(env.nt.messages) != 0 { + t.Fatalf("messages = %v, want none", env.nt.messages) + } + + run := env.repo.run(t, "1") + if run.Outcome != domain.OutcomeUnsustained { + t.Fatalf("Outcome = %q, want %q", run.Outcome, domain.OutcomeUnsustained) + } + if run.InternetOK == nil || *run.InternetOK { + t.Fatalf("InternetOK = %v, want false (the verdict stands, only the action is withheld)", run.InternetOK) + } + }) + + t.Run("soft, confirmed outage the previous run also saw: reboots", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.settings.RebootMinStreak = 2 + outage := false + env.repo.recentRuns = []domain.RunSummary{{InternetOK: &outage, Outcome: domain.OutcomeUnsustained}} + env.chk.checkResults = []domain.Result{ + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, + {State: domain.StateUp, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}}}, + } + env.chk.gatewayResults = []domain.TargetResult{{Target: "192.168.1.1:80", Kind: domain.CheckKindGateway, OK: true}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitOK { + t.Fatalf("Execute() = %d, want ExitOK", code) + } + if env.rb.calls != 1 { + t.Fatalf("rebooter.Reboot calls = %d, want 1 (the outage spans two runs)", env.rb.calls) + } + }) + + t.Run("soft, a hard-mode run in the history does not corroborate an outage", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.settings.RebootMinStreak = 2 + env.repo.recentRuns = []domain.RunSummary{{InternetOK: nil, Outcome: domain.OutcomeOK}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitNoAction { + t.Fatalf("Execute() = %d, want ExitNoAction (a run that never probed cannot corroborate)", code) + } + if env.rb.calls != 0 { + t.Fatalf("rebooter.Reboot calls = %d, want 0", env.rb.calls) + } + }) + + t.Run("soft, unreadable history is fail-closed: no reboot", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.settings.RebootMinStreak = 2 + env.repo.recentRunsErr = errors.New("store: disk i/o error") + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitNoAction { + t.Fatalf("Execute() = %d, want ExitNoAction", code) + } + if env.rb.calls != 0 { + t.Fatalf("rebooter.Reboot calls = %d, want 0 (an unreadable history must not fall through to a reboot)", env.rb.calls) + } + }) + + t.Run("soft, cooldown skip repeated after a skipped run: no repeat message", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.repo.lastRebootStartedAt = env.clk.now.Add(-30 * time.Minute) + env.repo.lastRebootOK = true + env.repo.recentRuns = []domain.RunSummary{{Outcome: domain.OutcomeSkipped}} + env.chk.checkResults = []domain.Result{{State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitSkippedCooldown { + t.Fatalf("Execute() = %d, want ExitSkippedCooldown", code) + } + if len(env.nt.messages) != 0 { + t.Fatalf("messages = %v, want none (the cooldown skip was already announced)", env.nt.messages) + } + + run := env.repo.run(t, "1") + if run.Outcome != domain.OutcomeSkipped { + t.Fatalf("Outcome = %q, want %q", run.Outcome, domain.OutcomeSkipped) + } + }) + + t.Run("soft, recovery ends on a degraded verdict rather than waiting out the timeout", func(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + env.chk.checkResults = []domain.Result{ + {State: domain.StateDown, Targets: []domain.TargetResult{{Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: false}}}, // initial + {State: domain.StateDegraded, Targets: []domain.TargetResult{ // tick 1: link back, DNS still broken + {Target: "1.1.1.1:443", Kind: domain.CheckKindIP, OK: true}, + {Target: "https://a.example/", Kind: domain.CheckKindDomain, OK: false}, + }}, + } + env.chk.gatewayResults = []domain.TargetResult{{Target: "192.168.1.1:80", Kind: domain.CheckKindGateway, OK: true}} + + code := Execute(t.Context(), env.settings, env.repo, env.mr, env.pr, env.chk, env.rb, env.nt, env.mc, env.pc, env.clk, domain.ModeSoft) + + if code != domain.ExitOK { + t.Fatalf("Execute() = %d, want ExitOK (the link the reboot targeted is back)", code) + } + + run := env.repo.run(t, "1") + if run.InternetRestoredAt == nil { + t.Fatal("InternetRestoredAt is nil, want set") + } + }) +} + +// equalStrings reports whether two string slices hold the same values in the +// same order, so a phase-sequence assertion reads as one comparison instead +// of a hand-rolled loop at every call site. +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true } // errRebootFailed is a stand-in for gateway/router's real reboot errors; @@ -709,6 +981,10 @@ type testEnv struct { // newTestEnv builds a testEnv with sane defaults (2h cooldown, 60s recovery // interval, 15m recovery timeout, reboot, metrics, and ping all enabled — the // design's own defaults) that a subtest can override before calling Execute. +// The issue #6 false-positive guards are left at their "off" values +// (CheckConfirmations 0, RebootMinStreak 0) so every pre-existing scenario +// keeps asserting the flow it was written for; the guards have their own +// subtests, which set them explicitly. func newTestEnv(t *testing.T) *testEnv { t.Helper() diff --git a/services/ports.go b/services/ports.go index cf71006..184abc7 100644 --- a/services/ports.go +++ b/services/ports.go @@ -56,11 +56,18 @@ type Clock interface { // infrastructure.Store satisfies it. InsertRun's returned id and UpdateRun's // id parameter are opaque UUIDv7 strings (issue #4) that the orchestrator // only ever threads through, never inspects. +// RecentRuns returns the runs immediately preceding beforeID, newest first, +// at most limit of them — the history the issue #6 guards corroborate a +// present outage against. beforeID is the caller's own run id: ids are +// time-ordered UUIDv7 strings (issue #4), so "preceding" is a plain id +// comparison and the caller's own freshly inserted row is never returned to +// it. Fewer than limit rows simply means the history is that short. type RunRepository interface { InsertRun(ctx context.Context, startedAt time.Time, mode string, internetOK *bool) (string, error) UpdateRun(ctx context.Context, id string, u domain.RunUpdate) error InsertCheck(ctx context.Context, c domain.Check) error GetLastRebootStartedAt(ctx context.Context) (time.Time, bool, error) + RecentRuns(ctx context.Context, beforeID string, limit int) ([]domain.RunSummary, error) } // MetricsCollector takes one host telemetry snapshot (design @@ -118,12 +125,21 @@ type Sender interface { // domain types (plans/003-host-telemetry.md Trade-off T1): domain owns the // vocabulary the toggles drive (ActionSkippedDisabled, OutcomeRebootDisabled, // the Collector consts), not the toggle itself. +// CheckConfirmations, CheckConfirmDelay, and RebootMinStreak (issue #6) are +// the two independent guards between a down verdict and a reboot: the first +// pair re-probes within the run, the third requires the preceding runs to have +// seen the outage too. Both are counted in samples rather than wall time +// because the collector is a cron process with no memory of its own — the +// interval between runs is cron's business, not the orchestrator's. type Settings struct { - Label string - RebootCooldown time.Duration - RecoveryInterval time.Duration - RecoveryTimeout time.Duration - RebootEnabled bool - MetricsEnabled bool - PingEnabled bool + Label string + RebootCooldown time.Duration + RecoveryInterval time.Duration + RecoveryTimeout time.Duration + CheckConfirmations int + CheckConfirmDelay time.Duration + RebootMinStreak int + RebootEnabled bool + MetricsEnabled bool + PingEnabled bool } diff --git a/yarddog.env.example b/yarddog.env.example index 41fab73..0269a7f 100644 --- a/yarddog.env.example +++ b/yarddog.env.example @@ -26,6 +26,20 @@ YARDDOG_ROUTER_PASS=change-me #YARDDOG_RECOVERY_INTERVAL=60s #YARDDOG_RECOVERY_TIMEOUT=15m #YARDDOG_REBOOT_COOLDOWN=2h + +# false-positive guards between a "down" verdict and a reboot. +# a down verdict is re-probed this many more times, this far apart, and a single +# healthy sample calls the whole thing off (outcome "unconfirmed", no message). +# 0 confirmations restores the old "act on the first sample" behaviour. +#YARDDOG_CHECK_CONFIRMATIONS=2 +# ^ clamped to [0, 5] +#YARDDOG_CHECK_CONFIRM_DELAY=20s +# ^ clamped to [1s, 60s] +# how many consecutive runs (this one included) must have seen the internet down +# before a reboot is allowed. 2 means the previous run has to agree; 1 disables +# the cross-run check. a run that falls short is recorded as "unsustained". +#YARDDOG_REBOOT_MIN_STREAK=2 +# ^ clamped to [1, 10] # a run stays in the fast "hot" tables while younger than this; older runs # roll into the *_archive tables at every collector startup (clamped to >=1) #YARDDOG_HOT_WINDOW_DAYS=30