From 01eb32811b931d6b279e452ef60a001f9af722b3 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Sat, 26 Sep 2026 10:58:34 -0400 Subject: [PATCH] feat: alert when an instance's fetch keeps failing The stuck checks deliberately skip stale data, so a revoked API token or an unreachable Coolify instance froze every row on screen and sent nothing to the webhook feed. evaluateStuck now also takes the instance states and emits fetch_failed for an instance whose fetch has been failing past the stuck threshold, firing once and re-arming on recovery. The error rides in detail so the payload tells a credential problem from an outage. Ported from aws-green #56. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++ internal/poller/poller.go | 2 +- internal/poller/stuck.go | 27 +++++++++- internal/poller/stuck_test.go | 93 ++++++++++++++++++++++++++++------- internal/webhooks/webhooks.go | 2 + 5 files changed, 107 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 4d427ed..bbd6d57 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,10 @@ so an expired token is not reported as a fleet-wide outage. `event` is `deployment_stuck` or `resource_stuck`; `reason` is one of `deploy_failed`, `deploy_in_progress`, `resource_down`, `resource_unhealthy`. +An instance whose API fetch keeps failing past the threshold (a revoked token, an unreachable host) +sends `event` and `reason` `fetch_failed`, with `resource_type` `instance` and the error in `detail`. +It fires once per outage and re-arms when the fetch recovers. + When a webhook has a `secret`, the request is signed with HMAC-SHA256 over the raw body and sent as `X-Coolify-Green-Signature: sha256=` — verify it before trusting the payload. Delivery failures are never retried and never interrupt polling; run with `COOLIFY_GREEN_DEBUG=1` to see them on diff --git a/internal/poller/poller.go b/internal/poller/poller.go index 847bd68..25f63de 100644 --- a/internal/poller/poller.go +++ b/internal/poller/poller.go @@ -213,7 +213,7 @@ func (p *Poller) Poll(ctx context.Context, ch chan<- state.Snapshot) { if len(inventory) > 0 || len(instances) > 0 { p.inventory = inventory } - events := p.evaluateStuck(projects) + events := p.evaluateStuck(projects, instances) p.mu.Unlock() // Dispatch outside the lock: a slow webhook endpoint must not stall diff --git a/internal/poller/stuck.go b/internal/poller/stuck.go index 5580885..b25e060 100644 --- a/internal/poller/stuck.go +++ b/internal/poller/stuck.go @@ -24,7 +24,7 @@ func stuckKey(ps state.ProjectState, resourceUUID, kind string) string { // // Callers must hold p.mu. Dispatch is left to the caller so the HTTP calls // happen outside the lock. -func (p *Poller) evaluateStuck(projects []state.ProjectState) []webhooks.Event { +func (p *Poller) evaluateStuck(projects []state.ProjectState, instances []state.InstanceState) []webhooks.Event { threshold := time.Duration(p.cfg.Settings.StuckThresholdMinutes) * time.Minute now := p.now() seen := make(map[string]struct{}, len(p.stuck)) @@ -48,6 +48,31 @@ func (p *Poller) evaluateStuck(projects []state.ProjectState) []webhooks.Event { events = append(events, build(entry.since)) } + // A fetch that keeps failing is its own kind of stuck. Every row for the + // instance is frozen at whatever it showed when the fetch died, and the + // checks below deliberately skip that stale data, so without this a + // revoked token or a dead instance alerts nowhere at all. + for _, instance := range instances { + inst := instance + if inst.Err == nil { + continue + } + key := "instance|" + inst.Name + "|fetch" + track(key, webhooks.ReasonFetchFailed, func(since time.Time) webhooks.Event { + return webhooks.Event{ + Event: webhooks.EventFetchFailed, + Reason: webhooks.ReasonFetchFailed, + Instance: inst.Name, + ResourceType: "instance", + Resource: inst.Name, + Detail: inst.Err.Error(), + URL: inst.URL, + StuckSince: since, + Timestamp: now, + } + }) + } + for _, proj := range projects { ps := proj // Stale data is carried forward from an earlier poll. Alerting on it diff --git a/internal/poller/stuck_test.go b/internal/poller/stuck_test.go index aea5f2f..86a28af 100644 --- a/internal/poller/stuck_test.go +++ b/internal/poller/stuck_test.go @@ -1,6 +1,7 @@ package poller import ( + "errors" "testing" "time" @@ -51,17 +52,17 @@ func TestStuckFiresOnceAfterThreshold(t *testing.T) { p, clock := stuckPoller(t) projects := failedDeployProject() - if events := p.evaluateStuck(projects); len(events) != 0 { + if events := p.evaluateStuck(projects, nil); len(events) != 0 { t.Fatalf("alerted before the threshold: %+v", events) } *clock = clock.Add(29 * time.Minute) - if events := p.evaluateStuck(projects); len(events) != 0 { + if events := p.evaluateStuck(projects, nil); len(events) != 0 { t.Fatalf("alerted one minute early: %+v", events) } *clock = clock.Add(2 * time.Minute) - events := p.evaluateStuck(projects) + events := p.evaluateStuck(projects, nil) if len(events) != 1 { t.Fatalf("got %d events, want 1", len(events)) } @@ -80,7 +81,7 @@ func TestStuckFiresOnceAfterThreshold(t *testing.T) { } *clock = clock.Add(time.Hour) - if events := p.evaluateStuck(projects); len(events) != 0 { + if events := p.evaluateStuck(projects, nil); len(events) != 0 { t.Fatalf("re-alerted while still stuck: %+v", events) } } @@ -89,9 +90,9 @@ func TestRecoveryRearmsTheAlert(t *testing.T) { p, clock := stuckPoller(t) projects := failedDeployProject() - p.evaluateStuck(projects) // first sighting starts the clock + p.evaluateStuck(projects, nil) // first sighting starts the clock *clock = clock.Add(31 * time.Minute) - if len(p.evaluateStuck(projects)) != 1 { + if len(p.evaluateStuck(projects, nil)) != 1 { t.Fatal("first alert did not fire") } @@ -99,16 +100,16 @@ func TestRecoveryRearmsTheAlert(t *testing.T) { healthy[0].Resources[0].Deploy.Status = "finished" healthy[0].Resources[0].Deploy.Stoplight = aggregator.StoplightGreen *clock = clock.Add(time.Minute) - if events := p.evaluateStuck(healthy); len(events) != 0 { + if events := p.evaluateStuck(healthy, nil); len(events) != 0 { t.Fatalf("recovery should not alert: %+v", events) } *clock = clock.Add(time.Minute) - if events := p.evaluateStuck(projects); len(events) != 0 { + if events := p.evaluateStuck(projects, nil); len(events) != 0 { t.Fatalf("clock should restart on the new incident: %+v", events) } *clock = clock.Add(31 * time.Minute) - if events := p.evaluateStuck(projects); len(events) != 1 { + if events := p.evaluateStuck(projects, nil); len(events) != 1 { t.Fatalf("a fresh incident must alert again, got %d", len(events)) } } @@ -119,9 +120,9 @@ func TestChangedReasonRestartsTheClock(t *testing.T) { building[0].Resources[0].Deploy.Status = "in_progress" building[0].Resources[0].Deploy.Stoplight = aggregator.StoplightYellow - p.evaluateStuck(building) // first sighting starts the clock + p.evaluateStuck(building, nil) // first sighting starts the clock *clock = clock.Add(31 * time.Minute) - events := p.evaluateStuck(building) + events := p.evaluateStuck(building, nil) if len(events) != 1 || events[0].Reason != webhooks.ReasonDeployInProgress { t.Fatalf("events = %+v", events) } @@ -130,11 +131,11 @@ func TestChangedReasonRestartsTheClock(t *testing.T) { // threshold rather than firing instantly off the old timer. failed := failedDeployProject() *clock = clock.Add(time.Minute) - if events := p.evaluateStuck(failed); len(events) != 0 { + if events := p.evaluateStuck(failed, nil); len(events) != 0 { t.Fatalf("new reason should restart the clock: %+v", events) } *clock = clock.Add(31 * time.Minute) - if events := p.evaluateStuck(failed); len(events) != 1 { + if events := p.evaluateStuck(failed, nil); len(events) != 1 { t.Fatalf("want one event after the new threshold, got %d", len(events)) } } @@ -147,7 +148,7 @@ func TestStaleProjectsDoNotAlert(t *testing.T) { projects[0].Err = errContext *clock = clock.Add(2 * time.Hour) - if events := p.evaluateStuck(projects); len(events) != 0 { + if events := p.evaluateStuck(projects, nil); len(events) != 0 { t.Fatalf("stale carried-forward data must not alert: %+v", events) } } @@ -167,9 +168,9 @@ func TestDownContainerAlerts(t *testing.T) { Status: "exited", ContainerLight: aggregator.StoplightRed, }}, }} - p.evaluateStuck(projects) // first sighting starts the clock + p.evaluateStuck(projects, nil) // first sighting starts the clock *clock = clock.Add(31 * time.Minute) - events := p.evaluateStuck(projects) + events := p.evaluateStuck(projects, nil) if len(events) != 1 { t.Fatalf("got %d events", len(events)) } @@ -190,9 +191,9 @@ func TestUnhealthyContainerHasItsOwnReason(t *testing.T) { Status: "running:unhealthy", ContainerLight: aggregator.StoplightRed, }}, }} - p.evaluateStuck(projects) // first sighting starts the clock + p.evaluateStuck(projects, nil) // first sighting starts the clock *clock = clock.Add(31 * time.Minute) - events := p.evaluateStuck(projects) + events := p.evaluateStuck(projects, nil) if len(events) != 1 || events[0].Reason != webhooks.ReasonResourceUnhealth { t.Fatalf("events = %+v", events) } @@ -209,7 +210,7 @@ func TestHealthyStateNeverAlerts(t *testing.T) { }}, }} *clock = clock.Add(24 * time.Hour) - if events := p.evaluateStuck(projects); len(events) != 0 { + if events := p.evaluateStuck(projects, nil); len(events) != 0 { t.Fatalf("healthy state alerted: %+v", events) } } @@ -221,3 +222,57 @@ func TestStuckKeysAreInstanceQualified(t *testing.T) { t.Error("same resource on two instances must not share a stuck key") } } + +func failingInstance(err error) []state.InstanceState { + return []state.InstanceState{{Name: "studio", URL: "https://coolify.test", Err: err}} +} + +// A dead API token freezes every row on screen, and the checks above skip that +// stale data, so the failing fetch has to alert on its own. +func TestFailingFetchAlertsOnceAfterThreshold(t *testing.T) { + p, clock := stuckPoller(t) + down := failingInstance(errors.New("coolify: unauthorized")) + + if events := p.evaluateStuck(nil, down); len(events) != 0 { + t.Fatalf("alerted before the threshold: %+v", events) + } + *clock = clock.Add(30 * time.Minute) + events := p.evaluateStuck(nil, down) + if len(events) != 1 { + t.Fatalf("got %d events at the threshold, want 1: %+v", len(events), events) + } + e := events[0] + if e.Event != webhooks.EventFetchFailed || e.Reason != webhooks.ReasonFetchFailed { + t.Errorf("event/reason = %q/%q, want fetch_failed", e.Event, e.Reason) + } + if e.Instance != "studio" || e.ResourceType != "instance" || e.Detail != "coolify: unauthorized" { + t.Errorf("event = %+v, want instance studio with the error in detail", e) + } + if !e.StuckSince.Equal(clock.Add(-30 * time.Minute)) { + t.Errorf("StuckSince = %v, want first failure", e.StuckSince) + } + + *clock = clock.Add(10 * time.Minute) + if events := p.evaluateStuck(nil, down); len(events) != 0 { + t.Fatalf("alerted again while still failing: %+v", events) + } +} + +func TestRecoveredFetchRearmsTheAlert(t *testing.T) { + p, clock := stuckPoller(t) + down := failingInstance(errors.New("dial tcp: timeout")) + p.evaluateStuck(nil, down) + *clock = clock.Add(30 * time.Minute) + if len(p.evaluateStuck(nil, down)) != 1 { + t.Fatal("expected the first alert") + } + + if events := p.evaluateStuck(nil, failingInstance(nil)); len(events) != 0 { + t.Fatalf("recovery alerted: %+v", events) + } + p.evaluateStuck(nil, down) + *clock = clock.Add(30 * time.Minute) + if len(p.evaluateStuck(nil, down)) != 1 { + t.Fatal("a new outage after recovery should alert again") + } +} diff --git a/internal/webhooks/webhooks.go b/internal/webhooks/webhooks.go index c0d7397..1407f7a 100644 --- a/internal/webhooks/webhooks.go +++ b/internal/webhooks/webhooks.go @@ -23,6 +23,7 @@ const SignatureHeader = "X-Coolify-Green-Signature" const ( EventDeploymentStuck = "deployment_stuck" EventResourceStuck = "resource_stuck" + EventFetchFailed = "fetch_failed" ) // Reasons carried in Event.Reason. @@ -31,6 +32,7 @@ const ( ReasonDeployInProgress = "deploy_in_progress" ReasonResourceDown = "resource_down" ReasonResourceUnhealth = "resource_unhealthy" + ReasonFetchFailed = "fetch_failed" ) // Event is the JSON payload POSTed to each webhook.