diff --git a/changelog.d/fixed-10559.md b/changelog.d/fixed-10559.md new file mode 100644 index 0000000000..bec73d0ab6 --- /dev/null +++ b/changelog.d/fixed-10559.md @@ -0,0 +1 @@ +- Pull-only clusters (such as a firewalled GPU pool registered `pull_only: true`) get their node stats from spoke heartbeats again ([#10559](https://github.com/hivecommons/hive/issues/10559)). Spokes now attach cluster node health whenever a cluster id is configured, whether it comes from `hub.cluster_id` in hive.yaml or from `HIVE_CLUSTER_ID`. Before this fix only the environment variable counted, so a spoke configured through hive.yaml never sent node stats, and the hub card showed `0 nodes` under a red "cluster is pull-only: not reachable" banner. A pull-only cluster with no heartbeat health yet now shows a neutral `pull-only · awaiting spoke heartbeat (last seen: never/)` line. The card turns red only when a spoke that was reporting has gone stale. diff --git a/src/cmd/hive/boot_heartbeat_cluster_health_test.go b/src/cmd/hive/boot_heartbeat_cluster_health_test.go new file mode 100644 index 0000000000..8ef4a537b3 --- /dev/null +++ b/src/cmd/hive/boot_heartbeat_cluster_health_test.go @@ -0,0 +1,46 @@ +package main + +import ( + "testing" + + spoke "github.com/hivecommons/hive/pkg/hub/spoke" +) + +// A spoke told its cluster through hive.yaml (hub.cluster_id) — the usual +// setup for a firewalled spoke on a pull-only cluster — must still attach its +// node health to the heartbeat. Gating on HIVE_CLUSTER_ID alone dropped it, +// leaving the hub nothing to show for the cluster (#10559). +func TestHeartbeatClusterHealth_GatedOnResolvedClusterID(t *testing.T) { + report := &spoke.HeartbeatClusterHealthReport{Summary: spoke.HeartbeatClusterSummary{TotalNodes: 2}} + calls := 0 + collect := func() *spoke.HeartbeatClusterHealthReport { + calls++ + return report + } + + // cluster id from hive.yaml only; HIVE_CLUSTER_ID unset. + t.Setenv("HIVE_CLUSTER_ID", "") + cfgHub := bootHeartbeatConfig().Hub + cfgHub.ClusterID = "vllm-d" + tgt := resolveHubTarget(cfgHub, "", "") + if got := heartbeatClusterHealth(tgt.clusterID, collect); got != report { + t.Fatalf("hive.yaml cluster_id: got %v, want the collected report", got) + } + + // cluster id from the environment. + if got := heartbeatClusterHealth(resolveHubTarget(bootHeartbeatConfig().Hub, "", "vllm-d").clusterID, collect); got != report { + t.Fatalf("HIVE_CLUSTER_ID: got %v, want the collected report", got) + } + + // No cluster id anywhere: never guess, and never pay for the collection. + before := calls + if got := heartbeatClusterHealth("", collect); got != nil { + t.Fatalf("no cluster id: got %v, want nil", got) + } + if got := heartbeatClusterHealth(" ", collect); got != nil { + t.Fatalf("blank cluster id: got %v, want nil", got) + } + if calls != before { + t.Fatalf("collector ran %d time(s) with no cluster id, want 0", calls-before) + } +} diff --git a/src/cmd/hive/boot_heartbeat_payload_test.go b/src/cmd/hive/boot_heartbeat_payload_test.go index 1e3534532d..14199c6ea6 100644 --- a/src/cmd/hive/boot_heartbeat_payload_test.go +++ b/src/cmd/hive/boot_heartbeat_payload_test.go @@ -131,7 +131,8 @@ func TestBootHeartbeatCollect_FreshBootGatesUnmeasuredSignalsToNil(t *testing.T) if p.Governor.WorkSource != "" { t.Fatalf("Governor.WorkSource = %q for a default (github) source, want empty", p.Governor.WorkSource) } - // Cluster health is gated on HIVE_CLUSTER_ID, which the fixture clears: + // Cluster health is gated on the resolved cluster id (hub.cluster_id or + // HIVE_CLUSTER_ID), both of which the fixture leaves empty: // never guess health for a cluster this spoke was not told it is on. if p.ClusterHealth != nil { t.Fatalf("ClusterHealth=%v with no cluster id, want nil", p.ClusterHealth) diff --git a/src/cmd/hive/boot_seams.go b/src/cmd/hive/boot_seams.go index 79bd85c8b9..4e3989168e 100644 --- a/src/cmd/hive/boot_seams.go +++ b/src/cmd/hive/boot_seams.go @@ -2,8 +2,10 @@ package main import ( "strconv" + "strings" "github.com/hivecommons/hive/pkg/config" + spoke "github.com/hivecommons/hive/pkg/hub/spoke" ) // hiveConfigEnv is the environment variable entrypoint.sh uses to redirect @@ -68,6 +70,21 @@ func resolveHubTarget(hub config.HubConfig, envHubURL, envClusterID string) hubT return t } +// heartbeatClusterHealth returns the node-health report a spoke attaches to +// its heartbeat. It is gated on the RESOLVED cluster id (hive.yaml +// hub.cluster_id, overridden by HIVE_CLUSTER_ID) — the same id the payload +// carries as cluster_id and the hub files the report under. Gating on the +// environment variable alone silently dropped node stats for any spoke told +// its cluster through hive.yaml, which is how a firewalled spoke on a +// pull-only cluster is typically configured: the hub then had nothing to fall +// back to and showed the cluster with no nodes (#10559). +func heartbeatClusterHealth(clusterID string, collect func() *spoke.HeartbeatClusterHealthReport) *spoke.HeartbeatClusterHealthReport { + if strings.TrimSpace(clusterID) == "" || collect == nil { + return nil + } + return collect() +} + // heartbeatsToHub reports whether main() should start the hub heartbeat: the // hub must be enabled AND reachable by URL. func (t hubTarget) heartbeatsToHub() bool { diff --git a/src/cmd/hive/main.go b/src/cmd/hive/main.go index 2ce6361c6a..fbfe19c4f9 100644 --- a/src/cmd/hive/main.go +++ b/src/cmd/hive/main.go @@ -4597,12 +4597,9 @@ func (b *boot) bootHeartbeatWith(deps bootHeartbeatDeps) { GitHubAppHTTPStatus: ghAppHTTPStatus, PendingGitHubAppInstall: b.dashSrv.IsPendingGitHubAppInstall(), AutoUpgrade: b.cfg.Hub.AutoUpgrade, - ClusterHealth: func() *spoke.HeartbeatClusterHealthReport { - if os.Getenv("HIVE_CLUSTER_ID") == "" { - return nil - } + ClusterHealth: heartbeatClusterHealth(b.cfg.Hub.ClusterID, func() *spoke.HeartbeatClusterHealthReport { return spoke.CollectClusterHealth(b.logger) - }(), + }), PRsMerged90d: prsMerged, PRsRejected90d: prsRejected, CVEsClosed: cvesClosed, diff --git a/src/pkg/hub/assets/dashboard.html b/src/pkg/hub/assets/dashboard.html index aa91f89869..ffd85a9c57 100644 --- a/src/pkg/hub/assets/dashboard.html +++ b/src/pkg/hub/assets/dashboard.html @@ -141,6 +141,8 @@ .fleet-tile-label { font-size: var(--fs-2xs); color: var(--muted); text-transform: uppercase; letter-spacing: 0.04em; white-space: nowrap; } .fleet-tile.warn { border-color: rgba(245,158,11,0.4); } .fleet-tile.bad { border-color: rgba(248,81,73,0.45); } + .cluster-note { margin: var(--sp-4) var(--sp-0); padding: var(--sp-3) var(--sp-5); background: var(--surface); border: 1px solid var(--border); border-radius: var(--r); font-size: var(--fs-sm); color: var(--text-muted); } + .cluster-note.stale { color: var(--red); border-color: var(--red); } .alert-panel { border: 1px solid var(--border); border-radius: 10px; background: var(--surface); padding: var(--sp-5) var(--sp-6); } .alert-panel.has-critical { border-color: rgba(248,81,73,0.45); background: rgba(248,81,73,0.06); } .alert-panel.has-warning { border-color: rgba(245,158,11,0.4); background: rgba(245,158,11,0.05); } @@ -9272,6 +9274,16 @@

' + esc(c.error) + '' : ''; + // Pull-only clusters (#10559): the hub never reaches them, so no + // heartbeat data yet is the expected state — informational, not + // red. Red is kept for a spoke that WAS reporting and went stale. + if (!c.error && c.awaiting_heartbeat) { + errorLine = '
' + + 'pull-only · awaiting spoke heartbeat (last seen: ' + esc(c.last_seen || 'never') + ')
'; + } else if (!c.error && c.data_stale) { + errorLine = '
' + + (c.pull_only ? 'pull-only · ' : '') + 'spoke heartbeat stale — node stats last reported ' + esc(c.data_age || 'a while ago') + '
'; + } var headerHtml = '
' + clusterBadge(c.id, c.name) + '' + esc(cLabel) + '' + diff --git a/src/pkg/hub/cluster_health_pull_only_test.go b/src/pkg/hub/cluster_health_pull_only_test.go new file mode 100644 index 0000000000..8500a0bb1d --- /dev/null +++ b/src/pkg/hub/cluster_health_pull_only_test.go @@ -0,0 +1,107 @@ +package hub + +import ( + "log/slog" + "strings" + "testing" + "time" +) + +func resetClusterHealthCacheForTest() { + clusterHealthCacheMu.Lock() + clusterHealthCache = nil + clusterHealthCacheMu.Unlock() +} + +func findClusterHealth(t *testing.T, resp *ClusterHealthResponse, id string) PerClusterHealth { + t.Helper() + for _, c := range resp.Clusters { + if c.ID == id { + return c + } + } + t.Fatalf("cluster %q missing from health response: %+v", id, resp.Clusters) + return PerClusterHealth{} +} + +// A configured pull-only cluster with no heartbeat health yet is an expected, +// informational state — not the red "cluster is pull-only: not reachable" +// error banner (#10559). +func TestBuildClusterHealth_PullOnlyAwaitingHeartbeatIsNotAnError(t *testing.T) { + cleanup := helperSetupTempDirs(t) + defer cleanup() + resetClusterHealthCacheForTest() + + s := &HubServer{ + logger: slog.Default(), + clusters: map[string]ClusterConfig{"vllm-d": {ID: "vllm-d", Name: "vLLM-d (GPU)", PullOnly: true}}, + heartbeatHealth: make(map[string]*HeartbeatHealthEntry), + } + + resp, err := buildClusterHealth(s) + if err != nil { + t.Fatalf("buildClusterHealth: %v", err) + } + c := findClusterHealth(t, resp, "vllm-d") + if c.Error != "" { + t.Errorf("Error = %q, want empty for a pull-only cluster awaiting its first heartbeat", c.Error) + } + if !c.PullOnly || !c.AwaitingHeartbeat { + t.Errorf("PullOnly=%v AwaitingHeartbeat=%v, want both true", c.PullOnly, c.AwaitingHeartbeat) + } + if c.LastSeen != "never" { + t.Errorf("LastSeen = %q, want %q with no spoke registered on the cluster", c.LastSeen, "never") + } + if c.Name != "vLLM-d (GPU)" { + t.Errorf("Name = %q, want the configured cluster name", c.Name) + } +} + +// Once a spoke on the pull-only cluster reports health over the heartbeat, +// the card shows its node stats and is no longer "awaiting". +func TestBuildClusterHealth_PullOnlyUsesHeartbeatHealth(t *testing.T) { + cleanup := helperSetupTempDirs(t) + defer cleanup() + resetClusterHealthCacheForTest() + + s := &HubServer{ + logger: slog.Default(), + clusters: map[string]ClusterConfig{"vllm-d": {ID: "vllm-d", Name: "vLLM-d (GPU)", PullOnly: true}}, + heartbeatHealth: make(map[string]*HeartbeatHealthEntry), + } + s.heartbeatHealth["vllm-d"] = &HeartbeatHealthEntry{Report: sampleHeartbeatReport(), ReceivedAt: time.Now()} + + resp, err := buildClusterHealth(s) + if err != nil { + t.Fatalf("buildClusterHealth: %v", err) + } + c := findClusterHealth(t, resp, "vllm-d") + if c.Error != "" || c.AwaitingHeartbeat { + t.Errorf("Error=%q AwaitingHeartbeat=%v, want heartbeat-backed health", c.Error, c.AwaitingHeartbeat) + } + if !c.PullOnly || c.DataSource != "heartbeat" { + t.Errorf("PullOnly=%v DataSource=%q, want true/heartbeat", c.PullOnly, c.DataSource) + } + if c.Summary.TotalNodes != 1 || len(c.Nodes) != 1 { + t.Errorf("nodes = %d (summary %d), want the heartbeat-reported node", len(c.Nodes), c.Summary.TotalNodes) + } +} + +func TestLastSpokeHeartbeatForCluster(t *testing.T) { + now := time.Date(2026, 10, 5, 12, 0, 0, 0, time.UTC) + s := &HubServer{registry: Registry{Hives: []RegistryEntry{ + {ID: "a", ClusterID: "vllm-d", LastHeartbeat: now.Add(-90 * time.Minute).Format(time.RFC3339)}, + {ID: "b", ClusterID: "vllm-d", LastHeartbeat: now.Add(-7 * time.Minute).Format(time.RFC3339)}, + {ID: "c", ClusterID: "hive-oke", LastHeartbeat: now.Format(time.RFC3339)}, + {ID: "d", ClusterID: "vllm-d", LastHeartbeat: "garbage"}, + }}} + if got := s.lastSpokeHeartbeatForCluster("vllm-d", now); got != "7m ago" { + t.Errorf("vllm-d last seen = %q, want the most recent spoke on that cluster (7m ago)", got) + } + if got := s.lastSpokeHeartbeatForCluster("other", now); got != "never" { + t.Errorf("other last seen = %q, want never", got) + } + if got := s.lastSpokeHeartbeatForCluster("hive-oke", now); !strings.Contains(got, "just now") { + t.Errorf("hive-oke last seen = %q, want just now", got) + } +} diff --git a/src/pkg/hub/saas_cluster_health.go b/src/pkg/hub/saas_cluster_health.go index 90179ec925..d63399ea4b 100644 --- a/src/pkg/hub/saas_cluster_health.go +++ b/src/pkg/hub/saas_cluster_health.go @@ -131,6 +131,16 @@ type PerClusterHealth struct { DataSource string `json:"data_source,omitempty"` // "heartbeat" when data comes from spoke heartbeat instead of kubectl DataStale bool `json:"data_stale,omitempty"` // true when heartbeat data is older than heartbeatHealthStaleness DataAge string `json:"data_age,omitempty"` // human-readable age or collection timestamp + // PullOnly marks a cluster the hub cannot reach (ClusterConfig.PullOnly): + // its health exists only as what its spokes report over the heartbeat. + PullOnly bool `json:"pull_only,omitempty"` + // AwaitingHeartbeat is set for a pull-only cluster the hub holds no + // heartbeat health for yet. It is an informational state, not a fault — + // the hub cannot query the cluster by design — so Error stays empty and the + // dashboard renders it neutrally. LastSeen says when any spoke on the + // cluster last heartbeated ("never" when none has since the hub started). + AwaitingHeartbeat bool `json:"awaiting_heartbeat,omitempty"` + LastSeen string `json:"last_seen,omitempty"` // StuckPods reports hive-namespace pods stuck Terminating — the residue of // nodes disappearing without draining (#5328 item 3). Nil means the hub // could not determine it (unreachable cluster, pull-only pool, failed @@ -298,9 +308,11 @@ func buildClusterHealth(s *HubServer) (*ClusterHealthResponse, error) { } else { s.logger.Warn("cluster health query failed", "cluster", cID, "error", res.err) } + pullOnly := errors.Is(res.err, errClusterPullOnly) // Fall back to heartbeat-reported health if available. if hbHealth := s.getHeartbeatHealthForCluster(cID); hbHealth != nil { pch := convertHeartbeatToPerClusterHealth(cID, s.clusterNameForID(cID), hbHealth, hiveCounts[cID]) + pch.PullOnly = pullOnly perCluster = append(perCluster, pch) allNodes = append(allNodes, pch.Nodes...) aggCPUCores += pch.Summary.TotalCPUCores @@ -311,6 +323,12 @@ func buildClusterHealth(s *HubServer) (*ClusterHealthResponse, error) { s.logger.Info("cluster health: using heartbeat fallback", "cluster", cID) continue } + if pullOnly { + // Unreachable by design and no spoke has reported yet: say + // so neutrally rather than as an error banner. + perCluster = append(perCluster, awaitingHeartbeatHealth(cID, s.clusterNameForID(cID), hiveCounts[cID], s.lastSpokeHeartbeatForCluster(cID, time.Now()))) + continue + } perCluster = append(perCluster, PerClusterHealth{ ID: cID, Name: s.clusterNameForID(cID), @@ -804,6 +822,55 @@ func (s *HubServer) getHeartbeatHealthForCluster(clusterID string) *HeartbeatHea return entry } +// awaitingHeartbeatHealth is the health entry for a pull-only cluster the hub +// holds no heartbeat-reported health for. It carries no Error: the hub not +// reaching the cluster is the configured design, not a fault. +func awaitingHeartbeatHealth(clusterID, clusterName string, hiveCount int, lastSeen string) PerClusterHealth { + return PerClusterHealth{ + ID: clusterID, + Name: clusterName, + Summary: ClusterHealthSummary{HiveCount: hiveCount}, + HiveCount: hiveCount, + DataSource: "heartbeat", + PullOnly: true, + AwaitingHeartbeat: true, + LastSeen: lastSeen, + } +} + +// lastSpokeHeartbeatForCluster reports when any registry hive on clusterID +// last heartbeated, as " ago", or "never" when none has. It answers the +// operator's real question for a pull-only cluster with no health data: is a +// spoke there talking to the hub at all (and simply not reporting node +// metrics), or is nothing heartbeating from it? +func (s *HubServer) lastSpokeHeartbeatForCluster(clusterID string, now time.Time) string { + var latest time.Time + s.mu.RLock() + for _, h := range s.registry.Hives { + if h.ClusterID != clusterID || h.LastHeartbeat == "" { + continue + } + if t, err := time.Parse(time.RFC3339, h.LastHeartbeat); err == nil && t.After(latest) { + latest = t + } + } + s.mu.RUnlock() + if latest.IsZero() { + return "never" + } + age := now.Sub(latest) + switch { + case age < time.Minute: + return "just now" + case age < time.Hour: + return fmt.Sprintf("%dm ago", int(age.Minutes())) + case age < 48*time.Hour: + return fmt.Sprintf("%dh ago", int(age.Hours())) + default: + return fmt.Sprintf("%dd ago", int(age.Hours()/24)) + } +} + // convertHeartbeatToPerClusterHealth converts heartbeat-reported health data // into the hub's PerClusterHealth format for display. If the data is older // than heartbeatHealthStaleness, it is marked with a staleness warning.