From b6b9bad49c65016e696e88acf52dc656307877f2 Mon Sep 17 00:00:00 2001 From: Douglas Adolph <116844282+dcadolph@users.noreply.github.com> Date: Sat, 31 Oct 2026 06:02:54 -0500 Subject: [PATCH 01/13] Read scanner fields through one observation accessor --- internal/report/findings.go | 14 +- internal/report/findings_health.go | 63 +++--- internal/report/observe.go | 147 +++++++++++++ internal/report/observe_test.go | 330 +++++++++++++++++++++++++++++ internal/report/report.go | 4 + 5 files changed, 526 insertions(+), 32 deletions(-) create mode 100644 internal/report/observe.go create mode 100644 internal/report/observe_test.go diff --git a/internal/report/findings.go b/internal/report/findings.go index 5ff1d2b..6e8870c 100644 --- a/internal/report/findings.go +++ b/internal/report/findings.go @@ -428,13 +428,13 @@ func generateClusterHealth(r *Report, findings []Finding) []ClusterHealth { h.Status = "degraded" } - h.KubernetesVersion = getStringField(r, "version", cluster, "git_version") - h.NodeCount = getIntField(r, "node-health", cluster, "node_count") - h.HealthyNodes = getIntField(r, "node-health", cluster, "healthy_nodes") - h.AvgCPU = getMetricsFloat(r, cluster, "avg_cpu_percent") - h.AvgMemory = getMetricsFloat(r, cluster, "avg_memory_percent") - h.WarningEvents = getIntField(r, "events", cluster, "warning_events") - h.NamespaceCount = getIntField(r, "namespaces", cluster, "count") + h.KubernetesVersion, _ = observeString(r, "version", cluster, "git_version") + h.NodeCount, _ = observeInt(r, "node-health", cluster, "node_count") + h.HealthyNodes, _ = observeInt(r, "node-health", cluster, "healthy_nodes") + h.AvgCPU, _ = observe(r, "metrics", cluster, "avg_cpu_percent") + h.AvgMemory, _ = observe(r, "metrics", cluster, "avg_memory_percent") + h.WarningEvents, _ = observeInt(r, "events", cluster, "warning_events") + h.NamespaceCount, _ = observeInt(r, "namespaces", cluster, "count") h.DegradedScanners = degradedByCluster[cluster] healths = append(healths, h) diff --git a/internal/report/findings_health.go b/internal/report/findings_health.go index e9ed89e..0e26f15 100644 --- a/internal/report/findings_health.go +++ b/internal/report/findings_health.go @@ -19,17 +19,17 @@ func nodeHealthFindings(r *Report) []Finding { var findings []Finding for _, cluster := range r.Clusters { nodes := extractNodeNames(r, cluster, "node-health") - memPressure := getIntField(r, "node-health", cluster, "memory_pressure_nodes") - diskPressure := getIntField(r, "node-health", cluster, "disk_pressure_nodes") - notReady := getIntField(r, "node-health", cluster, "not_ready_nodes") - total := getIntField(r, "node-health", cluster, "node_count") + memPressure, memObs := observeInt(r, "node-health", cluster, "memory_pressure_nodes") + diskPressure, diskObs := observeInt(r, "node-health", cluster, "disk_pressure_nodes") + notReady, notReadyObs := observeInt(r, "node-health", cluster, "not_ready_nodes") + total, _ := observeInt(r, "node-health", cluster, "node_count") - if memPressure > 0 { + if memObs.seen() && memPressure > 0 { affected := nodes.byCondition("MemoryPressure") findings = append(findings, Finding{ Rule: "node-health/memory-pressure", Title: fmt.Sprintf("%s has %d node(s) under memory pressure", cluster, memPressure), - Description: fmt.Sprintf("%d of %d nodes report MemoryPressure=True. Pods on these nodes risk OOM kills and the scheduler will avoid them.", memPressure, total), + Description: fmt.Sprintf("%d of %d nodes report MemoryPressure=True. Pods on these nodes risk OOM kills and the scheduler will avoid them.", memPressure, total) + memObs.note(), Severity: SeverityCritical, Cluster: cluster, Scanner: "node-health", @@ -39,12 +39,12 @@ func nodeHealthFindings(r *Report) []Finding { }, }) } - if diskPressure > 0 { + if diskObs.seen() && diskPressure > 0 { affected := nodes.byCondition("DiskPressure") findings = append(findings, Finding{ Rule: "node-health/disk-pressure", Title: fmt.Sprintf("%s has %d node(s) under disk pressure", cluster, diskPressure), - Description: fmt.Sprintf("%d of %d nodes report DiskPressure=True. Pods may be evicted to free disk space.", diskPressure, total), + Description: fmt.Sprintf("%d of %d nodes report DiskPressure=True. Pods may be evicted to free disk space.", diskPressure, total) + diskObs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "node-health", @@ -54,12 +54,12 @@ func nodeHealthFindings(r *Report) []Finding { }, }) } - if notReady > 0 { + if notReadyObs.seen() && notReady > 0 { affected := nodes.notReady() findings = append(findings, Finding{ Rule: "node-health/not-ready", Title: fmt.Sprintf("%s has %d node(s) not ready", cluster, notReady), - Description: fmt.Sprintf("%d of %d nodes are not reporting Ready=True. Workloads cannot be scheduled to these nodes.", notReady, total), + Description: fmt.Sprintf("%d of %d nodes are not reporting Ready=True. Workloads cannot be scheduled to these nodes.", notReady, total) + notReadyObs.note(), Severity: SeverityCritical, Cluster: cluster, Scanner: "node-health", @@ -116,8 +116,8 @@ func (ns nodeSet) notReady() []string { // unmarshal generically so this function does not depend on the scanner // package's concrete types. func extractNodeNames(r *Report, cluster, scannerName string) nodeSet { - m := clusterData(r, scannerName, cluster) - if m == nil { + m, obs := clusterView(r, scannerName, cluster) + if !obs.seen() { return nil } rawNodes, _ := m["nodes"].([]any) @@ -203,7 +203,9 @@ func capacityFindings(r *Report) []Finding { } // quotaFindings reports clusters with no ResourceQuotas defined and ships a -// baseline manifest in the remediation block. +// baseline manifest in the remediation block. Zero quotas is an absence claim, so it +// needs a completed quota run. The namespace count only shows namespaces exist, so a +// degraded namespace run is enough and marks the finding partial. func quotaFindings(r *Report) []Finding { sec := r.Sections["resource-quotas"] if sec == nil { @@ -211,13 +213,13 @@ func quotaFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - quotas := getIntField(r, "resource-quotas", cluster, "namespaces_with_quotas") - nsCount := getIntField(r, "namespaces", cluster, "count") - if nsCount > 0 && quotas == 0 { + quotas, quotaObs := observeInt(r, "resource-quotas", cluster, "namespaces_with_quotas") + nsCount, nsObs := observeInt(r, "namespaces", cluster, "count") + if quotaObs == observed && nsObs.seen() && nsCount > 0 && quotas == 0 { findings = append(findings, Finding{ Rule: "resource-quotas/no-quota", Title: fmt.Sprintf("%s has no resource quotas defined", cluster), - Description: "No namespaces have ResourceQuota objects. There are no limits on resource consumption per namespace.", + Description: "No namespaces have ResourceQuota objects. There are no limits on resource consumption per namespace." + nsObs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "resource-quotas", @@ -248,7 +250,9 @@ spec: // eventFindings normalizes the warning-event count by cluster size so a // 200-node cluster is not falsely flagged for the same threshold a 5-node -// cluster trips at. Top reasons are included when the scanner provided them. +// cluster trips at. Top reasons are included when the scanner provided them. An +// unobserved node count is not read as one node: the rate is left out and only the +// absolute thresholds apply. func eventFindings(r *Report) []Finding { sec := r.Sections["events"] if sec == nil { @@ -256,12 +260,15 @@ func eventFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - warningCount := getIntField(r, "events", cluster, "warning_events") - if warningCount == 0 { + warningCount, eventsObs := observeInt(r, "events", cluster, "warning_events") + if !eventsObs.seen() || warningCount == 0 { continue } - nodes := max(getIntField(r, "node-health", cluster, "node_count"), 1) - perNode := float64(warningCount) / float64(nodes) + nodes, nodesObs := observeInt(r, "node-health", cluster, "node_count") + perNode := 0.0 + if nodesObs.seen() { + perNode = float64(warningCount) / float64(max(nodes, 1)) + } topReasons := topEventReasons(r, cluster, 3) sev := "" @@ -273,10 +280,16 @@ func eventFindings(r *Report) []Finding { default: sev = SeverityInfo } - desc := fmt.Sprintf("%d warning events in the last hour across %d node(s) (%.1f per node).", warningCount, nodes, perNode) + desc := fmt.Sprintf("%d warning events in the last hour. The node count was not observed, so no per-node rate.", + warningCount) + if nodesObs.seen() { + desc = fmt.Sprintf("%d warning events in the last hour across %d node(s) (%.1f per node).", + warningCount, max(nodes, 1), perNode) + } if len(topReasons) > 0 { desc += " Top reasons: " + strings.Join(topReasons, ", ") + "." } + desc += eventsObs.note() findings = append(findings, Finding{ Rule: "events/warning-volume", Title: fmt.Sprintf("%s has %d warning event(s) in the last hour", cluster, warningCount), @@ -297,8 +310,8 @@ func eventFindings(r *Report) []Finding { // cluster, formatted as "Reason (N)". Returns nil if the scanner did not // emit a TopWarningReasons slice. func topEventReasons(r *Report, cluster string, n int) []string { - m := clusterData(r, "events", cluster) - if m == nil { + m, obs := clusterView(r, "events", cluster) + if !obs.seen() { return nil } raw, _ := m["top_warning_reasons"].([]any) diff --git a/internal/report/observe.go b/internal/report/observe.go new file mode 100644 index 0000000..5626b94 --- /dev/null +++ b/internal/report/observe.go @@ -0,0 +1,147 @@ +package report + +import ( + "github.com/dcadolph/fleetsweeper/internal/scanner" + "github.com/dcadolph/fleetsweeper/internal/util" +) + +// observation says whether a scanner field was measured on a cluster. The zero value is +// unobserved, so a lookup that fails for any reason never reads as a measurement. +type observation int + +const ( + // unobserved means no scanner measured the value: the run errored, never ran on the + // cluster, or did not emit the field. + unobserved observation = iota + // partial means the field is present but the run was degraded, so other reads in the + // same run failed. + partial + // observed means the field is present and the run completed, or the run found the + // scanned feature not served, which is a trustworthy "nothing here". + observed +) + +// partialScanNote is appended to a finding's description when it rests on a degraded run. +const partialScanNote = " (partial scan)" + +// runKey identifies one scanner run on one cluster. +type runKey struct { + // scanner is the scanner name. + scanner string + // cluster is the cluster name. + cluster string +} + +// indexRuns derives every run's state from r.Degraded and the section payloads. A run listed +// in Degraded carries that state, a run with a payload and no Degraded entry is OK, and a run +// with neither is absent from the map. A report decoded from JSON yields the same table, so +// the accessor needs nothing restored after a load. +func indexRuns(r *Report) map[runKey]scanner.State { + runs := make(map[runKey]scanner.State) + for name, sec := range r.Sections { + if sec == nil { + continue + } + for cluster := range sec.PerCluster { + runs[runKey{scanner: name, cluster: cluster}] = scanner.StateOK + } + } + for _, d := range r.Degraded { + runs[runKey{scanner: d.Scanner, cluster: d.Cluster}] = scanner.State(d.State) + } + return runs +} + +// runState returns the state of scannerName's run on cluster and whether the run exists. It +// reads the table Build fills and derives the state from the report when the table is absent. +func runState(r *Report, scannerName, cluster string) (scanner.State, bool) { + runs := r.runs + if runs == nil { + runs = indexRuns(r) + } + st, ok := runs[runKey{scanner: scannerName, cluster: cluster}] + return st, ok +} + +// clusterView returns a cluster's scanner payload as a generic map and how far it can be +// trusted. An errored or absent run, or a payload that does not convert, is unobserved with a +// nil map. The map may be the report's own, so callers must not modify it. +func clusterView(r *Report, scannerName, cluster string) (map[string]any, observation) { + st, ok := runState(r, scannerName, cluster) + if !ok || st == scanner.StateErrored { + return nil, unobserved + } + sec := r.Sections[scannerName] + if sec == nil { + return nil, unobserved + } + data, ok := sec.PerCluster[cluster] + if !ok { + return nil, unobserved + } + m, _ := genericMap(data) + if m == nil { + return nil, unobserved + } + if st == scanner.StateDegraded { + return m, partial + } + return m, observed +} + +// observe returns a numeric field from a cluster's scanner payload and its observation. A +// missing or non-numeric field is unobserved with a zero value the caller must not use. +func observe(r *Report, scannerName, cluster, field string) (float64, observation) { + m, obs := clusterView(r, scannerName, cluster) + if obs == unobserved { + return 0, unobserved + } + v, ok := util.Float64(m[field]) + if !ok { + return 0, unobserved + } + return v, obs +} + +// observeInt returns observe's value truncated to an int. +func observeInt(r *Report, scannerName, cluster, field string) (int, observation) { + v, obs := observe(r, scannerName, cluster, field) + return int(v), obs +} + +// observeString returns a string field from a cluster's scanner payload and its observation. +// A missing or non-string field is unobserved. +func observeString(r *Report, scannerName, cluster, field string) (string, observation) { + m, obs := clusterView(r, scannerName, cluster) + if obs == unobserved { + return "", unobserved + } + s, ok := m[field].(string) + if !ok { + return "", unobserved + } + return s, obs +} + +// seen reports whether obs carries a value, observed or partial. Presence rules use it. +func (obs observation) seen() bool { + return obs != unobserved +} + +// note returns the description suffix for a finding resting on obs. +func (obs observation) note() string { + if obs == partial { + return partialScanNote + } + return "" +} + +// weakest returns the least trustworthy of obs, so a finding resting on several fields +// reports the weakest of them. +func weakest(obs ...observation) observation { + out := observed + for _, o := range obs { + out = min(out, o) + } + return out +} diff --git a/internal/report/observe_test.go b/internal/report/observe_test.go new file mode 100644 index 0000000..d3ca6af --- /dev/null +++ b/internal/report/observe_test.go @@ -0,0 +1,330 @@ +package report + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/dcadolph/fleetsweeper/internal/scanner" +) + +// okRun returns an OK scanner result carrying data. +func okRun(name string, data map[string]any) scanner.Result { + return scanner.Result{Scanner: name, Data: data} +} + +// degradedRun returns a degraded scanner result carrying partial data. +func degradedRun(name string, data map[string]any) scanner.Result { + return scanner.Result{Scanner: name, State: scanner.StateDegraded, Reason: "list failed", Data: data} +} + +// erroredRun returns an errored scanner result with no data. +func erroredRun(name string) scanner.Result { + return scanner.ErroredResult(name, errors.New("forbidden")) +} + +// findingsWithRule returns the findings in fs whose rule is rule. +func findingsWithRule(fs []Finding, rule string) []Finding { + var out []Finding + for _, f := range fs { + if f.Rule == rule { + out = append(out, f) + } + } + return out +} + +// TestObserve reads numeric fields through the accessor on a built and a decoded report. +func TestObserve(t *testing.T) { + t.Parallel() + results := map[string]map[string]scanner.Result{ + "c1": { + "namespaces": okRun("namespaces", map[string]any{"count": 3, "name": "a"}), + "resource-quotas": degradedRun("resource-quotas", map[string]any{"namespaces_with_quotas": 0}), + "certs": erroredRun("certs"), + "crds": {Scanner: "crds", State: scanner.StateUnavailable, Data: map[string]any{"count": 0}}, + }, + } + built := Build([]string{"c1"}, results) + blob, err := json.Marshal(built) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded Report + if err := json.Unmarshal(blob, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + tests := []struct { + Scanner string + Field string + WantValue float64 + WantObs observation + }{{ // Test 0: An OK run's field is observed. + Scanner: "namespaces", Field: "count", WantValue: 3, WantObs: observed, + }, { // Test 1: A degraded run's field is partial. + Scanner: "resource-quotas", Field: "namespaces_with_quotas", WantValue: 0, WantObs: partial, + }, { // Test 2: An errored run is unobserved. + Scanner: "certs", Field: "expiring", WantValue: 0, WantObs: unobserved, + }, { // Test 3: A scanner that never ran on the cluster is unobserved. + Scanner: "events", Field: "warning_events", WantValue: 0, WantObs: unobserved, + }, { // Test 4: A field the run did not emit is unobserved. + Scanner: "namespaces", Field: "missing", WantValue: 0, WantObs: unobserved, + }, { // Test 5: A non-numeric field is unobserved. + Scanner: "namespaces", Field: "name", WantValue: 0, WantObs: unobserved, + }, { // Test 6: An unavailable run is a trustworthy nothing, so observed. + Scanner: "crds", Field: "count", WantValue: 0, WantObs: observed, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + for _, r := range []*Report{built, &decoded} { + gotValue, gotObs := observe(r, test.Scanner, "c1", test.Field) + if diff := cmp.Diff(test.WantValue, gotValue); diff != "" { + t.Errorf("value mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantObs, gotObs); diff != "" { + t.Errorf("observation mismatch (-want +got):\n%s", diff) + } + } + }) + } +} + +// TestObserveString reads string fields through the accessor across run states. +func TestObserveString(t *testing.T) { + t.Parallel() + results := map[string]map[string]scanner.Result{ + "c1": {"version": okRun("version", map[string]any{"git_version": "v1.30.0", "major": 1})}, + "c2": {"version": degradedRun("version", map[string]any{"git_version": "v1.29.0"})}, + "c3": {"version": erroredRun("version")}, + } + r := Build([]string{"c1", "c2", "c3"}, results) + tests := []struct { + Cluster string + Field string + WantValue string + WantObs observation + }{{ // Test 0: An OK string field is observed. + Cluster: "c1", Field: "git_version", WantValue: "v1.30.0", WantObs: observed, + }, { // Test 1: A degraded string field is partial. + Cluster: "c2", Field: "git_version", WantValue: "v1.29.0", WantObs: partial, + }, { // Test 2: An errored run is unobserved. + Cluster: "c3", Field: "git_version", WantValue: "", WantObs: unobserved, + }, { // Test 3: A non-string field is unobserved. + Cluster: "c1", Field: "major", WantValue: "", WantObs: unobserved, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + gotValue, gotObs := observeString(r, "version", test.Cluster, test.Field) + if diff := cmp.Diff(test.WantValue, gotValue); diff != "" { + t.Errorf("value mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantObs, gotObs); diff != "" { + t.Errorf("observation mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestWeakest picks the least trustworthy observation and its note. +func TestWeakest(t *testing.T) { + t.Parallel() + tests := []struct { + In []observation + WantObs observation + WantNote string + }{{ // Test 0: No inputs is observed. + In: nil, WantObs: observed, WantNote: "", + }, { // Test 1: One partial input makes the result partial. + In: []observation{observed, partial}, WantObs: partial, WantNote: partialScanNote, + }, { // Test 2: One unobserved input makes the result unobserved. + In: []observation{partial, unobserved, observed}, WantObs: unobserved, WantNote: "", + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + got := weakest(test.In...) + if diff := cmp.Diff(test.WantObs, got); diff != "" { + t.Errorf("observation mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantNote, got.note()); diff != "" { + t.Errorf("note mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestQuotaFindingsCoverage keeps the no-quota claim off clusters whose quota run was not complete. +func TestQuotaFindingsCoverage(t *testing.T) { + t.Parallel() + ns := okRun("namespaces", map[string]any{"count": 3}) + tests := []struct { + Runs map[string]scanner.Result + WantPartial bool + WantCount int + }{{ // Test 0: An errored quota run with live namespaces claims nothing (R-5). + Runs: map[string]scanner.Result{"namespaces": ns, "resource-quotas": erroredRun("resource-quotas")}, + WantCount: 0, + }, { // Test 1: A degraded quota run reading zero is not proof of no quotas. + Runs: map[string]scanner.Result{"namespaces": ns, "resource-quotas": degradedRun("resource-quotas", + map[string]any{"namespaces_with_quotas": 0})}, + WantCount: 0, + }, { // Test 2: An OK quota run reading zero with live namespaces is the finding. + Runs: map[string]scanner.Result{"namespaces": ns, "resource-quotas": okRun("resource-quotas", + map[string]any{"namespaces_with_quotas": 0})}, + WantCount: 1, + }, { // Test 3: Namespaces from a degraded run still prove namespaces exist. + Runs: map[string]scanner.Result{ + "namespaces": degradedRun("namespaces", map[string]any{"count": 3}), + "resource-quotas": okRun("resource-quotas", map[string]any{"namespaces_with_quotas": 0}), + }, + WantPartial: true, + WantCount: 1, + }, { // Test 4: An errored namespace run leaves the namespace count unknown. + Runs: map[string]scanner.Result{ + "namespaces": erroredRun("namespaces"), + "resource-quotas": okRun("resource-quotas", map[string]any{"namespaces_with_quotas": 0}), + }, + WantCount: 0, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + r := Build([]string{"c1"}, map[string]map[string]scanner.Result{"c1": test.Runs}) + got := findingsWithRule(quotaFindings(r), "resource-quotas/no-quota") + if diff := cmp.Diff(test.WantCount, len(got)); diff != "" { + t.Fatalf("finding count mismatch (-want +got):\n%s", diff) + } + if len(got) == 1 { + gotPartial := strings.HasSuffix(got[0].Description, partialScanNote) + if diff := cmp.Diff(test.WantPartial, gotPartial); diff != "" { + t.Errorf("partial note mismatch (-want +got):\n%s", diff) + } + } + }) + } +} + +// TestNodeHealthFindingsCoverage reports node pressure from OK and degraded runs only. +func TestNodeHealthFindingsCoverage(t *testing.T) { + t.Parallel() + pressure := map[string]any{"node_count": 3, "memory_pressure_nodes": 1, "not_ready_nodes": 1} + tests := []struct { + Run scanner.Result + WantPartial bool + WantCount int + }{{ // Test 0: An OK run's pressure counts produce findings. + Run: okRun("node-health", pressure), WantCount: 2, + }, { // Test 1: A degraded run's positive counts still produce findings, marked partial. + Run: degradedRun("node-health", pressure), WantPartial: true, WantCount: 2, + }, { // Test 2: An errored run produces nothing. + Run: erroredRun("node-health"), WantCount: 0, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + r := Build([]string{"c1"}, map[string]map[string]scanner.Result{"c1": {"node-health": test.Run}}) + got := nodeHealthFindings(r) + if diff := cmp.Diff(test.WantCount, len(got)); diff != "" { + t.Fatalf("finding count mismatch (-want +got):\n%s", diff) + } + for _, f := range got { + gotPartial := strings.HasSuffix(f.Description, partialScanNote) + if diff := cmp.Diff(test.WantPartial, gotPartial); diff != "" { + t.Errorf("%s partial note mismatch (-want +got):\n%s", f.Rule, diff) + } + } + }) + } +} + +// TestEventFindingsCoverage rates warning events per node only when the node count was observed. +func TestEventFindingsCoverage(t *testing.T) { + t.Parallel() + nodes := okRun("node-health", map[string]any{"node_count": 100}) + tests := []struct { + Runs map[string]scanner.Result + WantSeverity string + WantDesc string + WantCount int + }{{ // Test 0: A known node count rates events per node. + Runs: map[string]scanner.Result{"node-health": nodes, + "events": okRun("events", map[string]any{"warning_events": 300})}, + WantSeverity: SeverityWarning, + WantDesc: "300 warning events in the last hour across 100 node(s) (3.0 per node).", + WantCount: 1, + }, { // Test 1: An unknown node count is not read as one node. + Runs: map[string]scanner.Result{"node-health": erroredRun("node-health"), + "events": okRun("events", map[string]any{"warning_events": 150})}, + WantSeverity: SeverityInfo, + WantDesc: "150 warning events in the last hour. The node count was not observed, so no per-node rate.", + WantCount: 1, + }, { // Test 2: A degraded events run is marked partial. + Runs: map[string]scanner.Result{"node-health": nodes, + "events": degradedRun("events", map[string]any{"warning_events": 300})}, + WantSeverity: SeverityWarning, + WantDesc: "300 warning events in the last hour across 100 node(s) (3.0 per node)." + partialScanNote, + WantCount: 1, + }, { // Test 3: An errored events run produces nothing. + Runs: map[string]scanner.Result{"node-health": nodes, "events": erroredRun("events")}, + WantCount: 0, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + r := Build([]string{"c1"}, map[string]map[string]scanner.Result{"c1": test.Runs}) + got := eventFindings(r) + if diff := cmp.Diff(test.WantCount, len(got)); diff != "" { + t.Fatalf("finding count mismatch (-want +got):\n%s", diff) + } + if len(got) == 0 { + return + } + if diff := cmp.Diff(test.WantSeverity, got[0].Severity); diff != "" { + t.Errorf("severity mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantDesc, got[0].Description); diff != "" { + t.Errorf("description mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestClusterHealthCoverage fills cluster health fields only from runs that observed them. +func TestClusterHealthCoverage(t *testing.T) { + t.Parallel() + results := map[string]map[string]scanner.Result{ + "c1": { + "version": okRun("version", map[string]any{"git_version": "v1.30.0"}), + "node-health": degradedRun("node-health", map[string]any{"node_count": 4, "healthy_nodes": 4}), + }, + "c2": { + "version": erroredRun("version"), + "node-health": erroredRun("node-health"), + }, + } + r := Build([]string{"c1", "c2"}, results) + type view struct { + Version string + Nodes int + Healthy int + } + got := map[string]view{} + for _, h := range r.ClusterHealths { + got[h.Name] = view{Version: h.KubernetesVersion, Nodes: h.NodeCount, Healthy: h.HealthyNodes} + } + want := map[string]view{ + // Test 0: Observed and partial fields carry their values. + "c1": {Version: "v1.30.0", Nodes: 4, Healthy: 4}, + // Test 1: Unobserved fields stay empty. + "c2": {}, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("cluster health mismatch (-want +got):\n%s", diff) + } +} diff --git a/internal/report/report.go b/internal/report/report.go index a22f4a9..834f401 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -104,6 +104,9 @@ type Report struct { // Notes explain analysis the report could not run, such as outlier // detection on a fleet too small to have a norm. Notes []string `json:"notes,omitempty"` + // runs holds each scanner run's state by scanner and cluster. Build fills it, and a + // decoded report derives the same table from Degraded and Sections on demand. + runs map[runKey]scanner.State } // ScannerStatus records a scanner run that did not return complete, @@ -225,6 +228,7 @@ func Build(clusters []string, results map[string]map[string]scanner.Result, opts r.Sections[name] = section } sortScannerStatuses(r.Degraded) + r.runs = indexRuns(r) r.Categories = buildCategories(scannerNames) r.Summary = buildSummary(clusters, r.Sections) From fa3990414df1cd013063ae7da406c78e80e142b3 Mon Sep 17 00:00:00 2001 From: Douglas Adolph <116844282+dcadolph@users.noreply.github.com> Date: Sat, 31 Oct 2026 06:06:31 -0500 Subject: [PATCH 02/13] Hold every finding generator to the observation rules --- internal/report/findings.go | 6 - internal/report/findings_cluster.go | 41 +++-- internal/report/findings_security.go | 93 ++++++----- internal/report/findings_workloads.go | 27 +-- internal/report/observe_generators_test.go | 182 +++++++++++++++++++++ 5 files changed, 271 insertions(+), 78 deletions(-) create mode 100644 internal/report/observe_generators_test.go diff --git a/internal/report/findings.go b/internal/report/findings.go index 6e8870c..365ad9a 100644 --- a/internal/report/findings.go +++ b/internal/report/findings.go @@ -470,12 +470,6 @@ func clusterData(r *Report, scannerName, cluster string) map[string]any { return m } -// getStringField extracts a string field from a cluster's scanner data. -func getStringField(r *Report, scannerName, cluster, field string) string { - s, _ := clusterData(r, scannerName, cluster)[field].(string) - return s -} - // getIntField extracts an integer field from a cluster's scanner data. func getIntField(r *Report, scannerName, cluster, field string) int { n, _ := util.Float64(clusterData(r, scannerName, cluster)[field]) diff --git a/internal/report/findings_cluster.go b/internal/report/findings_cluster.go index bc84b62..16f812a 100644 --- a/internal/report/findings_cluster.go +++ b/internal/report/findings_cluster.go @@ -62,7 +62,10 @@ func versionFindings(r *Report) []Finding { uniqueVersions := make([]string, 0, len(r.Clusters)) seen := make(map[string]bool) for _, cluster := range r.Clusters { - v := getStringField(r, "version", cluster, "git_version") + v, obs := observeString(r, "version", cluster, "git_version") + if !obs.seen() { + continue + } versions[cluster] = v if v != "" && !seen[v] { seen[v] = true @@ -159,7 +162,9 @@ func namespaceFindings(r *Report) []Finding { } counts := make(map[string]int, len(r.Clusters)) for _, cluster := range r.Clusters { - counts[cluster] = countNonSystemNamespaces(r, cluster) + if n, obs := countNonSystemNamespaces(r, cluster); obs.seen() { + counts[cluster] = n + } } if len(counts) < 2 { return nil @@ -195,11 +200,12 @@ func namespaceFindings(r *Report) []Finding { // countNonSystemNamespaces reads the namespace scanner's per-namespace list // when present and counts those not in the system allowlist. Falls back to -// the bare count when the scanner did not emit names. -func countNonSystemNamespaces(r *Report, cluster string) int { - m := clusterData(r, "namespaces", cluster) - if m == nil { - return 0 +// the bare count when the scanner did not emit names. An unobserved cluster reports its +// observation so the caller can leave it out of the comparison. +func countNonSystemNamespaces(r *Report, cluster string) (int, observation) { + m, obs := clusterView(r, "namespaces", cluster) + if !obs.seen() { + return 0, unobserved } if raw, ok := m["namespaces"].([]any); ok { count := 0 @@ -213,10 +219,13 @@ func countNonSystemNamespaces(r *Report, cluster string) int { count++ } } - return count + return count, obs + } + count, ok := util.Float64(m["count"]) + if !ok { + return 0, unobserved } - count, _ := util.Float64(m["count"]) - return int(count) + return int(count), obs } // rbacFindings emits a fleet-level warning when RBAC objects diverge. @@ -248,8 +257,8 @@ func deprecatedAPIFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - m := clusterData(r, "deprecated-apis", cluster) - if m == nil { + m, obs := clusterView(r, "deprecated-apis", cluster) + if !obs.seen() { continue } raw, _ := m["deprecated"].([]any) @@ -272,7 +281,7 @@ func deprecatedAPIFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "deprecated-apis/in-use", Title: fmt.Sprintf("%s uses %d deprecated API version(s)", cluster, len(raw)), - Description: "These API versions have been deprecated or removed in newer Kubernetes minor releases. Migrate before the next upgrade or workloads will fail.", + Description: "These API versions have been deprecated or removed in newer Kubernetes minor releases. Migrate before the next upgrade or workloads will fail." + obs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "deprecated-apis", @@ -295,8 +304,8 @@ func clusterInfoFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - m := clusterData(r, "cluster-info", cluster) - if m == nil { + m, obs := clusterView(r, "cluster-info", cluster) + if !obs.seen() { continue } hasDrift, _ := m["has_drift"].(bool) @@ -319,7 +328,7 @@ func clusterInfoFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "cluster-info/node-platform-drift", Title: fmt.Sprintf("%s has node-level platform drift", cluster), - Description: "Different nodes report different platform identities. Common cause: rolling node-pool replacement, stale AMI, or partially-completed upgrade.", + Description: "Different nodes report different platform identities. Common cause: rolling node-pool replacement, stale AMI, or partially-completed upgrade." + obs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "cluster-info", diff --git a/internal/report/findings_security.go b/internal/report/findings_security.go index 549d600..becc99d 100644 --- a/internal/report/findings_security.go +++ b/internal/report/findings_security.go @@ -17,14 +17,16 @@ func securityFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - unenforced := getIntField(r, "security", cluster, "unenforced_count") - total := getIntField(r, "security", cluster, "namespace_count") - if unenforced > 0 { + unenforced, unenforcedObs := observeInt(r, "security", cluster, "unenforced_count") + total, totalObs := observeInt(r, "security", cluster, "namespace_count") + // Every namespace unenforced is an absence claim, so it needs a completed run. + allUnenforced := totalObs.seen() && total > 0 && unenforced == total + if unenforcedObs.seen() && unenforced > 0 && (!allUnenforced || weakest(unenforcedObs, totalObs) == observed) { // A namespace total below the unenforced count means the total // is missing, so the share is unknown rather than over 100%. share := fmt.Sprintf("%d namespaces", unenforced) sev := SeverityWarning - if total >= unenforced { + if totalObs.seen() && total >= unenforced { pct := float64(unenforced) / float64(total) * 100 share = fmt.Sprintf("%d of %d namespaces (%.0f%%)", unenforced, total, pct) if pct > 50 { @@ -42,7 +44,7 @@ func securityFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "security/psa-unenforced", Title: fmt.Sprintf("%s has %d namespace(s) without Pod Security enforcement", cluster, unenforced), - Description: share + " have no Pod Security Standards enforce label or enforce only privileged. Pods in these namespaces can run with any privileges.", + Description: share + " have no Pod Security Standards enforce label or enforce only privileged. Pods in these namespaces can run with any privileges." + weakest(unenforcedObs, totalObs).note(), Severity: sev, Cluster: cluster, Scanner: "security", @@ -57,8 +59,13 @@ func securityFindings(r *Report) []Finding { // unenforcedNamespaces returns the namespaces in a cluster's security payload that enforce // nothing, split into those with no enforce label and those that enforce privileged. The // remediation labels only the first, since a namespace set to privileged was set on purpose. +// An unobserved payload returns nothing. func unenforcedNamespaces(r *Report, cluster string) (unlabeled, privileged []string) { - raw, _ := clusterData(r, "security", cluster)["namespaces"].([]any) + m, obs := clusterView(r, "security", cluster) + if !obs.seen() { + return nil, nil + } + raw, _ := m["namespaces"].([]any) for _, item := range raw { entry, ok := item.(map[string]any) if !ok { @@ -86,8 +93,8 @@ func networkFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - without := getIntField(r, "network-policies", cluster, "namespaces_without_policies") - if without > 0 { + without, withoutObs := observeInt(r, "network-policies", cluster, "namespaces_without_policies") + if withoutObs == observed && without > 0 { findings = append(findings, Finding{ Rule: "network-policies/no-policy", Title: fmt.Sprintf("%s has %d namespace(s) with no network policies", cluster, without), @@ -130,19 +137,19 @@ func workloadSecFindings(r *Report) []Finding { var findings []Finding for _, cluster := range r.Clusters { risks := highRiskPods(r, cluster) - privCount := getIntField(r, "workload-security", cluster, "privileged_containers") - hostNet := getIntField(r, "workload-security", cluster, "host_network_pods") - hostPID := getIntField(r, "workload-security", cluster, "host_pid_pods") - rootCount := getIntField(r, "workload-security", cluster, "run_as_root_containers") - capAdd := getIntField(r, "workload-security", cluster, "capability_additions") - total := getIntField(r, "workload-security", cluster, "total_pods") + privCount, privObs := observeInt(r, "workload-security", cluster, "privileged_containers") + hostNet, hostNetObs := observeInt(r, "workload-security", cluster, "host_network_pods") + hostPID, hostPIDObs := observeInt(r, "workload-security", cluster, "host_pid_pods") + rootCount, rootObs := observeInt(r, "workload-security", cluster, "run_as_root_containers") + capAdd, capObs := observeInt(r, "workload-security", cluster, "capability_additions") + total, totalObs := observeInt(r, "workload-security", cluster, "total_pods") - if privCount > 0 { + if privObs.seen() && privCount > 0 { affected := filterRiskNames(risks, "privileged") findings = append(findings, Finding{ Rule: "workload-security/privileged", Title: fmt.Sprintf("%s has %d privileged container(s)", cluster, privCount), - Description: fmt.Sprintf("Privileged containers have full host access. Review whether these workloads genuinely require it. Total pods: %d.", total), + Description: fmt.Sprintf("Privileged containers have full host access. Review whether these workloads genuinely require it. Total pods: %d.", total) + privObs.note(), Severity: SeverityCritical, Cluster: cluster, Scanner: "workload-security", @@ -152,19 +159,19 @@ func workloadSecFindings(r *Report) []Finding { }, }) } - if hostNet > 0 || hostPID > 0 { + if hostNetObs.seen() && hostPIDObs.seen() && (hostNet > 0 || hostPID > 0) { affected := append(filterRiskNames(risks, "host-network"), filterRiskNames(risks, "host-pid")...) findings = append(findings, Finding{ Rule: "workload-security/host-namespaces", Title: fmt.Sprintf("%s has pods using host namespaces (network: %d, PID: %d)", cluster, hostNet, hostPID), - Description: "Pods sharing the host network or PID namespace can see and interact with other processes and traffic on the node.", + Description: "Pods sharing the host network or PID namespace can see and interact with other processes and traffic on the node." + weakest(hostNetObs, hostPIDObs).note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "workload-security", Affected: dedup(affected), }) } - if rootCount > 0 && total > 0 { + if rootObs.seen() && totalObs.seen() && rootCount > 0 && total > 0 { pct := float64(rootCount) / float64(total) * 100 sev := SeverityInfo if pct > 50 { @@ -173,18 +180,18 @@ func workloadSecFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "workload-security/run-as-root", Title: fmt.Sprintf("%s has %d container(s) explicitly running as root (%.0f%%)", cluster, rootCount, pct), - Description: "These containers have an effective runAsUser of 0, set on the container or inherited from the pod. Container escape vulnerabilities have higher impact when the workload is root.", + Description: "These containers have an effective runAsUser of 0, set on the container or inherited from the pod. Container escape vulnerabilities have higher impact when the workload is root." + weakest(rootObs, totalObs).note(), Severity: sev, Cluster: cluster, Scanner: "workload-security", }) } - if capAdd > 0 { + if capObs.seen() && capAdd > 0 { affected := filterRiskNames(risks, "capabilities-add") findings = append(findings, Finding{ Rule: "workload-security/capability-add", Title: fmt.Sprintf("%s has %d container(s) with added Linux capabilities", cluster, capAdd), - Description: "Containers requesting capabilities such as NET_ADMIN or SYS_PTRACE have elevated kernel access beyond the default set.", + Description: "Containers requesting capabilities such as NET_ADMIN or SYS_PTRACE have elevated kernel access beyond the default set." + capObs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "workload-security", @@ -208,8 +215,8 @@ type highRiskPod struct { // highRiskPods extracts the HighRiskPods slice from a cluster's workloadsec // data so finding messages can name specific offenders. func highRiskPods(r *Report, cluster string) []highRiskPod { - m := clusterData(r, "workload-security", cluster) - if m == nil { + m, obs := clusterView(r, "workload-security", cluster) + if !obs.seen() { return nil } raw, _ := m["high_risk_pods"].([]any) @@ -278,16 +285,16 @@ func rbacAuditFindings(r *Report) []Finding { var findings []Finding for _, cluster := range r.Clusters { risk := riskBindings(r, cluster) - adminBindings := getIntField(r, "rbac-audit", cluster, "cluster_admin_bindings") - wildcardRules := getIntField(r, "rbac-audit", cluster, "wildcard_rules") - defaultSA := getIntField(r, "rbac-audit", cluster, "default_sa_bindings") + adminBindings, adminObs := observeInt(r, "rbac-audit", cluster, "cluster_admin_bindings") + wildcardRules, wildcardObs := observeInt(r, "rbac-audit", cluster, "wildcard_rules") + defaultSA, defaultObs := observeInt(r, "rbac-audit", cluster, "default_sa_bindings") - if adminBindings > 0 { + if adminObs.seen() && adminBindings > 0 { affected := filterBindingsByRisk(risk, "cluster-admin-to-non-system") findings = append(findings, Finding{ Rule: "rbac-audit/cluster-admin-binding", Title: fmt.Sprintf("%s has %d non-system cluster-admin binding(s)", cluster, adminBindings), - Description: "ClusterRoleBindings granting cluster-admin to non-system principals give unrestricted access to every resource in the cluster.", + Description: "ClusterRoleBindings granting cluster-admin to non-system principals give unrestricted access to every resource in the cluster." + adminObs.note(), Severity: SeverityCritical, Cluster: cluster, Scanner: "rbac-audit", @@ -297,22 +304,22 @@ func rbacAuditFindings(r *Report) []Finding { }, }) } - if wildcardRules > 0 { + if wildcardObs.seen() && wildcardRules > 0 { findings = append(findings, Finding{ Rule: "rbac-audit/wildcard-rule", Title: fmt.Sprintf("%s has %d RBAC rule(s) with wildcard permissions", cluster, wildcardRules), - Description: "Rules using '*' for verbs, resources, or apiGroups grant broader access than most workloads need.", + Description: "Rules using '*' for verbs, resources, or apiGroups grant broader access than most workloads need." + wildcardObs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "rbac-audit", }) } - if defaultSA > 0 { + if defaultObs.seen() && defaultSA > 0 { affected := filterBindingsByRisk(risk, "grants-to-default-sa") findings = append(findings, Finding{ Rule: "rbac-audit/default-sa-binding", Title: fmt.Sprintf("%s has %d binding(s) granting permissions to a default service account", cluster, defaultSA), - Description: "The default service account is used by any pod that does not specify a different one. Granting it permissions means all unattributed workloads inherit them.", + Description: "The default service account is used by any pod that does not specify a different one. Granting it permissions means all unattributed workloads inherit them." + defaultObs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "rbac-audit", @@ -337,8 +344,8 @@ type riskBinding struct { // riskBindings extracts the rbac-audit scanner's risk_bindings slice. func riskBindings(r *Report, cluster string) []riskBinding { - m := clusterData(r, "rbac-audit", cluster) - if m == nil { + m, obs := clusterView(r, "rbac-audit", cluster) + if !obs.seen() { return nil } raw, _ := m["risk_bindings"].([]any) @@ -385,8 +392,8 @@ func certFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - m := clusterData(r, "certs", cluster) - if m == nil { + m, obs := clusterView(r, "certs", cluster) + if !obs.seen() { continue } raw, _ := m["certs"].([]any) @@ -420,7 +427,7 @@ func certFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "certs/expiring-7d", Title: fmt.Sprintf("%s has %d certificate(s) expiring in fewer than 7 days", cluster, len(critical)), - Description: "These TLS certificates will expire imminently. Renew them before they break webhooks, ingress, or metrics endpoints.", + Description: "These TLS certificates will expire imminently. Renew them before they break webhooks, ingress, or metrics endpoints." + obs.note(), Severity: SeverityCritical, Cluster: cluster, Scanner: "certs", @@ -434,7 +441,7 @@ func certFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "certs/expiring-30d", Title: fmt.Sprintf("%s has %d certificate(s) expiring within 30 days", cluster, len(warning)), - Description: "Schedule renewals now to avoid expiry-driven outages.", + Description: "Schedule renewals now to avoid expiry-driven outages." + obs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "certs", @@ -506,8 +513,8 @@ func admissionFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - m := clusterData(r, "admission", cluster) - if m == nil { + m, obs := clusterView(r, "admission", cluster) + if !obs.seen() { continue } raw, _ := m["webhooks"].([]any) @@ -545,7 +552,7 @@ func admissionFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "admission/no-healthy-endpoints", Title: fmt.Sprintf("%s has %d admission webhook(s) with no healthy endpoints", cluster, len(unhealthy)), - Description: "When failurePolicy=Fail, every API request matching these webhooks will be rejected. Even with failurePolicy=Ignore, mutations and validations are silently skipped.", + Description: "When failurePolicy=Fail, every API request matching these webhooks will be rejected. Even with failurePolicy=Ignore, mutations and validations are silently skipped." + obs.note(), Severity: SeverityCritical, Cluster: cluster, Scanner: "admission", @@ -557,7 +564,7 @@ func admissionFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "admission/ca-expiring", Title: fmt.Sprintf("%s has %d admission webhook(s) with CA bundles expiring soon", cluster, len(expiring)), - Description: "When a webhook CA bundle expires, the apiserver refuses to talk to the webhook. Renew or rotate before the listed deadlines.", + Description: "When a webhook CA bundle expires, the apiserver refuses to talk to the webhook. Renew or rotate before the listed deadlines." + obs.note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "admission", diff --git a/internal/report/findings_workloads.go b/internal/report/findings_workloads.go index 944e69b..d12601a 100644 --- a/internal/report/findings_workloads.go +++ b/internal/report/findings_workloads.go @@ -17,23 +17,24 @@ func imageAuditFindings(r *Report) []Finding { var findings []Finding for _, cluster := range r.Clusters { risks := imageRisks(r, cluster) - latestTag := getIntField(r, "image-audit", cluster, "latest_tag") - noDigest := getIntField(r, "image-audit", cluster, "no_digest") - total := getIntField(r, "image-audit", cluster, "total_containers") + latestTag, latestObs := observeInt(r, "image-audit", cluster, "latest_tag") + noDigest, noDigestObs := observeInt(r, "image-audit", cluster, "no_digest") + total, totalObs := observeInt(r, "image-audit", cluster, "total_containers") - if latestTag > 0 { + if latestObs.seen() && latestTag > 0 { affected := filterImagesByRisk(risks, "latest-tag") findings = append(findings, Finding{ Rule: "image-audit/latest-tag", Title: fmt.Sprintf("%s has %d container(s) using :latest or no tag", cluster, latestTag), - Description: fmt.Sprintf("Out of %d containers, %d use :latest or omit a tag. Without a pinned tag, rollbacks and audits are unreliable.", total, latestTag), + Description: fmt.Sprintf("Out of %d containers, %d use :latest or omit a tag. Without a pinned tag, rollbacks and audits are unreliable.", total, latestTag) + weakest(latestObs, totalObs).note(), Severity: SeverityWarning, Cluster: cluster, Scanner: "image-audit", Affected: affected, }) } - if noDigest > 0 && total > 0 { + digestObs := weakest(noDigestObs, totalObs) + if digestObs.seen() && noDigest > 0 && total > 0 { pct := float64(noDigest) / float64(total) * 100 sev := SeverityInfo if pct > 80 { @@ -42,7 +43,7 @@ func imageAuditFindings(r *Report) []Finding { findings = append(findings, Finding{ Rule: "image-audit/no-digest", Title: fmt.Sprintf("%s has %d container(s) without digest pins (%.0f%%)", cluster, noDigest, pct), - Description: "Images without @sha256: digest pins can change content without changing the tag. Pinning digests ensures reproducible deployments.", + Description: "Images without @sha256: digest pins can change content without changing the tag. Pinning digests ensures reproducible deployments." + digestObs.note(), Severity: sev, Cluster: cluster, Scanner: "image-audit", @@ -64,8 +65,8 @@ type imageRisk struct { // imageRisks extracts the image-audit scanner's image_risks slice. func imageRisks(r *Report, cluster string) []imageRisk { - m := clusterData(r, "image-audit", cluster) - if m == nil { + m, obs := clusterView(r, "image-audit", cluster) + if !obs.seen() { return nil } raw, _ := m["image_risks"].([]any) @@ -114,8 +115,8 @@ func coverageFindings(r *Report) []Finding { } var findings []Finding for _, cluster := range r.Clusters { - m := clusterData(r, "workload-coverage", cluster) - if m == nil { + m, obs := clusterView(r, "workload-coverage", cluster) + if obs != observed { continue } raw, _ := m["gaps"].([]any) @@ -261,8 +262,8 @@ func namespacesWithImageRisk(r *Report, cluster string) map[string]struct{} { // include any of the supplied reasons. func namespacesWithEventReasons(r *Report, cluster string, reasons map[string]bool) map[string]struct{} { out := make(map[string]struct{}) - m := clusterData(r, "events", cluster) - if m == nil { + m, obs := clusterView(r, "events", cluster) + if !obs.seen() { return out } rec, _ := m["recent_warnings"].([]any) diff --git a/internal/report/observe_generators_test.go b/internal/report/observe_generators_test.go new file mode 100644 index 0000000..1206bbb --- /dev/null +++ b/internal/report/observe_generators_test.go @@ -0,0 +1,182 @@ +package report + +import ( + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/dcadolph/fleetsweeper/internal/scanner" +) + +// TestGeneratorCoverage checks each converted generator against an errored, degraded, and +// completed run. Absence rules stay silent unless the run completed. Presence rules fire on a +// degraded run and mark the description partial. +func TestGeneratorCoverage(t *testing.T) { + t.Parallel() + tests := []struct { + Gen func(*Report) []Finding + Data map[string]any + Scanner string + Rule string + Absence bool + }{{ // Test 0: Unenforced namespaces are a presence rule. + Gen: securityFindings, Scanner: "security", Rule: "security/psa-unenforced", + Data: map[string]any{"unenforced_count": 2, "namespace_count": 5}, + }, { // Test 1: Every namespace unenforced is an absence claim. + Gen: securityFindings, Scanner: "security", Rule: "security/psa-unenforced", Absence: true, + Data: map[string]any{"unenforced_count": 5, "namespace_count": 5}, + }, { // Test 2: Namespaces without network policies is an absence rule. + Gen: networkFindings, Scanner: "network-policies", Rule: "network-policies/no-policy", + Absence: true, Data: map[string]any{"namespaces_without_policies": 2}, + }, { // Test 3: Privileged containers are a presence rule. + Gen: workloadSecFindings, Scanner: "workload-security", Rule: "workload-security/privileged", + Data: map[string]any{"privileged_containers": 1, "total_pods": 4}, + }, { // Test 4: Host namespaces are a presence rule. + Gen: workloadSecFindings, Scanner: "workload-security", + Rule: "workload-security/host-namespaces", + Data: map[string]any{"host_network_pods": 1, "host_pid_pods": 0, "total_pods": 4}, + }, { // Test 5: Root containers are a presence rule. + Gen: workloadSecFindings, Scanner: "workload-security", Rule: "workload-security/run-as-root", + Data: map[string]any{"run_as_root_containers": 1, "total_pods": 4}, + }, { // Test 6: Added capabilities are a presence rule. + Gen: workloadSecFindings, Scanner: "workload-security", + Rule: "workload-security/capability-add", + Data: map[string]any{"capability_additions": 1, "total_pods": 4}, + }, { // Test 7: Cluster-admin bindings are a presence rule. + Gen: rbacAuditFindings, Scanner: "rbac-audit", Rule: "rbac-audit/cluster-admin-binding", + Data: map[string]any{"cluster_admin_bindings": 1}, + }, { // Test 8: Wildcard rules are a presence rule. + Gen: rbacAuditFindings, Scanner: "rbac-audit", Rule: "rbac-audit/wildcard-rule", + Data: map[string]any{"wildcard_rules": 1}, + }, { // Test 9: Default service account bindings are a presence rule. + Gen: rbacAuditFindings, Scanner: "rbac-audit", Rule: "rbac-audit/default-sa-binding", + Data: map[string]any{"default_sa_bindings": 1}, + }, { // Test 10: Expiring certificates are a presence rule. + Gen: certFindings, Scanner: "certs", Rule: "certs/expiring-7d", + Data: map[string]any{"certs": []any{map[string]any{"name": "a", "days_remaining": 3}}}, + }, { // Test 11: Unhealthy webhooks are a presence rule. + Gen: admissionFindings, Scanner: "admission", Rule: "admission/no-healthy-endpoints", + Data: map[string]any{"webhooks": []any{map[string]any{ + "configuration": "c", "webhook": "w", "service": "ns/svc", "ca_bundle_days_remaining": 90, + }}}, + }, { // Test 12: Latest-tag images are a presence rule. + Gen: imageAuditFindings, Scanner: "image-audit", Rule: "image-audit/latest-tag", + Data: map[string]any{"latest_tag": 2, "total_containers": 10}, + }, { // Test 13: Images without digests are a presence rule. + Gen: imageAuditFindings, Scanner: "image-audit", Rule: "image-audit/no-digest", + Data: map[string]any{"no_digest": 2, "total_containers": 10}, + }, { // Test 14: A missing PDB is an absence rule. + Gen: coverageFindings, Scanner: "workload-coverage", Rule: "workload-coverage/missing-pdb", + Absence: true, Data: map[string]any{"gaps": []any{map[string]any{ + "namespace": "ns", "name": "app", "kind": "Deployment", "has_pdb": false, "has_hpa": true, + }}}, + }, { // Test 15: A missing HPA is an absence rule. + Gen: coverageFindings, Scanner: "workload-coverage", Rule: "workload-coverage/missing-hpa", + Absence: true, Data: map[string]any{"gaps": []any{map[string]any{ + "namespace": "ns", "name": "app", "kind": "Deployment", "has_pdb": true, "has_hpa": false, + }}}, + }, { // Test 16: Deprecated APIs are a presence rule. + Gen: deprecatedAPIFindings, Scanner: "deprecated-apis", Rule: "deprecated-apis/in-use", + Data: map[string]any{"deprecated": []any{map[string]any{"api_version": "v1beta1", "kind": "X"}}}, + }, { // Test 17: Node platform drift is a presence rule. + Gen: clusterInfoFindings, Scanner: "cluster-info", Rule: "cluster-info/node-platform-drift", + Data: map[string]any{"has_drift": true}, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + runs := map[string]scanner.Result{ + "errored": erroredRun(test.Scanner), + "degraded": degradedRun(test.Scanner, test.Data), + "ok": okRun(test.Scanner, test.Data), + } + wantCount := map[string]int{"errored": 0, "degraded": 1, "ok": 1} + if test.Absence { + wantCount["degraded"] = 0 + } + for name, run := range runs { + r := Build([]string{"c1"}, map[string]map[string]scanner.Result{ + "c1": {test.Scanner: run}, + }) + got := findingsWithRule(test.Gen(r), test.Rule) + if diff := cmp.Diff(wantCount[name], len(got)); diff != "" { + t.Fatalf("%s finding count mismatch (-want +got):\n%s", name, diff) + } + if len(got) == 0 { + continue + } + gotPartial := strings.HasSuffix(got[0].Description, partialScanNote) + if diff := cmp.Diff(name == "degraded", gotPartial); diff != "" { + t.Errorf("%s partial note mismatch (-want +got):\n%s", name, diff) + } + } + }) + } +} + +// TestVersionFindingsCoverage leaves clusters without an observed version out of the skew set. +func TestVersionFindingsCoverage(t *testing.T) { + t.Parallel() + ver := func(v string) scanner.Result { + return okRun("version", map[string]any{"git_version": v}) + } + tests := []struct { + Third scanner.Result + WantAffected []string + WantCount int + }{{ // Test 0: An errored third cluster is not a distinct version. + Third: erroredRun("version"), WantCount: 1, WantAffected: []string{"c1=v1.28.0", "c2=v1.30.0"}, + }, { // Test 1: A degraded third cluster still read its version, so it joins the skew set. + Third: degradedRun("version", map[string]any{"git_version": "v1.20.0"}), + WantCount: 1, WantAffected: []string{"c1=v1.28.0", "c2=v1.30.0", "c3=v1.20.0"}, + }, { // Test 2: A completed third cluster joins the skew set. + Third: ver("v1.20.0"), WantCount: 1, + WantAffected: []string{"c1=v1.28.0", "c2=v1.30.0", "c3=v1.20.0"}, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + r := Build([]string{"c1", "c2", "c3"}, map[string]map[string]scanner.Result{ + "c1": {"version": ver("v1.28.0")}, + "c2": {"version": ver("v1.30.0")}, + "c3": {"version": test.Third}, + }) + got := versionFindings(r) + if diff := cmp.Diff(test.WantCount, len(got)); diff != "" { + t.Fatalf("finding count mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantAffected, got[0].Affected); diff != "" { + t.Errorf("affected mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestNamespaceFindingsCoverage leaves unobserved clusters out of the namespace comparison. +func TestNamespaceFindingsCoverage(t *testing.T) { + t.Parallel() + ns := func(n int) scanner.Result { return okRun("namespaces", map[string]any{"count": n}) } + tests := []struct { + Third scanner.Result + WantCount int + }{{ // Test 0: An errored cluster is not compared as zero namespaces. + Third: erroredRun("namespaces"), WantCount: 0, + }, { // Test 1: A completed cluster with zero namespaces is compared. + Third: ns(0), WantCount: 1, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + r := Build([]string{"c1", "c2", "c3"}, map[string]map[string]scanner.Result{ + "c1": {"namespaces": ns(5)}, + "c2": {"namespaces": ns(6)}, + "c3": {"namespaces": test.Third}, + }) + if diff := cmp.Diff(test.WantCount, len(namespaceFindings(r))); diff != "" { + t.Errorf("finding count mismatch (-want +got):\n%s", diff) + } + }) + } +} From 81732393209ca55cb5668afc2005164b12df09f3 Mon Sep 17 00:00:00 2001 From: Douglas Adolph <116844282+dcadolph@users.noreply.github.com> Date: Sat, 31 Oct 2026 06:21:09 -0500 Subject: [PATCH 03/13] Report unknown capacity instead of inventing headroom --- client/types.go | 16 +- internal/report/capacity.go | 156 +++++++++++++------- internal/report/capacity_test.go | 118 +++++++++++---- internal/report/findings.go | 28 ---- internal/report/findings_health.go | 4 +- internal/report/findings_test.go | 3 +- internal/scanner/metrics/metrics.go | 78 +++++----- internal/scanner/metrics/metrics_test.go | 58 ++++++-- internal/server/demo_results.go | 15 +- internal/server/openapi.yaml | 4 +- internal/server/static/js/views/capacity.js | 19 ++- internal/server/static/js/views/clusters.js | 4 +- internal/server/static/js/views/detail.js | 8 +- 13 files changed, 334 insertions(+), 177 deletions(-) diff --git a/client/types.go b/client/types.go index 4d8df7a..fb63d00 100644 --- a/client/types.go +++ b/client/types.go @@ -240,18 +240,18 @@ type CapacityAnalysis struct { Cluster string `json:"cluster,omitempty"` // Status summarizes capacity pressure. Status string `json:"status,omitempty"` - // CPUUtilization is the current CPU utilization. - CPUUtilization float64 `json:"cpu_utilization,omitempty"` - // MemoryUtilization is the current memory utilization. - MemoryUtilization float64 `json:"memory_utilization,omitempty"` + // CPUUtilization is the current CPU utilization, nil when no scanner observed it. + CPUUtilization *float64 `json:"cpu_utilization,omitempty"` + // MemoryUtilization is the current memory utilization, nil when no scanner observed it. + MemoryUtilization *float64 `json:"memory_utilization,omitempty"` // NodeCount is the number of nodes. NodeCount int `json:"node_count,omitempty"` // HealthyNodes is the number of ready nodes. HealthyNodes int `json:"healthy_nodes,omitempty"` - // HeadroomCPU is the remaining CPU headroom. - HeadroomCPU float64 `json:"headroom_cpu,omitempty"` - // HeadroomMemory is the remaining memory headroom. - HeadroomMemory float64 `json:"headroom_memory,omitempty"` + // HeadroomCPU is the remaining CPU headroom, nil when utilization was not observed. + HeadroomCPU *float64 `json:"headroom_cpu,omitempty"` + // HeadroomMemory is the remaining memory headroom, nil when utilization was not observed. + HeadroomMemory *float64 `json:"headroom_memory,omitempty"` // HasMemoryPressure reports memory pressure on the cluster. HasMemoryPressure bool `json:"has_memory_pressure,omitempty"` // HasDiskPressure reports disk pressure on the cluster. diff --git a/internal/report/capacity.go b/internal/report/capacity.go index 8535538..fad756d 100644 --- a/internal/report/capacity.go +++ b/internal/report/capacity.go @@ -5,6 +5,7 @@ import ( "math" "sort" + "github.com/dcadolph/fleetsweeper/internal/scanner" "github.com/dcadolph/fleetsweeper/internal/util" ) @@ -14,6 +15,9 @@ const ( capacityStrained = "strained" capacityBusy = "busy" capacityHealthy = "healthy" + // capacityUnknown marks a cluster whose utilization no scanner observed and whose other + // signals show no strain, so neither healthy nor busy can be claimed. + capacityUnknown = "unknown" ) // CapacityAnalysis holds smart analysis for a single cluster, correlating @@ -21,10 +25,10 @@ const ( type CapacityAnalysis struct { // Cluster is the cluster name. Cluster string `json:"cluster"` - // CPUUtilization is the average CPU percentage. - CPUUtilization float64 `json:"cpu_utilization"` - // MemoryUtilization is the average memory percentage. - MemoryUtilization float64 `json:"memory_utilization"` + // CPUUtilization is the average CPU percentage, nil when no scanner observed it. + CPUUtilization *float64 `json:"cpu_utilization,omitempty"` + // MemoryUtilization is the average memory percentage, nil when no scanner observed it. + MemoryUtilization *float64 `json:"memory_utilization,omitempty"` // HasMemoryPressure is true when any node reports MemoryPressure=True. HasMemoryPressure bool `json:"has_memory_pressure"` // HasDiskPressure is true when any node reports DiskPressure=True. @@ -37,11 +41,11 @@ type CapacityAnalysis struct { NodeCount int `json:"node_count"` // HealthyNodes is the number of healthy nodes. HealthyNodes int `json:"healthy_nodes"` - // HeadroomCPU is the percentage of CPU capacity still available. - HeadroomCPU float64 `json:"headroom_cpu"` - // HeadroomMemory is the percentage of memory capacity still available. - HeadroomMemory float64 `json:"headroom_memory"` - // Status is the assessed state: healthy, busy, strained, critical. + // HeadroomCPU is the percentage of CPU capacity still available, nil when unobserved. + HeadroomCPU *float64 `json:"headroom_cpu,omitempty"` + // HeadroomMemory is the percentage of memory capacity still available, nil when unobserved. + HeadroomMemory *float64 `json:"headroom_memory,omitempty"` + // Status is the assessed state: healthy, busy, strained, critical, or unknown. Status string `json:"status"` // Recommendation is a plain English capacity recommendation. Recommendation string `json:"recommendation,omitempty"` @@ -53,15 +57,15 @@ type CapacityAnalysis struct { // clusterSignals holds extracted signals for capacity analysis. type clusterSignals struct { - // hasMetrics is true when the metrics scanner read the cluster, so the - // utilization fields are measurements rather than zero for unknown. - hasMetrics bool + // utilObs is how far cpuPct and memPct can be trusted. Unobserved means both are zero + // placeholders that must never be read as a measurement. + utilObs observation + // utilGap names the read that left utilization unobserved, empty when it was observed. + utilGap string // cpuPct is the average CPU utilization percent. cpuPct float64 // memPct is the average memory utilization percent. memPct float64 - // maxMemPct is the highest node memory utilization percent. - maxMemPct float64 // nodeCount is the number of nodes. nodeCount int // healthyNodes is the number of Ready nodes without pressure. @@ -76,8 +80,6 @@ type clusterSignals struct { oomEvents bool // schedFail is true when recent warnings include FailedScheduling. schedFail bool - // warnEvents is the warning event count. - warnEvents int } // analyzeCapacity runs smart capacity analysis across all clusters in a report. @@ -104,20 +106,22 @@ func analyzeCapacity(r *Report, groups map[string][]string) []CapacityAnalysis { ca := CapacityAnalysis{ Cluster: cluster, - CPUUtilization: sig.cpuPct, - MemoryUtilization: sig.memPct, HasMemoryPressure: sig.memPressure > 0, HasDiskPressure: sig.diskPressure > 0, HasOOMEvents: sig.oomEvents, HasSchedulingFailures: sig.schedFail, NodeCount: sig.nodeCount, HealthyNodes: sig.healthyNodes, - HeadroomCPU: 100 - sig.cpuPct, - HeadroomMemory: 100 - sig.memPct, + } + if sig.utilObs.seen() { + ca.CPUUtilization = new(sig.cpuPct) + ca.MemoryUtilization = new(sig.memPct) + ca.HeadroomCPU = new(100 - sig.cpuPct) + ca.HeadroomMemory = new(100 - sig.memPct) } ca.Status = assessStatus(sig) - ca.Recommendation = recommend(sig, ca.Status) + ca.Recommendation = recommend(sig, ca.Status) + sig.utilObs.note() ca.NodesNeededForTarget = nodesNeeded(sig, 70.0) // Compare to group peers. @@ -138,7 +142,8 @@ func analyzeCapacity(r *Report, groups map[string][]string) []CapacityAnalysis { sort.Slice(analyzes, func(i, j int) bool { order := map[string]int{ - capacityCritical: 0, capacityStrained: 1, capacityBusy: 2, capacityHealthy: 3, + capacityCritical: 0, capacityStrained: 1, capacityBusy: 2, capacityUnknown: 3, + capacityHealthy: 4, } return order[analyzes[i].Status] < order[analyzes[j].Status] }) @@ -146,13 +151,16 @@ func analyzeCapacity(r *Report, groups map[string][]string) []CapacityAnalysis { return analyzes } -// assessStatus determines cluster state by correlating signals, not just -// reading utilization numbers. +// assessStatus determines cluster state by correlating signals, not just reading utilization +// numbers. Pressure and failure events still mark a cluster strained or critical without a +// utilization reading, but healthy and busy rest on utilization, so without one the status is +// unknown. func assessStatus(sig clusterSignals) string { + seen := sig.utilObs.seen() hasPressure := sig.memPressure > 0 || sig.diskPressure > 0 hasFailures := sig.oomEvents || sig.schedFail - highUtil := sig.memPct > 85 || sig.cpuPct > 85 - elevatedUtil := sig.memPct > 70 || sig.cpuPct > 70 + highUtil := seen && (sig.memPct > 85 || sig.cpuPct > 85) + elevatedUtil := seen && (sig.memPct > 70 || sig.cpuPct > 70) // Critical: pressure + failures, or nodes down + high utilization. if hasPressure && hasFailures { @@ -177,13 +185,18 @@ func assessStatus(sig clusterSignals) string { if elevatedUtil { return capacityBusy } - + if !seen { + return capacityUnknown + } return capacityHealthy } -// recommend generates a capacity recommendation based on correlated signals. +// recommend generates a capacity recommendation based on correlated signals. It states a +// utilization figure only when one was observed. func recommend(sig clusterSignals, status string) string { switch status { + case capacityUnknown: + return "Utilization is unknown: " + sig.utilGap case capacityCritical: needed := nodesNeeded(sig, 70.0) if sig.oomEvents && sig.memPressure > 0 && needed == 0 { @@ -209,6 +222,10 @@ func recommend(sig clusterSignals, status string) string { "%d of %d nodes report MemoryPressure. Add %d node(s) to relieve pressure or evict low-priority workloads.", sig.memPressure, sig.nodeCount, needed) } + if sig.notReady > 0 && !sig.utilObs.seen() { + return fmt.Sprintf( + "%d node(s) are not Ready. Investigate node conditions before scaling.", sig.notReady) + } if sig.notReady > 0 { return fmt.Sprintf( "%d node(s) are not Ready. Investigate node conditions before scaling. Remaining nodes are overloaded at %.0f%% memory.", @@ -232,6 +249,9 @@ func recommend(sig clusterSignals, status string) string { "FailedScheduling events indicate insufficient capacity. Add %d node(s) or reduce workload resource requests.", needed) } + if !sig.utilObs.seen() { + return "Nodes report pressure or failure events, but utilization is unknown: " + sig.utilGap + } if sig.memPct > 85 { return fmt.Sprintf( "Memory at %.0f%% with no pressure yet, but little headroom remains. Consider adding %d node(s) proactively.", @@ -258,11 +278,19 @@ func recommend(sig clusterSignals, status string) string { } } -// memClause returns ", while average memory is N%" for a cluster with a -// memory reading, or nothing when metrics were unavailable and the reading -// is zero. +// pctText formats a utilization percentage as a whole number, or "unknown" when it was not +// observed. +func pctText(p *float64) string { + if p == nil { + return "unknown" + } + return fmt.Sprintf("%.0f%%", *p) +} + +// memClause returns " while average memory is N%" for a cluster with an observed memory +// reading, or nothing when utilization was not observed. func memClause(sig clusterSignals) string { - if sig.memPct <= 0 { + if !sig.utilObs.seen() { return "" } return fmt.Sprintf(" while average memory is %.0f%%", sig.memPct) @@ -270,9 +298,10 @@ func memClause(sig clusterSignals) string { // nodesNeeded calculates how many additional nodes are required to bring // memory utilization below the target percentage, assuming each new node -// adds the same capacity as existing nodes. +// adds the same capacity as existing nodes. Without an observed utilization it +// returns 0, since no count can be derived from an unknown. func nodesNeeded(sig clusterSignals, targetPct float64) int { - if sig.nodeCount == 0 || sig.memPct <= targetPct { + if !sig.utilObs.seen() || sig.nodeCount == 0 || sig.memPct <= targetPct { return 0 } // Total memory units currently used = memPct/100 * nodeCount. @@ -289,7 +318,7 @@ func nodesNeeded(sig clusterSignals, targetPct float64) int { // without one is left out, so an unknown never counts as zero utilization. func groupComparison(cluster, groupName string, peers []string, signals map[string]clusterSignals) string { sig := signals[cluster] - if !sig.hasMetrics { + if !sig.utilObs.seen() { return "" } var peerMem []float64 @@ -298,7 +327,7 @@ func groupComparison(cluster, groupName string, peers []string, signals map[stri if p == cluster { continue } - if s, ok := signals[p]; ok && s.hasMetrics { + if s, ok := signals[p]; ok && s.utilObs.seen() { peerMem = append(peerMem, s.memPct) peerCPU = append(peerCPU, s.cpuPct) } @@ -336,37 +365,56 @@ func groupComparison(cluster, groupName string, peers []string, signals map[stri return parts } -// extractAllSignals pulls capacity-relevant data from the report for all clusters. +// extractAllSignals pulls capacity-relevant data from the report for all clusters. Every +// cluster gets an entry. Utilization carries its observation, and a cluster without one names +// the read that failed. Node and event counts are presence signals, so an unobserved count +// stands as no evidence of pressure. func extractAllSignals(r *Report) map[string]clusterSignals { out := make(map[string]clusterSignals, len(r.Clusters)) for _, cluster := range r.Clusters { var sig clusterSignals - if sec := r.Sections["metrics"]; sec != nil { - _, sig.hasMetrics = sec.PerCluster[cluster] + var cpuObs, memObs observation + sig.cpuPct, cpuObs = observe(r, "metrics", cluster, "avg_cpu_percent") + sig.memPct, memObs = observe(r, "metrics", cluster, "avg_memory_percent") + sig.utilObs = weakest(cpuObs, memObs) + if !sig.utilObs.seen() { + sig.cpuPct, sig.memPct = 0, 0 + sig.utilGap = utilizationGap(r, cluster) } - sig.cpuPct = getMetricsFloat(r, cluster, "avg_cpu_percent") - sig.memPct = getMetricsFloat(r, cluster, "avg_memory_percent") - sig.maxMemPct = getMetricsFloat(r, cluster, "max_memory_percent") - sig.nodeCount = getIntField(r, "node-health", cluster, "node_count") - sig.healthyNodes = getIntField(r, "node-health", cluster, "healthy_nodes") - sig.memPressure = getIntField(r, "node-health", cluster, "memory_pressure_nodes") - sig.diskPressure = getIntField(r, "node-health", cluster, "disk_pressure_nodes") - sig.notReady = getIntField(r, "node-health", cluster, "not_ready_nodes") - sig.warnEvents = getIntField(r, "events", cluster, "warning_events") - - // Check for OOM and scheduling events in the event data. + sig.nodeCount, _ = observeInt(r, "node-health", cluster, "node_count") + sig.healthyNodes, _ = observeInt(r, "node-health", cluster, "healthy_nodes") + sig.memPressure, _ = observeInt(r, "node-health", cluster, "memory_pressure_nodes") + sig.diskPressure, _ = observeInt(r, "node-health", cluster, "disk_pressure_nodes") + sig.notReady, _ = observeInt(r, "node-health", cluster, "not_ready_nodes") sig.oomEvents = hasEventReason(r, cluster, "OOMKilling") sig.schedFail = hasEventReason(r, cluster, "FailedScheduling") - out[cluster] = sig } return out } -// hasEventReason checks if a cluster has warning events with a specific reason. +// utilizationGap names the metrics read that left a cluster's utilization unobserved. +func utilizationGap(r *Report, cluster string) string { + st, ok := runState(r, "metrics", cluster) + switch { + case !ok: + return "the metrics scanner did not run on this cluster, so headroom cannot be assessed." + case st == scanner.StateErrored: + return "the node metrics read failed, so headroom cannot be assessed. " + + "Check that the scanner can list nodes in metrics.k8s.io." + case st == scanner.StateUnavailable: + return "the cluster does not serve the metrics.k8s.io API. " + + "Install metrics-server to measure headroom." + } + return "no node's usage resolved to a percentage of its allocatable capacity, " + + "so headroom cannot be assessed. Check that the scanner can list nodes." +} + +// hasEventReason reports whether a cluster's warning events include reason. An unobserved +// events run is no evidence either way and reports false. func hasEventReason(r *Report, cluster, reason string) bool { - m := clusterData(r, "events", cluster) - if m == nil { + m, obs := clusterView(r, "events", cluster) + if !obs.seen() { return false } reasons, ok := m["top_warning_reasons"].([]any) diff --git a/internal/report/capacity_test.go b/internal/report/capacity_test.go index 58e7fb3..4495520 100644 --- a/internal/report/capacity_test.go +++ b/internal/report/capacity_test.go @@ -18,23 +18,31 @@ func TestRecommendNodeCount(t *testing.T) { Status string Sig clusterSignals }{{ // Test 0: OOM kills under pressure with memory above target. - Status: "critical", Sig: clusterSignals{nodeCount: 4, memPct: 90, memPressure: 2, oomEvents: true}, WantText: "Add 2 node(s)", + Status: "critical", Sig: clusterSignals{utilObs: observed, nodeCount: 4, memPct: 90, memPressure: 2, oomEvents: true}, WantText: "Add 2 node(s)", }, { // Test 1: OOM kills under pressure with memory below target. - Status: "critical", Sig: clusterSignals{nodeCount: 4, memPct: 40, memPressure: 1, oomEvents: true}, WantText: "load is uneven", + Status: "critical", Sig: clusterSignals{utilObs: observed, nodeCount: 4, memPct: 40, memPressure: 1, oomEvents: true}, WantText: "load is uneven", }, { // Test 2: Memory pressure with memory below target. - Status: "critical", Sig: clusterSignals{nodeCount: 4, memPct: 40, memPressure: 1}, WantText: "while average memory is 40%", + Status: "critical", Sig: clusterSignals{utilObs: observed, nodeCount: 4, memPct: 40, memPressure: 1}, WantText: "while average memory is 40%", }, { // Test 3: Exhaustion with memory below target. - Status: "critical", Sig: clusterSignals{nodeCount: 4, memPct: 40}, WantText: "Check node conditions", + Status: "critical", Sig: clusterSignals{utilObs: observed, nodeCount: 4, memPct: 40}, WantText: "Check node conditions", }, { // Test 4: Exhaustion with memory above target. - Status: "critical", Sig: clusterSignals{nodeCount: 4, memPct: 95}, WantText: "Add 2 node(s)", - }, { // Test 5: Scheduling failures with memory below target. - Status: "strained", Sig: clusterSignals{nodeCount: 1, memPct: 0, schedFail: true}, WantText: "Check pod requests", + Status: "critical", Sig: clusterSignals{utilObs: observed, nodeCount: 4, memPct: 95}, WantText: "Add 2 node(s)", + }, { // Test 5: Scheduling failures with memory unobserved. + Status: "strained", Sig: clusterSignals{nodeCount: 1, schedFail: true}, WantText: "Check pod requests", }, { // Test 6: Scheduling failures with memory above target. - Status: "strained", Sig: clusterSignals{nodeCount: 3, memPct: 80, schedFail: true}, WantText: "Add 1 node(s)", + Status: "strained", Sig: clusterSignals{utilObs: observed, nodeCount: 3, memPct: 80, schedFail: true}, WantText: "Add 1 node(s)", }, { // Test 7: Elevated CPU with memory below target. - Status: "strained", Sig: clusterSignals{nodeCount: 3, cpuPct: 92, memPct: 50}, WantText: "Monitor closely.", + Status: "strained", Sig: clusterSignals{utilObs: observed, nodeCount: 3, cpuPct: 92, memPct: 50}, WantText: "Monitor closely.", }, { // Test 8: No nodes reported. Status: "strained", Sig: clusterSignals{schedFail: true}, WantText: "Check pod requests", + }, { // Test 9: Disk pressure with utilization unobserved names the gap, not a zero. + Status: "strained", Sig: clusterSignals{diskPressure: 1, utilGap: "the gap"}, + WantText: "utilization is unknown: the gap", + }, { // Test 10: Critical with nodes down and utilization unobserved prints no load. + Status: "critical", Sig: clusterSignals{nodeCount: 3, notReady: 1, diskPressure: 1, oomEvents: true}, + WantText: "1 node(s) are not Ready. Investigate node conditions before scaling.", + }, { // Test 11: An unknown status names the missing read. + Status: capacityUnknown, Sig: clusterSignals{utilGap: "the gap"}, WantText: "Utilization is unknown: the gap", }} for testNum, test := range tests { t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { @@ -46,8 +54,8 @@ func TestRecommendNodeCount(t *testing.T) { if strings.Contains(got, "Add 0 node") || strings.Contains(got, " 0 additional node") { t.Errorf("recommendation %q prescribes zero nodes", got) } - if strings.Contains(got, "memory is 0%") { - t.Errorf("recommendation %q reports a missing memory reading as zero", got) + if !test.Sig.utilObs.seen() && strings.Contains(got, "%") { + t.Errorf("recommendation %q reports a percentage it never observed", got) } }) } @@ -59,7 +67,7 @@ func TestRecommendNodeCount(t *testing.T) { func TestGroupComparison(t *testing.T) { t.Parallel() read := func(cpu, mem float64) clusterSignals { - return clusterSignals{hasMetrics: true, cpuPct: cpu, memPct: mem} + return clusterSignals{utilObs: observed, cpuPct: cpu, memPct: mem} } peers := []string{"a", "b", "c", "d", "e"} tests := []struct { @@ -112,17 +120,23 @@ func TestCapacityHealthyRaisesNoFinding(t *testing.T) { } } -// TestAnalyzeCapacity verifies status assignment and event signal detection -// from report sections. +// TestAnalyzeCapacity verifies status assignment, event signal detection, and that a cluster +// without observed utilization gets no utilization or headroom figure and an unknown status +// when nothing else marks it strained or critical. func TestAnalyzeCapacity(t *testing.T) { t.Parallel() oom := map[string]any{"top_warning_reasons": []any{map[string]any{"reason": "OOMKilling"}}} sched := map[string]any{ "top_warning_reasons": []any{map[string]any{"reason": "FailedScheduling"}}, } + nodes := map[string]any{"node_count": 3, "healthy_nodes": 3} tests := []struct { Sections map[string]map[string]any + Degraded []ScannerStatus + WantHeadroomCPU *float64 + WantHeadroomMem *float64 WantStatus string + WantText string WantOOM bool WantSchedFailed bool }{{ // Test 0: OOMKilling with memory pressure is critical. @@ -136,31 +150,66 @@ func TestAnalyzeCapacity(t *testing.T) { WantStatus: capacityStrained, WantSchedFailed: true, }, { // Test 2: Memory at 75 percent is busy. Sections: map[string]map[string]any{ - "metrics": {"c1": map[string]any{"avg_memory_percent": 75.0}}, + "metrics": {"c1": map[string]any{"avg_cpu_percent": 40.0, "avg_memory_percent": 75.0}}, }, - WantStatus: capacityBusy, + WantStatus: capacityBusy, WantHeadroomCPU: new(60.0), WantHeadroomMem: new(25.0), }, { // Test 3: A quiet cluster is healthy. Sections: map[string]map[string]any{ - "metrics": {"c1": map[string]any{"avg_memory_percent": 30.0}}, + "metrics": {"c1": map[string]any{"avg_cpu_percent": 20.0, "avg_memory_percent": 30.0}}, }, - WantStatus: capacityHealthy, + WantStatus: capacityHealthy, WantHeadroomCPU: new(80.0), WantHeadroomMem: new(70.0), }, { // Test 4: Events for another cluster leave this one healthy. Sections: map[string]map[string]any{ - "metrics": {"c1": map[string]any{"avg_memory_percent": 30.0}}, + "metrics": {"c1": map[string]any{"avg_cpu_percent": 20.0, "avg_memory_percent": 30.0}}, "events": {"c2": oom}, }, - WantStatus: capacityHealthy, - }, { // Test 5: Event data that is not an object is ignored. + WantStatus: capacityHealthy, WantHeadroomCPU: new(80.0), WantHeadroomMem: new(70.0), + }, { // Test 5: Event data that is not an object is ignored, and no metrics is unknown. Sections: map[string]map[string]any{"events": {"c1": "bad"}}, - WantStatus: capacityHealthy, - }, { // Test 6: Event data without warning reasons is ignored. + WantStatus: capacityUnknown, + }, { // Test 6: Event data without warning reasons is ignored, and no metrics is unknown. Sections: map[string]map[string]any{"events": {"c1": map[string]any{}}}, - WantStatus: capacityHealthy, + WantStatus: capacityUnknown, + }, { // Test 7: No metrics section invents no headroom and names the scanner that never ran. + Sections: map[string]map[string]any{"node-health": {"c1": nodes}}, + WantStatus: capacityUnknown, WantText: "the metrics scanner did not run", + }, { // Test 8: A degraded metrics run with no node percentages is unknown. + Sections: map[string]map[string]any{ + "node-health": {"c1": nodes}, + "metrics": {"c1": map[string]any{"available": true, "node_count": 3, "nodes_with_usage": 0}}, + }, + Degraded: []ScannerStatus{{Cluster: "c1", Scanner: "metrics", State: "degraded"}}, + WantStatus: capacityUnknown, WantText: "list nodes", + }, { // Test 9: An errored metrics run is unknown and names the failed read. + Sections: map[string]map[string]any{"node-health": {"c1": nodes}}, + Degraded: []ScannerStatus{{Cluster: "c1", Scanner: "metrics", State: "errored"}}, + WantStatus: capacityUnknown, WantText: "node metrics read failed", + }, { // Test 10: A cluster that does not serve the metrics API is unknown. + Sections: map[string]map[string]any{ + "node-health": {"c1": nodes}, + "metrics": {"c1": map[string]any{"available": false, "node_count": 0}}, + }, + Degraded: []ScannerStatus{{Cluster: "c1", Scanner: "metrics", State: "unavailable"}}, + WantStatus: capacityUnknown, WantText: "does not serve the metrics.k8s.io API", + }, { // Test 11: Disk pressure without metrics is strained with no invented percentage. + Sections: map[string]map[string]any{ + "node-health": {"c1": map[string]any{"node_count": 3, "disk_pressure_nodes": 1}}, + }, + WantStatus: capacityStrained, WantText: "utilization is unknown", + }, { // Test 12: Utilization from a degraded run keeps its figure and says so. + Sections: map[string]map[string]any{ + "metrics": {"c1": map[string]any{"avg_cpu_percent": 20.0, "avg_memory_percent": 30.0}}, + }, + Degraded: []ScannerStatus{{Cluster: "c1", Scanner: "metrics", State: "degraded"}}, + WantStatus: capacityHealthy, WantHeadroomCPU: new(80.0), WantHeadroomMem: new(70.0), + WantText: partialScanNote, }} for testNum, test := range tests { t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { t.Parallel() - got := analyzeCapacity(testReport(test.Sections), nil) + r := testReport(test.Sections) + r.Degraded = test.Degraded + got := analyzeCapacity(r, nil) var ca CapacityAnalysis for _, a := range got { if a.Cluster == "c1" { @@ -179,6 +228,25 @@ func TestAnalyzeCapacity(t *testing.T) { if diff := cmp.Diff(test.WantSchedFailed, ca.HasSchedulingFailures); diff != "" { t.Errorf("scheduling mismatch (-want +got):\n%s", diff) } + if diff := cmp.Diff(test.WantHeadroomCPU, ca.HeadroomCPU); diff != "" { + t.Errorf("cpu headroom mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantHeadroomMem, ca.HeadroomMemory); diff != "" { + t.Errorf("memory headroom mismatch (-want +got):\n%s", diff) + } + if test.WantHeadroomMem == nil && (ca.CPUUtilization != nil || ca.MemoryUtilization != nil) { + t.Errorf("utilization reported without an observed reading: cpu %v, memory %v", + ca.CPUUtilization, ca.MemoryUtilization) + } + if !strings.Contains(ca.Recommendation, test.WantText) { + t.Errorf("recommendation %q does not contain %q", ca.Recommendation, test.WantText) + } + if test.WantHeadroomMem == nil && strings.Contains(ca.Recommendation, "%") { + t.Errorf("recommendation %q reports a percentage it never observed", ca.Recommendation) + } + if test.WantStatus == capacityUnknown && ca.NodesNeededForTarget != 0 { + t.Errorf("unknown status prescribes %d nodes", ca.NodesNeededForTarget) + } }) } } diff --git a/internal/report/findings.go b/internal/report/findings.go index 365ad9a..4020992 100644 --- a/internal/report/findings.go +++ b/internal/report/findings.go @@ -453,31 +453,3 @@ func capList(items []string) []string { out = append(out, fmt.Sprintf("...and %d more", len(items)-maxAffected)) return out } - -// clusterData returns a cluster's scanner payload as a generic map, or nil when the section, -// cluster, or a valid conversion is absent. The map may be the report's own, so callers must not -// modify it. -func clusterData(r *Report, scannerName, cluster string) map[string]any { - sec := r.Sections[scannerName] - if sec == nil { - return nil - } - data, ok := sec.PerCluster[cluster] - if !ok { - return nil - } - m, _ := genericMap(data) - return m -} - -// getIntField extracts an integer field from a cluster's scanner data. -func getIntField(r *Report, scannerName, cluster, field string) int { - n, _ := util.Float64(clusterData(r, scannerName, cluster)[field]) - return int(n) -} - -// getMetricsFloat extracts a float field from a cluster's metrics scanner data. -func getMetricsFloat(r *Report, cluster, field string) float64 { - n, _ := util.Float64(clusterData(r, "metrics", cluster)[field]) - return n -} diff --git a/internal/report/findings_health.go b/internal/report/findings_health.go index 0e26f15..def80f5 100644 --- a/internal/report/findings_health.go +++ b/internal/report/findings_health.go @@ -175,9 +175,11 @@ func capacityFindings(r *Report) []Finding { Scanner: "metrics", }) case capacityBusy: + title := fmt.Sprintf("%s is busy but healthy (CPU %s, memory %s)", + ca.Cluster, pctText(ca.CPUUtilization), pctText(ca.MemoryUtilization)) findings = append(findings, Finding{ Rule: "metrics/capacity-busy", - Title: fmt.Sprintf("%s is busy but healthy (CPU %.0f%%, memory %.0f%%)", ca.Cluster, ca.CPUUtilization, ca.MemoryUtilization), + Title: title, Description: ca.Recommendation, Severity: SeverityInfo, Cluster: ca.Cluster, diff --git a/internal/report/findings_test.go b/internal/report/findings_test.go index bafb281..decfda4 100644 --- a/internal/report/findings_test.go +++ b/internal/report/findings_test.go @@ -394,7 +394,8 @@ func TestFindingGenerators(t *testing.T) { //nolint:funlen // Test function. Report: &Report{Capacity: []CapacityAnalysis{ {Cluster: "a", Status: "critical", Recommendation: "r1"}, { - Cluster: "b", Status: "busy", CPUUtilization: 72, MemoryUtilization: 68, Recommendation: "r2", + Cluster: "b", Status: "busy", CPUUtilization: new(72.0), MemoryUtilization: new(68.0), + Recommendation: "r2", GroupDeviation: `Memory is 20% points higher than group "g" average (80% vs 60%). `, }, }}, diff --git a/internal/scanner/metrics/metrics.go b/internal/scanner/metrics/metrics.go index bd13a9f..1cbaf7b 100644 --- a/internal/scanner/metrics/metrics.go +++ b/internal/scanner/metrics/metrics.go @@ -47,14 +47,17 @@ type Data struct { Available bool `json:"available"` // NodeCount is the number of nodes with metrics. NodeCount int `json:"node_count"` - // AvgCPUPercent is the average CPU utilization across all nodes. - AvgCPUPercent float64 `json:"avg_cpu_percent"` - // AvgMemoryPercent is the average memory utilization across all nodes. - AvgMemoryPercent float64 `json:"avg_memory_percent"` - // MaxCPUPercent is the highest CPU utilization on any single node. - MaxCPUPercent float64 `json:"max_cpu_percent"` - // MaxMemoryPercent is the highest memory utilization on any single node. - MaxMemoryPercent float64 `json:"max_memory_percent"` + // NodesWithUsage is the number of nodes whose CPU and memory usage both resolved to a + // percentage of allocatable. The averages and maxima cover only these nodes. + NodesWithUsage int `json:"nodes_with_usage"` + // AvgCPUPercent is the average CPU utilization across NodesWithUsage, nil when none. + AvgCPUPercent *float64 `json:"avg_cpu_percent,omitempty"` + // AvgMemoryPercent is the average memory utilization across NodesWithUsage, nil when none. + AvgMemoryPercent *float64 `json:"avg_memory_percent,omitempty"` + // MaxCPUPercent is the highest CPU utilization on any resolved node, nil when none. + MaxCPUPercent *float64 `json:"max_cpu_percent,omitempty"` + // MaxMemoryPercent is the highest memory utilization on any resolved node, nil when none. + MaxMemoryPercent *float64 `json:"max_memory_percent,omitempty"` // MaxCPUNode is the name of the node with the highest CPU usage. MaxCPUNode string `json:"max_cpu_node,omitempty"` // MaxMemoryNode is the name of the node with the highest memory usage. @@ -66,7 +69,8 @@ type Data struct { // NewScanner returns a scanner that collects node resource utilization from the metrics API. // A metrics API the cluster does not serve yields StateUnavailable. A denied or failed metrics // read errors, so it is never mistaken for a healthy, zero-utilization cluster. A failed node -// read leaves usage without percentages and marks the result StateDegraded. +// read leaves usage without percentages and marks the result StateDegraded. A cluster with no +// node resolved to a percentage omits its averages and maxima, so unknown never reads as zero. func NewScanner() scanner.Scanner { return scanner.Func(func(ctx context.Context, client *kube.Client) (scanner.Result, error) { dyn := client.Dynamic() @@ -100,33 +104,10 @@ func NewScanner() scanner.Scanner { Available: true, NodeCount: len(metricsItems), } - - var totalCPUPct, totalMemPct float64 - for _, item := range metricsItems { - nm := extractNodeMetrics(item.Object, allocatable) - data.Nodes = append(data.Nodes, nm) - - if nm.CPUPercent >= 0 { - totalCPUPct += nm.CPUPercent - } - if nm.MemoryPercent >= 0 { - totalMemPct += nm.MemoryPercent - } - if nm.CPUPercent > data.MaxCPUPercent { - data.MaxCPUPercent = nm.CPUPercent - data.MaxCPUNode = nm.Name - } - if nm.MemoryPercent > data.MaxMemoryPercent { - data.MaxMemoryPercent = nm.MemoryPercent - data.MaxMemoryNode = nm.Name - } - } - - if data.NodeCount > 0 { - data.AvgCPUPercent = totalCPUPct / float64(data.NodeCount) - data.AvgMemoryPercent = totalMemPct / float64(data.NodeCount) + data.Nodes = append(data.Nodes, extractNodeMetrics(item.Object, allocatable)) } + data.aggregate() sort.Slice(data.Nodes, func(i, j int) bool { return data.Nodes[i].Name < data.Nodes[j].Name @@ -136,6 +117,35 @@ func NewScanner() scanner.Scanner { }) } +// aggregate fills the usage count, averages, and maxima from the nodes whose CPU and memory +// both resolved to a percentage. With none resolved they stay nil. +func (d *Data) aggregate() { + var totalCPU, totalMem, maxCPU, maxMem float64 + for _, nm := range d.Nodes { + if nm.CPUPercent < 0 || nm.MemoryPercent < 0 { + continue + } + d.NodesWithUsage++ + totalCPU += nm.CPUPercent + totalMem += nm.MemoryPercent + first := d.NodesWithUsage == 1 + if first || nm.CPUPercent > maxCPU { + maxCPU, d.MaxCPUNode = nm.CPUPercent, nm.Name + } + if first || nm.MemoryPercent > maxMem { + maxMem, d.MaxMemoryNode = nm.MemoryPercent, nm.Name + } + } + if d.NodesWithUsage == 0 { + return + } + n := float64(d.NodesWithUsage) + d.AvgCPUPercent = new(totalCPU / n) + d.AvgMemoryPercent = new(totalMem / n) + d.MaxCPUPercent = new(maxCPU) + d.MaxMemoryPercent = new(maxMem) +} + // allocInfo captures the allocatable CPU (millicores) and memory (bytes) for // one node, used to convert raw usage into percentages. type allocInfo struct { diff --git a/internal/scanner/metrics/metrics_test.go b/internal/scanner/metrics/metrics_test.go index 47340d8..ac723ca 100644 --- a/internal/scanner/metrics/metrics_test.go +++ b/internal/scanner/metrics/metrics_test.go @@ -2,6 +2,7 @@ package metrics import ( "context" + "encoding/json" "errors" "fmt" "testing" @@ -181,18 +182,24 @@ func TestExtractNodeMetrics(t *testing.T) { // TestScan drives the full metrics scanner over the dynamic and typed fakes, // covering the missing dynamic client, a missing metrics API (NoMatch and // NotFound), a 403, a generic list error, the resolved-percentage -// aggregation, a denied node read, and metrics without matching nodes. +// aggregation, a denied node read, metrics without matching nodes, and a mix +// of resolved and unresolved nodes. A run with no resolved percentage must omit +// its averages and maxima from the JSON rather than report zero utilization. func TestScan(t *testing.T) { t.Parallel() errDenied := errors.New("rbac denied") + unresolvedKeys := []string{ + "avg_cpu_percent", "avg_memory_percent", "max_cpu_percent", "max_memory_percent", + } errBoom := errors.New("boom") tests := []struct { - Client *kube.Client - Want error - WantState scanner.State - WantData Data + Client *kube.Client + Want error + WantState scanner.State + WantData Data + WantOmitted []string }{{ // Test 0: No dynamic client is a blind read, never a clean zero. Client: nilDynamicClient(), Want: scanner.ErrScan, WantState: scanner.StateErrored, }, { // Test 1: A NoMatch error means the metrics API is absent, not a failure. @@ -224,10 +231,10 @@ func TestScan(t *testing.T) { ), ), WantData: Data{ - Available: true, NodeCount: 2, - AvgCPUPercent: 37.5, AvgMemoryPercent: 37.5, - MaxCPUPercent: 50, MaxCPUNode: "node2", - MaxMemoryPercent: 50, MaxMemoryNode: "node1", + Available: true, NodeCount: 2, NodesWithUsage: 2, + AvgCPUPercent: new(37.5), AvgMemoryPercent: new(37.5), + MaxCPUPercent: new(50.0), MaxCPUNode: "node2", + MaxMemoryPercent: new(50.0), MaxMemoryNode: "node1", Nodes: []NodeMetrics{ {Name: "node1", CPUUsage: "250m", MemoryUsage: "1000Mi", CPUPercent: 25, MemoryPercent: 50}, {Name: "node2", CPUUsage: "1000m", MemoryUsage: "1000Mi", CPUPercent: 50, MemoryPercent: 25}, @@ -243,6 +250,7 @@ func TestScan(t *testing.T) { {Name: "node1", CPUUsage: "250m", MemoryUsage: "1000Mi", CPUPercent: -1, MemoryPercent: -1}, }, }, + WantOmitted: unresolvedKeys, }, { // Test 7: Metrics without matching nodes stay unresolved but available. Client: kube.NewTestClientWithDynamic("test", fakeclientset.NewSimpleClientset(), @@ -254,6 +262,25 @@ func TestScan(t *testing.T) { {Name: "orphan", CPUUsage: "250m", MemoryUsage: "100Mi", CPUPercent: -1, MemoryPercent: -1}, }, }, + WantOmitted: unresolvedKeys, + }, { // Test 8: Averages cover only the nodes that resolved, not every node listed. + Client: kube.NewTestClientWithDynamic("test", + fakeclientset.NewSimpleClientset(allocNode("node1", "1", "2000Mi")), + newMetricsDynamic( + nodeMetric("node1", "250m", "1000Mi"), + nodeMetric("orphan", "250m", "100Mi"), + ), + ), + WantData: Data{ + Available: true, NodeCount: 2, NodesWithUsage: 1, + AvgCPUPercent: new(25.0), AvgMemoryPercent: new(50.0), + MaxCPUPercent: new(25.0), MaxCPUNode: "node1", + MaxMemoryPercent: new(50.0), MaxMemoryNode: "node1", + Nodes: []NodeMetrics{ + {Name: "node1", CPUUsage: "250m", MemoryUsage: "1000Mi", CPUPercent: 25, MemoryPercent: 50}, + {Name: "orphan", CPUUsage: "250m", MemoryUsage: "100Mi", CPUPercent: -1, MemoryPercent: -1}, + }, + }, }} for testNum, test := range tests { @@ -280,6 +307,19 @@ func TestScan(t *testing.T) { if diff := cmp.Diff(test.WantData, data, opts); diff != "" { t.Errorf("data mismatch (-want +got):\n%s", diff) } + b, err := json.Marshal(data) + if err != nil { + t.Fatalf("marshal data: %v", err) + } + var keys map[string]any + if err := json.Unmarshal(b, &keys); err != nil { + t.Fatalf("unmarshal data: %v", err) + } + for _, k := range test.WantOmitted { + if v, ok := keys[k]; ok { + t.Errorf("json key %q present as %v, want omitted", k, v) + } + } }) } } diff --git a/internal/server/demo_results.go b/internal/server/demo_results.go index e505f25..bb04f9b 100644 --- a/internal/server/demo_results.go +++ b/internal/server/demo_results.go @@ -326,7 +326,11 @@ func demoResources(p demoProfile) resources.Data { func demoMetrics(p demoProfile) metrics.Data { cpu, mem := p.utilization() names := p.nodeNames() - d := metrics.Data{Available: true, NodeCount: len(names), AvgCPUPercent: cpu, AvgMemoryPercent: mem} + d := metrics.Data{ + Available: true, NodeCount: len(names), NodesWithUsage: len(names), + AvgCPUPercent: new(cpu), AvgMemoryPercent: new(mem), + } + var maxCPU, maxMem float64 for i, name := range names { offset := 1.5 * (float64(i) - float64(len(names)-1)/2) nc := min(cpu+offset, 99) @@ -338,13 +342,14 @@ func demoMetrics(p demoProfile) metrics.Data { Name: name, CPUPercent: nc, MemoryPercent: nm, CPUUsage: fmt.Sprintf("%dm", int(nc*80)), MemoryUsage: fmt.Sprintf("%dMi", int(nm*320)), }) - if nc > d.MaxCPUPercent { - d.MaxCPUPercent, d.MaxCPUNode = nc, name + if nc > maxCPU { + maxCPU, d.MaxCPUNode = nc, name } - if nm > d.MaxMemoryPercent { - d.MaxMemoryPercent, d.MaxMemoryNode = nm, name + if nm > maxMem { + maxMem, d.MaxMemoryNode = nm, name } } + d.MaxCPUPercent, d.MaxMemoryPercent = new(maxCPU), new(maxMem) return d } diff --git a/internal/server/openapi.yaml b/internal/server/openapi.yaml index 7ae7690..3bbea38 100644 --- a/internal/server/openapi.yaml +++ b/internal/server/openapi.yaml @@ -204,9 +204,11 @@ components: CapacityAnalysis: type: object + description: >- + Utilization and headroom are absent when no scanner observed the cluster's utilization. properties: cluster: {type: string} - status: {type: string} + status: {type: string, description: "critical, strained, busy, healthy, or unknown."} cpu_utilization: {type: number} memory_utilization: {type: number} node_count: {type: integer} diff --git a/internal/server/static/js/views/capacity.js b/internal/server/static/js/views/capacity.js index 34f9698..d5db491 100644 --- a/internal/server/static/js/views/capacity.js +++ b/internal/server/static/js/views/capacity.js @@ -19,12 +19,21 @@ export async function renderCapacity() { return { html: h + '
', post: loadCostPanel }; } -// capacityCards renders one card per cluster. +// utilTone maps a utilization percentage to a tone, or muted text when no +// scanner observed it. +function utilTone(v) { + return v == null ? 't3' : threshTone(v); +} + +// capacityCards renders one card per cluster. A cluster whose utilization no +// scanner observed shows N/A for it and its headroom, and an unknown status +// whose recommendation names the missing read. function capacityCards(capacity) { if (!capacity || !capacity.length) return emptyState('The latest scan read no capacity data from any cluster.'); let h = '| Cluster | ' + - 'Status | ' + + 'Status | ' + `Version | ` + 'Nodes | ' + `CPU | Memory | ` + @@ -144,7 +144,7 @@ function renderTable() { // rowHTML renders one comparison row. function rowHTML(r) { - const hrTone = r.headroom < 15 ? 'crit' : r.headroom < 30 ? 'warn' : 'ok'; + const hrTone = r.headroom == null ? 't3' : r.headroom < 15 ? 'crit' : r.headroom < 30 ? 'warn' : 'ok'; return `
|---|---|---|---|---|---|---|
| ${esc(r.name)} | ` + `${esc(r.status)} | ` + `${esc(r.ver)} | ${r.nodes} | ` + diff --git a/internal/server/static/js/views/detail.js b/internal/server/static/js/views/detail.js index 0feea37..f141418 100644 --- a/internal/server/static/js/views/detail.js +++ b/internal/server/static/js/views/detail.js @@ -113,10 +113,10 @@ function sections(sd) { if (sd.metrics && sd.metrics.available) { const m = sd.metrics; let c = '|||
| ${esc(n.name)} | ${esc(n.cpu_usage)} | ` + From ae5b4fa8f2e2e5f415e42690118bbd32e901a382 Mon Sep 17 00:00:00 2001 From: Douglas Adolph <116844282+dcadolph@users.noreply.github.com> Date: Sat, 31 Oct 2026 06:24:00 -0500 Subject: [PATCH 04/13] Keep nodes_with_usage from restating the metrics node count --- internal/report/fields.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/report/fields.go b/internal/report/fields.go index 6c70565..0a5673f 100644 --- a/internal/report/fields.go +++ b/internal/report/fields.go @@ -78,6 +78,11 @@ var fieldSpecs = []fieldSpec{ Polarity: UpIsBad, Trend: true, }, + {FieldRef: ref("metrics", "node_count")}, + { + FieldRef: ref("metrics", "nodes_with_usage"), + RedundantWith: []FieldRef{ref("metrics", "node_count")}, + }, { FieldRef: ref("node-health", "unhealthy_nodes"), From cbb048622ccd6cf6a2ec4f6ada2a3b7777fdf441 Mon Sep 17 00:00:00 2001 From: Douglas Adolph <116844282+dcadolph@users.noreply.github.com> Date: Sat, 31 Oct 2026 07:05:03 -0500 Subject: [PATCH 05/13] Grade the fleet only when scanners observed enough of it --- docs/concepts/fleet-score.md | 37 ++++++ internal/report/brief.go | 31 +++-- internal/report/brief_test.go | 76 +++++++++++- internal/report/score.go | 184 +++++++++++++++++++++++++-- internal/report/score_test.go | 228 ++++++++++++++++++++++++++++++++-- 5 files changed, 528 insertions(+), 28 deletions(-) diff --git a/docs/concepts/fleet-score.md b/docs/concepts/fleet-score.md index 72e96b0..34eab4b 100644 --- a/docs/concepts/fleet-score.md +++ b/docs/concepts/fleet-score.md @@ -38,6 +38,35 @@ with every rule and cluster it charged and what each cap forgave, is The exact weights live in `internal/report/score.go` and are documented inline. +## Coverage gate + +A score is only graded when enough of the fleet was observed to back it. +Every report counts its scanner runs: one expected run per listed cluster +for every scanner that ran on at least one cluster. A run is observed when +it finished OK or found its feature not served (unavailable), partial when +it was degraded, and blind when it errored or never ran on that cluster. + +The score is graded when both hold: + +- Every listed cluster has at least one observed run. +- At least 80 percent of expected runs are observed. The floor is the + constant `minCoverageFraction` in `internal/report/score.go`. + +An ungraded score has `graded: false` and an empty `grade`. Its headline +leads with the gap and labels the number, for example "Not enough data: 6 +of 6 scanner runs did not observe the fleet, and 2 of 2 clusters had no +complete scanner run. Observed-only score 100." The number is still +computed from what was observed, so it is there for inspection, but it is +not the fleet's health. The brief leads with the same sentence. + +A graded score with blind or partial runs names them after the band, and +its top band reads "nothing flagged in what was observed" in place of "no +meaningful drift", because a blind run cannot vouch for anything. A fully +observed fleet reads exactly as before. + +The counts travel in `coverage`: `expected_runs`, `observed_runs`, +`blind_runs`, `blind_clusters`, and `fraction` (observed over expected). + ## Cluster score Each cluster also gets a 0-100 score on the same scale. It charges the @@ -49,6 +78,12 @@ once in the Fleet Score. This is the same rule that sets cluster status, so a cluster that reads healthy never loses points to a critical or warning finding. +A cluster with any blind scanner run (errored, or never run on it) that no +finding made degraded or critical reads `unknown` instead of `healthy`. Its +cluster score pays the degraded status cost, and its headline reads +"Unknown. N scanner runs did not observe this cluster." The Fleet Score +does not charge unknown clusters. The coverage gate covers them instead. + ## Provenance A score never travels alone. Every surface that reports it (the report @@ -60,6 +95,8 @@ metadata, and the Prometheus and OpenTelemetry metrics) also carries: one cluster. - `excluded_clusters`: scanned clusters that produced no usable data and were left out of the statistical population. +- `coverage`: the scanner runs behind the score and how many observed the + fleet (see [Coverage gate](#coverage-gate)). A score computed across six clusters and one computed across sixty are different claims. The provenance says which one you are reading. diff --git a/internal/report/brief.go b/internal/report/brief.go index 25b62c2..5780ba2 100644 --- a/internal/report/brief.go +++ b/internal/report/brief.go @@ -19,28 +19,38 @@ type Brief struct { // generateBrief synthesizes an executive summary from a fully built report: the // score, the worst incident, cohort drift, degraded coverage, and the fixes -// that matter most. +// that matter most. An ungraded score leads with the coverage sentence and +// labels the score observed-only on the next line. func generateBrief(r *Report) Brief { crit := countFindingsBySeverity(r.Findings, SeverityCritical) warn := countFindingsBySeverity(r.Findings, SeverityWarning) + fs := r.FleetScore + coverage := coverageSentence(fs.Coverage, len(r.Clusters)) - b := Brief{ - Headline: fmt.Sprintf( + var b Brief + if fs.Graded { + b.Headline = fmt.Sprintf( "Fleet score %d (%s) from %d clusters across %d dimensions. "+ "%d critical and %d warning findings across %d incidents.", - r.FleetScore.Score, r.FleetScore.Grade, r.FleetScore.SampleSize, - r.FleetScore.Dimensions, crit, warn, len(r.Incidents), - ), + fs.Score, fs.Grade, fs.SampleSize, fs.Dimensions, crit, warn, len(r.Incidents), + ) + } else { + b.Headline = "Not enough data: " + coverage + b.Lines = append(b.Lines, fmt.Sprintf( + "Observed-only score %d from %d clusters across %d dimensions. "+ + "%d critical and %d warning findings across %d incidents.", + fs.Score, fs.SampleSize, fs.Dimensions, crit, warn, len(r.Incidents), + )) } if line := normLine(r.Findings); line != "" { b.Lines = append(b.Lines, line) } - if r.FleetScore.ExcludedClusters > 0 { + if fs.Graded && fs.ExcludedClusters > 0 { b.Lines = append(b.Lines, fmt.Sprintf( "%d cluster(s) excluded from the score for insufficient data.", - r.FleetScore.ExcludedClusters, + fs.ExcludedClusters, )) } @@ -58,7 +68,10 @@ func generateBrief(r *Report) Brief { b.Lines = append(b.Lines, fmt.Sprintf("%d cluster metric(s) drifted from their cohort baseline.", cohortOutliers)) } - if len(r.Degraded) > 0 { + switch { + case fs.Graded && coverage != "": + b.Lines = append(b.Lines, coverage) + case fs.Graded && len(r.Degraded) > 0: b.Lines = append(b.Lines, fmt.Sprintf( "Coverage is partial: %d scanner run(s) degraded on %d cluster(s).", len(r.Degraded), len(r.DegradedByCluster()), diff --git a/internal/report/brief_test.go b/internal/report/brief_test.go index 3402dc2..3cbe71c 100644 --- a/internal/report/brief_test.go +++ b/internal/report/brief_test.go @@ -1,8 +1,13 @@ package report import ( + "fmt" "strings" "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/dcadolph/fleetsweeper/internal/scanner" ) // TestGenerateBrief verifies the executive summary reflects the report's score, @@ -13,7 +18,7 @@ func TestGenerateBrief(t *testing.T) { r := &Report{ Clusters: []string{"a", "b", "c"}, FleetScore: FleetScore{ - Score: 73, Grade: "C", SampleSize: 3, Dimensions: 24, ExcludedClusters: 1, + Score: 73, Grade: "C", Graded: true, SampleSize: 3, Dimensions: 24, ExcludedClusters: 1, }, Findings: []Finding{ {Title: "a webhook down", Severity: SeverityCritical, Cluster: "a", Scanner: "admission", @@ -60,3 +65,72 @@ func TestGenerateBrief(t *testing.T) { t.Errorf("fix line missing remediation command:\n%s", joined) } } + +// TestGenerateBriefCoverage verifies the brief leads with coverage when the score is not graded +// and names blind runs when it is. +func TestGenerateBriefCoverage(t *testing.T) { + t.Parallel() + clusters := []string{"c1", "c2"} + allErrored := map[runKey]*scanner.Result{} + for _, c := range clusters { + for _, s := range []string{"s1", "s2", "s3"} { + allErrored[runKey{scanner: s, cluster: c}] = runOf(erroredRun(s)) + } + } + tests := []struct { + Name string + Override map[runKey]*scanner.Result + WantHeadline string + WantFirst string + WantLine string + WantNoLine string + }{{ // Test 0: Every run errored, so the coverage sentence is the first line. + Name: "all errored", + Override: allErrored, + WantHeadline: "Not enough data: 6 of 6 scanner runs did not observe the fleet, " + + "and 2 of 2 clusters had no complete scanner run.", + WantFirst: "Observed-only score 100", + WantNoLine: "Coverage is partial", + }, { // Test 1: One blind run on a graded fleet adds the coverage sentence to the lines. + Name: "graded with a blind run", + Override: map[runKey]*scanner.Result{ + {scanner: "s1", cluster: "c1"}: runOf(erroredRun("s1")), + }, + WantHeadline: "Fleet score 100 (A)", + WantLine: "1 of 6 scanner runs did not observe the fleet.", + WantNoLine: "Coverage is partial", + }, { // Test 2: A fully observed fleet carries no coverage sentence. + Name: "fully observed", + WantHeadline: "Fleet score 100 (A)", + WantNoLine: "scanner runs", + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d: %s", testNum, test.Name), func(t *testing.T) { + t.Parallel() + r := Build(clusters, coverageResults(clusters, 3, test.Override)) + b := r.Brief + if !strings.HasPrefix(b.Headline, test.WantHeadline) { + t.Errorf("brief headline %q, want prefix %q", b.Headline, test.WantHeadline) + } + if test.WantFirst != "" { + if len(b.Lines) == 0 { + t.Fatalf("brief has no lines, want first %q", test.WantFirst) + } + if !strings.HasPrefix(b.Lines[0], test.WantFirst) { + t.Errorf("first line %q, want prefix %q", b.Lines[0], test.WantFirst) + } + } + joined := strings.Join(b.Lines, "\n") + if test.WantLine != "" && !strings.Contains(joined, test.WantLine) { + t.Errorf("brief lines missing %q:\n%s", test.WantLine, joined) + } + if test.WantNoLine != "" && strings.Contains(joined, test.WantNoLine) { + t.Errorf("brief lines carry %q:\n%s", test.WantNoLine, joined) + } + gradedHead := !strings.HasPrefix(b.Headline, "Not enough data") + if diff := cmp.Diff(r.FleetScore.Graded, gradedHead); diff != "" { + t.Errorf("brief headline disagrees with Graded (-want +got):\n%s", diff) + } + }) + } +} diff --git a/internal/report/score.go b/internal/report/score.go index 063f1fb..1516489 100644 --- a/internal/report/score.go +++ b/internal/report/score.go @@ -3,7 +3,9 @@ package report import ( "fmt" "math" + "sort" + "github.com/dcadolph/fleetsweeper/internal/scanner" "github.com/dcadolph/fleetsweeper/internal/util" ) @@ -12,12 +14,21 @@ import ( // asymmetric: clusters in trouble pull it down faster than warnings do, and // individual findings are capped so a noisy fleet does not collapse to zero on // volume alone. The Drivers list names the top reasons the score sits where it -// does so operators can read the number and the explanation in one glance. +// does so operators can read the number and the explanation in one glance. A +// score resting on too little observed data is not graded: Graded is false, Grade +// is empty, and the headline says how much of the fleet went unobserved. type FleetScore struct { - // Score is the rounded 0-100 health score. + // Score is the rounded 0-100 health score. When Graded is false it is computed from the + // observed data only and must not be read as the fleet's health. Score int `json:"score"` - // Grade is a single-letter rollup of Score: A 90+, B 80+, C 70+, D 60+, F under 60. + // Grade is a single-letter rollup of Score: A 90+, B 80+, C 70+, D 60+, F under 60. Empty + // when Graded is false. Grade string `json:"grade"` + // Graded is true when coverage clears the floor: every listed cluster has at least one + // observed scanner run, and at least minCoverageFraction of expected runs are observed. + Graded bool `json:"graded"` + // Coverage counts the scanner runs behind the score. + Coverage FleetCoverage `json:"coverage"` // Headline is a one-line plain-English summary suitable for a hero card. Headline string `json:"headline"` // Drivers names the top three factors pulling the score below 100, in @@ -35,6 +46,26 @@ type FleetScore struct { ExcludedClusters int `json:"excluded_clusters"` } +// FleetCoverage counts how many of the scanner runs a report expected actually observed the +// fleet. A run is observed when its state is OK or unavailable, partial when degraded, and blind +// when it errored or never ran on the cluster. +type FleetCoverage struct { + // ExpectedRuns is the listed clusters times the scanners that ran on at least one cluster. + ExpectedRuns int `json:"expected_runs"` + // ObservedRuns is how many expected runs finished OK or found the feature unavailable. + ObservedRuns int `json:"observed_runs"` + // BlindRuns is how many expected runs errored or never ran on their cluster. + BlindRuns int `json:"blind_runs"` + // BlindClusters is how many listed clusters have no observed run. + BlindClusters int `json:"blind_clusters"` + // Fraction is ObservedRuns over ExpectedRuns, zero when no run was expected. + Fraction float64 `json:"fraction"` +} + +// minCoverageFraction is the share of expected scanner runs that must be observed before the +// fleet score is graded. +const minCoverageFraction = 0.8 + // FleetScoreDriver describes a single factor reducing the fleet score. type FleetScoreDriver struct { // Reason is a short human-readable label. @@ -80,22 +111,134 @@ func Rescore(r *Report) { // floored at zero, and the drivers are the ledger's top three terms. func scoreWithAttribution(r *Report) (FleetScore, ScoreAttribution) { if r == nil { - return FleetScore{Score: 100, Grade: "A", Headline: "No data yet."}, ScoreAttribution{} + return FleetScore{Score: 100, Headline: "No data yet."}, ScoreAttribution{} } a := computeScoreAttribution(r) raw := math.Max(100.0-a.Total, 0) score := int(math.Round(raw)) sample, dims, excluded := scoreProvenance(r) - return FleetScore{ + cov := fleetCoverage(r) + fs := FleetScore{ Score: score, - Grade: grade(score), - Headline: headline(score, r), + Graded: cov.clearsFloor(), + Coverage: cov, Drivers: a.Drivers(), SampleSize: sample, Dimensions: dims, ExcludedClusters: excluded, - }, a + } + if fs.Graded { + fs.Grade = grade(score) + } + fs.Headline = headline(fs, len(r.Clusters)) + return fs, a +} + +// fleetCoverage counts r's expected, observed, and blind scanner runs from the run-state +// table. +func fleetCoverage(r *Report) FleetCoverage { + scanners := expectedScanners(r) + var c FleetCoverage + for _, cluster := range r.Clusters { + observed, blind := clusterRunCounts(r, cluster, scanners) + c.ExpectedRuns += len(scanners) + c.ObservedRuns += observed + c.BlindRuns += blind + if observed == 0 { + c.BlindClusters++ + } + } + if c.ExpectedRuns > 0 { + c.Fraction = float64(c.ObservedRuns) / float64(c.ExpectedRuns) + } + return c +} + +// expectedScanners returns every scanner with a section or a degraded entry in r, so a +// scanner that errored on every cluster is still expected everywhere. +func expectedScanners(r *Report) []string { + seen := make(map[string]struct{}, len(r.Sections)) + for name := range r.Sections { + seen[name] = struct{}{} + } + for _, d := range r.Degraded { + seen[d.Scanner] = struct{}{} + } + names := make([]string, 0, len(seen)) + for name := range seen { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// clusterRunCounts returns how many of scanners' runs on cluster observed it (OK or +// unavailable) and how many were blind (errored or never ran). Degraded runs are neither. +func clusterRunCounts(r *Report, cluster string, scanners []string) (observed, blind int) { + for _, name := range scanners { + st, ok := runState(r, name, cluster) + switch { + case !ok || st == scanner.StateErrored: + blind++ + case st == scanner.StateOK || st == scanner.StateUnavailable: + observed++ + } + } + return observed, blind +} + +// clearsFloor reports whether c is enough coverage to grade: no listed cluster went unobserved +// and the observed share is at least minCoverageFraction. A report that expected no runs and +// lists no clusters has nothing missing and clears it. +func (c FleetCoverage) clearsFloor() bool { + if c.BlindClusters > 0 { + return false + } + return c.ExpectedRuns == 0 || c.Fraction >= minCoverageFraction +} + +// partialRuns returns how many expected runs were degraded: neither observed nor blind. +func (c FleetCoverage) partialRuns() int { + return c.ExpectedRuns - c.ObservedRuns - c.BlindRuns +} + +// coverageSentence describes the runs that did not fully observe a fleet of clusters, such as +// "3 of 40 scanner runs did not observe the fleet." It returns "" when every run was observed. +func coverageSentence(c FleetCoverage, clusters int) string { + partial := c.partialRuns() + var s string + switch { + case c.ExpectedRuns == 0 && c.BlindClusters == 0: + return "" + case c.ExpectedRuns == 0: + return fmt.Sprintf("No scanner ran on %d listed cluster%s.", + c.BlindClusters, util.Plural(c.BlindClusters)) + case c.BlindRuns > 0 && partial > 0: + s = fmt.Sprintf("%d of %d scanner runs did not observe the fleet and %d %s partial", + c.BlindRuns, c.ExpectedRuns, partial, wasWere(partial)) + case c.BlindRuns > 0: + s = fmt.Sprintf("%d of %d scanner runs did not observe the fleet", + c.BlindRuns, c.ExpectedRuns) + case partial > 0: + s = fmt.Sprintf("%d of %d scanner runs %s partial", + partial, c.ExpectedRuns, wasWere(partial)) + default: + return "" + } + if c.BlindClusters > 0 { + s += fmt.Sprintf(", and %d of %d cluster%s had no complete scanner run", + c.BlindClusters, clusters, util.Plural(clusters)) + } + return s + "." +} + +// wasWere returns the past tense of "to be" agreeing with n. +func wasWere(n int) string { + if n == 1 { + return "was" + } + return "were" } // scoreProvenance derives the sample size, contributing dimension count, and @@ -139,10 +282,29 @@ func grade(score int) string { } // headline returns a one-line plain-English summary suitable for the hero -// card on the dashboard. -func headline(score int, r *Report) string { - clusters := len(r.Clusters) +// card on the dashboard. An ungraded score leads with the missing coverage and +// labels the number observed-only. A graded score with unobserved runs names them +// after the band and never claims the fleet is free of drift. +func headline(fs FleetScore, clusters int) string { + cov := coverageSentence(fs.Coverage, clusters) + if !fs.Graded { + return fmt.Sprintf("Not enough data: %s Observed-only score %d.", cov, fs.Score) + } + band := scoreBand(fs.Score, clusters, cov != "") + if cov == "" { + return band + } + return band + " " + cov +} + +// scoreBand returns the headline for a graded score of a fleet of clusters. When some runs +// went unobserved the top band says nothing was flagged in what was observed rather than +// claiming no drift. +func scoreBand(score, clusters int, unobserved bool) string { switch { + case score >= 95 && unobserved: + return fmt.Sprintf("%d cluster%s, nothing flagged in what was observed.", + clusters, util.Plural(clusters)) case score >= 95: return fmt.Sprintf("%d cluster%s, no meaningful drift.", clusters, util.Plural(clusters)) case score >= 85: diff --git a/internal/report/score_test.go b/internal/report/score_test.go index 034cb04..647a5fe 100644 --- a/internal/report/score_test.go +++ b/internal/report/score_test.go @@ -1,13 +1,28 @@ package report import ( + "encoding/json" "fmt" + "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/dcadolph/fleetsweeper/internal/scanner" ) +// observedBy returns sections in which one OK scanner observed every named cluster, so a +// hand-built report clears the coverage gate. +func observedBy(clusters ...string) map[string]*SectionReport { + per := make(map[string]any, len(clusters)) + for _, c := range clusters { + per[c] = map[string]any{"n": 1} + } + return map[string]*SectionReport{"s1": {PerCluster: per}} +} + +// TestScoreWithAttribution verifies score, grade, and top driver on hand-built reports. func TestScoreWithAttribution(t *testing.T) { t.Parallel() tests := []struct { @@ -17,11 +32,10 @@ func TestScoreWithAttribution(t *testing.T) { WantGrade string WantTopDr string }{ - { // Test 0: Nil report scores 100 with placeholder headline. + { // Test 0: Nil report scores 100 ungraded with a placeholder headline. Name: "nil report", In: nil, WantScore: 100, - WantGrade: "A", }, { // Test 1: Empty report scores 100 with no drivers. Name: "empty report", @@ -33,6 +47,7 @@ func TestScoreWithAttribution(t *testing.T) { Name: "one warning", In: &Report{ Clusters: []string{"c1"}, + Sections: observedBy("c1"), Findings: []Finding{{Severity: SeverityWarning}}, }, WantScore: 99, @@ -44,6 +59,7 @@ func TestScoreWithAttribution(t *testing.T) { Name: "five criticals", In: &Report{ Clusters: []string{"c1", "c2", "c3"}, + Sections: observedBy("c1", "c2", "c3"), Findings: []Finding{ {Severity: SeverityCritical}, {Severity: SeverityCritical}, @@ -60,6 +76,7 @@ func TestScoreWithAttribution(t *testing.T) { Name: "single critical cluster", In: &Report{ Clusters: []string{"c1"}, + Sections: observedBy("c1"), ClusterHealths: []ClusterHealth{ {Name: "c1", Status: "critical", NodeCount: 3, HealthyNodes: 3}, }, @@ -73,6 +90,7 @@ func TestScoreWithAttribution(t *testing.T) { Name: "all caps applied", In: &Report{ Clusters: []string{"c1", "c2"}, + Sections: observedBy("c1", "c2"), Findings: makeFindings(SeverityCritical, 200), ClusterHealths: []ClusterHealth{ {Name: "c1", Status: "critical", NodeCount: 10, HealthyNodes: 0, @@ -88,6 +106,7 @@ func TestScoreWithAttribution(t *testing.T) { Name: "critical version skew", In: &Report{ Clusters: []string{"c1", "c2", "c3"}, + Sections: observedBy("c1", "c2", "c3"), ClusterHealths: []ClusterHealth{ {Name: "c1", KubernetesVersion: "v1.31.2", NodeCount: 5, HealthyNodes: 5}, {Name: "c2", KubernetesVersion: "v1.30.6", NodeCount: 5, HealthyNodes: 5}, @@ -102,6 +121,7 @@ func TestScoreWithAttribution(t *testing.T) { Name: "worst node cluster surfaced", In: &Report{ Clusters: []string{"a", "b"}, + Sections: observedBy("a", "b"), ClusterHealths: []ClusterHealth{ {Name: "a", NodeCount: 10, HealthyNodes: 10}, {Name: "b", NodeCount: 10, HealthyNodes: 5}, @@ -245,7 +265,7 @@ func TestFleetScoreProvenance(t *testing.T) { } opts := []cmp.Option{ cmpopts.EquateEmpty(), - cmpopts.IgnoreFields(FleetScore{}, "Score", "Grade", "Headline", "Drivers"), + cmpopts.IgnoreFields(FleetScore{}, "Score", "Grade", "Headline", "Drivers", "Graded", "Coverage"), } if diff := cmp.Diff(want, got, opts...); diff != "" { t.Errorf("provenance mismatch (-want +got):\n%s", diff) @@ -382,10 +402,13 @@ func TestFleetScoreGolden(t *testing.T) { //nolint:funlen // Test function. t.Run(fmt.Sprintf("test %d: %s", testNum, test.Name), func(t *testing.T) { t.Parallel() got, _ := scoreWithAttribution(test.In) - want := FleetScore{Score: test.WantScore, Grade: test.WantGrade, Drivers: test.WantDrivers} + want := FleetScore{ + Score: test.WantScore, Grade: test.WantGrade, Graded: true, Drivers: test.WantDrivers, + } opts := []cmp.Option{ cmpopts.EquateEmpty(), - cmpopts.IgnoreFields(FleetScore{}, "Headline", "SampleSize", "Dimensions", "ExcludedClusters"), + cmpopts.IgnoreFields(FleetScore{}, + "Headline", "SampleSize", "Dimensions", "ExcludedClusters", "Coverage"), } if diff := cmp.Diff(want, got, opts...); diff != "" { t.Errorf("score mismatch (-want +got):\n%s", diff) @@ -415,10 +438,201 @@ func TestHeadlineThresholds(t *testing.T) { for testNum, test := range tests { t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { t.Parallel() - r := &Report{Clusters: make([]string, test.Clusters)} - if diff := cmp.Diff(test.WantHeadline, headline(test.Score, r)); diff != "" { + fs := FleetScore{Score: test.Score, Graded: true} + if diff := cmp.Diff(test.WantHeadline, headline(fs, test.Clusters)); diff != "" { t.Errorf("headline mismatch (-want +got):\n%s", diff) } }) } } + +// coverageResults builds per-cluster results for scanners s1 to sN, all OK, then replaces the +// runs named in override, where a missing scanner.Result removes the run. +func coverageResults( + clusters []string, scanners int, override map[runKey]*scanner.Result, +) map[string]map[string]scanner.Result { + out := make(map[string]map[string]scanner.Result, len(clusters)) + for _, c := range clusters { + out[c] = make(map[string]scanner.Result, scanners) + for i := 1; i <= scanners; i++ { + name := fmt.Sprintf("s%d", i) + out[c][name] = okRun(name, map[string]any{"n": 1}) + } + } + for k, res := range override { + if res == nil { + delete(out[k.cluster], k.scanner) + continue + } + out[k.cluster][k.scanner] = *res + } + return out +} + +// runOf returns a pointer to res so coverage overrides read as one expression. +func runOf(res scanner.Result) *scanner.Result { + return &res +} + +// TestFleetCoverage verifies the coverage counts and the grading gate on built reports. +func TestFleetCoverage(t *testing.T) { //nolint:funlen // Test function. + t.Parallel() + two := []string{"c1", "c2"} + five := []string{"c1", "c2", "c3", "c4", "c5"} + allErrored := map[runKey]*scanner.Result{} + for _, c := range two { + for _, s := range []string{"s1", "s2", "s3"} { + allErrored[runKey{scanner: s, cluster: c}] = runOf(erroredRun(s)) + } + } + tests := []struct { + Name string + Clusters []string + Scanners int + Override map[runKey]*scanner.Result + WantCoverage FleetCoverage + WantGraded bool + WantGrade string + WantHeadlineHead string + }{{ // Test 0: Every run errored, so nothing is graded and the headline says so. + Name: "all errored", Clusters: two, Scanners: 3, Override: allErrored, + WantCoverage: FleetCoverage{ExpectedRuns: 6, BlindRuns: 6, BlindClusters: 2}, + WantHeadlineHead: "Not enough data: 6 of 6 scanner runs did not observe the fleet", + }, { // Test 1: Every run OK grades normally with full coverage. + Name: "all ok", Clusters: two, Scanners: 2, + WantCoverage: FleetCoverage{ExpectedRuns: 4, ObservedRuns: 4, Fraction: 1}, + WantGraded: true, + WantGrade: "A", + WantHeadlineHead: "2 clusters, no meaningful drift.", + }, { // Test 2: One errored run of ten stays above the floor and grades. + Name: "one errored of ten", Clusters: two, Scanners: 5, + Override: map[runKey]*scanner.Result{ + {scanner: "s3", cluster: "c1"}: runOf(erroredRun("s3")), + }, + WantCoverage: FleetCoverage{ + ExpectedRuns: 10, ObservedRuns: 9, BlindRuns: 1, Fraction: 0.9, + }, + WantGraded: true, + WantGrade: "A", + WantHeadlineHead: "2 clusters, nothing flagged in what was observed.", + }, { // Test 3: Three errored runs of ten fall under the floor. + Name: "three errored of ten", Clusters: two, Scanners: 5, + Override: map[runKey]*scanner.Result{ + {scanner: "s1", cluster: "c1"}: runOf(erroredRun("s1")), + {scanner: "s2", cluster: "c2"}: runOf(erroredRun("s2")), + {scanner: "s3", cluster: "c1"}: runOf(erroredRun("s3")), + }, + WantCoverage: FleetCoverage{ + ExpectedRuns: 10, ObservedRuns: 7, BlindRuns: 3, Fraction: 0.7, + }, + WantHeadlineHead: "Not enough data: 3 of 10 scanner runs did not observe the fleet.", + }, { // Test 4: A cluster no scanner observed blocks the grade even at the floor. + Name: "one cluster blind", Clusters: five, Scanners: 2, + Override: map[runKey]*scanner.Result{ + {scanner: "s1", cluster: "c5"}: runOf(erroredRun("s1")), + {scanner: "s2", cluster: "c5"}: runOf(erroredRun("s2")), + }, + WantCoverage: FleetCoverage{ + ExpectedRuns: 10, ObservedRuns: 8, BlindRuns: 2, BlindClusters: 1, Fraction: 0.8, + }, + WantHeadlineHead: "Not enough data: 2 of 10 scanner runs did not observe the fleet, " + + "and 1 of 5 clusters had no complete scanner run.", + }, { // Test 5: Unavailable counts as observed, degraded as neither observed nor blind. + Name: "unavailable and degraded", Clusters: two, Scanners: 5, + Override: map[runKey]*scanner.Result{ + {scanner: "s1", cluster: "c1"}: { + Scanner: "s1", State: scanner.StateUnavailable, Data: map[string]any{"n": 0}, + }, + {scanner: "s2", cluster: "c2"}: runOf(degradedRun("s2", map[string]any{"n": 1})), + }, + WantCoverage: FleetCoverage{ExpectedRuns: 10, ObservedRuns: 9, Fraction: 0.9}, + WantGraded: true, + WantGrade: "A", + WantHeadlineHead: "2 clusters, nothing flagged in what was observed. " + + "1 of 10 scanner runs was partial.", + }, { // Test 6: A scanner that never ran on a cluster counts as a blind run. + Name: "absent run", Clusters: two, Scanners: 2, + Override: map[runKey]*scanner.Result{{scanner: "s2", cluster: "c2"}: nil}, + WantCoverage: FleetCoverage{ + ExpectedRuns: 4, ObservedRuns: 3, BlindRuns: 1, Fraction: 0.75, + }, + WantHeadlineHead: "Not enough data: 1 of 4 scanner runs did not observe the fleet.", + }, { // Test 7: Exactly the floor with every cluster observed grades. + Name: "at the floor", Clusters: five, Scanners: 2, + Override: map[runKey]*scanner.Result{ + {scanner: "s1", cluster: "c1"}: runOf(erroredRun("s1")), + {scanner: "s2", cluster: "c2"}: runOf(erroredRun("s2")), + }, + WantCoverage: FleetCoverage{ + ExpectedRuns: 10, ObservedRuns: 8, BlindRuns: 2, Fraction: 0.8, + }, + WantGraded: true, + WantGrade: "A", + WantHeadlineHead: "5 clusters, nothing flagged in what was observed. " + + "2 of 10 scanner runs did not observe the fleet.", + }, { // Test 8: Clusters with no scanner at all are blind and ungraded. + Name: "no scanners", Clusters: two, Scanners: 0, + WantCoverage: FleetCoverage{BlindClusters: 2}, + WantHeadlineHead: "Not enough data: No scanner ran on 2 listed clusters.", + }, { // Test 9: Blind and partial runs are both named. + Name: "blind and partial", Clusters: two, Scanners: 5, + Override: map[runKey]*scanner.Result{ + {scanner: "s1", cluster: "c1"}: runOf(erroredRun("s1")), + {scanner: "s2", cluster: "c2"}: runOf(degradedRun("s2", map[string]any{"n": 1})), + }, + WantCoverage: FleetCoverage{ + ExpectedRuns: 10, ObservedRuns: 8, BlindRuns: 1, Fraction: 0.8, + }, + WantGraded: true, + WantGrade: "A", + WantHeadlineHead: "2 clusters, nothing flagged in what was observed. " + + "1 of 10 scanner runs did not observe the fleet and 1 was partial.", + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d: %s", testNum, test.Name), func(t *testing.T) { + t.Parallel() + r := Build(test.Clusters, coverageResults(test.Clusters, test.Scanners, test.Override)) + got := r.FleetScore + if diff := cmp.Diff(test.WantCoverage, got.Coverage); diff != "" { + t.Errorf("coverage mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantGraded, got.Graded); diff != "" { + t.Errorf("graded mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.WantGrade, got.Grade); diff != "" { + t.Errorf("grade mismatch (-want +got):\n%s", diff) + } + if !strings.HasPrefix(got.Headline, test.WantHeadlineHead) { + t.Errorf("headline %q, want prefix %q", got.Headline, test.WantHeadlineHead) + } + label := fmt.Sprintf(" Observed-only score %d.", got.Score) + if !got.Graded && !strings.HasSuffix(got.Headline, label) { + t.Errorf("ungraded headline %q does not label the observed-only score", got.Headline) + } + if got.Coverage.BlindRuns > 0 && strings.Contains(got.Headline, "no meaningful drift") { + t.Errorf("headline %q claims no drift with blind runs", got.Headline) + } + }) + } +} + +// TestFleetCoverageDecoded verifies a report decoded from JSON yields the coverage Build +// computed, so a stored report rescored after a load grades the same way. +func TestFleetCoverageDecoded(t *testing.T) { + t.Parallel() + clusters := []string{"c1", "c2"} + built := Build(clusters, coverageResults(clusters, 3, map[runKey]*scanner.Result{ + {scanner: "s1", cluster: "c1"}: runOf(erroredRun("s1")), + })) + blob, err := json.Marshal(built) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded Report + if err := json.Unmarshal(blob, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if diff := cmp.Diff(built.FleetScore.Coverage, fleetCoverage(&decoded)); diff != "" { + t.Errorf("decoded coverage mismatch (-want +got):\n%s", diff) + } +} From 7bc9cd4aebc5fb65a96184fc7f7d040dedd49d1b Mon Sep 17 00:00:00 2001 From: Douglas Adolph <116844282+dcadolph@users.noreply.github.com> Date: Sat, 31 Oct 2026 07:05:03 -0500 Subject: [PATCH 06/13] Mark clusters unknown when a scanner run did not observe them --- cmd/cmd_top.go | 4 +- cmd/cmd_top_test.go | 12 ++-- docs/integrations/prometheus.md | 2 +- internal/compare/compare.go | 4 +- internal/compare/compare_test.go | 29 ++++++++++ internal/report/cluster_score.go | 13 ++++- internal/report/cluster_score_test.go | 31 ++++++++++ internal/report/findings.go | 15 ++++- internal/report/findings_test.go | 81 +++++++++++++++++++++++++++ internal/server/geo_handlers.go | 2 +- internal/server/metrics.go | 9 ++- internal/server/metrics_otel.go | 3 +- internal/server/metrics_test.go | 3 + internal/server/openapi.yaml | 2 +- 14 files changed, 191 insertions(+), 19 deletions(-) diff --git a/cmd/cmd_top.go b/cmd/cmd_top.go index 5682a35..e8cdf8c 100644 --- a/cmd/cmd_top.go +++ b/cmd/cmd_top.go @@ -270,7 +270,7 @@ func (s *topState) lessFn(rows []topRow) func(i, j int) bool { case sortByName: return func(i, j int) bool { return rows[i].Cluster < rows[j].Cluster } case sortByStatus: - order := map[string]int{"critical": 0, "degraded": 1, "busy": 2, "healthy": 3} + order := map[string]int{"critical": 0, "degraded": 1, "busy": 2, "unknown": 3, "healthy": 4} return func(i, j int) bool { if order[rows[i].Status] != order[rows[j].Status] { return order[rows[i].Status] < order[rows[j].Status] @@ -507,6 +507,8 @@ func statusColor(status string) string { return "33" case "busy": return "34" + case "unknown": + return "90" default: return "32" } diff --git a/cmd/cmd_top_test.go b/cmd/cmd_top_test.go index 5d74514..5354da2 100644 --- a/cmd/cmd_top_test.go +++ b/cmd/cmd_top_test.go @@ -70,7 +70,7 @@ func TestColorCodeHelpers(t *testing.T) { // Test 0: statusColor mapping. statusCases := map[string]string{ "critical": "31", "degraded": "33", "strained": "33", - "busy": "34", "healthy": "32", "unknown": "32", + "busy": "34", "healthy": "32", "unknown": "90", } for status, want := range statusCases { if got := statusColor(status); got != want { @@ -214,15 +214,19 @@ func TestLessFn(t *testing.T) { {Cluster: "beta", Status: "healthy", Score: 90}, {Cluster: "alpha", Status: "critical", Score: 40}, {Cluster: "gamma", Status: "busy", Score: 70}, + {Cluster: "delta", Status: "unknown", Score: 95}, } tests := []struct { WantOrder []string Mode topSortMode }{ - {WantOrder: []string{"alpha", "gamma", "beta"}, Mode: sortByScore}, // Test 0: Worst score first. - {WantOrder: []string{"alpha", "beta", "gamma"}, Mode: sortByName}, // Test 1: Alphabetical. - {WantOrder: []string{"alpha", "gamma", "beta"}, Mode: sortByStatus}, // Test 2: critical