diff --git a/internal/hostagent/ceph.go b/internal/hostagent/ceph.go index 4a86cd1f4..28a19ed41 100644 --- a/internal/hostagent/ceph.go +++ b/internal/hostagent/ceph.go @@ -240,9 +240,14 @@ func parseCephStatus(data []byte) (*CephClusterStatus, error) { } `json:"detail"` } `json:"checks"` } `json:"health"` - MonMap struct { + // quorum/quorum_names sit at the top level of `ceph status` output. + QuorumNames []string `json:"quorum_names"` + Quorum []json.RawMessage `json:"quorum"` + MonMap struct { Epoch int `json:"epoch"` - Mons []struct { + // Quincy+ reports num_mons without a mons array. + NumMons int `json:"num_mons"` + Mons []struct { Name string `json:"name"` Rank int `json:"rank"` Addr string `json:"addr"` @@ -252,7 +257,9 @@ func parseCephStatus(data []byte) (*CephClusterStatus, error) { Available bool `json:"available"` NumActive int `json:"num_active_name,omitempty"` ActiveName string `json:"active_name"` - Standbys []struct { + // Quincy+ reports num_standbys without a standbys array. + NumStandbys int `json:"num_standbys"` + Standbys []struct { Name string `json:"name"` } `json:"standbys"` } `json:"mgrmap"` @@ -281,6 +288,13 @@ func parseCephStatus(data []byte) (*CephClusterStatus, error) { return nil, fmt.Errorf("ceph.parseStatus: unmarshal ceph status JSON: %w", err) } + numMons := max(raw.MonMap.NumMons, len(raw.MonMap.Mons), len(raw.QuorumNames), len(raw.Quorum)) + activeMgrs := 0 + if raw.MgrMap.Available || raw.MgrMap.ActiveName != "" { + activeMgrs = 1 + } + standbyMgrs := max(raw.MgrMap.NumStandbys, len(raw.MgrMap.Standbys)) + status := &CephClusterStatus{ FSID: raw.FSID, Health: CephHealthStatus{ @@ -289,13 +303,13 @@ func parseCephStatus(data []byte) (*CephClusterStatus, error) { }, MonMap: CephMonitorMap{ Epoch: raw.MonMap.Epoch, - NumMons: len(raw.MonMap.Mons), + NumMons: numMons, }, MgrMap: CephManagerMap{ Available: raw.MgrMap.Available, - NumMgrs: 1 + len(raw.MgrMap.Standbys), + NumMgrs: activeMgrs + standbyMgrs, ActiveMgr: raw.MgrMap.ActiveName, - Standbys: len(raw.MgrMap.Standbys), + Standbys: standbyMgrs, }, OSDMap: CephOSDMap{ Epoch: raw.OSDMap.Epoch, @@ -347,9 +361,14 @@ func parseCephStatus(data []byte) (*CephClusterStatus, error) { } } - // Build service summary + // Build service summary. Monitors in quorum are the ones known to be running; + // fall back to the total count when quorum data is absent. + monsRunning := max(len(raw.QuorumNames), len(raw.Quorum)) + if monsRunning == 0 { + monsRunning = numMons + } status.Services = []CephServiceInfo{ - {Type: "mon", Running: len(raw.MonMap.Mons), Total: len(raw.MonMap.Mons)}, + {Type: "mon", Running: monsRunning, Total: numMons}, {Type: "mgr", Running: cephBoolToInt(raw.MgrMap.Available), Total: status.MgrMap.NumMgrs}, {Type: "osd", Running: raw.OSDMap.NumUp, Total: raw.OSDMap.NumOSDs}, } diff --git a/internal/hostagent/issue1626_ceph_squid_status_test.go b/internal/hostagent/issue1626_ceph_squid_status_test.go new file mode 100644 index 000000000..47ccef9c1 --- /dev/null +++ b/internal/hostagent/issue1626_ceph_squid_status_test.go @@ -0,0 +1,125 @@ +package hostagent + +import ( + "context" + "testing" +) + +// Ceph Quincy+ (including Squid on PVE 9) reports monmap.num_mons and +// mgrmap.num_standbys instead of the mons/standbys arrays the legacy schema +// used. Issue #1626: the parser only read the arrays, so MON count came back 0 +// and MGR count came back 1 on a 3-node cluster with an active + standby mgr. +const issue1626SquidStatusJSON = `{ + "fsid": "9d4c2f0a-1626-4f6e-9b7a-squid0000001", + "health": {"status": "HEALTH_OK", "checks": {}, "mutes": []}, + "election_epoch": 148, + "quorum": [0, 1, 2], + "quorum_names": ["pve1", "pve2", "pve3"], + "quorum_age": 4161, + "monmap": { + "epoch": 3, + "min_mon_release_name": "squid", + "num_mons": 3 + }, + "mgrmap": { + "available": true, + "num_standbys": 1, + "modules": ["balancer", "crash", "devicehealth", "orchestrator"], + "services": {} + }, + "osdmap": {"epoch": 214, "num_osds": 6, "num_up_osds": 6, "num_in_osds": 6, "num_remapped_pgs": 0}, + "pgmap": { + "num_pgs": 129, + "bytes_total": 12002349744128, + "bytes_used": 3000587436032, + "bytes_avail": 9001762308096, + "data_bytes": 999862272000 + } +}` + +func TestIssue1626ParseCephStatusSquidSchema(t *testing.T) { + status, err := parseCephStatus([]byte(issue1626SquidStatusJSON)) + if err != nil { + t.Fatalf("parseCephStatus returned error: %v", err) + } + + if status.MonMap.NumMons != 3 { + t.Errorf("MonMap.NumMons = %d, want 3", status.MonMap.NumMons) + } + if status.MgrMap.NumMgrs != 2 { + t.Errorf("MgrMap.NumMgrs = %d, want 2 (active + num_standbys)", status.MgrMap.NumMgrs) + } + if status.MgrMap.Standbys != 1 { + t.Errorf("MgrMap.Standbys = %d, want 1", status.MgrMap.Standbys) + } + if !status.MgrMap.Available { + t.Error("MgrMap.Available = false, want true") + } + + var monService, mgrService *CephServiceInfo + for i := range status.Services { + switch status.Services[i].Type { + case "mon": + monService = &status.Services[i] + case "mgr": + mgrService = &status.Services[i] + } + } + if monService == nil || monService.Running != 3 || monService.Total != 3 { + t.Errorf("mon service = %+v, want Running 3 / Total 3", monService) + } + if mgrService == nil || mgrService.Running != 1 || mgrService.Total != 2 { + t.Errorf("mgr service = %+v, want Running 1 / Total 2", mgrService) + } +} + +func TestIssue1626CollectCephSquidSchema(t *testing.T) { + withLookPath(t, func(file string) (string, error) { return fakeCephBinary, nil }) + withCommandRunner(t, func(ctx context.Context, name string, args ...string) ([]byte, []byte, error) { + if len(args) > 0 && args[0] == "status" { + return []byte(issue1626SquidStatusJSON), nil, nil + } + return []byte(`{"stats":{},"pools":[]}`), nil, nil + }) + + status, err := CollectCeph(context.Background()) + if err != nil { + t.Fatalf("CollectCeph returned error: %v", err) + } + if status == nil { + t.Fatal("CollectCeph returned nil status") + } + if status.MonMap.NumMons != 3 { + t.Errorf("MonMap.NumMons = %d, want 3", status.MonMap.NumMons) + } + if status.MgrMap.NumMgrs != 2 { + t.Errorf("MgrMap.NumMgrs = %d, want 2", status.MgrMap.NumMgrs) + } +} + +// Legacy (pre-Quincy) payloads with mons/standbys arrays must keep working. +func TestIssue1626ParseCephStatusLegacySchemaUnchanged(t *testing.T) { + legacy := []byte(`{ + "fsid": "legacy-fsid", + "health": {"status": "HEALTH_OK", "checks": {}}, + "monmap": {"epoch": 7, "mons": [ + {"name": "a", "rank": 0, "addr": "10.0.0.1"}, + {"name": "b", "rank": 1, "addr": "10.0.0.2"}, + {"name": "c", "rank": 2, "addr": "10.0.0.3"} + ]}, + "mgrmap": {"available": true, "active_name": "mgr-a", "standbys": [{"name": "mgr-b"}]}, + "osdmap": {"epoch": 3, "num_osds": 3, "num_up_osds": 3, "num_in_osds": 3}, + "pgmap": {"num_pgs": 64, "bytes_total": 1000, "bytes_used": 250, "bytes_avail": 750} + }`) + + status, err := parseCephStatus(legacy) + if err != nil { + t.Fatalf("parseCephStatus returned error: %v", err) + } + if status.MonMap.NumMons != 3 || len(status.MonMap.Monitors) != 3 { + t.Errorf("legacy MonMap = %+v, want 3 monitors", status.MonMap) + } + if status.MgrMap.NumMgrs != 2 || status.MgrMap.ActiveMgr != "mgr-a" || status.MgrMap.Standbys != 1 { + t.Errorf("legacy MgrMap = %+v, want 2 mgrs with active mgr-a", status.MgrMap) + } +} diff --git a/internal/models/ceph_cluster_identity.go b/internal/models/ceph_cluster_identity.go index 2c189d08c..b6e027f89 100644 --- a/internal/models/ceph_cluster_identity.go +++ b/internal/models/ceph_cluster_identity.go @@ -121,10 +121,12 @@ func supplementCephCluster(primary, supplemental CephCluster) CephCluster { primary.AvailableBytes = supplemental.AvailableBytes primary.UsagePercent = supplemental.UsagePercent } - if primary.NumMons == 0 { + // Prefer the larger daemon counts: sources parsing legacy schemas can + // undercount MONs/MGRs on modern Ceph releases (issue #1626). + if supplemental.NumMons > primary.NumMons { primary.NumMons = supplemental.NumMons } - if primary.NumMgrs == 0 { + if supplemental.NumMgrs > primary.NumMgrs { primary.NumMgrs = supplemental.NumMgrs } if primary.NumOSDs == 0 { diff --git a/internal/monitoring/ceph.go b/internal/monitoring/ceph.go index aab8c9bc2..354cce2d2 100644 --- a/internal/monitoring/ceph.go +++ b/internal/monitoring/ceph.go @@ -9,6 +9,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/models" + monerrors "github.com/rcourtman/pulse-go-rewrite/internal/monitoring/errors" "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" "github.com/rs/zerolog/log" ) @@ -40,7 +41,12 @@ func (m *Monitor) pollCephCluster(ctx context.Context, instanceName string, clie status, err := client.GetCephStatus(cephCtx) if err != nil { - log.Debug().Err(err).Str("instance", instanceName).Msg("ceph status unavailable – preserving previous ceph state") + if monerrors.IsAuthError(err) { + log.Warn().Err(err).Str("instance", instanceName). + Msg("ceph status request denied (401/403) – grant the Pulse token Sys.Audit on path / to enable Ceph monitoring") + } else { + log.Debug().Err(err).Str("instance", instanceName).Msg("ceph status unavailable – preserving previous ceph state") + } return } if status == nil { @@ -238,6 +244,13 @@ func countCephMonitorDaemons(status *proxmox.CephStatus) int { if len(status.MonMap.Quorum) > 0 { return len(status.MonMap.Quorum) } + // Ceph places quorum membership at the top level of the status payload. + if len(status.QuorumNames) > 0 { + return len(status.QuorumNames) + } + if len(status.Quorum) > 0 { + return len(status.Quorum) + } return countServiceDaemons(status.ServiceMap.Services, "mon") } @@ -248,8 +261,15 @@ func countCephManagerDaemons(status *proxmox.CephStatus) int { if status.MgrMap.NumMgrs > 0 { return status.MgrMap.NumMgrs } - if status.MgrMap.ActiveName != "" { - return 1 + len(status.MgrMap.Standbys) + // Quincy+ drops num_mgrs/active_name from mgrmap; derive the count from + // availability plus standbys (num_standbys on modern releases). + active := 0 + if status.MgrMap.Available || status.MgrMap.ActiveName != "" { + active = 1 + } + standbys := max(status.MgrMap.NumStandbys, len(status.MgrMap.Standbys)) + if active+standbys > 0 { + return active + standbys } return countServiceDaemons(status.ServiceMap.Services, "mgr") } diff --git a/internal/monitoring/issue1626_ceph_squid_status_test.go b/internal/monitoring/issue1626_ceph_squid_status_test.go new file mode 100644 index 000000000..b4eadefff --- /dev/null +++ b/internal/monitoring/issue1626_ceph_squid_status_test.go @@ -0,0 +1,82 @@ +package monitoring + +import ( + "encoding/json" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" + agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" +) + +// Issue #1626: on PVE 9 / Ceph Squid the status payload has no monmap mons +// array, no mgrmap num_mgrs/active_name/standbys, and quorum membership lives +// at the top level. A 3-node cluster with 2 managers rendered as 0 MONs / 1 MGR. +const issue1626SquidStatusJSON = `{ + "fsid": "9d4c2f0a-1626-4f6e-9b7a-squid0000001", + "health": {"status": "HEALTH_OK", "checks": {}, "mutes": []}, + "election_epoch": 148, + "quorum": [0, 1, 2], + "quorum_names": ["pve1", "pve2", "pve3"], + "quorum_age": 4161, + "monmap": { + "epoch": 3, + "min_mon_release_name": "squid" + }, + "mgrmap": { + "available": true, + "num_standbys": 1, + "modules": ["balancer", "crash", "devicehealth"], + "services": {} + }, + "servicemap": {"services": {}}, + "osdmap": {"num_osds": 6, "num_up_osds": 6, "num_in_osds": 6}, + "pgmap": {"num_pgs": 129, "bytes_total": 12002349744128, "bytes_used": 3000587436032, "bytes_avail": 9001762308096} +}` + +func TestIssue1626BuildCephClusterModelSquidSchema(t *testing.T) { + var status proxmox.CephStatus + if err := json.Unmarshal([]byte(issue1626SquidStatusJSON), &status); err != nil { + t.Fatalf("unmarshal squid status: %v", err) + } + + cluster := buildCephClusterModel("pve-squid", &status, nil) + + if cluster.NumMons != 3 { + t.Errorf("NumMons = %d, want 3", cluster.NumMons) + } + if cluster.NumMgrs != 2 { + t.Errorf("NumMgrs = %d, want 2 (available active + num_standbys)", cluster.NumMgrs) + } + if cluster.NumOSDs != 6 || cluster.NumOSDsUp != 6 { + t.Errorf("OSD counts = %d/%d up, want 6/6", cluster.NumOSDs, cluster.NumOSDsUp) + } +} + +func TestIssue1626ConvertAgentCephSquidCounts(t *testing.T) { + // Counts as produced by the fixed hostagent parser for the same + // Squid-shaped payload (see internal/hostagent/issue1626_ceph_squid_status_test.go). + agentCeph := &agentshost.CephCluster{ + FSID: "9d4c2f0a-1626-4f6e-9b7a-squid0000001", + Health: agentshost.CephHealth{ + Status: "HEALTH_OK", + }, + MonMap: agentshost.CephMonitorMap{Epoch: 3, NumMons: 3}, + MgrMap: agentshost.CephManagerMap{Available: true, NumMgrs: 2, Standbys: 1}, + OSDMap: agentshost.CephOSDMap{NumOSDs: 6, NumUp: 6, NumIn: 6}, + PGMap: agentshost.CephPGMap{NumPGs: 129}, + } + + cluster := convertAgentCephToGlobalCluster(agentCeph, "pve1", "host-1", time.Now()) + + if cluster.NumMons != 3 { + t.Errorf("NumMons = %d, want 3", cluster.NumMons) + } + if cluster.NumMgrs != 2 { + t.Errorf("NumMgrs = %d, want 2", cluster.NumMgrs) + } + if cluster.Source != models.CephClusterSourceHostAgent { + t.Errorf("Source = %q, want %q", cluster.Source, models.CephClusterSourceHostAgent) + } +} diff --git a/pkg/proxmox/ceph.go b/pkg/proxmox/ceph.go index 87a15dc82..64480db98 100644 --- a/pkg/proxmox/ceph.go +++ b/pkg/proxmox/ceph.go @@ -16,6 +16,11 @@ type CephStatus struct { MgrMap CephMgrMap `json:"mgrmap"` OSDMap CephOSDMap `json:"osdmap"` PGMap CephPGMap `json:"pgmap"` + // Ceph reports quorum membership at the top level of the status payload, + // not inside monmap. Quincy+ omits the monmap mons array, so these are the + // only per-monitor signals available there. + QuorumNames []string `json:"quorum_names,omitempty"` + Quorum []json.RawMessage `json:"quorum,omitempty"` } // CephHealth captures cluster health status and summaries. @@ -160,18 +165,21 @@ func maxCephMonitorCount(values ...int) int { // CephMgrMap captures manager summary information. type CephMgrMap struct { - Available bool `json:"available"` - NumMgrs int `json:"num_mgrs"` - ActiveName string `json:"active_name"` - Standbys []string `json:"standbys"` + Available bool `json:"available"` + NumMgrs int `json:"num_mgrs"` + ActiveName string `json:"active_name"` + // Quincy+ reports num_standbys without a standbys array. + NumStandbys int `json:"num_standbys"` + Standbys []string `json:"standbys"` } func (m *CephMgrMap) UnmarshalJSON(data []byte) error { type rawCephMgrMap struct { - Available bool `json:"available"` - NumMgrs int `json:"num_mgrs"` - ActiveName string `json:"active_name"` - Standbys []json.RawMessage `json:"standbys"` + Available bool `json:"available"` + NumMgrs int `json:"num_mgrs"` + ActiveName string `json:"active_name"` + NumStandbys int `json:"num_standbys"` + Standbys []json.RawMessage `json:"standbys"` } var raw rawCephMgrMap @@ -205,6 +213,8 @@ func (m *CephMgrMap) UnmarshalJSON(data []byte) error { } } + m.NumStandbys = max(raw.NumStandbys, len(m.Standbys)) + return nil } diff --git a/pkg/proxmox/issue1626_ceph_squid_status_test.go b/pkg/proxmox/issue1626_ceph_squid_status_test.go new file mode 100644 index 000000000..7e652b805 --- /dev/null +++ b/pkg/proxmox/issue1626_ceph_squid_status_test.go @@ -0,0 +1,84 @@ +package proxmox + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" +) + +// Issue #1626: PVE 9 / Ceph Squid payloads omit mgrmap num_mgrs/active_name +// and the monmap mons array. The Squid mgrmap only carries available + +// num_standbys, and quorum membership lives at the top level of the payload. +const issue1626SquidAPIPayload = `{ + "data": { + "fsid": "9d4c2f0a-1626-4f6e-9b7a-squid0000001", + "health": {"status": "HEALTH_OK", "checks": {}, "mutes": []}, + "election_epoch": 148, + "quorum": [0, 1, 2], + "quorum_names": ["pve1", "pve2", "pve3"], + "quorum_age": 4161, + "monmap": { + "epoch": 3, + "min_mon_release_name": "squid", + "num_mons": 3 + }, + "mgrmap": { + "available": true, + "num_standbys": 1, + "modules": ["balancer", "crash", "devicehealth"], + "services": {} + }, + "servicemap": {"services": {}}, + "osdmap": {"num_osds": 6, "num_up_osds": 6, "num_in_osds": 6}, + "pgmap": {"num_pgs": 129, "bytes_total": 12002349744128, "bytes_used": 3000587436032, "bytes_avail": 9001762308096} + } +}` + +func TestIssue1626GetCephStatusSquidSchema(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/cluster/ceph/status" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(issue1626SquidAPIPayload)) + })) + defer server.Close() + + client := &Client{baseURL: server.URL, httpClient: server.Client()} + status, err := client.GetCephStatus(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if status.MonMap.NumMons != 3 { + t.Errorf("MonMap.NumMons = %d, want 3", status.MonMap.NumMons) + } + if !status.MgrMap.Available { + t.Error("MgrMap.Available = false, want true") + } + if status.MgrMap.NumStandbys != 1 { + t.Errorf("MgrMap.NumStandbys = %d, want 1", status.MgrMap.NumStandbys) + } + if len(status.QuorumNames) != 3 { + t.Errorf("QuorumNames = %v, want 3 entries", status.QuorumNames) + } + if len(status.Quorum) != 3 { + t.Errorf("Quorum = %v, want 3 entries", status.Quorum) + } +} + +// Legacy payloads with a standbys array must still populate NumStandbys. +func TestIssue1626CephMgrMapLegacyStandbysArray(t *testing.T) { + var mgrMap CephMgrMap + if err := mgrMap.UnmarshalJSON([]byte(`{"available": true, "active_name": "mgr-a", "standbys": ["mgr-b", "mgr-c"]}`)); err != nil { + t.Fatalf("UnmarshalJSON returned error: %v", err) + } + if mgrMap.NumStandbys != 2 { + t.Errorf("NumStandbys = %d, want 2 (derived from standbys array)", mgrMap.NumStandbys) + } + if len(mgrMap.Standbys) != 2 { + t.Errorf("Standbys = %v, want 2 entries", mgrMap.Standbys) + } +}