diff --git a/CHANGELOG.md b/CHANGELOG.md index dee3c350..6c7861c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ ## Unreleased +- Export drain-marked cluster members as their own snapshot field and + Prometheus series, independent of held-out (offline) members. A full + online drain keeps platform health up and still fires queue and assigned + pages; `gha_fleet_visibility_drain_marked_members` is now the count of + that authorized hold instead of inferring it from health staying one. + Only a reason with the `drained: ` prefix counts; an empty member listing + is not a full drain. + +- Keep `fleet_health_flapping` evaluable on a healthy fleet: sum the bool + of `healthy < 1` over thirty one-minute steps instead of counting a + filter that this backend drops when health is one. Host-signal tickets + and dashboards use last-min of the cumulative counter, not unsigned + max-min, so a collector restart or series reset is not a burst. + +- Align queued-delivery, slow-burn and created-unbound operator copy with + the shipped thresholds (thirty minutes, five-to-thirty band, fifteen + minutes). Thresholds themselves are unchanged. + - Keep platform health up when every cluster member is drain-marked, including an online drain. `gha_fleet_visibility_held_out_members` still counts only offline drained members, so queue and assigned pages keep diff --git a/config/observability-dashboards.yaml b/config/observability-dashboards.yaml index 79549fc9..f7067746 100644 --- a/config/observability-dashboards.yaml +++ b/config/observability-dashboards.yaml @@ -136,25 +136,25 @@ dashboards: - id: all_host_signals title: Host signals by class and member kind: timeseries - query: sum by (host_name, signal_class) (max_over_time(gha_fleet_host_signal_events{aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) + query: sum by (host_name, signal_class) (last_over_time(gha_fleet_host_signal_events{aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) unit: count description: OTEL-classified host events observed in each rolling hour without retaining duplicate raw logs. - id: audit_suppression title: Audit callback suppressions kind: timeseries - query: sum by (host_name) (max_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) + query: sum by (host_name) (last_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) unit: count description: Kernel notices that audit callbacks were suppressed during high event volume. - id: kernel_workqueue_hog title: Kernel workqueue hog events kind: timeseries - query: sum by (host_name) (max_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[24h]) - min_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[24h])) + query: sum by (host_name) (last_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[24h]) - min_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[24h])) unit: count description: Kernel workqueue CPU-hog signals that may precede host latency or pressure. - id: ufw_blocks title: UFW block events kind: timeseries - query: sum by (host_name) (max_over_time(gha_fleet_host_signal_events{signal_class="ufw_block",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{signal_class="ufw_block",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) + query: sum by (host_name) (last_over_time(gha_fleet_host_signal_events{signal_class="ufw_block",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{signal_class="ufw_block",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) unit: count description: Firewall block volume retained as an OTEL metric rather than repeated kernel log records. - id: host_utilization diff --git a/config/observability-rules.yaml b/config/observability-rules.yaml index c397844e..daad96fe 100644 --- a/config/observability-rules.yaml +++ b/config/observability-rules.yaml @@ -72,7 +72,13 @@ rules: # # A window with no events now returns an empty result, which is the honest # answer to "how many events", evaluates without error, and does not fire. - expression: sum by (host_name) (max_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[15m]) - min_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[15m])) + # + # last_over_time - min_over_time, not max - min: a collector restart that + # republishes the cumulative total, or a series that resets Nā†’0, has the + # same unsigned max-min span as N new events. last-min is 0 across that + # reset and the true delta when the counter moves up. Same treatment as + # host_oom_detected. + expression: sum by (host_name) (last_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[15m]) - min_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[15m])) operator: ">" threshold: 30 # No outer subquery: evaluation and hold are equal, so the renderer emits @@ -219,11 +225,19 @@ rules: # beside each failed one. Counting the unhealthy minutes catches the shape # the page is built to miss, and it is a ticket: a fleet that recovers by # itself every minute is not an outage, it is a defect with a heartbeat. + # + # Written as sum_over_time of a bool, not count_over_time of a filter. + # This backend drops `(healthy < 1)` when healthy is 1, so the inner + # range is empty on a quiet fleet; hold > eval then wraps that empty + # vector and raises error 20008, which pages as alert_evaluation_failed. + # `X < bool 1` returns 0 when healthy and 1 when not, so the range is + # never empty. sum_over_time of those 0/1 steps is the unhealthy-minute + # count; count_over_time would count every step, healthy or not. - id: fleet_health_flapping severity: ticket query_language: promql stream_name: gha_fleet_platform_healthy - expression: count_over_time((min by (host_name) (gha_fleet_platform_healthy) < 1)[30m:1m]) + expression: sum_over_time((min by (host_name) (gha_fleet_platform_healthy) < bool 1)[30m:1m]) operator: ">=" threshold: 3 evaluation_seconds: 300 @@ -486,7 +500,9 @@ rules: query_language: promql stream_name: gha_fleet_host_signal_events # Same `or vector(0)` removal as audit_suppression_burst; see there. - expression: sum by (host_name) (max_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) + # last_over_time - min_over_time, not max - min: same unsigned-reset + # treatment as host_oom_detected and audit_suppression_burst. + expression: sum by (host_name) (last_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h]) - min_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog",aggregation_temporality="AGGREGATION_TEMPORALITY_CUMULATIVE"}[1h])) operator: ">" threshold: 10 # No outer subquery: evaluation and hold are equal, so the renderer emits @@ -548,7 +564,7 @@ rules: runbook: https://github.com/NDDev-OpenNetwork/github-actions/blob/main/docs/runbooks/fleet-alerts.md summary: A created worker has no running job identity beyond transition grace. action: Verify GitHub absent/offline state before allowing the normal reaper to delete it. - recovery: No created lease remains unbound beyond five minutes. + recovery: No created lease remains unbound beyond fifteen minutes. - id: lifecycle_inventory_gap severity: page query_language: promql @@ -614,9 +630,9 @@ rules: enabled: true owner: fleet-operations runbook: https://github.com/NDDev-OpenNetwork/github-actions/blob/main/docs/runbooks/fleet-alerts.md - summary: An eligible queued job has not produced runner delivery progress within five minutes. + summary: An eligible queued job has not produced runner delivery progress within thirty minutes. action: Preserve the job and correlate queue intent, provider retry, admission, worker creation and GitHub registration before recovery. - recovery: Queued age remains below five minutes or the exact job advances without operator cancellation. + recovery: Queued wait remains below thirty minutes or the exact job advances without operator cancellation. - id: memory_psi_slow_burn severity: ticket query_language: promql @@ -680,10 +696,10 @@ rules: # # A ticket must hold at least ten minutes -- that is the rule package's own # policy for the class, and it is right -- so the threshold has to come down - # to where a ten-minute hold can reach. 300 s is also the boundary this - # rule's own summary names: "between the two-minute objective and the - # five-minute page". Held ten minutes it fires once on a day like that one, - # which is what a slow-burn ticket should cost. + # to where a ten-minute hold can reach. 300 s is the slow-burn floor; + # lifecycle_queued_delivery_stall owns waits past thirty minutes. Held ten + # minutes it fires once on a day like that one, which is what a slow-burn + # ticket should cost. # The wait a developer feels, closed: first queued to runner running, over # the jobs that started in the last fifteen minutes. Every other wait rule # here reads a gauge that stops when the provider assigns the intent, so on @@ -735,9 +751,9 @@ rules: enabled: true owner: fleet-performance runbook: https://github.com/NDDev-OpenNetwork/github-actions/blob/main/docs/runbooks/fleet-alerts.md - summary: Queue latency is persistently between the two-minute objective and the five-minute page. + summary: Queue latency is persistently between five minutes and the thirty-minute page. action: Create a capacity or fairness ticket using phase-specific and class metrics. - recovery: Queue age remains below two minutes for the hold window, or a stuck job crosses five minutes and lifecycle_queued_delivery_stall owns it instead. + recovery: Queue age remains below five minutes for the hold window, or a stuck job crosses thirty minutes and lifecycle_queued_delivery_stall owns it instead. - id: snapshot_stale severity: page query_language: promql diff --git a/docs/maintenance-windows.md b/docs/maintenance-windows.md index 73f5074e..0e43ffce 100644 --- a/docs/maintenance-windows.md +++ b/docs/maintenance-windows.md @@ -27,6 +27,11 @@ config, and the observer reads it there to tell maintenance from an incident. ## What the observer does during a hold +Every drain-marked member, online or offline, is named on +`gha_fleet_visibility_drain_marked_members` and the per-member +`gha_fleet_visibility_drain_marked` rows. That series is how an authorized +hold is observed; it does not change queue or assigned pages. + While an offline member carries a `drained: ` reason, the member is **held out**: inventory gap counts (orphan / missing / uncovered-beyond-grace) move to `*_unattributable` snapshot fields, a loud listing failure moves to @@ -37,6 +42,12 @@ ticket pages when a hold outlives half an hour, so the suppression can never hide a member that failed to come back. An offline member **without** a drain reason fails collection immediately. +An **online** drain leaves the listing complete: empty on that member is the +truth, so HeldOutMembers stays empty and queue/assigned pages still fire. If +every reported cluster member is drain-marked, uncovered-beyond-grace does +not fail platform health ā€” there is no eligible placement left on purpose. A +partial drain with an undrained sibling still fails health on that gap. + ## Stopping the Incus daemon for real The daemon is socket-activated, and the pressure timer connects to the local diff --git a/docs/observability-dashboards.md b/docs/observability-dashboards.md index 0c6f076f..73e66877 100644 --- a/docs/observability-dashboards.md +++ b/docs/observability-dashboards.md @@ -29,13 +29,15 @@ The reconciler owns only dashboards carrying the versioned `managed-by:gds` description marker. It preserves manually created dashboards, uses OpenObserve hash preconditions for updates, and requires a clean post-apply read-back. -The nine managed dashboards cover: +The eleven managed dashboards cover: - capacity and pressure; - correlation integrity; - diagnostic storage retention; - host package, reboot, kernel and SRSO compliance; - classified host signals without duplicate raw-log storage; +- host CPU, RAM, swap occupancy and major paging; +- job lifecycle transitions; - lifecycle phase latency; - provider reliability; - priority/class fairness; @@ -47,6 +49,6 @@ human decision surfaces used by alert response and CD health verification; a private deployment must not silently edit them. Dashboards do not prove alerts deliver. `config/observability-rules.yaml` owns -the twenty-five-rule separately tested alert contract, and a real backend lifecycle record must +the thirty-one-rule separately tested alert contract, and a real backend lifecycle record must prove Collector buffering, recovered records and explicit loss counters before the observability product is accepted. diff --git a/docs/runbooks/fleet-alerts.md b/docs/runbooks/fleet-alerts.md index 5eaff05f..0cf49cc5 100644 --- a/docs/runbooks/fleet-alerts.md +++ b/docs/runbooks/fleet-alerts.md @@ -79,10 +79,10 @@ gha-fleet reconcile-openobserve-alerts \ `constraint=CONSTRAINT_NONE` journal line on the current boot before treating the page as a live kill. - Inventory-gap pages: a 30-second `missing_instances=1` listing blip is not - the page. The rule requires each of orphan, missing and uncovered-beyond-grace - to stay non-zero for two minutes of raw samples. Preserve all three states - before recovering; a member reboot without a drain marker is still a gap if - it lasts that long. + the page. The rule is a sum of two-minute mins: any of orphan, missing or + uncovered-beyond-grace staying non-zero for two minutes of raw samples + fires. Preserve all three states before recovering; a member reboot without + a drain marker is still a gap if it lasts that long. - Host-signal tickets: use `gha_fleet_host_signal_events` cumulative deltas. LVM activation and overlay `xino=off` are workload-volume context; audit suppression and workqueue-hog alerts act only on their bounded burst budget. @@ -105,14 +105,23 @@ gha-fleet reconcile-openobserve-alerts \ until normal successful consolidation removes or supersedes it. - Slow-burn tickets: inspect class/tenant percentiles and capacity evidence; do not page an operator for a trend without an immediate action. -- Visibility-degraded tickets: a drain-marked member is offline, so the - cluster listing is partial by design. Inventory gap counts move to their +- Platform-health pages during an authorized full-fleet drain: if every + reported cluster member carries a drain reason, including an online drain, + uncovered-beyond-grace does not zero `gha_fleet_platform_healthy`. + `gha_fleet_visibility_drain_marked_members` is the count. Queue and + assigned pages still fire because an online drain leaves the listing + complete (`gha_fleet_visibility_held_out_members` stays 0). A partial + drain with an undrained sibling still fails platform health. +- Visibility-degraded tickets: a drain-marked member that is offline makes + the cluster listing partial by design. Inventory gap counts move to their `*_unattributable` snapshot fields and a loud listing failure moves to `listing_unavailable` -- suppressed with attribution, never hidden -- while - `gha_fleet_visibility_held_out_members` says so. The ticket fires when the - hold outlives half an hour: finish the maintenance and restore the member, - or find out why it did not come back. An offline member WITHOUT a drain - reason is an incident and fails the platform immediately. + `gha_fleet_visibility_held_out_members` says so. Online drain-marked + members appear only on `gha_fleet_visibility_drain_marked_members`; they + do not suppress gaps. The ticket fires when an offline hold outlives half + an hour: finish the maintenance and restore the member, or find out why it + did not come back. An offline member WITHOUT a drain reason is an incident + and fails the platform immediately. - Pressure-staleness pages during maintenance: a member drained through the drain marker keeps publishing a fresh closed state carrying `drained: ` on every timer tick, so staleness stays silent for the diff --git a/internal/fleetobserve/metrics.go b/internal/fleetobserve/metrics.go index 1de4c9de..6513396a 100644 --- a/internal/fleetobserve/metrics.go +++ b/internal/fleetobserve/metrics.go @@ -118,6 +118,11 @@ func RenderPrometheus(snapshot Snapshot, now time.Time, maxStaleness time.Durati for _, member := range snapshot.HeldOutMembers { metric(&output, "gha_fleet_visibility_degraded", map[string]string{"member": member}, 1) } + gauge(&output, "gha_fleet_visibility_drain_marked_members", "Cluster members whose gate carries a drain reason, online or offline. Held-out members are the offline subset; an online drain leaves the listing complete.", float64(len(snapshot.DrainMarkedMembers))) + labeledGaugeHeader(&output, "gha_fleet_visibility_drain_marked", "One row per drain-marked member, for attribution.") + for _, member := range snapshot.DrainMarkedMembers { + metric(&output, "gha_fleet_visibility_drain_marked", map[string]string{"member": member}, 1) + } gauge(&output, "gha_fleet_queue_uncovered_running_beyond_grace", "Running queue intents still uncovered after the broker reclaim window, which is the gap nothing is coming to close.", float64(snapshot.Queue.UncoveredRunningBeyondGrace)) gauge(&output, "gha_fleet_queue_uncovered_running_oldest_age_seconds", "Age of the oldest running queue intent whose runner holds no execution lease.", float64(snapshot.Queue.OldestUncoveredRunningAgeSeconds)) gauge(&output, "gha_fleet_queue_running_without_runner_identity", "Running queue intents written before or without an exact JobStarted runner identity.", float64(snapshot.Queue.RunningWithoutRunnerIdentity)) diff --git a/internal/fleetobserve/metrics_test.go b/internal/fleetobserve/metrics_test.go index 2aaf09bf..9bb336a4 100644 --- a/internal/fleetobserve/metrics_test.go +++ b/internal/fleetobserve/metrics_test.go @@ -1,6 +1,7 @@ package fleetobserve import ( + "context" "strings" "testing" "time" @@ -49,6 +50,8 @@ func TestRenderPrometheusIsDeterministicAndBounded(t *testing.T) { "gha_fleet_queue_terminal_tombstone_next_expiry_seconds 0\n", "gha_fleet_queue_intents_in_flight 0\n", "gha_fleet_queue_uncovered_running 0\n", + "gha_fleet_visibility_held_out_members 0\n", + "gha_fleet_visibility_drain_marked_members 0\n", "gha_fleet_queue_missing_runner_request_id 0\n", "gha_fleet_queue_direct_jit_without_runner_request_id 0\n", "gha_fleet_queue_unbound_repository_beyond_grace 0\n", @@ -151,6 +154,35 @@ func TestRenderPrometheusFailsClosedForFutureSample(t *testing.T) { } } +func TestRenderPrometheusExposesDrainMarkedIndependentlyOfHeldOut(t *testing.T) { + collector := strandedUncoveredCollector(t) + collector.Members = func(context.Context) ([]MemberVisibility, error) { + return []MemberVisibility{ + {Name: "gha-runner-1", Online: true, DrainReason: "drained: image bake"}, + {Name: "gha-runner-2", Online: true, DrainReason: "drained: image bake"}, + {Name: "gha-runner-3", Online: true, DrainReason: "drained: image bake"}, + {Name: "gha-runner-4", Online: true, DrainReason: "drained: image bake"}, + }, nil + } + metrics := RenderPrometheus(collector.Collect(t.Context()), observationTime, 45*time.Second) + for _, wanted := range []string{ + "gha_fleet_platform_healthy 1\n", + "gha_fleet_visibility_held_out_members 0\n", + "gha_fleet_visibility_drain_marked_members 4\n", + `gha_fleet_visibility_drain_marked{member="gha-runner-1"} 1`, + `gha_fleet_visibility_drain_marked{member="gha-runner-2"} 1`, + `gha_fleet_visibility_drain_marked{member="gha-runner-3"} 1`, + `gha_fleet_visibility_drain_marked{member="gha-runner-4"} 1`, + } { + if !strings.Contains(metrics, wanted) { + t.Errorf("drain-marked metrics missing %q\n%s", wanted, metrics) + } + } + if strings.Contains(metrics, "gha_fleet_visibility_degraded{") { + t.Fatalf("online drain leaked a held-out series\n%s", metrics) + } +} + func TestMetricLabelsAreEscaped(t *testing.T) { var output strings.Builder metric(&output, "test_metric", map[string]string{"label": "line\nslash\\quote\""}, 1) diff --git a/internal/fleetobserve/observe.go b/internal/fleetobserve/observe.go index 536200a1..1fdda817 100644 --- a/internal/fleetobserve/observe.go +++ b/internal/fleetobserve/observe.go @@ -18,6 +18,9 @@ import ( ) const ( + // 16 exports drain-marked members (online or offline) independently of + // held-out (offline) members, so a full online drain is observable rather + // than inferred from platform health staying up. // 15 separates transient pre-JobAssigned correlation gaps from identities // that remain incomplete beyond the bounded convergence window. // 14 separates first-observed created-to-deleting inventory convergence from @@ -27,7 +30,7 @@ const ( // running identity telemetry. Version 8 added role-correct central exporter // health, container admission readiness, phase ages and rollback-compatible // WAL progress semantics. - SchemaVersion = 15 + SchemaVersion = 16 queueCorrelationGracePeriod = 2 * time.Minute // A running intent is retired by the cache broker's reclaim, which waits // five minutes for the two ledgers to agree and then runs once a minute. @@ -137,6 +140,15 @@ type ProviderRetrySource func(time.Time) (providerretry.Snapshot, error) type QueueIntentSource func(context.Context) (queueintent.Snapshot, error) type InstanceSource func(context.Context) ([]string, error) +// drainReasonPrefix is the gate's authorized-hold marker. The provider only +// copies a reason that already carries it; the observer requires the same +// prefix so a pressure-closed or operator-typed string cannot look like a drain. +const drainReasonPrefix = "drained: " + +func isDrainMarked(reason string) bool { + return strings.HasPrefix(reason, drainReasonPrefix) +} + // MemberVisibility is one cluster member as the collector needs it: name, // availability, and the drain reason its gate publishes when it is held out. type MemberVisibility struct { @@ -251,6 +263,12 @@ type Snapshot struct { // is held out, inventory gap counts are unattributable and move to their // *_unattributable fields rather than paging. HeldOutMembers []string `json:"held_out_members,omitempty"` + // DrainMarkedMembers are cluster members whose published gate reason + // carries the drain prefix, online or offline. HeldOutMembers is the + // offline subset: only those make the listing partial. A full online + // drain leaves HeldOutMembers empty and DrainMarkedMembers equal to the + // reported cluster. + DrainMarkedMembers []string `json:"drain_marked_members,omitempty"` } type DiagnosticExportSync struct { @@ -482,24 +500,25 @@ func (c Collector) Collect(ctx context.Context) Snapshot { // // An online drain is different: the listing is complete (empty on that // member is the truth), so HeldOutMembers stays empty and queue/assigned - // pages still fire. If every member is drain-marked, there is no eligible - // placement left on purpose; uncovered-beyond-grace must not fail platform - // health, or an authorized bake drain pages fleet_platform_unhealthy. + // pages still fire. DrainMarkedMembers still names every drained member so + // the authorized hold is observable. If every member is drain-marked, there + // is no eligible placement left on purpose; uncovered-beyond-grace must not + // fail platform health, or an authorized bake drain pages + // fleet_platform_unhealthy. visibilityDegraded := false allMembersDrainMarked := false if c.Members != nil { if membersErr != nil { snapshot.CollectionErrors = append(snapshot.CollectionErrors, safeError("cluster members", membersErr)) } else { - marked := 0 for _, member := range members { - if member.DrainReason != "" { - marked++ + if isDrainMarked(member.DrainReason) { + snapshot.DrainMarkedMembers = append(snapshot.DrainMarkedMembers, member.Name) } if member.Online { continue } - if member.DrainReason != "" { + if isDrainMarked(member.DrainReason) { snapshot.HeldOutMembers = append(snapshot.HeldOutMembers, member.Name) visibilityDegraded = true continue @@ -507,8 +526,9 @@ func (c Collector) Collect(ctx context.Context) Snapshot { snapshot.CollectionErrors = append(snapshot.CollectionErrors, "incus: cluster member "+member.Name+" is offline without a drain") } - allMembersDrainMarked = len(members) > 0 && marked == len(members) + allMembersDrainMarked = len(members) > 0 && len(snapshot.DrainMarkedMembers) == len(members) sort.Strings(snapshot.HeldOutMembers) + sort.Strings(snapshot.DrainMarkedMembers) } } diff --git a/internal/fleetobserve/visibility_test.go b/internal/fleetobserve/visibility_test.go index b0eaba51..2fc13f4e 100644 --- a/internal/fleetobserve/visibility_test.go +++ b/internal/fleetobserve/visibility_test.go @@ -32,6 +32,9 @@ func TestHeldOutMemberSuppressesGapsWithoutHidingThem(t *testing.T) { if len(snapshot.HeldOutMembers) != 1 || snapshot.HeldOutMembers[0] != "gha-runner-3" { t.Fatalf("held out = %v", snapshot.HeldOutMembers) } + if len(snapshot.DrainMarkedMembers) != 1 || snapshot.DrainMarkedMembers[0] != "gha-runner-3" { + t.Fatalf("drain marked = %v", snapshot.DrainMarkedMembers) + } if snapshot.Incus.MissingInstances != 0 { t.Fatalf("a gap paged during a marked hold: %#v", snapshot.Incus) } @@ -67,6 +70,9 @@ func TestOfflineMemberWithoutADrainFailsCollection(t *testing.T) { if len(snapshot.HeldOutMembers) != 0 { t.Fatalf("an undrained member was treated as held out: %v", snapshot.HeldOutMembers) } + if len(snapshot.DrainMarkedMembers) != 0 { + t.Fatalf("an undrained member was treated as drain-marked: %v", snapshot.DrainMarkedMembers) + } } // Online members change nothing: the healthy fixture stays byte-identical in @@ -80,7 +86,7 @@ func TestOnlineMembersLeaveTheSnapshotUntouched(t *testing.T) { }, nil } snapshot := collector.Collect(context.Background()) - if !snapshot.Healthy || len(snapshot.HeldOutMembers) != 0 || len(snapshot.CollectionErrors) != 0 { + if !snapshot.Healthy || len(snapshot.HeldOutMembers) != 0 || len(snapshot.DrainMarkedMembers) != 0 || len(snapshot.CollectionErrors) != 0 { t.Fatalf("online members disturbed the snapshot: %#v", snapshot) } } @@ -163,6 +169,9 @@ func TestOnlineDrainOfEveryMemberDoesNotFailPlatformHealth(t *testing.T) { if len(snapshot.HeldOutMembers) != 0 { t.Fatalf("online drain was counted as a listing hold-out: %v", snapshot.HeldOutMembers) } + if len(snapshot.DrainMarkedMembers) != 4 { + t.Fatalf("online drain was not named: %v", snapshot.DrainMarkedMembers) + } if !snapshot.Healthy { t.Fatalf("a full online drain failed the platform: errors=%v uncovered=%d", snapshot.CollectionErrors, snapshot.Queue.UncoveredRunningBeyondGrace) @@ -186,4 +195,82 @@ func TestPartialOnlineDrainStillFailsPlatformHealthOnUncoveredRunning(t *testing t.Fatalf("partial drain hid an uncovered running gap: healthy=%v uncovered=%d", snapshot.Healthy, snapshot.Queue.UncoveredRunningBeyondGrace) } + if len(snapshot.DrainMarkedMembers) != 3 { + t.Fatalf("partial drain marked = %v", snapshot.DrainMarkedMembers) + } +} + +// One offline drain-marked sibling makes the listing partial, so gaps move to +// unattributable. The other three stay online and drain-marked. Held-out is +// the offline subset; drain-marked is the whole authorized hold. +func TestMixedOnlineAndOfflineFullDrainKeepsPlatformHealth(t *testing.T) { + collector := strandedUncoveredCollector(t) + collector.Members = func(context.Context) ([]MemberVisibility, error) { + return []MemberVisibility{ + {Name: "gha-runner-1", Online: true, DrainReason: "drained: image bake"}, + {Name: "gha-runner-2", Online: true, DrainReason: "drained: image bake"}, + {Name: "gha-runner-3", Online: false, DrainReason: "drained: image bake"}, + {Name: "gha-runner-4", Online: true, DrainReason: "drained: image bake"}, + }, nil + } + snapshot := collector.Collect(context.Background()) + if !snapshot.Healthy { + t.Fatalf("a full mixed drain failed the platform: errors=%v", snapshot.CollectionErrors) + } + if len(snapshot.HeldOutMembers) != 1 || snapshot.HeldOutMembers[0] != "gha-runner-3" { + t.Fatalf("held out = %v", snapshot.HeldOutMembers) + } + if len(snapshot.DrainMarkedMembers) != 4 { + t.Fatalf("drain marked = %v", snapshot.DrainMarkedMembers) + } + if snapshot.Queue.UncoveredRunningBeyondGrace != 0 || snapshot.Queue.UncoveredUnattributable == 0 { + t.Fatalf("offline hold did not attribute the uncovered gap: %#v", snapshot.Queue) + } +} + +// A blank cluster listing is not an authorized full drain: there is no +// evidence every member was marked. The stranded uncovered intent must still +// fail platform health. +func TestEmptyMemberListingDoesNotCountAsFullDrain(t *testing.T) { + collector := strandedUncoveredCollector(t) + collector.Members = func(context.Context) ([]MemberVisibility, error) { + return []MemberVisibility{}, nil + } + snapshot := collector.Collect(context.Background()) + if snapshot.Healthy { + t.Fatal("an empty member listing left the platform healthy") + } + if len(snapshot.DrainMarkedMembers) != 0 || len(snapshot.HeldOutMembers) != 0 { + t.Fatalf("empty listing was treated as a drain: drain=%v held=%v", + snapshot.DrainMarkedMembers, snapshot.HeldOutMembers) + } +} + +// Only the drained: prefix is an authorized hold. A pressure-closed or +// operator-typed reason must not suppress gaps or keep health up. +func TestNonDrainReasonDoesNotHoldOutOrMark(t *testing.T) { + collector := strandedUncoveredCollector(t) + collector.Members = func(context.Context) ([]MemberVisibility, error) { + return []MemberVisibility{ + {Name: "gha-runner-1", Online: true, DrainReason: "pressure-closed"}, + {Name: "gha-runner-2", Online: false, DrainReason: "manual"}, + }, nil + } + snapshot := collector.Collect(context.Background()) + if snapshot.Healthy { + t.Fatal("a non-drain reason left the platform healthy") + } + if len(snapshot.DrainMarkedMembers) != 0 || len(snapshot.HeldOutMembers) != 0 { + t.Fatalf("non-drain reason was treated as a drain: drain=%v held=%v", + snapshot.DrainMarkedMembers, snapshot.HeldOutMembers) + } + found := false + for _, message := range snapshot.CollectionErrors { + if strings.Contains(message, "gha-runner-2") && strings.Contains(message, "offline without a drain") { + found = true + } + } + if !found { + t.Fatalf("collection errors do not name the offline member: %v", snapshot.CollectionErrors) + } } diff --git a/internal/observabilityrules/rules_test.go b/internal/observabilityrules/rules_test.go index da11438f..6daeda20 100644 --- a/internal/observabilityrules/rules_test.go +++ b/internal/observabilityrules/rules_test.go @@ -31,8 +31,9 @@ func TestRepositoryRulesUseCurrentMetricSemantics(t *testing.T) { "compute_pressure_state_stale": "gha_fleet_pressure_observer_up", "compute_root_disk_low": "gha_fleet_host_root_free_percent", "kernel_slab_unreclaimable": "gha_fleet_host_slab_unreclaimable_attributed_bytes", - "audit_suppression_burst": `signal_class="audit_suppressed"`, - "kernel_workqueue_hog": `signal_class="kernel_workqueue_hog"`, + "audit_suppression_burst": `last_over_time(gha_fleet_host_signal_events{signal_class="audit_suppressed"`, + "kernel_workqueue_hog": `last_over_time(gha_fleet_host_signal_events{signal_class="kernel_workqueue_hog"`, + "fleet_health_flapping": "< bool 1", "host_compliance_observer_missing": "last_over_time(gha_fleet_host_compliance_observer_up[10m])", "host_oom_detected": "round(max by (host_name) (last_over_time(gha_fleet_host_oom_kills_total[5m]) - min_over_time(gha_fleet_host_oom_kills_total[5m])))", "lifecycle_inventory_gap": "min_over_time(gha_fleet_journal_missing_instances[2m])", @@ -128,8 +129,8 @@ func TestHostSignalSlowBurnsRemainVectorsForOpenObserveSubqueries(t *testing.T) // three series of 4, 4 and 3). Summing by host keeps the subject // the notification prints and cannot collapse it to a scalar. body := withoutSubject(alert.QueryCondition.PromQL) - if !strings.HasPrefix(body, "sum by (host_name) (max_over_time(") { - t.Fatalf("%s is not the windowed delta summed by host: %s", id, alert.QueryCondition.PromQL) + if !strings.HasPrefix(body, "sum by (host_name) (last_over_time(") { + t.Fatalf("%s is not the windowed last-min delta summed by host: %s", id, alert.QueryCondition.PromQL) } if !strings.Contains(body, "- min_over_time(") { t.Fatalf("%s lost the min_over_time half of its delta: %s", id, alert.QueryCondition.PromQL)