From d7225a45a009001abc8bc32d1d353e10b50c06a7 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Tue, 5 May 2026 14:55:26 +0100 Subject: [PATCH] Fix Proxmox guest memory fallbacks Also fixes Ceph pool threshold resource identity. Refs #1341 --- .../v6/internal/subsystems/alerts.md | 9 +- .../subsystems/frontend-primitives.md | 15 +-- .../v6/internal/subsystems/monitoring.md | 5 + .../internal/subsystems/unified-resources.md | 6 ++ .../Workloads/__tests__/DiskList.test.tsx | 9 +- .../__tests__/alertOverridesModel.test.ts | 44 +++++++- .../__tests__/useAlertOverridesState.test.tsx | 6 +- .../features/alerts/alertOverridesModel.ts | 49 +++++++-- .../hooks/useThresholdsInfrastructureData.ts | 13 ++- .../pages/__tests__/Alerts.helpers.test.ts | 6 +- .../utils/__tests__/metricThresholds.test.ts | 4 +- .../workloadGuestPresentation.test.ts | 3 + frontend-modern/src/utils/metricThresholds.ts | 5 +- .../src/utils/workloadGuestPresentation.ts | 47 ++++---- internal/models/ceph_pool_storage_test.go | 33 ++++++ internal/models/models.go | 63 +++++++++++ .../monitoring/canonical_guardrails_test.go | 4 +- internal/monitoring/ceph.go | 27 +++++ internal/monitoring/ceph_test.go | 56 ++++++++++ internal/monitoring/diagnostic_snapshots.go | 9 ++ internal/monitoring/guest_agent_evidence.go | 2 +- internal/monitoring/guest_disk_stability.go | 3 - .../monitoring/guest_disk_stability_test.go | 46 +++++++- internal/monitoring/guest_memory_sources.go | 48 +++++++-- .../monitoring/guest_memory_sources_test.go | 14 +++ internal/monitoring/guest_memory_stability.go | 2 +- internal/monitoring/guest_metadata.go | 2 +- .../memory_trust_characterization_test.go | 14 +++ internal/monitoring/monitor_polling_vm.go | 11 +- .../monitoring/monitor_pve_guest_builders.go | 11 +- .../unifiedresources/code_standards_test.go | 34 ++++++ internal/unifiedresources/registry.go | 3 + internal/unifiedresources/registry_test.go | 48 +++++++++ pkg/proxmox/client.go | 102 +++++++++++++----- pkg/proxmox/client_test.go | 46 +++++++- .../tests/66-ceph-alert-thresholds.spec.ts | 50 ++++++++- 36 files changed, 744 insertions(+), 105 deletions(-) create mode 100644 internal/models/ceph_pool_storage_test.go diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 754792817..d7ab83c18 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -638,10 +638,11 @@ config normalization, factory defaults, docker-gap validation, and save-payload serialization, `frontend-modern/src/features/alerts/alertOverridesModel.ts` for raw override normalization plus resource-backed override projection. That override owner -must canonicalize legacy per-node shared-storage keys onto the current -cluster-scoped storage resource id before the thresholds surface rebuilds, -so old Ceph/shared-datastore overrides still surface on the live v6 editor -instead of disappearing after the feature-shell migration, and +must canonicalize legacy per-node shared-storage keys and hashed storage +resource ids onto the storage metrics target id before the thresholds surface +rebuilds, so old Ceph/shared-datastore overrides and newly projected Ceph pool +overrides still surface on the live v6 editor instead of disappearing after +the feature-shell migration, and `frontend-modern/src/features/alerts/useAlertOverridesState.ts` for reactive override state, derived resource lists, and overview handoff, and `frontend-modern/src/features/alerts/alertDestinationsModel.ts` for email and diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 48e9456b1..990667005 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -1046,9 +1046,11 @@ That same shared alerts feature boundary now also owns legacy shared-storage override migration. `frontend-modern/src/features/alerts/alertOverridesModel.ts` and `frontend-modern/src/features/alerts/useAlertOverridesState.ts` must canonicalize per-node shared-storage override keys such as -`Main-pve1-ceph-pool` onto the current cluster-scoped storage resource id -before the thresholds table derives rows, so old Ceph override records survive -the v6 feature-shell path instead of silently disappearing from the live editor. +`Main-pve1-ceph-pool`, hashed `/api/resources` storage ids, and Ceph pool +storage rows onto the storage metrics target id before the thresholds table +derives rows, so old Ceph override records and newly projected Ceph pool +overrides survive the v6 feature-shell path instead of silently disappearing +from the live editor. The frontend already has several guardrail tests. The next step is to keep turning repeated local patterns into explicit shared primitives with hard usage @@ -2435,9 +2437,10 @@ normalization, factory defaults, docker-gap validation, and payload serialization, `frontend-modern/src/features/alerts/alertOverridesModel.ts` for override normalization and resource-backed projection. That shared feature-model boundary must also canonicalize legacy shared-storage override -keys onto the current storage resource id before thresholds rows are derived, -so migrated Ceph/shared-datastore overrides survive the feature-shell path -instead of dropping out of the live editor, and +keys and hashed storage resource ids onto the storage metrics target id before +thresholds rows are derived, so migrated Ceph/shared-datastore overrides and +Ceph pool overrides survive the feature-shell path instead of dropping out of +the live editor, and `frontend-modern/src/features/alerts/useAlertOverridesState.ts` for reactive override state and thresholds-facing resource selectors, and `frontend-modern/src/features/alerts/alertDestinationsModel.ts` for diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 4dff9a9fe..7889e9d1d 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -359,6 +359,11 @@ through the resolved unified-resource metrics target, so seeded history, runtime writes, storage summary hover selection, and detail charts all extend one series instead of splitting between canonical resource IDs and source-native metric IDs. +Proxmox Ceph pools are part of that same storage-series contract. When Ceph DF +exposes pools, monitoring must project each pool through the shared +`models.CephPoolStorage` helper, write storage history under that pool storage +id, and evaluate alerts through `CheckStorage` so per-pool thresholds, active +alerts, and charts all use the same storage series identity. That same chart boundary also owns provider-backed workload bridging. Workload-chart consumers may query VM and system-container history through the resolved unified-resource metrics target, but the emitted series identity must diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 741034352..3cb505143 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -638,6 +638,12 @@ selection, `/api/storage-charts`, and `/api/charts/storage-summary` all see the same canonical history identity instead of splitting between view-cache resource IDs and API serialization-time metric IDs. +Proxmox Ceph pools are canonical storage resources when present in +`CephCluster.Pools`. `internal/models.CephPoolStorageID` owns the stable +source and metrics identity, and `internal/unifiedresources/registry.go` must +project each pool through normal storage ingest so API resources expose a +storage metrics target whose resource id is the storage runtime metric id +rather than the hashed unified-resource id. That same VMware contract now also includes the identity rule. VMware managed object identifiers are phase-1 provider identities, but they must be scoped by the owning `vCenter` connection or discovered vCenter identity so bare object diff --git a/frontend-modern/src/components/Workloads/__tests__/DiskList.test.tsx b/frontend-modern/src/components/Workloads/__tests__/DiskList.test.tsx index 3fe6bec86..a363ad538 100644 --- a/frontend-modern/src/components/Workloads/__tests__/DiskList.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/DiskList.test.tsx @@ -1,10 +1,7 @@ import { describe, it, expect, afterEach } from 'vitest'; import { render, screen, cleanup } from '@solidjs/testing-library'; import { DiskList } from '../DiskList'; -import { - buildWorkloadsDiskPresentation, - getWorkloadsDiskUsagePercent, -} from '../diskListModel'; +import { buildWorkloadsDiskPresentation, getWorkloadsDiskUsagePercent } from '../diskListModel'; import type { Disk } from '@/types/api'; function makeDisk(overrides: Partial = {}): Disk { @@ -62,6 +59,10 @@ describe('DiskList', () => { ], ['agent-error', 'Error communicating with guest agent.'], ['no-data', 'No disk data available from Proxmox API.'], + [ + 'prev-no-filesystems', + 'Using last known disk stats. No filesystems found. VM may be booting or using a Live ISO.', + ], ] as const)('shows correct tooltip for diskStatusReason="%s"', (reason, expected) => { render(() => ); expect(screen.getByText('-')).toHaveAttribute('title', expected); diff --git a/frontend-modern/src/features/alerts/__tests__/alertOverridesModel.test.ts b/frontend-modern/src/features/alerts/__tests__/alertOverridesModel.test.ts index feb929989..19f1143a4 100644 --- a/frontend-modern/src/features/alerts/__tests__/alertOverridesModel.test.ts +++ b/frontend-modern/src/features/alerts/__tests__/alertOverridesModel.test.ts @@ -33,10 +33,14 @@ describe('alertOverridesModel', () => { it('normalizes legacy shared-storage override ids into canonical storage resource ids', () => { const storage = makeResource({ - id: 'Main-cluster-ceph-pool', + id: 'storage-4a40f1c6', name: 'ceph-pool', type: 'storage', platformId: 'Main', + metricsTarget: { + resourceType: 'storage', + resourceId: 'Main-cluster-ceph-pool', + }, proxmox: { instance: 'Main', node: 'cluster', @@ -65,6 +69,38 @@ describe('alertOverridesModel', () => { }); }); + it('normalizes hashed storage resource ids into storage metrics target ids', () => { + const storage = makeResource({ + id: 'storage-4a40f1c6', + name: 'data_replication', + type: 'storage', + metricsTarget: { + resourceType: 'storage', + resourceId: 'Main-ceph-pool-data_replication', + }, + storage: { + shared: true, + isCeph: true, + type: 'ceph', + }, + }); + + expect( + normalizeRawOverridesConfig( + { + 'storage-4a40f1c6': { + usage: { trigger: 70, clear: 65 }, + } as any, + }, + [storage], + ), + ).toEqual({ + 'Main-ceph-pool-data_replication': { + usage: { trigger: 70, clear: 65 }, + }, + }); + }); + it('projects guest overrides without requiring agent-backed resources', () => { const guest = makeResource({ id: 'cluster-a:node-2:100', @@ -209,11 +245,15 @@ describe('alertOverridesModel', () => { it('projects shared-storage overrides through the canonical storage resource id', () => { const storage = makeResource({ - id: 'Main-cluster-ceph-pool', + id: 'storage-4a40f1c6', name: 'ceph-pool', displayName: 'ceph-pool', type: 'storage', platformId: 'Main', + metricsTarget: { + resourceType: 'storage', + resourceId: 'Main-cluster-ceph-pool', + }, proxmox: { instance: 'Main', node: 'cluster', diff --git a/frontend-modern/src/features/alerts/__tests__/useAlertOverridesState.test.tsx b/frontend-modern/src/features/alerts/__tests__/useAlertOverridesState.test.tsx index c6f6a4079..0b6b3bfce 100644 --- a/frontend-modern/src/features/alerts/__tests__/useAlertOverridesState.test.tsx +++ b/frontend-modern/src/features/alerts/__tests__/useAlertOverridesState.test.tsx @@ -128,11 +128,15 @@ describe('useAlertOverridesState', () => { const [overviewOverrides, setOverviewOverrides] = createSignal([]); const resources = [ makeResource({ - id: 'Main-cluster-ceph-pool', + id: 'storage-4a40f1c6', name: 'ceph-pool', displayName: 'ceph-pool', type: 'storage', platformId: 'Main', + metricsTarget: { + resourceType: 'storage', + resourceId: 'Main-cluster-ceph-pool', + }, proxmox: { instance: 'Main', node: 'cluster', diff --git a/frontend-modern/src/features/alerts/alertOverridesModel.ts b/frontend-modern/src/features/alerts/alertOverridesModel.ts index f70988407..be3532227 100644 --- a/frontend-modern/src/features/alerts/alertOverridesModel.ts +++ b/frontend-modern/src/features/alerts/alertOverridesModel.ts @@ -46,7 +46,7 @@ const buildSharedStorageLegacyKeyMap = (storageResources: Resource[]): Map + uniqueIds( + resource.metricsTarget?.resourceType === 'storage' + ? resource.metricsTarget.resourceId + : undefined, + resource.id, + ); + +export const storageOverrideActionId = (resource: Resource): string => + storageOverrideIdCandidates(resource)[0] || resource.id; + +const buildStorageOverrideKeyMap = (storageResources: Resource[]): Map => { + const keyMap = new Map(); + + storageResources.forEach((resource) => { + const canonicalID = storageOverrideActionId(resource); + if (!canonicalID) return; + + storageOverrideIdCandidates(resource).forEach((candidate) => { + if (candidate !== canonicalID) { + keyMap.set(candidate, canonicalID); + } + }); + }); + + buildSharedStorageLegacyKeyMap(storageResources).forEach((canonicalID, legacyID) => { + keyMap.set(legacyID, canonicalID); + }); + + return keyMap; +}; + export const normalizeRawOverridesConfig = ( rawOverrides: Record, storageResources: Resource[] = [], ): Record => { const cleanedOverrides: Record = {}; const priorityByKey = new Map(); - const sharedStorageLegacyKeyMap = buildSharedStorageLegacyKeyMap(storageResources); + const storageOverrideKeyMap = buildStorageOverrideKeyMap(storageResources); for (const [key, value] of Object.entries(rawOverrides)) { const normalizedGuestKey = normalizeGuestOverrideKey(key); @@ -91,9 +123,9 @@ export const normalizeRawOverridesConfig = ( .replace(/^-|-$/g, '') || 'unknown'; normalizedKey = diskMatch[1] + normalized; } - const sharedStorageKey = sharedStorageLegacyKeyMap.get(normalizedKey); - if (sharedStorageKey) { - normalizedKey = sharedStorageKey; + const storageOverrideKey = storageOverrideKeyMap.get(normalizedKey); + if (storageOverrideKey) { + normalizedKey = storageOverrideKey; } const priority = normalizedKey === key ? 1 : 0; @@ -255,10 +287,13 @@ export const buildProjectedOverrides = ({ }); storageResources.forEach((storageResource) => { - const canonicalID = asString(storageResource.id); + const canonicalID = storageOverrideActionId(storageResource); if (canonicalID) { storageMap.set(canonicalID, storageResource); } + storageOverrideIdCandidates(storageResource).forEach((candidate) => { + storageMap.set(candidate, storageResource); + }); }); buildSharedStorageLegacyKeyMap(storageResources).forEach((canonicalID, legacyID) => { @@ -418,7 +453,7 @@ export const buildProjectedOverrides = ({ if (storage) { const coords = storageCoords(storage); upsertProjectedOverride({ - id: storage.id, + id: storageOverrideActionId(storage), name: getAlertResourceDisplayLabel(storage), type: 'storage', resourceType: 'Storage', diff --git a/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsInfrastructureData.ts b/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsInfrastructureData.ts index a2f027056..eabfb61fb 100644 --- a/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsInfrastructureData.ts +++ b/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsInfrastructureData.ts @@ -8,10 +8,15 @@ import type { Resource as TableResource } from '../tableTypes'; import { ThresholdsDataInputs } from '../thresholdsResourceModel'; import { createOverridesMap, + findOverrideByCandidates, hasThresholdDiff, normalizeStorageStatus, storageCoords, } from '../thresholdsResourceModel'; +import { + storageOverrideActionId as getStorageOverrideActionId, + storageOverrideIdCandidates as getStorageOverrideIdCandidates, +} from '@/features/alerts/alertOverridesModel'; export function useThresholdsInfrastructureData(inputs: ThresholdsDataInputs) { const { props, editingId, searchTerm } = inputs; @@ -130,12 +135,16 @@ export function useThresholdsInfrastructureData(inputs: ThresholdsDataInputs) { const overridesMap = createOverridesMap(props.overrides()); const storageDevices = (props.storage ?? []).map((storage) => { - const override = overridesMap.get(storage.id); + const storageActionId = getStorageOverrideActionId(storage); + const override = findOverrideByCandidates( + overridesMap, + getStorageOverrideIdCandidates(storage), + ); const coords = storageCoords(storage); const hasCustomThresholds = hasThresholdDiff(override, { usage: props.storageDefault() }); return { - id: storage.id, + id: storageActionId, name: getAlertResourceDisplayLabel(storage), displayName: getAlertResourceDisplayLabel(storage), rawName: storage.name, diff --git a/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts b/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts index 800fffb4b..9975ae8f3 100644 --- a/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts +++ b/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts @@ -202,7 +202,7 @@ describe('alert resource display labels', () => { describe('shared storage override migration', () => { it('keeps legacy Ceph override ids visible on the v6 thresholds surface', () => { const storage = { - id: 'Main-cluster-ceph-pool', + id: 'storage-4a40f1c6', name: 'ceph-pool', displayName: 'ceph-pool', type: 'storage', @@ -211,6 +211,10 @@ describe('shared storage override migration', () => { sourceType: 'api', status: 'online', lastSeen: Date.now(), + metricsTarget: { + resourceType: 'storage', + resourceId: 'Main-cluster-ceph-pool', + }, proxmox: { instance: 'Main', node: 'cluster', diff --git a/frontend-modern/src/utils/__tests__/metricThresholds.test.ts b/frontend-modern/src/utils/__tests__/metricThresholds.test.ts index cf7412069..c1e4a5a47 100644 --- a/frontend-modern/src/utils/__tests__/metricThresholds.test.ts +++ b/frontend-modern/src/utils/__tests__/metricThresholds.test.ts @@ -198,7 +198,7 @@ describe('metricThresholds', () => { }); }); - it('treats disabled thresholds as normal display coloring', () => { + it('uses default display coloring when alert thresholds are disabled', () => { const config = { enabled: true, guestDefaults: {}, @@ -214,7 +214,7 @@ describe('metricThresholds', () => { expect( resolveMetricDisplayThresholds(config, 'guest', 'disk', 'guest:cluster-a:100'), ).toBeNull(); - expect(getMetricColorClass(99, 'disk', null)).toContain('bg-metric-normal-bg'); + expect(getMetricColorClass(99, 'disk', null)).toContain('bg-metric-critical-bg'); }); it('honors disabled docker defaults and storage usage aliases', () => { diff --git a/frontend-modern/src/utils/__tests__/workloadGuestPresentation.test.ts b/frontend-modern/src/utils/__tests__/workloadGuestPresentation.test.ts index c1560e0d1..a42d734b2 100644 --- a/frontend-modern/src/utils/__tests__/workloadGuestPresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/workloadGuestPresentation.test.ts @@ -33,5 +33,8 @@ describe('workloadGuestPresentation', () => { expect(getWorkloadGuestDiskStatusMessage()).toBe( 'Disk stats unavailable. Guest agent may not be installed.', ); + expect(getWorkloadGuestDiskStatusMessage('prev-no-filesystems')).toBe( + 'Using last known disk stats. No filesystems found. VM may be booting or using a Live ISO.', + ); }); }); diff --git a/frontend-modern/src/utils/metricThresholds.ts b/frontend-modern/src/utils/metricThresholds.ts index 00ae217ab..63d754804 100644 --- a/frontend-modern/src/utils/metricThresholds.ts +++ b/frontend-modern/src/utils/metricThresholds.ts @@ -242,10 +242,7 @@ export function getMetricSeverity( metric: MetricType, thresholds?: MetricDisplayThresholds | null, ): MetricSeverity { - const t = thresholds === undefined ? METRIC_THRESHOLDS[metric] : thresholds; - if (!t) { - return 'normal'; - } + const t = thresholds ?? METRIC_THRESHOLDS[metric]; if (value >= t.critical) return 'critical'; if (value >= t.warning) return 'warning'; return 'normal'; diff --git a/frontend-modern/src/utils/workloadGuestPresentation.ts b/frontend-modern/src/utils/workloadGuestPresentation.ts index 81f227e60..8bc8079fc 100644 --- a/frontend-modern/src/utils/workloadGuestPresentation.ts +++ b/frontend-modern/src/utils/workloadGuestPresentation.ts @@ -50,24 +50,31 @@ export function getWorkloadsGuestNetworkEmptyState(): string { } export function getWorkloadGuestDiskStatusMessage(reason?: string): string { - switch (reason) { - case 'agent-not-running': - return 'Guest agent not running. Install and start qemu-guest-agent in the VM.'; - case 'agent-timeout': - return 'Guest agent timeout. Agent may need to be restarted.'; - case 'permission-denied': - return 'Permission denied. Check that your Pulse user/token has VM.Monitor permission (PVE 8) or VM.GuestAgent.Audit permission (PVE 9).'; - case 'agent-disabled': - return 'Guest agent is disabled in VM configuration. Enable it in VM Options.'; - case 'no-filesystems': - return 'No filesystems found. VM may be booting or using a Live ISO.'; - case 'special-filesystems-only': - return 'Only special filesystems detected (ISO/squashfs). This is normal for Live systems.'; - case 'agent-error': - return 'Error communicating with guest agent.'; - case 'no-data': - return 'No disk data available from Proxmox API.'; - default: - return 'Disk stats unavailable. Guest agent may not be installed.'; - } + const carriedForward = reason?.startsWith('prev-') ?? false; + const normalizedReason = carriedForward ? reason?.slice(5) : reason; + + const message = (() => { + switch (normalizedReason) { + case 'agent-not-running': + return 'Guest agent not running. Install and start qemu-guest-agent in the VM.'; + case 'agent-timeout': + return 'Guest agent timeout. Agent may need to be restarted.'; + case 'permission-denied': + return 'Permission denied. Check that your Pulse user/token has VM.Monitor permission (PVE 8) or VM.GuestAgent.Audit permission (PVE 9).'; + case 'agent-disabled': + return 'Guest agent is disabled in VM configuration. Enable it in VM Options.'; + case 'no-filesystems': + return 'No filesystems found. VM may be booting or using a Live ISO.'; + case 'special-filesystems-only': + return 'Only special filesystems detected (ISO/squashfs). This is normal for Live systems.'; + case 'agent-error': + return 'Error communicating with guest agent.'; + case 'no-data': + return 'No disk data available from Proxmox API.'; + default: + return 'Disk stats unavailable. Guest agent may not be installed.'; + } + })(); + + return carriedForward ? `Using last known disk stats. ${message}` : message; } diff --git a/internal/models/ceph_pool_storage_test.go b/internal/models/ceph_pool_storage_test.go new file mode 100644 index 000000000..fdc6c929d --- /dev/null +++ b/internal/models/ceph_pool_storage_test.go @@ -0,0 +1,33 @@ +package models + +import "testing" + +func TestStorageFromCephPool(t *testing.T) { + cluster := CephCluster{ + Instance: "Main", + Health: "HEALTH_OK", + } + pool := CephPool{ + ID: 7, + Name: "data_replication", + StoredBytes: 70, + AvailableBytes: 30, + Objects: 12, + PercentUsed: 70, + } + + storage := StorageFromCephPool(cluster, pool) + + if storage.ID != "Main-ceph-pool-data_replication" { + t.Fatalf("ID = %q, want Main-ceph-pool-data_replication", storage.ID) + } + if storage.Name != "data_replication" || storage.Pool != "data_replication" { + t.Fatalf("name/pool = %q/%q, want data_replication", storage.Name, storage.Pool) + } + if storage.Type != "ceph" || !storage.Shared || !storage.Enabled || !storage.Active { + t.Fatalf("unexpected storage flags: %+v", storage) + } + if storage.Total != 100 || storage.Used != 70 || storage.Free != 30 || storage.Usage != 70 { + t.Fatalf("unexpected capacity projection: %+v", storage) + } +} diff --git a/internal/models/models.go b/internal/models/models.go index 5bb88eff8..eac099070 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -1111,6 +1111,69 @@ type Storage struct { ZFSPool *ZFSPool `json:"zfsPool,omitempty"` // ZFS pool details if this is ZFS storage } +func CephPoolStorageID(instanceName, poolName string) string { + instance := strings.TrimSpace(instanceName) + if instance == "" { + instance = "ceph" + } + name := strings.TrimSpace(poolName) + if name == "" { + name = "pool" + } + return fmt.Sprintf("%s-ceph-pool-%s", instance, name) +} + +func StorageFromCephPool(cluster CephCluster, pool CephPool) Storage { + name := strings.TrimSpace(pool.Name) + if name == "" { + name = fmt.Sprintf("pool-%d", pool.ID) + } + total := pool.StoredBytes + pool.AvailableBytes + free := pool.AvailableBytes + if free < 0 { + free = 0 + } + usage := pool.PercentUsed + if usage == 0 && total > 0 && pool.StoredBytes > 0 { + usage = (float64(pool.StoredBytes) / float64(total)) * 100 + } + + status := "available" + switch strings.ToUpper(strings.TrimSpace(cluster.Health)) { + case "HEALTH_ERR", "ERR", "ERROR": + status = "unavailable" + } + + return Storage{ + ID: CephPoolStorageID(cluster.Instance, name), + Name: name, + Node: "cluster", + Instance: cluster.Instance, + Type: "ceph", + Status: status, + Pool: name, + Total: total, + Used: pool.StoredBytes, + Free: free, + Usage: usage, + Content: "ceph", + Shared: true, + Enabled: true, + Active: status == "available", + } +} + +func CephPoolStorage(cluster CephCluster) []Storage { + if len(cluster.Pools) == 0 { + return nil + } + out := make([]Storage, 0, len(cluster.Pools)) + for _, pool := range cluster.Pools { + out = append(out, StorageFromCephPool(cluster, pool)) + } + return out +} + func (s Storage) NormalizeCollections() Storage { if s.Nodes == nil { s.Nodes = []string{} diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 6a2379695..da7ccc409 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -302,7 +302,9 @@ func TestGuestMemoryFallbackUsesCanonicalLowTrustSelector(t *testing.T) { for _, snippet := range []string{ "func effectiveGuestFreeMemTotal(memTotal uint64, status *proxmox.VMStatus) uint64 {", - "if status.Balloon > 0 && status.Balloon <= memTotal && status.FreeMem <= status.Balloon {", + "func guestStatusFreeMem(status *proxmox.VMStatus) uint64 {", + "if status.BalloonInfo != nil {", + "if status.Balloon > 0 && status.Balloon <= memTotal && freeMem <= status.Balloon {", "func selectGuestLowTrustUsedMemory(memTotal uint64, status *proxmox.VMStatus) (uint64, string) {", "if statusMemPlusFree > freeMemTotal+guestStatusMemoryMismatchTolerance {", } { diff --git a/internal/monitoring/ceph.go b/internal/monitoring/ceph.go index 314f7752e..befe1f14c 100644 --- a/internal/monitoring/ceph.go +++ b/internal/monitoring/ceph.go @@ -60,6 +60,33 @@ func (m *Monitor) pollCephCluster(ctx context.Context, instanceName string, clie } m.state.UpdateCephClustersForInstance(instanceName, []models.CephCluster{cluster}) + m.checkCephPoolStorage(cluster) +} + +func (m *Monitor) checkCephPoolStorage(cluster models.CephCluster) { + storagePools := models.CephPoolStorage(cluster) + if len(storagePools) == 0 { + return + } + + timestamp := time.Now() + for _, storage := range storagePools { + if m.metricsHistory != nil && !shouldSkipNativeMockStateMetricWrites() { + m.metricsHistory.AddStorageMetric(storage.ID, "usage", storage.Usage, timestamp) + m.metricsHistory.AddStorageMetric(storage.ID, "used", float64(storage.Used), timestamp) + m.metricsHistory.AddStorageMetric(storage.ID, "total", float64(storage.Total), timestamp) + m.metricsHistory.AddStorageMetric(storage.ID, "avail", float64(storage.Free), timestamp) + if m.metricsStore != nil { + m.metricsStore.Write("storage", storage.ID, "usage", storage.Usage, timestamp) + m.metricsStore.Write("storage", storage.ID, "used", float64(storage.Used), timestamp) + m.metricsStore.Write("storage", storage.ID, "total", float64(storage.Total), timestamp) + m.metricsStore.Write("storage", storage.ID, "avail", float64(storage.Free), timestamp) + } + } + if m.alertManager != nil { + m.alertManager.CheckStorage(storage) + } + } } // buildCephClusterModel converts the proxmox Ceph responses into the shared model representation. diff --git a/internal/monitoring/ceph_test.go b/internal/monitoring/ceph_test.go index 005719a5e..1898122a9 100644 --- a/internal/monitoring/ceph_test.go +++ b/internal/monitoring/ceph_test.go @@ -6,6 +6,7 @@ import ( "fmt" "testing" + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" "github.com/stretchr/testify/assert" @@ -113,6 +114,61 @@ func TestPollCephCluster(t *testing.T) { }) } +func TestPollCephClusterChecksPoolStorageThresholds(t *testing.T) { + manager := alerts.NewManagerWithDataDir(t.TempDir()) + manager.UpdateConfig(alerts.AlertConfig{ + Enabled: true, + ActivationState: alerts.ActivationActive, + StorageDefault: alerts.HysteresisThreshold{Trigger: 80, Clear: 75}, + TimeThresholds: map[string]int{"storage": 0}, + Overrides: map[string]alerts.ThresholdConfig{ + models.CephPoolStorageID("pve1", "data_replication"): { + Usage: &alerts.HysteresisThreshold{Trigger: 50, Clear: 45}, + }, + }, + }) + + m := &Monitor{ + state: models.NewState(), + alertManager: manager, + } + client := &mockCephPVEClient{} + status := &proxmox.CephStatus{ + FSID: "fsid123", + Health: proxmox.CephHealth{Status: "HEALTH_OK"}, + } + df := &proxmox.CephDF{ + Data: proxmox.CephDFData{ + Pools: []proxmox.CephDFPool{ + { + ID: 1, + Name: "data_replication", + Stats: proxmox.CephDFPoolStat{ + BytesUsed: 70, + MaxAvail: 30, + PercentUsed: 70, + }, + }, + }, + }, + } + client.On("GetCephStatus", mock.Anything).Return(status, nil) + client.On("GetCephDF", mock.Anything).Return(df, nil) + + m.pollCephCluster(context.Background(), "pve1", client, true) + + active := manager.GetActiveAlerts() + if len(active) != 1 { + t.Fatalf("active alerts = %+v, want one Ceph pool usage alert", active) + } + if active[0].ResourceID != models.CephPoolStorageID("pve1", "data_replication") { + t.Fatalf("ResourceID = %q, want %q", active[0].ResourceID, models.CephPoolStorageID("pve1", "data_replication")) + } + if active[0].Threshold != 50 { + t.Fatalf("Threshold = %v, want override threshold 50", active[0].Threshold) + } +} + func TestIsCephStorageType(t *testing.T) { t.Parallel() diff --git a/internal/monitoring/diagnostic_snapshots.go b/internal/monitoring/diagnostic_snapshots.go index d0c4d8158..6524e5eca 100644 --- a/internal/monitoring/diagnostic_snapshots.go +++ b/internal/monitoring/diagnostic_snapshots.go @@ -52,6 +52,9 @@ type VMMemoryRaw struct { StatusMaxMem uint64 `json:"statusMaxmem,omitempty"` Balloon uint64 `json:"balloon,omitempty"` BalloonMin uint64 `json:"balloonMin,omitempty"` + BalloonInfoFreeMem uint64 `json:"balloonInfoFreeMem,omitempty"` + BalloonInfoTotalMem uint64 `json:"balloonInfoTotalMem,omitempty"` + BalloonInfoActual uint64 `json:"balloonInfoActual,omitempty"` MemInfoUsed uint64 `json:"meminfoUsed,omitempty"` MemInfoFree uint64 `json:"meminfoFree,omitempty"` MemInfoTotal uint64 `json:"meminfoTotal,omitempty"` @@ -241,6 +244,12 @@ func (m *Monitor) logGuestMemorySource(instance, guestType, node string, vmid in if snapshot.Raw.StatusMaxMem > 0 { evt = evt.Uint64("statusMaxMem", snapshot.Raw.StatusMaxMem) } + if snapshot.Raw.BalloonInfoFreeMem > 0 { + evt = evt.Uint64("balloonInfoFreeMem", snapshot.Raw.BalloonInfoFreeMem) + } + if snapshot.Raw.BalloonInfoTotalMem > 0 { + evt = evt.Uint64("balloonInfoTotalMem", snapshot.Raw.BalloonInfoTotalMem) + } if snapshot.Raw.MemInfoAvailable > 0 { evt = evt.Uint64("memInfoAvailable", snapshot.Raw.MemInfoAvailable) } diff --git a/internal/monitoring/guest_agent_evidence.go b/internal/monitoring/guest_agent_evidence.go index 61e173b19..7a1f0f272 100644 --- a/internal/monitoring/guest_agent_evidence.go +++ b/internal/monitoring/guest_agent_evidence.go @@ -38,7 +38,7 @@ func hasRecentGuestAgentEvidence(prev *models.VM, now time.Time) bool { func shouldQueryGuestAgent(vmStatus *proxmox.VMStatus, prev *models.VM, now time.Time) bool { if vmStatus != nil { - return vmStatus.Agent.Value > 0 + return vmStatus.Agent.IsAvailable() } return hasRecentGuestAgentEvidence(prev, now) } diff --git a/internal/monitoring/guest_disk_stability.go b/internal/monitoring/guest_disk_stability.go index f6586b8b0..87cb04f5b 100644 --- a/internal/monitoring/guest_disk_stability.go +++ b/internal/monitoring/guest_disk_stability.go @@ -66,9 +66,6 @@ func stabilizeGuestLowTrustDisk( if prev == nil || prev.Type != "qemu" || !hasRecentGuestAgentEvidence(prev, now) { return diskTotal, diskUsed, diskFree, diskUsage, individualDisks, diskStatusReason } - if strings.HasPrefix(prev.DiskStatusReason, "prev-") { - return diskTotal, diskUsed, diskFree, diskUsage, individualDisks, diskStatusReason - } if prev.Disk.Total <= 0 || prev.Disk.Used < 0 || prev.Disk.Used > prev.Disk.Total || prev.Disk.Usage < 0 { return diskTotal, diskUsed, diskFree, diskUsage, individualDisks, diskStatusReason } diff --git a/internal/monitoring/guest_disk_stability_test.go b/internal/monitoring/guest_disk_stability_test.go index b265c38e4..c61818ee1 100644 --- a/internal/monitoring/guest_disk_stability_test.go +++ b/internal/monitoring/guest_disk_stability_test.go @@ -49,13 +49,12 @@ func TestStabilizeGuestLowTrustDiskCarriesForwardPreviousSnapshot(t *testing.T) } } -func TestStabilizeGuestLowTrustDiskRejectsPreviouslyCarriedForwardSnapshot(t *testing.T) { +func TestStabilizeGuestLowTrustDiskRejectsPreviouslyCarriedForwardSnapshotWithoutAgentEvidence(t *testing.T) { now := time.Now() prev := &models.VM{ Type: "qemu", Status: "running", LastSeen: now.Add(-time.Minute), - AgentVersion: "8.2.0", DiskStatusReason: "prev-no-filesystems", Disk: models.Disk{ Total: 1000, @@ -91,3 +90,46 @@ func TestStabilizeGuestLowTrustDiskRejectsPreviouslyCarriedForwardSnapshot(t *te t.Fatalf("reason = %q, want no-filesystems", reason) } } + +func TestStabilizeGuestLowTrustDiskCarriesPreviouslyForwardedSnapshotWithAgentEvidence(t *testing.T) { + now := time.Now() + prev := &models.VM{ + Type: "qemu", + Status: "running", + LastSeen: now.Add(-time.Minute), + AgentVersion: "8.2.0", + DiskStatusReason: "prev-no-filesystems", + Disk: models.Disk{ + Total: 1000, + Used: 400, + Free: 600, + Usage: 40, + }, + Disks: []models.Disk{ + {Total: 1000, Used: 400, Free: 600, Usage: 40, Mountpoint: "/", Type: "ext4", Device: "/dev/vda"}, + }, + } + + total, used, free, usage, disks, reason := stabilizeGuestLowTrustDisk( + prev, + "running", + 1000, + 0, + 1000, + -1, + nil, + "no-filesystems", + false, + now, + ) + + if total != 1000 || used != 400 || free != 600 || usage != 40 { + t.Fatalf("unexpected carried-forward disk summary: total=%d used=%d free=%d usage=%.2f", total, used, free, usage) + } + if len(disks) != 1 || disks[0].Device != "/dev/vda" { + t.Fatalf("expected previous disks to be cloned, got %#v", disks) + } + if reason != "prev-no-filesystems" { + t.Fatalf("reason = %q, want prev-no-filesystems", reason) + } +} diff --git a/internal/monitoring/guest_memory_sources.go b/internal/monitoring/guest_memory_sources.go index 96059f188..109d84d88 100644 --- a/internal/monitoring/guest_memory_sources.go +++ b/internal/monitoring/guest_memory_sources.go @@ -95,11 +95,41 @@ func saturatingAddUint64(lhs, rhs uint64) uint64 { return lhs + rhs } +func guestStatusFreeMem(status *proxmox.VMStatus) uint64 { + if status == nil { + return 0 + } + if status.FreeMem > 0 { + return status.FreeMem + } + if status.BalloonInfo != nil { + return status.BalloonInfo.FreeMem + } + return 0 +} + func effectiveGuestFreeMemTotal(memTotal uint64, status *proxmox.VMStatus) uint64 { if status == nil { return memTotal } - if status.Balloon > 0 && status.Balloon <= memTotal && status.FreeMem <= status.Balloon { + + freeMem := guestStatusFreeMem(status) + if status.BalloonInfo != nil { + if status.BalloonInfo.TotalMem > 0 && freeMem <= status.BalloonInfo.TotalMem { + if memTotal > 0 && status.BalloonInfo.TotalMem > memTotal { + return memTotal + } + return status.BalloonInfo.TotalMem + } + if status.BalloonInfo.Actual > 0 && freeMem <= status.BalloonInfo.Actual { + if memTotal > 0 && status.BalloonInfo.Actual > memTotal { + return memTotal + } + return status.BalloonInfo.Actual + } + } + + if status.Balloon > 0 && status.Balloon <= memTotal && freeMem <= status.Balloon { return status.Balloon } return memTotal @@ -111,15 +141,16 @@ func selectGuestLowTrustUsedMemory(memTotal uint64, status *proxmox.VMStatus) (u } freeMemTotal := effectiveGuestFreeMemTotal(memTotal, status) - hasFreeFallback := status.FreeMem > 0 && freeMemTotal >= status.FreeMem + freeMem := guestStatusFreeMem(status) + hasFreeFallback := freeMem > 0 && freeMemTotal >= freeMem freeDerivedUsed := uint64(0) if hasFreeFallback { - freeDerivedUsed = freeMemTotal - status.FreeMem + freeDerivedUsed = freeMemTotal - freeMem } if status.Mem > 0 { if hasFreeFallback && freeDerivedUsed < status.Mem { - statusMemPlusFree := saturatingAddUint64(status.Mem, status.FreeMem) + statusMemPlusFree := saturatingAddUint64(status.Mem, freeMem) if status.Mem >= freeMemTotal && freeDerivedUsed < freeMemTotal { return freeDerivedUsed, "status-freemem" } @@ -162,10 +193,15 @@ func (m *Monitor) resolveGuestStatusMemory( if guestRaw != nil { guestRaw.StatusMaxMem = status.MaxMem guestRaw.StatusMem = status.Mem - guestRaw.StatusFreeMem = status.FreeMem + guestRaw.StatusFreeMem = guestStatusFreeMem(status) guestRaw.Balloon = status.Balloon guestRaw.BalloonMin = status.BalloonMin guestRaw.Agent = status.Agent.Value + if status.BalloonInfo != nil { + guestRaw.BalloonInfoFreeMem = status.BalloonInfo.FreeMem + guestRaw.BalloonInfoTotalMem = status.BalloonInfo.TotalMem + guestRaw.BalloonInfoActual = status.BalloonInfo.Actual + } } memAvailable := uint64(0) @@ -207,7 +243,7 @@ func (m *Monitor) resolveGuestStatusMemory( } } - if memAvailable == 0 && status.Agent.Value > 0 { + if memAvailable == 0 && status.Agent.IsAvailable() { if agentAvailable, agentErr := m.getVMAgentMemAvailable(ctx, client, instanceName, node, vmid); agentErr == nil && agentAvailable > 0 { memAvailable = agentAvailable memorySource = "guest-agent-meminfo" diff --git a/internal/monitoring/guest_memory_sources_test.go b/internal/monitoring/guest_memory_sources_test.go index 1cf8ccd2d..9a78a1c6a 100644 --- a/internal/monitoring/guest_memory_sources_test.go +++ b/internal/monitoring/guest_memory_sources_test.go @@ -76,6 +76,20 @@ func TestSelectGuestLowTrustUsedMemory(t *testing.T) { wantUsed: 3 * giB, wantSource: "status-freemem", }, + { + name: "uses ballooninfo free memory when status mem is falsely saturated", + memTotal: 8 * giB, + status: &proxmox.VMStatus{ + Mem: 8 * giB, + MaxMem: 8 * giB, + BalloonInfo: &proxmox.VMBalloonInfo{ + FreeMem: 5 * giB, + TotalMem: 8 * giB, + }, + }, + wantUsed: 3 * giB, + wantSource: "status-freemem", + }, { name: "returns empty selection without usable fields", memTotal: 8 * giB, diff --git a/internal/monitoring/guest_memory_stability.go b/internal/monitoring/guest_memory_stability.go index db1d4e637..52b3586f9 100644 --- a/internal/monitoring/guest_memory_stability.go +++ b/internal/monitoring/guest_memory_stability.go @@ -56,7 +56,7 @@ func guestAgentSignalsHealthy( networkInterfaces []models.GuestNetworkInterface, osName, osVersion, agentVersion string, ) bool { - if detailedStatus != nil && detailedStatus.Agent.Value > 0 { + if detailedStatus != nil && detailedStatus.Agent.IsAvailable() { return true } return diskFromAgent || diff --git a/internal/monitoring/guest_metadata.go b/internal/monitoring/guest_metadata.go index a4cfa23bc..08cfd57e5 100644 --- a/internal/monitoring/guest_metadata.go +++ b/internal/monitoring/guest_metadata.go @@ -228,7 +228,7 @@ func (m *Monitor) fetchGuestAgentMetadata(ctx context.Context, client PVEClientI cached, ok := m.guestMetadataCache[key] m.guestMetadataMu.RUnlock() - agentAvailable := client != nil && ((vmStatus != nil && vmStatus.Agent.Value > 0) || allowWithoutStatus) + agentAvailable := client != nil && ((vmStatus != nil && vmStatus.Agent.IsAvailable()) || allowWithoutStatus) if !agentAvailable { if ok && now.Sub(cached.fetchedAt) < guestMetadataCacheEntryTTL(cached) { return cloneStringSlice(cached.ipAddresses), cloneGuestNetworkInterfaces(cached.networkInterfaces), cached.osName, cached.osVersion, cached.agentVersion diff --git a/internal/monitoring/memory_trust_characterization_test.go b/internal/monitoring/memory_trust_characterization_test.go index 79d18102f..a6fb7ebb7 100644 --- a/internal/monitoring/memory_trust_characterization_test.go +++ b/internal/monitoring/memory_trust_characterization_test.go @@ -316,6 +316,20 @@ func TestHandleClusterVMResourceMemoryTrustCharacterization(t *testing.T) { wantSource: "status-freemem", wantUsed: 3 * gib, }, + { + name: "saturated VM derives freemem fallback from ballooninfo", + status: &proxmox.VMStatus{ + Status: "running", + MaxMem: 8 * gib, + Mem: 8 * gib, + BalloonInfo: &proxmox.VMBalloonInfo{ + FreeMem: 5 * gib, + TotalMem: 8 * gib, + }, + }, + wantSource: "status-freemem", + wantUsed: 3 * gib, + }, } for _, tt := range tests { diff --git a/internal/monitoring/monitor_polling_vm.go b/internal/monitoring/monitor_polling_vm.go index 6e896a8bf..4672276e8 100644 --- a/internal/monitoring/monitor_polling_vm.go +++ b/internal/monitoring/monitor_polling_vm.go @@ -254,13 +254,18 @@ func (m *Monitor) pollVMsWithNodes(ctx context.Context, instanceName string, clu } // Check if agent is enabled - if vmStatus != nil && vmStatus.Agent.Value == 0 { - diskStatusReason = "agent-disabled" + if vmStatus != nil && !vmStatus.Agent.IsAvailable() { + if vmStatus.Agent.IsEnabled() { + diskStatusReason = "agent-not-running" + } else { + diskStatusReason = "agent-disabled" + } if logging.IsLevelEnabled(zerolog.DebugLevel) { log.Debug(). Str("instance", instanceName). Str("vm", vm.Name). - Msg("Guest agent disabled in VM config") + Bool("agentEnabled", vmStatus.Agent.IsEnabled()). + Msg("Guest agent is not currently queryable") } } else { if logging.IsLevelEnabled(zerolog.DebugLevel) { diff --git a/internal/monitoring/monitor_pve_guest_builders.go b/internal/monitoring/monitor_pve_guest_builders.go index 05ad017d4..d9b956654 100644 --- a/internal/monitoring/monitor_pve_guest_builders.go +++ b/internal/monitoring/monitor_pve_guest_builders.go @@ -92,7 +92,7 @@ func (m *Monitor) applyVMStatusDetails( // Always try to get filesystem info if agent is enabled // Prefer guest agent data over cluster/resources data for accuracy - if status.Agent.Value > 0 { + if status.Agent.IsAvailable() { var fsDisks []models.Disk state.diskTotal, state.diskUsed, state.diskFree, state.diskUsage, fsDisks, state.diskFromAgent, state.diskStatusReason = m.updateVMDisksFromGuestAgentFSInfo( ctx, @@ -111,13 +111,18 @@ func (m *Monitor) applyVMStatusDetails( if state.diskTotal > 0 { state.diskUsage = -1 // Show as allocated size } - state.diskStatusReason = "agent-disabled" + if status.Agent.IsEnabled() { + state.diskStatusReason = "agent-not-running" + } else { + state.diskStatusReason = "agent-disabled" + } log.Debug(). Str("instance", instanceName). Str("vm", res.Name). Int("vmid", res.VMID). Int("agent", status.Agent.Value). - Msg("VM does not have guest agent enabled in config") + Bool("agentEnabled", status.Agent.IsEnabled()). + Msg("VM guest agent is not currently queryable") } } diff --git a/internal/unifiedresources/code_standards_test.go b/internal/unifiedresources/code_standards_test.go index 4488051e8..1adacf92c 100644 --- a/internal/unifiedresources/code_standards_test.go +++ b/internal/unifiedresources/code_standards_test.go @@ -286,6 +286,40 @@ func TestCanonicalStorageMetadataPreservesBackingPoolField(t *testing.T) { } } +func TestCephPoolsProjectThroughCanonicalStoragePath(t *testing.T) { + requiredSnippets := map[string][]string{ + filepath.Join("..", "models", "models.go"): { + "func CephPoolStorageID(instanceName, poolName string) string", + "func StorageFromCephPool(cluster CephCluster, pool CephPool) Storage", + `Type: "ceph",`, + "ID: CephPoolStorageID(cluster.Instance, name),", + }, + "registry.go": { + "models.CephPoolStorage(cluster)", + "rr.ingestStorage(storage)", + }, + filepath.Join("..", "monitoring", "ceph.go"): { + "m.checkCephPoolStorage(cluster)", + "models.CephPoolStorage(cluster)", + `m.metricsStore.Write("storage", storage.ID, "usage", storage.Usage, timestamp)`, + "m.alertManager.CheckStorage(storage)", + }, + } + + for path, snippets := range requiredSnippets { + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read %s: %v", path, err) + } + source := string(data) + for _, snippet := range snippets { + if !strings.Contains(source, snippet) { + t.Fatalf("%s must keep Ceph pool storage on canonical storage identity via %q", path, snippet) + } + } + } +} + func TestResourceAPIExposesDedicatedFacetReads(t *testing.T) { data, err := os.ReadFile(filepath.Join("..", "api", "resources.go")) if err != nil { diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 53a10250f..43ec178b3 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -165,6 +165,9 @@ func (rr *ResourceRegistry) IngestSnapshot(snapshot models.StateSnapshot) { } for _, cluster := range snapshot.CephClusters { rr.ingestCephCluster(cluster) + for _, storage := range models.CephPoolStorage(cluster) { + rr.ingestStorage(storage) + } } for _, dh := range snapshot.DockerHosts { for _, dc := range dh.Containers { diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index a1beacd24..5d28af918 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -1637,6 +1637,54 @@ func TestResourceRegistry_IngestSnapshotDerivesStorageConsumers(t *testing.T) { } } +func TestResourceRegistry_IngestSnapshotProjectsCephPoolsAsStorage(t *testing.T) { + now := time.Date(2026, 3, 8, 12, 0, 0, 0, time.UTC) + rr := NewRegistry(nil) + + rr.IngestSnapshot(models.StateSnapshot{ + CephClusters: []models.CephCluster{ + { + ID: "Main-fsid123", + Instance: "Main", + Name: "Main Ceph", + FSID: "fsid123", + Health: "HEALTH_OK", + LastUpdated: now, + Pools: []models.CephPool{ + { + ID: 1, + Name: "data_replication", + StoredBytes: 70, + AvailableBytes: 30, + PercentUsed: 70, + }, + }, + }, + }, + }) + + storageResources := rr.ListByType(ResourceTypeStorage) + pool := findStorageResource(storageResources, "data_replication", "cluster") + if pool.Storage == nil { + t.Fatalf("expected Ceph pool storage metadata, got %+v", pool) + } + if !pool.Storage.IsCeph || !pool.Storage.Shared || pool.Storage.Type != "ceph" { + t.Fatalf("unexpected Ceph pool storage metadata: %+v", pool.Storage) + } + if pool.Metrics == nil || pool.Metrics.Disk == nil || pool.Metrics.Disk.Percent != 70 { + t.Fatalf("expected Ceph pool disk metrics, got %+v", pool.Metrics) + } + + target := BuildMetricsTargetForRegistry(rr, pool.ID) + if target == nil { + t.Fatalf("expected metrics target for Ceph pool storage") + } + wantID := models.CephPoolStorageID("Main", "data_replication") + if target.ResourceType != "storage" || target.ResourceID != wantID { + t.Fatalf("metrics target = %+v, want storage/%s", target, wantID) + } +} + func TestResourceRegistry_IngestSnapshotDerivesPBSDatastoreConsumers(t *testing.T) { rr := NewRegistry(nil) now := time.Date(2026, 3, 8, 12, 0, 0, 0, time.UTC) diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 1943918a0..7ee7ad90a 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -2324,11 +2324,44 @@ type VMMemInfo struct { Shared uint64 `json:"shared,omitempty"` } +// VMBalloonInfo describes guest memory details returned by status/current when +// the virtio balloon driver can report guest-side memory state. +type VMBalloonInfo struct { + Actual uint64 `json:"actual,omitempty"` + FreeMem uint64 `json:"free_mem,omitempty"` + LastUpdate uint64 `json:"last_update,omitempty"` + MajorPageFaults uint64 `json:"major_page_faults,omitempty"` + MaxMem uint64 `json:"max_mem,omitempty"` + MemSwappedIn uint64 `json:"mem_swapped_in,omitempty"` + MemSwappedOut uint64 `json:"mem_swapped_out,omitempty"` + MinorPageFaults uint64 `json:"minor_page_faults,omitempty"` + TotalMem uint64 `json:"total_mem,omitempty"` +} + // VMAgentField handles the polymorphic agent field that changed in Proxmox 8.3+. // Older versions: integer (0 or 1) // Proxmox 8.3+: object {"enabled":1,"available":1} or similar type VMAgentField struct { - Value int + Value int + Enabled int + Available int + AvailabilityKnown bool +} + +// IsAvailable reports whether status/current says guest-agent commands are +// currently queryable. Legacy integer payloads only expose one bit, so they are +// treated as available when non-zero. +func (a VMAgentField) IsAvailable() bool { + return a.Value > 0 +} + +// IsEnabled reports whether the VM config has the guest agent enabled even if +// the current status says the in-guest service is unavailable. +func (a VMAgentField) IsEnabled() bool { + if a.Enabled > 0 { + return true + } + return !a.AvailabilityKnown && a.Value > 0 } // UnmarshalJSON implements custom JSON unmarshaling to handle both int and object formats @@ -2337,51 +2370,64 @@ func (a *VMAgentField) UnmarshalJSON(data []byte) error { var intValue int if err := json.Unmarshal(data, &intValue); err == nil { a.Value = intValue + a.Enabled = intValue + a.Available = intValue + a.AvailabilityKnown = false return nil } // Try parsing as object (Proxmox 8.3+) var objValue struct { - Enabled int `json:"enabled"` - Available int `json:"available"` + Enabled *int `json:"enabled"` + Available *int `json:"available"` } if err := json.Unmarshal(data, &objValue); err == nil { - // Agent is considered enabled if either field is > 0 - // Typically we want to check "available" for actual functionality - if objValue.Available > 0 { - a.Value = objValue.Available - } else if objValue.Enabled > 0 { - a.Value = objValue.Enabled - } else { - a.Value = 0 + a.Value = 0 + a.Enabled = 0 + a.Available = 0 + a.AvailabilityKnown = objValue.Available != nil + if objValue.Enabled != nil { + a.Enabled = *objValue.Enabled + } + if objValue.Available != nil { + a.Available = *objValue.Available + if a.Available > 0 { + a.Value = a.Available + } + } else if a.Enabled > 0 { + a.Value = a.Enabled } return nil } // If neither worked, default to 0 (agent disabled) a.Value = 0 + a.Enabled = 0 + a.Available = 0 + a.AvailabilityKnown = false return nil } // VMStatus represents detailed VM status returned by Proxmox. type VMStatus struct { - Status string `json:"status"` - CPU float64 `json:"cpu"` - CPUs int `json:"cpus"` - Mem uint64 `json:"mem"` - MaxMem uint64 `json:"maxmem"` - Balloon uint64 `json:"balloon"` - BalloonMin uint64 `json:"balloon_min"` - FreeMem uint64 `json:"freemem"` - MemInfo *VMMemInfo `json:"meminfo,omitempty"` - Disk uint64 `json:"disk"` - MaxDisk uint64 `json:"maxdisk"` - DiskRead uint64 `json:"diskread"` - DiskWrite uint64 `json:"diskwrite"` - NetIn uint64 `json:"netin"` - NetOut uint64 `json:"netout"` - Uptime uint64 `json:"uptime"` - Agent VMAgentField `json:"agent"` + Status string `json:"status"` + CPU float64 `json:"cpu"` + CPUs int `json:"cpus"` + Mem uint64 `json:"mem"` + MaxMem uint64 `json:"maxmem"` + Balloon uint64 `json:"balloon"` + BalloonMin uint64 `json:"balloon_min"` + BalloonInfo *VMBalloonInfo `json:"ballooninfo,omitempty"` + FreeMem uint64 `json:"freemem"` + MemInfo *VMMemInfo `json:"meminfo,omitempty"` + Disk uint64 `json:"disk"` + MaxDisk uint64 `json:"maxdisk"` + DiskRead uint64 `json:"diskread"` + DiskWrite uint64 `json:"diskwrite"` + NetIn uint64 `json:"netin"` + NetOut uint64 `json:"netout"` + Uptime uint64 `json:"uptime"` + Agent VMAgentField `json:"agent"` } // GetZFSPoolStatus gets the status of ZFS pools on a node diff --git a/pkg/proxmox/client_test.go b/pkg/proxmox/client_test.go index 556d5a4ad..2e8e614f9 100644 --- a/pkg/proxmox/client_test.go +++ b/pkg/proxmox/client_test.go @@ -1422,7 +1422,7 @@ func TestVMAgentFieldUnmarshalJSON(t *testing.T) { { name: "object format with enabled > 0 but available=0", input: `{"enabled":1,"available":0}`, - want: 1, + want: 0, }, { name: "object format with both 0", @@ -1492,7 +1492,7 @@ func TestVMAgentFieldUnmarshalJSON_InVMStatus(t *testing.T) { { name: "VMStatus with object agent enabled but not available", payload: `{"status":"stopped","cpu":0,"cpus":1,"mem":0,"maxmem":1073741824,"agent":{"enabled":1,"available":0}}`, - want: 1, + want: 0, }, { name: "VMStatus with object agent both zero", @@ -1519,6 +1519,48 @@ func TestVMAgentFieldUnmarshalJSON_InVMStatus(t *testing.T) { } } +func TestVMAgentFieldTracksEnabledAndAvailableSeparately(t *testing.T) { + var agent VMAgentField + if err := agent.UnmarshalJSON([]byte(`{"enabled":1,"available":0}`)); err != nil { + t.Fatalf("UnmarshalJSON returned error: %v", err) + } + if !agent.IsEnabled() { + t.Fatal("expected IsEnabled to preserve VM config state") + } + if agent.IsAvailable() { + t.Fatal("expected IsAvailable to reflect current guest-agent availability") + } +} + +func TestVMStatusUnmarshalJSON_BalloonInfo(t *testing.T) { + payload := []byte(`{ + "status":"running", + "mem":8589934592, + "maxmem":8589934592, + "agent":1, + "ballooninfo":{ + "actual":8589934592, + "free_mem":5368709120, + "max_mem":8589934592, + "total_mem":8200802304 + } + }`) + + var status VMStatus + if err := json.Unmarshal(payload, &status); err != nil { + t.Fatalf("unexpected error unmarshalling VMStatus: %v", err) + } + if status.BalloonInfo == nil { + t.Fatal("expected ballooninfo to be parsed") + } + if status.BalloonInfo.FreeMem != 5368709120 { + t.Fatalf("BalloonInfo.FreeMem = %d, want 5368709120", status.BalloonInfo.FreeMem) + } + if status.BalloonInfo.TotalMem != 8200802304 { + t.Fatalf("BalloonInfo.TotalMem = %d, want 8200802304", status.BalloonInfo.TotalMem) + } +} + func TestGetMHzString(t *testing.T) { tests := []struct { name string diff --git a/tests/integration/tests/66-ceph-alert-thresholds.spec.ts b/tests/integration/tests/66-ceph-alert-thresholds.spec.ts index a5b31bc0b..f651a0146 100644 --- a/tests/integration/tests/66-ceph-alert-thresholds.spec.ts +++ b/tests/integration/tests/66-ceph-alert-thresholds.spec.ts @@ -120,7 +120,7 @@ test.describe('Ceph alert thresholds', () => { }, }, { - id: 'Main-cluster-ceph-pool', + id: 'storage-4a40f1c6', type: 'storage', name: 'ceph-pool', displayName: 'ceph-pool', @@ -133,6 +133,10 @@ test.describe('Ceph alert thresholds', () => { displayName: 'ceph-pool', platformId: 'Main-cluster-ceph-pool', }, + metricsTarget: { + resourceType: 'storage', + resourceId: 'Main-cluster-ceph-pool', + }, storage: { type: 'rbd', shared: true, @@ -149,6 +153,41 @@ test.describe('Ceph alert thresholds', () => { sources: ['proxmox'], }, }, + { + id: 'storage-5f3b87d1', + type: 'storage', + name: 'data_replication', + displayName: 'data_replication', + platformId: 'Main', + platformType: 'proxmox-pve', + sourceType: 'api', + sources: ['proxmox'], + status: 'available', + canonicalIdentity: { + displayName: 'data_replication', + platformId: 'Main-ceph-pool-data_replication', + }, + metricsTarget: { + resourceType: 'storage', + resourceId: 'Main-ceph-pool-data_replication', + }, + storage: { + type: 'ceph', + shared: true, + isCeph: true, + pool: 'data_replication', + nodes: ['pve1', 'pve2'], + }, + proxmox: { + node: 'cluster', + instance: 'Main', + }, + platformData: { + node: 'cluster', + instance: 'Main', + sources: ['proxmox'], + }, + }, ], meta: { page: 1, @@ -178,6 +217,12 @@ test.describe('Ceph alert thresholds', () => { clear: 82, }, }, + 'Main-ceph-pool-data_replication': { + usage: { + trigger: 70, + clear: 65, + }, + }, }, }), }); @@ -228,7 +273,10 @@ test.describe('Ceph alert thresholds', () => { await expect(page.getByRole('heading', { name: 'Alert Thresholds' })).toBeVisible(); await expect(page.getByRole('heading', { name: 'Storage Devices' })).toBeVisible(); await expect(page.getByText('ceph-pool', { exact: true })).toBeVisible(); + await expect(page.getByText('data_replication', { exact: true })).toBeVisible(); await expect(page.getByRole('button', { name: 'Revert to defaults for ceph-pool' })).toBeVisible(); await expect(page.getByRole('button', { name: 'Edit thresholds for ceph-pool' })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Revert to defaults for data_replication' })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Edit thresholds for data_replication' })).toBeVisible(); }); });