From b6a74576bc4a6ee53c24a7aff62415a683aa136c Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 20 Jul 2026 14:54:36 +0100 Subject: [PATCH] Integrate trust-gate reliability fixes --- cmd/pulse-agent/main.go | 62 +- cmd/pulse-agent/main_test.go | 100 +++- docs/AI_PATROL_QUALIFICATION.md | 31 + .../v6/internal/subsystems/agent-lifecycle.md | 13 + .../subsystems/deployment-installability.md | 9 + .../v6/internal/subsystems/monitoring.md | 22 + .../internal/subsystems/security-privacy.md | 8 + frontend-modern/src/api/patrol.ts | 12 +- .../src/components/Storage/DiskDetail.tsx | 10 +- .../Storage/__tests__/DiskDetail.test.tsx | 31 + .../__tests__/useDiskDetailModel.test.ts | 31 + .../components/Storage/useDiskDetailModel.ts | 8 + .../features/alerts/alertOverridesModel.ts | 1 + .../guestThresholdOverrideMutationModel.ts | 28 +- .../truenasThresholdPersistence.test.tsx | 193 +++++++ .../hooks/useThresholdsPlatformData.ts | 5 +- .../features/alerts/thresholds/tableTypes.ts | 2 + .../__tests__/patrolRunAcceptance.test.ts | 155 +++++ .../usePatrolIntelligenceState.test.ts | 15 + .../features/patrol/patrolRunAcceptance.ts | 100 ++++ .../patrol/usePatrolIntelligenceState.ts | 116 ++-- .../__tests__/diskPresentation.test.ts | 43 ++ .../storageBackups/diskPresentation.ts | 42 +- .../__tests__/useUnifiedResources.test.ts | 2 + .../src/hooks/useUnifiedResources.ts | 1 + .../pages/__tests__/AIIntelligence.test.tsx | 7 +- frontend-modern/src/types/api.ts | 13 + frontend-modern/src/types/resource.ts | 32 ++ .../src/utils/resourceStateAdapters.ts | 2 + internal/ai/demo.go | 30 +- internal/ai/patrol.go | 35 +- internal/ai/patrol_findings.go | 26 +- internal/ai/patrol_manual_acceptance_test.go | 133 +++++ internal/ai/patrol_run.go | 117 +++- .../alerts/canonical_override_migration.go | 69 +++ .../canonical_override_migration_test.go | 68 +++ internal/api/ai_handlers.go | 107 ++-- ...handlers_patrol_actions_additional_test.go | 49 +- internal/hostagent/agent.go | 91 ++- internal/hostagent/enroll.go | 6 + internal/hostagent/enroll_test.go | 3 + .../issue1595_sas_collection_test.go | 272 +++++++++ internal/hostagent/smartctl.go | 228 +++++++- internal/models/converters.go | 16 +- internal/models/deepcopy.go | 12 + internal/models/models.go | 65 ++- internal/models/models_frontend.go | 30 +- .../issue1595_collection_trust_test.go | 314 +++++++++++ internal/monitoring/monitor.go | 126 ++++- internal/monitoring/monitor_agents.go | 12 +- .../monitor_alert_override_migration_test.go | 64 +++ internal/monitoring/monitor_alert_sync.go | 12 + internal/monitoring/monitor_helpers.go | 17 + internal/monitoring/monitor_pve.go | 88 ++- internal/truenas/contract_test.go | 57 ++ internal/unifiedresources/adapters.go | 29 + .../unifiedresources/canonical_identity.go | 16 +- internal/unifiedresources/clone.go | 12 + .../unifiedresources/code_standards_test.go | 5 +- .../unifiedresources/physical_disk_ids.go | 51 +- internal/unifiedresources/registry.go | 143 +++++ internal/unifiedresources/types.go | 109 ++-- internal/unifiedresources/views.go | 36 ++ pkg/agents/host/report.go | 32 +- pkg/diskinventory/identity.go | 56 ++ pkg/diskinventory/identity_test.go | 34 ++ pkg/diskinventory/status.go | 100 ++++ pkg/diskinventory/status_test.go | 26 + scripts/install.sh | 351 +++++++++--- .../agent_state_dir_lifecycle_test.go | 456 +++++++++++++++ scripts/installtests/install_sh_test.go | 532 +++++++++++++++++- testdata/issue1595_sas_topology.json | 47 ++ 72 files changed, 4755 insertions(+), 421 deletions(-) create mode 100644 frontend-modern/src/features/alerts/thresholds/hooks/__tests__/truenasThresholdPersistence.test.tsx create mode 100644 frontend-modern/src/features/patrol/__tests__/patrolRunAcceptance.test.ts create mode 100644 frontend-modern/src/features/patrol/patrolRunAcceptance.ts create mode 100644 internal/ai/patrol_manual_acceptance_test.go create mode 100644 internal/alerts/canonical_override_migration.go create mode 100644 internal/alerts/canonical_override_migration_test.go create mode 100644 internal/hostagent/issue1595_sas_collection_test.go create mode 100644 internal/monitoring/issue1595_collection_trust_test.go create mode 100644 internal/monitoring/monitor_alert_override_migration_test.go create mode 100644 pkg/diskinventory/identity.go create mode 100644 pkg/diskinventory/identity_test.go create mode 100644 pkg/diskinventory/status.go create mode 100644 pkg/diskinventory/status_test.go create mode 100644 scripts/installtests/agent_state_dir_lifecycle_test.go create mode 100644 testdata/issue1595_sas_topology.json diff --git a/cmd/pulse-agent/main.go b/cmd/pulse-agent/main.go index dbaf31a6c..e964d2b53 100644 --- a/cmd/pulse-agent/main.go +++ b/cmd/pulse-agent/main.go @@ -167,6 +167,13 @@ func run(ctx context.Context, args []string, getenv func(string) string) error { return nil } + if err := secureAgentStateDir(cfg.StateDir); err != nil { + logger.Warn(). + Err(err). + Str("path", cfg.StateDir). + Msg("Failed to enforce owner-only agent state directory permissions") + } + // 2b. Compute Agent ID if missing (needed for remote config) // We replicate the logic from hostagent.New to ensure we get the same ID lookupHostname := strings.TrimSpace(cfg.HostnameOverride) @@ -587,6 +594,20 @@ func configureAgentLogger(cfg Config) (zerolog.Logger, func(), error) { }, nil } +func secureAgentStateDir(path string) error { + path = strings.TrimSpace(path) + if path == "" { + return nil + } + if err := os.MkdirAll(path, 0o700); err != nil { + return fmt.Errorf("create state directory: %w", err) + } + if err := os.Chmod(path, 0o700); err != nil { + return fmt.Errorf("chmod state directory: %w", err) + } + return nil +} + // readAgentIDFile reads a persisted agent identifier from the given path. func readAgentIDFile(path string) (string, error) { if path == "" { @@ -1062,8 +1083,19 @@ func loadConfig(args []string, getenv func(string) string) (Config, error) { // URL; the startup warning is emitted once the logger exists in run(). securityutil.SetOperatorPlaintextHTTPConsent(*allowPlaintextHTTPFlag) - // Resolve token with priority: --token > --token-file > env > default file - token := resolveToken(*tokenFlag, *tokenFileFlag, envToken) + // Resolve the state directory before any implicit token or identity path. + // A custom instance must never borrow the default instance's credentials. + stateDir := strings.TrimSpace(*stateDirFlag) + if stateDir == "" { + stateDir = defaultAgentStateDir() + } + agentIDFile := strings.TrimSpace(*agentIDFileFlag) + if agentIDFile == "" { + agentIDFile = filepath.Join(stateDir, "agent-id") + } + + // Resolve token with priority: --token > --token-file > env > state-dir file. + token := resolveToken(*tokenFlag, *tokenFileFlag, envToken, stateDir) observers, err := agenttarget.LoadObservers(strings.TrimSpace(*observersFileFlag), pulseURL) if err != nil { return Config{}, fmt.Errorf("load observer destinations: %w", err) @@ -1071,11 +1103,7 @@ func loadConfig(args []string, getenv func(string) string) (Config, error) { // When --enroll is set and a runtime token already exists from a previous // enrollment, use it instead of the bootstrap token embedded in the service - // config. This ensures the agent survives restarts after enrollment. - stateDir := strings.TrimSpace(*stateDirFlag) - if stateDir == "" { - stateDir = defaultAgentStateDir() - } + // config. This ensures the agent survives process and server restarts. if *enrollFlag { runtimeTokenPath := filepath.Join(stateDir, "runtime.token") if content, err := os.ReadFile(runtimeTokenPath); err == nil { @@ -1141,7 +1169,7 @@ func loadConfig(args []string, getenv func(string) string) (Config, error) { Interval: interval, HostnameOverride: strings.TrimSpace(*hostnameFlag), AgentID: strings.TrimSpace(*agentIDFlag), - AgentIDFile: strings.TrimSpace(*agentIDFileFlag), + AgentIDFile: agentIDFile, Tags: tags, InsecureSkipVerify: *insecureFlag, AllowPlaintextHTTP: *allowPlaintextHTTPFlag, @@ -1294,11 +1322,11 @@ func resolveEnableCommands(enableFlag, disableFlag bool, envEnable, envDisable s // 1. --token flag (direct value) // 2. --token-file flag (read from file) // 3. PULSE_TOKEN environment variable -// 4. Default token file under the platform state directory +// 4. Token file under the resolved state directory // // Reading from a file is more secure than CLI args as tokens won't appear in `ps` output. -func resolveToken(tokenFlag, tokenFileFlag, envToken string) string { - return resolveTokenInternal(tokenFlag, tokenFileFlag, envToken, os.ReadFile) +func resolveToken(tokenFlag, tokenFileFlag, envToken, stateDir string) string { + return resolveTokenInternal(tokenFlag, tokenFileFlag, envToken, stateDir, os.ReadFile) } // defaultAgentStateDir mirrors where each platform's installer keeps agent @@ -1318,7 +1346,7 @@ func defaultTokenFilePath() string { return filepath.Join(defaultAgentStateDir(), "token") } -func resolveTokenInternal(tokenFlag, tokenFileFlag, envToken string, readFile func(string) ([]byte, error)) string { +func resolveTokenInternal(tokenFlag, tokenFileFlag, envToken, stateDir string, readFile func(string) ([]byte, error)) string { // 1. Direct token from --token flag if t := strings.TrimSpace(tokenFlag); t != "" { return t @@ -1338,9 +1366,13 @@ func resolveTokenInternal(tokenFlag, tokenFileFlag, envToken string, readFile fu return t } - // 4. Default token file (most secure method for service installs) - defaultTokenFile := defaultTokenFilePath() - if content, err := readFile(defaultTokenFile); err == nil { + // 4. Token file in the resolved state directory. When stateDir is custom, + // do not fall through to the default instance's token. + tokenFile := filepath.Join(strings.TrimSpace(stateDir), "token") + if strings.TrimSpace(stateDir) == "" { + tokenFile = defaultTokenFilePath() + } + if content, err := readFile(tokenFile); err == nil { if t := strings.TrimSpace(string(content)); t != "" { return t } diff --git a/cmd/pulse-agent/main_test.go b/cmd/pulse-agent/main_test.go index 83d6cec6e..0d1a87786 100644 --- a/cmd/pulse-agent/main_test.go +++ b/cmd/pulse-agent/main_test.go @@ -793,10 +793,14 @@ func TestResolveEnableCommands(t *testing.T) { } func TestResolveToken(t *testing.T) { + customStateDir := filepath.Join(string(filepath.Separator), "custom", "pulse-agent") fakeReadFile := func(path string) ([]byte, error) { if path == defaultTokenFilePath() { return []byte("default-token"), nil } + if path == filepath.Join(customStateDir, "token") { + return []byte("custom-token"), nil + } if path == "valid-file" { return []byte("file-token"), nil } @@ -808,12 +812,14 @@ func TestResolveToken(t *testing.T) { tokenFlag string tokenFileFlag string envToken string + stateDir string expected string }{ - {"flag priority", "flag-token", "valid-file", "env-token", "flag-token"}, - {"file priority", "", "valid-file", "env-token", "file-token"}, - {"env priority", "", "", "env-token", "env-token"}, - {"default file priority", "", "", "", "default-token"}, + {"flag priority", "flag-token", "valid-file", "env-token", customStateDir, "flag-token"}, + {"file priority", "", "valid-file", "env-token", customStateDir, "file-token"}, + {"env priority", "", "", "env-token", customStateDir, "env-token"}, + {"default file priority", "", "", "", defaultAgentStateDir(), "default-token"}, + {"custom state file priority", "", "", "", customStateDir, "custom-token"}, } // Update the test cases to avoid the default file if we want to test empty @@ -826,7 +832,7 @@ func TestResolveToken(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := resolveTokenInternal(tc.tokenFlag, tc.tokenFileFlag, tc.envToken, fakeReadFile) + got := resolveTokenInternal(tc.tokenFlag, tc.tokenFileFlag, tc.envToken, tc.stateDir, fakeReadFile) if got != tc.expected { t.Fatalf("%s: expected %q, got %q", tc.name, tc.expected, got) } @@ -834,11 +840,18 @@ func TestResolveToken(t *testing.T) { } t.Run("truly empty", func(t *testing.T) { - got := resolveTokenInternal("", "", "", fakeReadFileNoDefault) + got := resolveTokenInternal("", "", "", customStateDir, fakeReadFileNoDefault) if got != "" { t.Fatalf("expected empty, got %q", got) } }) + + t.Run("custom state never borrows default token", func(t *testing.T) { + got := resolveTokenInternal("", "", "", filepath.Join(string(filepath.Separator), "missing-custom"), fakeReadFile) + if got != "" { + t.Fatalf("custom state unexpectedly borrowed default token %q", got) + } + }) } func TestCleanupDockerAgent(t *testing.T) { @@ -923,6 +936,9 @@ func TestLoadConfig(t *testing.T) { if cfg.StateDir != defaultAgentStateDir() { t.Errorf("expected platform state directory %q, got %q", defaultAgentStateDir(), cfg.StateDir) } + if cfg.AgentIDFile != filepath.Join(defaultAgentStateDir(), "agent-id") { + t.Errorf("expected default agent ID file under state directory, got %q", cfg.AgentIDFile) + } }) t.Run("env overrides", func(t *testing.T) { @@ -995,6 +1011,61 @@ func TestLoadConfig(t *testing.T) { if cfg.StateDir != "/custom/pulse-state" { t.Errorf("expected explicit state directory, got %q", cfg.StateDir) } + if cfg.AgentIDFile != "/custom/pulse-state/agent-id" { + t.Errorf("expected agent ID file under explicit state directory, got %q", cfg.AgentIDFile) + } + }) + + t.Run("custom state directory supplies implicit token", func(t *testing.T) { + stateDir := t.TempDir() + if err := os.WriteFile(filepath.Join(stateDir, "token"), []byte("custom-state-token\n"), 0600); err != nil { + t.Fatal(err) + } + cfg, err := loadConfig([]string{ + "-url", "http://pulse.example.com", + "-state-dir", stateDir, + }, func(s string) string { return "" }) + if err != nil { + t.Fatal(err) + } + if cfg.APIToken != "custom-state-token" { + t.Fatalf("expected implicit custom-state token, got %q", cfg.APIToken) + } + }) + + t.Run("custom enrollment restart prefers persisted runtime token", func(t *testing.T) { + stateDir := t.TempDir() + if err := os.WriteFile(filepath.Join(stateDir, "token"), []byte("bootstrap-token"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(stateDir, "runtime.token"), []byte("runtime-token"), 0600); err != nil { + t.Fatal(err) + } + cfg, err := loadConfig([]string{ + "-url", "http://pulse.example.com", + "-state-dir", stateDir, + "-enroll", + }, func(s string) string { return "" }) + if err != nil { + t.Fatal(err) + } + if cfg.APIToken != "runtime-token" { + t.Fatalf("expected persisted runtime token after restart, got %q", cfg.APIToken) + } + }) + + t.Run("explicit agent ID file overrides state-derived path", func(t *testing.T) { + cfg, err := loadConfig([]string{ + "-token", "test-token", + "-state-dir", "/custom/pulse-state", + "-agent-id-file", "/identity/agent-id", + }, func(s string) string { return "" }) + if err != nil { + t.Fatal(err) + } + if cfg.AgentIDFile != "/identity/agent-id" { + t.Errorf("expected explicit agent ID file, got %q", cfg.AgentIDFile) + } }) t.Run("token optional when enrollment disabled", func(t *testing.T) { @@ -2187,6 +2258,23 @@ func TestAgentIDFilePersistence(t *testing.T) { }) } +func TestSecureAgentStateDir(t *testing.T) { + stateDir := filepath.Join(t.TempDir(), "state") + if err := os.MkdirAll(stateDir, 0755); err != nil { + t.Fatal(err) + } + if err := secureAgentStateDir(stateDir); err != nil { + t.Fatal(err) + } + info, err := os.Stat(stateDir) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); runtime.GOOS != "windows" && got != 0700 { + t.Fatalf("state directory mode = %o, want 700", got) + } +} + type stubTypedContainerUpdater struct { calls int } diff --git a/docs/AI_PATROL_QUALIFICATION.md b/docs/AI_PATROL_QUALIFICATION.md index 1429fb3c5..cf25fc0e9 100644 --- a/docs/AI_PATROL_QUALIFICATION.md +++ b/docs/AI_PATROL_QUALIFICATION.md @@ -284,6 +284,37 @@ go run ./cmd/patrol-qualify \ -artifacts tmp/patrol-qualification/ ``` +### Local-provider cold-start matrix + +Manual-run release validation must also cover a cold local model independently +of finding-quality qualification. This matrix needs no cloud API key: use a +local Ollama model that passes Patrol preflight (for example a locally installed +`qwen3:8b`) and an already authenticated local Pulse session. + +| Case | Preparation | First provider progress target | Required observations | +|---|---|---:|---| +| Warm control | Keep the model loaded | 0–5 seconds | POST returns one accepted `run_id`; status immediately reports the same `current_run_id` | +| Short cold load | Unload, then restart immediately | about 15 seconds | no start-timeout toast; one provider execution; one matching history record | +| Medium cold load | Unload and apply representative local memory pressure | about 30 seconds | accepted run remains running before provider progress; SSE reconnect does not retrigger POST | +| Long cold load | Use a deliberately cold model/runtime on representative hardware | about 60 seconds | no false error; completion or provider failure is recorded against the accepted `run_id` | + +For Ollama, unload without deleting the model: + +```sh +curl -fsS http://127.0.0.1:11434/api/generate \ + -H 'Content-Type: application/json' \ + -d '{"model":"qwen3:8b","keep_alive":0}' +``` + +For every row, capture the `POST /api/ai/patrol/run` response, poll +`GET /api/ai/patrol/status`, observe `/api/ai/patrol/stream`, and finally query +`GET /api/ai/patrol/runs?limit=30`. Pass only when the accepted ID is immediately +visible in status, the provider is invoked once, exactly one terminal history +record has that ID, browser/network and HTTP rejection errors remain distinct +from a recorded provider/runtime failure, and cancel/retry does not reuse stale +client tracking. The provider request timeout remains the terminal bound; a +quiet local model is never treated as a failed backend start. + `live-suite` selects every checked-in scenario for the requested track. The remediation track still requires `--authorize-remediation`; selecting the track does not broaden mutation authority. diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 44b7a5ff9..fda3b619c 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -338,6 +338,19 @@ update, profile rollout, command reachability, or fleet-control authority. restart or OS reboot persistence, and complete uninstall cleanup through the reusable lifecycle harness under `scripts/installtests/`. 26. `scripts/install.sh` shared with `deployment-installability`: the shell installer is both a deployment installability entry point and a canonical agent lifecycle runtime continuity boundary. + `--state-dir` is a whole-lifecycle ownership boundary, not only a runtime + flag. The resolved directory owns the protected bootstrap token, + enrollment runtime token, server-acknowledged `agent-id`, buffered and + command-receipt state, `connection.env`, and the saved offline installer. + Generated service definitions must carry that same directory and its token + file through install, process restart, server restart, update, + re-enrollment, and uninstall. Explicit state wins over discovered service + state, which wins over platform defaults; a custom instance must never + borrow token or identity files from the default instance. A changed + bootstrap token may clear the old enrollment runtime token to express + re-enrollment, while an unchanged token and tokenless update must preserve + it. Default platform paths remain valid migration inputs for installations + created before this contract. Legacy update recovery is cross-platform lifecycle continuity. Linux may read procfs or a systemd unit, while FreeBSD and pfSense must recover the same URL, token, feature, identity, and trust arguments from the live diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index a82eb45bc..b81864514 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -294,6 +294,15 @@ TLS floor in the dynamic config. path must run the reusable lifecycle harness rather than stopping at a parser check or foreground self-test. 8. `scripts/install.sh` shared with `agent-lifecycle`: the shell installer is both a deployment installability entry point and a canonical agent lifecycle runtime continuity boundary. + A caller-supplied `--state-dir` must remain canonical across rendered + systemd, launchd, OpenRC, rc.d, SysV, NAS wrapper, bootstrap, and reference + environment artifacts. Update and uninstall without a repeated custom path + must discover it from the active process or managed service before looking + at default-path state; explicit custom-path operations must not fall back + to another default instance. `connection.env` records the canonical state + and token-file paths without storing the token value, update rewrites the + same secure service shape, and uninstall removes the discovered canonical + directory rather than only `/var/lib/pulse-agent`. Existing-agent update commands copied from the settings UI must use the installer-owned `--update` mode rather than serializing a fresh enrollment token into platform notice links. In `--update` mode, `scripts/install.sh` diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 3f2a6d4ea..c906e73b1 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1266,6 +1266,28 @@ The Proxmox polling runtime in `internal/monitoring/monitor_pve.go` must evaluate disk alerts only after that merged disk view exists, so controller-backed disks do not lose health and endurance coverage between collection and alerting. +Wide SAS inventories use the same trust boundary. The host collector must +fan out per-disk SMART reads with bounded concurrency so a single report +deadline cannot truncate a controller-sized suffix of the inventory, and it +must preserve the Linux controller plus HCTL (or controller-member target) +alongside each reading. When Proxmox exposes a SAS address as `serial` while +smartctl exposes the drive serial, the smartctl serial is the canonical +hardware identity; exact device-path correlation is permitted only inside an +already-linked host/node parent and must fail closed when topology is +ambiguous. Direct SATA, SAS, and NVMe device fallback IDs retain their legacy +shape, while multiple controller members behind one block path add their +controller target to the fallback identity. Per-member I/O must never inherit +an aggregate controller counter. + +Disk identity, temperature, I/O, controller association, and pool membership +also carry typed collection state from `pkg/diskinventory`: `available`, +transiently `unavailable`, provider/controller `unsupported`, or unexpectedly +`missing`. Normalization may retain the last known value when the current +observation is not available, but it must preserve the current state and +reason so API and UI consumers do not present retained evidence as freshly +collected. Unified-resource physical-disk round trips must retain named +`StorageGroup` membership rather than degrading it to the generic `Used` +filesystem label. That same host-agent temperature boundary must prefer a recent linked host-agent payload over legacy SSH collection once the agent provides any usable CPU, NVMe, GPU, or SMART temperature reading. `internal/monitoring/monitor_polling_node_helpers.go` diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 02a1b344e..53d458b04 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -351,6 +351,14 @@ the `white_label` branding entitlement. retain that sink when remote configuration changes log level, and never place runtime tokens or enrollment secrets in the service command or log output. + Custom agent state directories share that same credential boundary: the + directory is owner-only, token, runtime-token, identity, and connection + files are mode `0600`, and implicit token lookup is confined to the + resolved directory instead of falling through to another instance's + default token. Installer health, lookup, and uninstall HTTP calls must feed + token headers through private curl configuration rather than exposing the + token in curl process arguments; generated service definitions and + `connection.env` may contain only protected token-file paths. Global resource timeline reads through `/api/resources/timeline` are adjacent monitoring-read surfaces, not a privacy bypass. Provider activity filters may expose backend-authored task/event metadata, but the endpoint diff --git a/frontend-modern/src/api/patrol.ts b/frontend-modern/src/api/patrol.ts index f56870ac0..1d41eff9c 100644 --- a/frontend-modern/src/api/patrol.ts +++ b/frontend-modern/src/api/patrol.ts @@ -276,6 +276,8 @@ export interface FindingsTrustSummary { export interface PatrolStatus { runtime_state: PatrolRuntimeState; running: boolean; + current_run_id?: string; + current_run_started_at?: string; enabled: boolean; last_patrol_at?: string; last_activity_at?: string; @@ -837,9 +839,13 @@ export interface PatrolRunScope { * Patrol behaviour). Pass a scope to run a manual Targeted check scoped to * specific resources — e.g. a single alert's resource. */ -export async function triggerPatrolRun( - scope?: PatrolRunScope, -): Promise<{ success: boolean; message: string }> { +export async function triggerPatrolRun(scope?: PatrolRunScope): Promise<{ + success: boolean; + accepted?: boolean; + message: string; + run_id?: string; + started_at?: string; +}> { const hasScope = !!scope && ((scope.resource_ids?.length ?? 0) > 0 || (scope.resource_types?.length ?? 0) > 0); return apiFetchJSON('/api/ai/patrol/run', { diff --git a/frontend-modern/src/components/Storage/DiskDetail.tsx b/frontend-modern/src/components/Storage/DiskDetail.tsx index e51164af8..2f9e3c317 100644 --- a/frontend-modern/src/components/Storage/DiskDetail.tsx +++ b/frontend-modern/src/components/Storage/DiskDetail.tsx @@ -56,6 +56,8 @@ export const DiskDetail: Component = (props) => { attributeCards, historyCharts, metricResourceId, + collectionMessages, + liveIOAvailable, } = useDiskDetailModel({ disk: () => props.disk, }); @@ -115,6 +117,12 @@ export const DiskDetail: Component = (props) => { + 0}> +
+ {(message) =>

{message}

}
+
+
+ {/* SMART attribute cards */}
@@ -131,7 +139,7 @@ export const DiskDetail: Component = (props) => { {/* Live Performance Sparklines */} - +

{ expect(Array.from(rangeSelector.options).map((option) => option.value)).not.toContain('14d'); expect(Array.from(rangeSelector.options).map((option) => option.value)).not.toContain('90d'); }); + + it('shows explicit collection status and does not render misleading live I/O', () => { + const disk = buildDisk(); + disk.physicalDisk!.collection = { + serial: { state: 'available', source: 'smartctl' }, + temperature: { + state: 'unavailable', + source: 'smartctl', + reason: 'collection deadline exceeded', + }, + io: { + state: 'unsupported', + source: 'controller', + reason: 'per-member counters unavailable', + }, + controller: { state: 'available', source: 'linux-sysfs' }, + pool: { state: 'available', source: 'zpool-status' }, + }; + + render(() => ); + + expect( + screen.getByText('Temperature is temporarily unavailable: collection deadline exceeded'), + ).toBeInTheDocument(); + expect( + screen.getByText('Disk I/O is unsupported: per-member counters unavailable'), + ).toBeInTheDocument(); + expect(screen.queryByText('Live I/O (30m)')).not.toBeInTheDocument(); + expect(screen.queryByText(/:diskread:/)).not.toBeInTheDocument(); + expect(screen.queryByText(/:diskwrite:/)).not.toBeInTheDocument(); + }); }); diff --git a/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts b/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts index 9cb4ad7c8..322e3024d 100644 --- a/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts +++ b/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts @@ -77,4 +77,35 @@ describe('useDiskDetailModel', () => { expect(result.historyResourceId()).toBe('disk:truenas-main:sda'); }); + + it('disables live I/O only when the collector explicitly reports it unavailable', () => { + const [disk] = createSignal( + buildDisk({ + physicalDisk: { + devPath: '/dev/sda', + model: 'SAS archive disk', + serial: 'ZR5TESTA0001', + diskType: 'sas', + temperature: 30, + collection: { + serial: { state: 'available', source: 'smartctl' }, + temperature: { state: 'available', source: 'smartctl' }, + io: { + state: 'unsupported', + source: 'controller', + reason: 'per-member counters unavailable', + }, + controller: { state: 'available', source: 'linux-sysfs' }, + pool: { state: 'available', source: 'zpool-status' }, + }, + }, + } as Partial), + ); + const { result } = renderHook(() => useDiskDetailModel({ disk })); + + expect(result.liveIOAvailable()).toBe(false); + expect(result.collectionMessages()).toEqual([ + 'Disk I/O is unsupported: per-member counters unavailable', + ]); + }); }); diff --git a/frontend-modern/src/components/Storage/useDiskDetailModel.ts b/frontend-modern/src/components/Storage/useDiskDetailModel.ts index 0fd8dc3aa..7ea36e2a6 100644 --- a/frontend-modern/src/components/Storage/useDiskDetailModel.ts +++ b/frontend-modern/src/components/Storage/useDiskDetailModel.ts @@ -1,6 +1,7 @@ import { Accessor, createMemo, createSignal } from 'solid-js'; import type { HistoryTimeRange } from '@/api/charts'; import { + getPhysicalDiskCollectionMessages, extractPhysicalDiskPresentationData, type PhysicalDiskPresentationData, } from '@/features/storageBackups/diskPresentation'; @@ -29,6 +30,11 @@ export const useDiskDetailModel = (options: UseDiskDetailModelOptions) => { ); const historyCharts = createMemo(() => getDiskDetailHistoryCharts(diskData())); const metricResourceId = createMemo(() => historyResourceId()); + const collectionMessages = createMemo(() => getPhysicalDiskCollectionMessages(diskData())); + const liveIOAvailable = createMemo(() => { + const state = diskData().collection?.io?.state; + return !state || state === 'available'; + }); return { chartRange, @@ -38,5 +44,7 @@ export const useDiskDetailModel = (options: UseDiskDetailModelOptions) => { attributeCards, historyCharts, metricResourceId, + collectionMessages, + liveIOAvailable, }; }; diff --git a/frontend-modern/src/features/alerts/alertOverridesModel.ts b/frontend-modern/src/features/alerts/alertOverridesModel.ts index 75f2b4aa8..9c4d146da 100644 --- a/frontend-modern/src/features/alerts/alertOverridesModel.ts +++ b/frontend-modern/src/features/alerts/alertOverridesModel.ts @@ -284,6 +284,7 @@ export const buildProjectedOverrides = ({ const alertResourceIdCandidates = (resource: Resource): string[] => uniqueIds( resource.id, + ...(resource.canonicalIdentity?.supersededIds ?? []), resource.metricsTarget?.resourceId, resource.discoveryTarget?.resourceId, resource.platformId, diff --git a/frontend-modern/src/features/alerts/thresholds/guestThresholdOverrideMutationModel.ts b/frontend-modern/src/features/alerts/thresholds/guestThresholdOverrideMutationModel.ts index 8ea588e27..9f859e1a4 100644 --- a/frontend-modern/src/features/alerts/thresholds/guestThresholdOverrideMutationModel.ts +++ b/frontend-modern/src/features/alerts/thresholds/guestThresholdOverrideMutationModel.ts @@ -4,14 +4,22 @@ import { guestOverrideIdCandidates, guestOverrideStorageId } from '../guestOverr import type { Resource as TableResource } from './tableTypes'; import type { Override } from './types'; -const exactOverrideIdentity = (resource?: Pick) => ({ - candidateIds: resource?.id ? [resource.id] : [], - storageId: resource?.id ?? '', +type OverrideIdentityResource = Pick< + TableResource, + 'id' | 'type' | 'vmid' | 'node' | 'instance' | 'overrideIdCandidates' | 'overrideStorageId' +>; + +const exactOverrideIdentity = (resource?: OverrideIdentityResource) => ({ + candidateIds: + resource?.overrideIdCandidates && resource.overrideIdCandidates.length > 0 + ? resource.overrideIdCandidates + : resource?.id + ? [resource.id] + : [], + storageId: resource?.overrideStorageId || resource?.id || '', }); -export const getOverridePersistenceIdentity = ( - resource?: Pick, -) => { +export const getOverridePersistenceIdentity = (resource?: OverrideIdentityResource) => { if (!resource || resource.type !== 'guest') { return exactOverrideIdentity(resource); } @@ -25,7 +33,7 @@ export const getOverridePersistenceIdentity = ( export const findOverrideForResource = ( overrides: Override[], - resource?: Pick, + resource?: OverrideIdentityResource, ): Override | undefined => { const { candidateIds } = getOverridePersistenceIdentity(resource); return overrides.find((override) => candidateIds.includes(override.id)); @@ -33,7 +41,7 @@ export const findOverrideForResource = ( export const findRawOverrideConfigForResource = ( rawOverridesConfig: Record, - resource?: Pick, + resource?: OverrideIdentityResource, ): RawOverrideConfig | undefined => { const { candidateIds } = getOverridePersistenceIdentity(resource); return candidateIds @@ -43,7 +51,7 @@ export const findRawOverrideConfigForResource = ( export const stripOverrideCandidates = ( overrides: Override[], - resource?: Pick, + resource?: OverrideIdentityResource, ): Override[] => { const { candidateIds } = getOverridePersistenceIdentity(resource); if (candidateIds.length === 0) { @@ -54,7 +62,7 @@ export const stripOverrideCandidates = ( export const stripRawOverrideCandidates = ( rawOverridesConfig: Record, - resource?: Pick, + resource?: OverrideIdentityResource, ): Record => { const nextRawConfig = { ...rawOverridesConfig }; const { candidateIds } = getOverridePersistenceIdentity(resource); diff --git a/frontend-modern/src/features/alerts/thresholds/hooks/__tests__/truenasThresholdPersistence.test.tsx b/frontend-modern/src/features/alerts/thresholds/hooks/__tests__/truenasThresholdPersistence.test.tsx new file mode 100644 index 000000000..c8815dd02 --- /dev/null +++ b/frontend-modern/src/features/alerts/thresholds/hooks/__tests__/truenasThresholdPersistence.test.tsx @@ -0,0 +1,193 @@ +import { renderHook } from '@solidjs/testing-library'; +import { createSignal } from 'solid-js'; +import { describe, expect, it, vi } from 'vitest'; + +import { buildProjectedOverrides } from '@/features/alerts/alertOverridesModel'; +import type { Override } from '@/features/alerts/types'; +import type { ThresholdsTableProps } from '@/features/alerts/thresholds/types'; +import type { RawOverrideConfig } from '@/types/alerts'; +import type { Resource } from '@/types/resource'; + +import { useThresholdsOverrideMutations } from '../useThresholdsOverrideMutations'; +import { useThresholdsPlatformData } from '../useThresholdsPlatformData'; + +const canonicalID = 'agent-b9ed6d0e20e94eaf'; +const legacyCanonicalID = 'agent-535886018cb53055'; +const connectionID = 'truenas-connection-1'; + +const connectionBackedTrueNASResource = (machineId: string): Resource => + ({ + id: canonicalID, + type: 'agent', + name: 'strawberrynas', + displayName: 'Strawberry NAS', + status: 'online', + platformType: 'truenas', + sources: ['truenas'], + identity: { + hostname: 'strawberrynas', + machineId, + }, + truenas: { + hostname: 'strawberrynas', + }, + metricsTarget: { + resourceType: 'agent', + resourceId: connectionID, + }, + canonicalIdentity: { + primaryId: `agent:${connectionID}`, + aliases: [canonicalID, connectionID, 'strawberrynas'], + supersededIds: [legacyCanonicalID], + }, + memory: { + current: 87, + }, + }) as Resource; + +const projectOverrides = ( + rawConfig: Record, + resource: Resource, +): Override[] => + buildProjectedOverrides({ + rawConfig, + nodeResources: [], + vmResources: [], + containerResources: [], + storageResources: [], + agentResourceList: [resource], + containerRuntimeResources: [resource], + getChildren: () => [], + pbsInstanceById: new Map(), + allResources: [resource], + }); + +describe('TrueNAS threshold persistence identity', () => { + it('re-homes a connection-target override onto the canonical ID and survives reload/refetch', () => { + const [resource, setResource] = createSignal( + connectionBackedTrueNASResource('serial-visible-on-first-poll'), + ); + const initialRawConfig: Record = { + [connectionID]: { + memory: { + trigger: 95, + clear: 90, + }, + }, + }; + const [rawOverridesConfig, setRawOverridesConfig] = createSignal(initialRawConfig); + const [overrides, setOverrides] = createSignal( + projectOverrides(initialRawConfig, resource()), + ); + const [editingId] = createSignal(null); + const [editingThresholds] = createSignal>({ + memory: 95, + }); + const [editingNote] = createSignal(''); + const [bulkEditIds] = createSignal([]); + const setHasUnsavedChanges = vi.fn(); + const cancelEdit = vi.fn(); + + const props = { + get allResources() { + return [resource()]; + }, + overrides, + setOverrides, + rawOverridesConfig, + setRawOverridesConfig, + trueNASDefaults: { + memory: 85, + }, + trueNASDiskDefaults: {}, + backupDefaults: () => ({ enabled: false, warningDays: 7, criticalDays: 14 }), + snapshotDefaults: () => ({ + enabled: false, + warningDays: 30, + criticalDays: 45, + warningSizeGiB: 0, + criticalSizeGiB: 0, + }), + guestDisableConnectivity: () => false, + guestPoweredOffSeverity: () => 'warning' as const, + dockerDisableConnectivity: () => false, + dockerPoweredOffSeverity: () => 'warning' as const, + setHasUnsavedChanges, + } as unknown as ThresholdsTableProps; + + const platform = renderHook(() => + useThresholdsPlatformData({ + props, + editingId, + searchTerm: () => '', + }), + ); + const mutations = renderHook(() => + useThresholdsOverrideMutations({ + props, + resources: { + nodesWithOverrides: () => [], + agentsWithOverrides: () => [], + agentDisksWithOverrides: () => [], + dockerHostsWithOverrides: () => [], + guestsFlat: () => [], + dockerContainersFlat: () => [], + pbsServersWithOverrides: () => [], + pmgServersWithOverrides: () => [], + storageWithOverrides: () => [], + trueNASSystemsWithOverrides: platform.result.trueNASSystemsWithOverrides, + }, + editingThresholds, + editingNote, + bulkEditIds, + cancelEdit, + updateBackupDefaults: vi.fn(), + updateSnapshotDefaults: vi.fn(), + }), + ); + + expect(platform.result.trueNASSystemsWithOverrides()).toEqual([ + expect.objectContaining({ + id: canonicalID, + hasOverride: true, + thresholds: { + memory: 95, + }, + }), + ]); + + mutations.result.saveEdit(canonicalID); + + expect(rawOverridesConfig()).toEqual({ + [canonicalID]: { + memory: { + trigger: 95, + clear: 90, + }, + }, + }); + expect(rawOverridesConfig()).not.toHaveProperty(connectionID); + + // Model the alerts API JSON round-trip, followed by a resource refetch where + // TrueNAS changes the reported machine serial. The configured connection + // remains the durable identity, so the canonical resource ID must not move. + const persisted = JSON.parse(JSON.stringify(rawOverridesConfig())) as Record< + string, + RawOverrideConfig + >; + setRawOverridesConfig(persisted); + setResource(connectionBackedTrueNASResource('serial-missing-on-next-poll')); + setOverrides(projectOverrides(persisted, resource())); + + expect(platform.result.trueNASSystemsWithOverrides()).toEqual([ + expect.objectContaining({ + id: canonicalID, + hasOverride: true, + thresholds: { + memory: 95, + }, + }), + ]); + expect(setHasUnsavedChanges).toHaveBeenCalledWith(true); + }); +}); diff --git a/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsPlatformData.ts b/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsPlatformData.ts index 367f376f9..ca3cb06f7 100644 --- a/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsPlatformData.ts +++ b/frontend-modern/src/features/alerts/thresholds/hooks/useThresholdsPlatformData.ts @@ -35,6 +35,7 @@ type AlertPlatformResourceType = const resourceAlertIdCandidates = (resource: Resource): string[] => uniqueIds( resource.id, + ...(resource.canonicalIdentity?.supersededIds ?? []), resource.metricsTarget?.resourceId, resource.discoveryTarget?.resourceId, resource.platformId, @@ -113,7 +114,9 @@ const toTableResource = ({ const note = typeof override?.note === 'string' ? override.note : undefined; return { - id: override?.id || candidates[0] || resource.id, + id: resourceAlertActionId(resource), + overrideIdCandidates: candidates, + overrideStorageId: resource.id, name: getAlertResourceDisplayLabel(resource), displayName: getAlertResourceDisplayLabel(resource), rawName: resource.name, diff --git a/frontend-modern/src/features/alerts/thresholds/tableTypes.ts b/frontend-modern/src/features/alerts/thresholds/tableTypes.ts index 09e703205..393506027 100644 --- a/frontend-modern/src/features/alerts/thresholds/tableTypes.ts +++ b/frontend-modern/src/features/alerts/thresholds/tableTypes.ts @@ -7,6 +7,8 @@ export type ThresholdsActiveTab = export interface Resource { id: string; + overrideIdCandidates?: string[]; + overrideStorageId?: string; name: string; displayName?: string; policy?: ResourcePolicy; diff --git a/frontend-modern/src/features/patrol/__tests__/patrolRunAcceptance.test.ts b/frontend-modern/src/features/patrol/__tests__/patrolRunAcceptance.test.ts new file mode 100644 index 000000000..f7401dc25 --- /dev/null +++ b/frontend-modern/src/features/patrol/__tests__/patrolRunAcceptance.test.ts @@ -0,0 +1,155 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { PatrolRunRecord, PatrolStatus } from '@/api/patrol'; +import { + schedulePatrolRunAcceptanceReconciliation, + type PatrolRunAcceptanceOutcome, +} from '../patrolRunAcceptance'; + +const runningStatus = (runId: string): PatrolStatus => + ({ + runtime_state: 'running', + running: true, + current_run_id: runId, + enabled: true, + }) as PatrolStatus; + +const completedRun = (runId: string, status: 'healthy' | 'error' = 'healthy'): PatrolRunRecord => + ({ + id: runId, + status, + error_count: status === 'error' ? 1 : 0, + }) as PatrolRunRecord; + +describe('Patrol accepted-run reconciliation', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + for (const firstProviderEventMs of [0, 15_000, 30_000, 60_000]) { + it(`keeps one accepted run authoritative when the first provider event arrives at ${firstProviderEventMs / 1000}s`, async () => { + const runId = `run-${firstProviderEventMs}`; + let providerEventObserved = false; + const history: PatrolRunRecord[] = []; + const errorToast = vi.fn(); + const onResult = vi.fn<(outcome: PatrolRunAcceptanceOutcome) => void>((outcome) => { + if (outcome.kind === 'missing') errorToast(); + }); + const getStatus = vi.fn(async () => + history.length > 0 + ? ({ ...runningStatus(runId), runtime_state: 'active', running: false } as PatrolStatus) + : runningStatus(runId), + ); + const getHistory = vi.fn(async () => [...history]); + let providerCalls = 0; + + providerCalls += 1; + setTimeout(() => { + providerEventObserved = true; + }, firstProviderEventMs); + setTimeout(() => { + history.push(completedRun(runId)); + }, firstProviderEventMs + 1_000); + + schedulePatrolRunAcceptanceReconciliation({ + runId, + delayMs: 15_000, + refreshTimeoutMs: 5_000, + getStatus, + getHistory, + isCurrent: () => true, + onResult, + }); + + await vi.advanceTimersByTimeAsync(15_000); + expect(onResult).toHaveBeenCalledTimes(1); + expect(onResult.mock.calls[0][0].kind).toMatch(/running|recorded/); + expect(onResult).not.toHaveBeenCalledWith(expect.objectContaining({ kind: 'missing' })); + expect(errorToast).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(Math.max(0, firstProviderEventMs + 1_000 - 15_000)); + expect(providerCalls).toBe(1); + expect(providerEventObserved).toBe(true); + expect(history).toHaveLength(1); + expect(history[0].id).toBe(runId); + expect(getStatus).toHaveBeenCalledTimes(1); + expect(getHistory).toHaveBeenCalledTimes(1); + }); + } + + it('preserves a provider/runtime failure as a recorded run instead of a start failure', async () => { + const onResult = vi.fn<(outcome: PatrolRunAcceptanceOutcome) => void>(); + schedulePatrolRunAcceptanceReconciliation({ + runId: 'run-provider-error', + delayMs: 15_000, + refreshTimeoutMs: 5_000, + getStatus: async () => ({ ...runningStatus('run-provider-error'), running: false }), + getHistory: async () => [completedRun('run-provider-error', 'error')], + isCurrent: () => true, + onResult, + }); + + await vi.advanceTimersByTimeAsync(15_000); + expect(onResult).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'recorded', + run: expect.objectContaining({ id: 'run-provider-error', status: 'error' }), + }), + ); + }); + + it('bounds hung reconciliation reads and reports a refresh failure', async () => { + const onResult = vi.fn<(outcome: PatrolRunAcceptanceOutcome) => void>(); + schedulePatrolRunAcceptanceReconciliation({ + runId: 'run-hung-read', + delayMs: 15_000, + refreshTimeoutMs: 5_000, + getStatus: () => new Promise(() => undefined), + getHistory: () => new Promise(() => undefined), + isCurrent: () => true, + onResult, + }); + + await vi.advanceTimersByTimeAsync(20_000); + expect(onResult).toHaveBeenCalledTimes(1); + expect(onResult.mock.calls[0][0].kind).toBe('refresh_failed'); + }); + + it('cancels without reading status or history and permits a clean retry', async () => { + const firstStatus = vi.fn(async () => runningStatus('run-cancelled')); + const firstHistory = vi.fn(async () => [] as PatrolRunRecord[]); + const firstResult = vi.fn<(outcome: PatrolRunAcceptanceOutcome) => void>(); + const cancel = schedulePatrolRunAcceptanceReconciliation({ + runId: 'run-cancelled', + delayMs: 15_000, + refreshTimeoutMs: 5_000, + getStatus: firstStatus, + getHistory: firstHistory, + isCurrent: () => true, + onResult: firstResult, + }); + cancel(); + + await vi.advanceTimersByTimeAsync(15_000); + expect(firstStatus).not.toHaveBeenCalled(); + expect(firstHistory).not.toHaveBeenCalled(); + expect(firstResult).not.toHaveBeenCalled(); + + const retryResult = vi.fn<(outcome: PatrolRunAcceptanceOutcome) => void>(); + schedulePatrolRunAcceptanceReconciliation({ + runId: 'run-retry', + delayMs: 15_000, + refreshTimeoutMs: 5_000, + getStatus: async () => runningStatus('run-retry'), + getHistory: async () => [], + isCurrent: () => true, + onResult: retryResult, + }); + await vi.advanceTimersByTimeAsync(15_000); + expect(retryResult).toHaveBeenCalledWith(expect.objectContaining({ kind: 'running' })); + }); +}); diff --git a/frontend-modern/src/features/patrol/__tests__/usePatrolIntelligenceState.test.ts b/frontend-modern/src/features/patrol/__tests__/usePatrolIntelligenceState.test.ts index c5b76d9bc..f22f88961 100644 --- a/frontend-modern/src/features/patrol/__tests__/usePatrolIntelligenceState.test.ts +++ b/frontend-modern/src/features/patrol/__tests__/usePatrolIntelligenceState.test.ts @@ -5,6 +5,7 @@ import { PATROL_REFRESH_TIMEOUT_MS, buildPatrolSettingsReadinessFailure, openPatrolAssistantWorkflowHandoff, + patrolStartFailureMessage, recordPatrolControlStarterActivity, recordPatrolWorkflowStarterActivity, resolvePatrolAutonomyLevelForSave, @@ -70,6 +71,20 @@ describe('usePatrolIntelligenceState', () => { expect(patrolIntelligenceStateSource).toContain('PATROL_MANUAL_SYNC_TIMEOUT_MS'); }); + it('keeps browser/network start failures distinct from backend rejections', () => { + expect(patrolStartFailureMessage(new TypeError('Failed to fetch'))).toBe( + 'Could not reach Pulse to start Patrol: Failed to fetch', + ); + expect( + patrolStartFailureMessage( + Object.assign(new Error('Patrol is already running.'), { + code: 'patrol_already_running', + status: 409, + }), + ), + ).toBe('Patrol is already running.'); + }); + it('fails soft when Patrol data refreshes reject', () => { expect(patrolIntelligenceStateSource).toContain('const [patrolLoadError, setPatrolLoadError]'); expect(patrolIntelligenceStateSource).toContain('rememberPatrolLoadError'); diff --git a/frontend-modern/src/features/patrol/patrolRunAcceptance.ts b/frontend-modern/src/features/patrol/patrolRunAcceptance.ts new file mode 100644 index 000000000..35ec3a519 --- /dev/null +++ b/frontend-modern/src/features/patrol/patrolRunAcceptance.ts @@ -0,0 +1,100 @@ +import type { PatrolRunRecord, PatrolStatus } from '@/api/patrol'; + +export type PatrolRunAcceptanceOutcome = + | { kind: 'running'; status: PatrolStatus } + | { kind: 'recorded'; run: PatrolRunRecord } + | { kind: 'missing' } + | { kind: 'refresh_failed'; error: unknown }; + +interface PatrolRunAcceptanceReconciliationOptions { + runId: string; + delayMs: number; + refreshTimeoutMs: number; + getStatus: () => Promise; + getHistory: () => Promise; + isCurrent: () => boolean; + onResult: (outcome: PatrolRunAcceptanceOutcome) => void; +} + +const timeoutError = () => new Error('Patrol acceptance reconciliation timed out'); + +async function settleReconciliationReads( + getStatus: () => Promise, + getHistory: () => Promise, + timeoutMs: number, +): Promise< + | { + status: PromiseSettledResult; + history: PromiseSettledResult; + } + | { error: unknown } +> { + let timeout: ReturnType | undefined; + try { + return await Promise.race([ + Promise.allSettled([getStatus(), getHistory()]).then(([status, history]) => ({ + status, + history, + })), + new Promise<{ error: unknown }>((resolve) => { + timeout = setTimeout(() => resolve({ error: timeoutError() }), timeoutMs); + }), + ]); + } finally { + if (timeout !== undefined) clearTimeout(timeout); + } +} + +export function schedulePatrolRunAcceptanceReconciliation( + options: PatrolRunAcceptanceReconciliationOptions, +): () => void { + let cancelled = false; + const timer = setTimeout(() => { + void settleReconciliationReads( + options.getStatus, + options.getHistory, + options.refreshTimeoutMs, + ).then((reads) => { + if (cancelled || !options.isCurrent()) return; + if ('error' in reads) { + options.onResult({ kind: 'refresh_failed', error: reads.error }); + return; + } + + const status = reads.status.status === 'fulfilled' ? reads.status.value : null; + const history = reads.history.status === 'fulfilled' ? reads.history.value : null; + const statusOwnsAcceptedRun = + status?.running === true && + (!options.runId || !status.current_run_id || status.current_run_id === options.runId); + if (statusOwnsAcceptedRun) { + options.onResult({ kind: 'running', status }); + return; + } + + const recordedRun = options.runId + ? history?.find((run) => run.id === options.runId) + : undefined; + if (recordedRun) { + options.onResult({ kind: 'recorded', run: recordedRun }); + return; + } + + const refreshError = + reads.status.status === 'rejected' + ? reads.status.reason + : reads.history.status === 'rejected' + ? reads.history.reason + : undefined; + if (refreshError !== undefined) { + options.onResult({ kind: 'refresh_failed', error: refreshError }); + return; + } + options.onResult({ kind: 'missing' }); + }); + }, options.delayMs); + + return () => { + cancelled = true; + clearTimeout(timer); + }; +} diff --git a/frontend-modern/src/features/patrol/usePatrolIntelligenceState.ts b/frontend-modern/src/features/patrol/usePatrolIntelligenceState.ts index d6ac6637c..c208b179b 100644 --- a/frontend-modern/src/features/patrol/usePatrolIntelligenceState.ts +++ b/frontend-modern/src/features/patrol/usePatrolIntelligenceState.ts @@ -40,6 +40,7 @@ import { } from '@/stores/license'; import { PATROL_AUTONOMY_FEATURE_KEY } from './patrolAutonomyAvailability'; import type { AISettings } from '@/types/ai'; +import { apiErrorStatus } from '@/api/responseUtils'; import { hasFindingInvestigationHandoffPointer, isPatrolRuntimeFinding, @@ -63,6 +64,7 @@ import { type PatrolAssistantFindingHandoff, type PatrolConfigurationFailureInput, } from './patrolInvestigationContextModel'; +import { schedulePatrolRunAcceptanceReconciliation } from './patrolRunAcceptance'; type PatrolTab = 'findings' | 'history'; @@ -93,6 +95,8 @@ const recordPatrolWorkflowStarterActivityForSurface = async ( export const PATROL_REFRESH_TIMEOUT_MS = 15000; export const PATROL_MANUAL_SYNC_TIMEOUT_MS = 5000; +export const PATROL_ACCEPTANCE_RECONCILE_MS = 15000; +export const PATROL_ACCEPTANCE_REFRESH_TIMEOUT_MS = 5000; export function recordPatrolWorkflowStarterActivity(): void { void recordPatrolWorkflowStarterActivityForSurface( @@ -132,6 +136,11 @@ export function openPatrolAssistantWorkflowHandoff( const patrolErrorMessage = (error: unknown, fallback: string) => error instanceof Error && error.message.trim() ? error.message : fallback; +export const patrolStartFailureMessage = (error: unknown): string => + apiErrorStatus(error) === null + ? `Could not reach Pulse to start Patrol: ${patrolErrorMessage(error, 'network request failed')}` + : patrolErrorMessage(error, 'Pulse rejected the Patrol run'); + const buildReadinessDetails = ( readiness: NonNullable, ): Record => { @@ -239,6 +248,7 @@ export function usePatrolIntelligenceState() { const [isUpdatingAutonomy, setIsUpdatingAutonomy] = createSignal(false); const [activityRefreshTrigger, setActivityRefreshTrigger] = createSignal(0); const [manualRunRequested, setManualRunRequested] = createSignal(false); + const [acceptedManualRunId, setAcceptedManualRunId] = createSignal(''); const [patrolEnabledLocal, setPatrolEnabledLocal] = createSignal(true); const [liveRunStartedAt, setLiveRunStartedAt] = createSignal(''); const [investigationBudget, setInvestigationBudget] = createSignal(15); @@ -250,22 +260,29 @@ export function usePatrolIntelligenceState() { const [assistantHandoffFindingId, setAssistantHandoffFindingId] = createSignal(''); const [patrolLoadError, setPatrolLoadError] = createSignal(''); - let safetyTimerRef: ReturnType | undefined; + let cancelAcceptanceReconciliation: (() => void) | undefined; let findingScrollTimerRef: ReturnType | undefined; let refreshTimeoutRef: ReturnType | undefined; let manualRefreshTimeoutRef: ReturnType | undefined; let refreshRequestId = 0; let manualRefreshRequestId = 0; + let manualRunRequestId = 0; let refreshInterval: ReturnType; let approvalPollInterval: ReturnType; - const clearSafetyTimer = () => { - if (safetyTimerRef !== undefined) { - clearTimeout(safetyTimerRef); - safetyTimerRef = undefined; + const clearAcceptanceReconcileTimer = () => { + if (cancelAcceptanceReconciliation !== undefined) { + cancelAcceptanceReconciliation(); + cancelAcceptanceReconciliation = undefined; } }; + const finishManualRunTracking = () => { + clearAcceptanceReconcileTimer(); + setAcceptedManualRunId(''); + setManualRunRequested(false); + }; + const clearRefreshTimeout = () => { if (refreshTimeoutRef !== undefined) { clearTimeout(refreshTimeoutRef); @@ -323,15 +340,12 @@ export function usePatrolIntelligenceState() { const patrolStream = usePatrolStream({ running: () => patrolEnabledLocal() && ((patrolStatus()?.running ?? false) || manualRunRequested()), - onStart: () => { - clearSafetyTimer(); - }, onComplete: () => { - setManualRunRequested(false); + finishManualRunTracking(); loadAllData(); }, onError: () => { - setManualRunRequested(false); + finishManualRunTracking(); loadAllData(); }, }); @@ -377,8 +391,9 @@ export function usePatrolIntelligenceState() { const newValue = !previousValue; setPatrolEnabledLocal(newValue); if (!newValue) { - setManualRunRequested(false); - clearSafetyTimer(); + manualRunRequestId += 1; + finishManualRunTracking(); + setIsTriggeringPatrol(false); } try { const data = await AIAPI.updateSettings({ patrol_enabled: newValue }); @@ -413,41 +428,60 @@ export function usePatrolIntelligenceState() { ) { return; } + const requestId = ++manualRunRequestId; setIsTriggeringPatrol(true); setManualRunRequested(true); - clearSafetyTimer(); - safetyTimerRef = setTimeout(() => { - safetyTimerRef = undefined; - if (!manualRunRequested() || patrolStream.isStreaming()) { - return; - } - // The trigger request already succeeded, so a quiet stream can also mean - // a slow provider (a self-hosted model cold-loading can take well over - // 15s to emit its first event). Only report a failure when the refreshed - // status confirms no run is in progress; otherwise let the running state - // carry the button. - void loadAllData() - .catch(() => undefined) - .then(() => { - setManualRunRequested(false); - if (!patrolStream.isStreaming() && !(patrolStatus()?.running ?? false)) { - notificationStore.error( - 'Patrol run did not start. The provider did not respond; check the AI provider settings or run the Patrol preflight.', - ); - } - }); - }, 15000); + clearAcceptanceReconcileTimer(); try { - await triggerPatrolRun(); + const acceptance = await triggerPatrolRun(); + if (requestId !== manualRunRequestId || !manualRunRequested()) { + return; + } + const runId = acceptance?.run_id?.trim() || ''; + setAcceptedManualRunId(runId); + cancelAcceptanceReconciliation = schedulePatrolRunAcceptanceReconciliation({ + runId, + delayMs: PATROL_ACCEPTANCE_RECONCILE_MS, + refreshTimeoutMs: PATROL_ACCEPTANCE_REFRESH_TIMEOUT_MS, + getStatus: refetchPatrolStatus, + getHistory: () => getPatrolRunHistory(30), + isCurrent: () => + requestId === manualRunRequestId && + manualRunRequested() && + acceptedManualRunId() === runId, + onResult: (outcome) => { + cancelAcceptanceReconciliation = undefined; + finishManualRunTracking(); + if (outcome.kind === 'running' || outcome.kind === 'recorded') { + setActivityRefreshTrigger((prev) => prev + 1); + return; + } + if (outcome.kind === 'refresh_failed') { + rememberPatrolLoadError( + outcome.error, + 'Patrol accepted the run, but its live status could not be refreshed.', + ); + return; + } + notificationStore.error( + runId + ? `Pulse accepted Patrol run ${runId}, but the backend no longer reports it as running and did not record it in history.` + : 'Pulse accepted the Patrol run, but the backend no longer reports it as running and did not record it in history.', + ); + }, + }); } catch (err) { console.error('Failed to trigger patrol run:', err); - setManualRunRequested(false); - notificationStore.error(patrolErrorMessage(err, 'Failed to start patrol run')); - clearSafetyTimer(); + if (requestId === manualRunRequestId) { + finishManualRunTracking(); + notificationStore.error(patrolStartFailureMessage(err)); + } return; } finally { - setIsTriggeringPatrol(false); + if (requestId === manualRunRequestId) { + setIsTriggeringPatrol(false); + } } void loadAllData().catch((err) => { @@ -993,7 +1027,8 @@ export function usePatrolIntelligenceState() { onCleanup(() => { stopPolling(); - clearSafetyTimer(); + manualRunRequestId += 1; + clearAcceptanceReconcileTimer(); clearManualRefreshTimeout(); if (findingScrollTimerRef !== undefined) { clearTimeout(findingScrollTimerRef); @@ -1005,6 +1040,7 @@ export function usePatrolIntelligenceState() { activeTab, activePatrolFindings, activityRefreshTrigger, + acceptedManualRunId, assistantHandoffFindingId, autonomyLevel, requestedAutonomyLevel, diff --git a/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts b/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts index e751f6984..cf98e27d5 100644 --- a/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts +++ b/frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts @@ -18,6 +18,8 @@ import { PHYSICAL_DISK_TABLE_CLASS, PHYSICAL_DISK_TABLE_ROW_HOVER_CLASS, getPhysicalDiskEmptyStatePresentation, + getPhysicalDiskCollectionMessages, + getPhysicalDiskFieldStatusMessage, getPhysicalDiskHealthStatus, getPhysicalDiskHealthSummary, getPhysicalDiskHostLabel, @@ -61,6 +63,47 @@ function makeDiskData( } describe('diskPresentation', () => { + it('distinguishes unsupported, unavailable, and unexpectedly missing disk evidence', () => { + expect( + getPhysicalDiskFieldStatusMessage('Disk I/O', { + state: 'unsupported', + source: 'controller', + reason: 'per-member counters unavailable', + }), + ).toBe('Disk I/O is unsupported: per-member counters unavailable'); + expect( + getPhysicalDiskFieldStatusMessage('Temperature', { + state: 'unavailable', + source: 'smartctl', + reason: 'collection deadline exceeded', + }), + ).toBe('Temperature is temporarily unavailable: collection deadline exceeded'); + expect( + getPhysicalDiskFieldStatusMessage('Serial number', { + state: 'missing', + source: 'smartctl', + reason: 'serial absent from successful response', + }), + ).toBe('Serial number is unexpectedly missing: serial absent from successful response'); + expect( + getPhysicalDiskCollectionMessages( + makeDiskData({ + collection: { + serial: { state: 'available', source: 'smartctl' }, + temperature: { state: 'unavailable', source: 'smartctl' }, + io: { state: 'unsupported', source: 'controller' }, + controller: { state: 'missing', source: 'linux-sysfs' }, + pool: { state: 'available', source: 'zpool-status' }, + }, + }), + ), + ).toEqual([ + 'Temperature is temporarily unavailable.', + 'Disk I/O is unsupported.', + 'Controller association is unexpectedly missing.', + ]); + }); + it('returns critical presentation for failed disks', () => { expect(PHYSICAL_DISK_EMPTY_CARD_CLASS).toBe('text-center'); expect(PHYSICAL_DISK_TABLE_CLASS).toBe('w-full table-fixed text-xs'); diff --git a/frontend-modern/src/features/storageBackups/diskPresentation.ts b/frontend-modern/src/features/storageBackups/diskPresentation.ts index 744d66b3b..f91f60aae 100644 --- a/frontend-modern/src/features/storageBackups/diskPresentation.ts +++ b/frontend-modern/src/features/storageBackups/diskPresentation.ts @@ -1,4 +1,8 @@ -import type { Resource } from '@/types/resource'; +import type { + PhysicalDiskCollectionStatus, + PhysicalDiskFieldStatus, + Resource, +} from '@/types/resource'; import { getSourcePlatformLabel, getSourcePlatformPresentation, @@ -54,9 +58,12 @@ export interface PhysicalDiskPresentationData { writeCount?: number; errorCount?: number; type: string; + controller?: string; + target?: string; temperature: number; rpm: number; used: string; + collection?: PhysicalDiskCollectionStatus; smartAttributes?: { powerOnHours?: number; powerCycles?: number; @@ -296,6 +303,8 @@ export function extractPhysicalDiskPresentationData( serial: pd.serial || '', wwn: pd.wwn || '', type: pd.diskType || '', + controller: pd.controller, + target: pd.target, size: pd.sizeBytes || 0, health: pd.health || 'UNKNOWN', wearout: pd.wearout ?? -1, @@ -309,6 +318,7 @@ export function extractPhysicalDiskPresentationData( readCount: pd.readCount, writeCount: pd.writeCount, errorCount: pd.errorCount, + collection: pd.collection, riskLevel: pd.risk?.level, riskReasons, smartAttributes: pd.smart @@ -328,6 +338,36 @@ export function extractPhysicalDiskPresentationData( }; } +export function getPhysicalDiskFieldStatusMessage( + label: string, + status: PhysicalDiskFieldStatus | null | undefined, +): string { + if (!status || status.state === 'available') return ''; + const reason = status.reason?.trim(); + switch (status.state) { + case 'unsupported': + return `${label} is unsupported${reason ? `: ${reason}` : '.'}`; + case 'unavailable': + return `${label} is temporarily unavailable${reason ? `: ${reason}` : '.'}`; + case 'missing': + return `${label} is unexpectedly missing${reason ? `: ${reason}` : '.'}`; + default: + return ''; + } +} + +export function getPhysicalDiskCollectionMessages(disk: PhysicalDiskPresentationData): string[] { + const collection = disk.collection; + if (!collection) return []; + return [ + getPhysicalDiskFieldStatusMessage('Serial number', collection.serial), + getPhysicalDiskFieldStatusMessage('Temperature', collection.temperature), + getPhysicalDiskFieldStatusMessage('Disk I/O', collection.io), + getPhysicalDiskFieldStatusMessage('Controller association', collection.controller), + getPhysicalDiskFieldStatusMessage('Pool membership', collection.pool), + ].filter((message): message is string => message.length > 0); +} + export function buildPhysicalDiskPresentationDataMap( disks: Resource[], ): Map { diff --git a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts index 9e9de416d..45e6085e6 100644 --- a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts +++ b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts @@ -1303,6 +1303,7 @@ describe('useUnifiedResources', () => { platformId: 'pve1', primaryId: 'node:instance-pve1', aliases: ['node:instance-pve1', 'instance-pve1', 'tower.local'], + supersededIds: ['agent-retired'], }, }, ], @@ -1324,6 +1325,7 @@ describe('useUnifiedResources', () => { expect(result!.resources()[0].canonicalIdentity).toMatchObject({ primaryId: 'node:instance-pve1', hostname: 'tower.local', + supersededIds: ['agent-retired'], }); dispose(); diff --git a/frontend-modern/src/hooks/useUnifiedResources.ts b/frontend-modern/src/hooks/useUnifiedResources.ts index 4f0aac581..1bf178ad4 100644 --- a/frontend-modern/src/hooks/useUnifiedResources.ts +++ b/frontend-modern/src/hooks/useUnifiedResources.ts @@ -469,6 +469,7 @@ type APIResource = { platformId?: string; primaryId?: string; aliases?: string[]; + supersededIds?: string[]; }; policy?: { sensitivity?: string; diff --git a/frontend-modern/src/pages/__tests__/AIIntelligence.test.tsx b/frontend-modern/src/pages/__tests__/AIIntelligence.test.tsx index 0c5c45f10..4a7d6b887 100644 --- a/frontend-modern/src/pages/__tests__/AIIntelligence.test.tsx +++ b/frontend-modern/src/pages/__tests__/AIIntelligence.test.tsx @@ -865,8 +865,11 @@ describe('AIIntelligence entitlement gating', () => { it('surfaces backend readiness rejection when a stale manual run request reaches the server', async () => { triggerPatrolRunMock.mockRejectedValue( - new Error( - 'The selected Patrol model is a reasoning-only model family that commonly does not emit tool calls.', + Object.assign( + new Error( + 'The selected Patrol model is a reasoning-only model family that commonly does not emit tool calls.', + ), + { code: 'patrol_readiness_not_ready', status: 409 }, ), ); diff --git a/frontend-modern/src/types/api.ts b/frontend-modern/src/types/api.ts index 70392648a..8e97a03f6 100644 --- a/frontend-modern/src/types/api.ts +++ b/frontend-modern/src/types/api.ts @@ -1126,6 +1126,8 @@ export interface PhysicalDisk { serial: string; wwn: string; type: 'nvme' | 'sata' | 'sas' | string; + controller?: string; + target?: string; size: number; health: 'PASSED' | 'FAILED' | 'UNKNOWN' | string; wearout: number; // 0-100, 100 is best, -1 when the controller doesn't report it @@ -1134,6 +1136,17 @@ export interface PhysicalDisk { used: string; lastChecked: string; smartAttributes?: SMARTAttributes; + io?: { + device?: string; + readBytes?: number; + writeBytes?: number; + readOps?: number; + writeOps?: number; + readTimeMs?: number; + writeTimeMs?: number; + ioTimeMs?: number; + }; + collection?: import('./resource').PhysicalDiskCollectionStatus; } export interface CPUInfo { diff --git a/frontend-modern/src/types/resource.ts b/frontend-modern/src/types/resource.ts index c1d56f76b..24b23d143 100644 --- a/frontend-modern/src/types/resource.ts +++ b/frontend-modern/src/types/resource.ts @@ -189,6 +189,7 @@ export interface ResourceCanonicalIdentity { platformId?: string; primaryId?: string; aliases?: string[]; + supersededIds?: string[]; } export type ResourceSensitivity = 'public' | 'internal' | 'sensitive' | 'restricted'; @@ -451,6 +452,33 @@ export interface ResourcePhysicalDiskRisk { reasons?: ResourceStorageRiskReason[]; } +export type PhysicalDiskFieldState = 'available' | 'unavailable' | 'unsupported' | 'missing'; + +export interface PhysicalDiskFieldStatus { + state: PhysicalDiskFieldState; + source?: string; + reason?: string; +} + +export interface PhysicalDiskCollectionStatus { + serial?: PhysicalDiskFieldStatus; + temperature?: PhysicalDiskFieldStatus; + io?: PhysicalDiskFieldStatus; + controller?: PhysicalDiskFieldStatus; + pool?: PhysicalDiskFieldStatus; +} + +export interface ResourcePhysicalDiskIO { + device?: string; + readBytes?: number; + writeBytes?: number; + readOps?: number; + writeOps?: number; + readTimeMs?: number; + writeTimeMs?: number; + ioTimeMs?: number; +} + export interface ResourceCephPoolMeta { name: string; storedBytes: number; @@ -485,6 +513,8 @@ export interface ResourcePhysicalDiskMeta { serial?: string; wwn?: string; diskType?: string; + controller?: string; + target?: string; sizeBytes?: number; health?: string; wearout?: number; @@ -498,6 +528,8 @@ export interface ResourcePhysicalDiskMeta { readCount?: number; writeCount?: number; errorCount?: number; + io?: ResourcePhysicalDiskIO; + collection?: PhysicalDiskCollectionStatus; smart?: { powerOnHours?: number; powerCycles?: number; diff --git a/frontend-modern/src/utils/resourceStateAdapters.ts b/frontend-modern/src/utils/resourceStateAdapters.ts index 75ea50982..6f4ccc6ca 100644 --- a/frontend-modern/src/utils/resourceStateAdapters.ts +++ b/frontend-modern/src/utils/resourceStateAdapters.ts @@ -693,10 +693,12 @@ const mergeCanonicalIdentity = ( if (!incoming) return existing; if (!existing) return incoming; const aliases = mergeStringArrays(incoming.aliases, existing.aliases); + const supersededIds = mergeStringArrays(incoming.supersededIds, existing.supersededIds); return { ...existing, ...incoming, aliases, + supersededIds, }; }; diff --git a/internal/ai/demo.go b/internal/ai/demo.go index 86cbb9769..a7c25a96c 100644 --- a/internal/ai/demo.go +++ b/internal/ai/demo.go @@ -51,14 +51,23 @@ func isDemoFindingID(id string) bool { // yet (mock disabled again, another run in flight, or mock state not // generated yet). func (p *PatrolService) runDemoPatrolCycle(trigger TriggerReason) bool { + return p.runDemoPatrolCycleWithStart(trigger, nil) +} + +func (p *PatrolService) runDemoPatrolCycleWithStart(trigger TriggerReason, acceptedStart *patrolRunStart) bool { if p == nil || p.findings == nil || p.runHistoryStore == nil { return false } if !IsDemoMode() { return false } - if !p.tryStartRun("full") { - return false + runStart := acceptedStart + if runStart == nil { + var accepted bool + runStart, accepted = p.beginRun("full") + if !accepted { + return false + } } defer p.endRun() @@ -114,6 +123,12 @@ func (p *PatrolService) runDemoPatrolCycle(trigger TriggerReason) bool { // retry or the next scheduled cycle will populate the surface once // state exists. log.Debug().Msg("demo patrol: no mock state available yet, skipping cycle") + if acceptedStart != nil { + p.recordAcceptedRunFailure(runStart, trigger, + "Patrol demo state unavailable", + "Pulse accepted the run, but demo infrastructure state was not available.") + return true + } return false } @@ -181,10 +196,17 @@ func (p *PatrolService) runDemoPatrolCycle(trigger TriggerReason) bool { // Presentational duration and token counts: deterministic, scaled to the // size of the mock fleet so the history reads like a real analysis pass. duration := time.Duration(35+resourceCount%40) * time.Second + runID := fmt.Sprintf("demo-run-%d", now.UnixNano()) + startedAt := now.Add(-duration) + if acceptedStart != nil { + runID = runStart.id + startedAt = runStart.startedAt + duration = now.Sub(startedAt) + } record := PatrolRunRecord{ - ID: fmt.Sprintf("demo-run-%d", now.UnixNano()), + ID: runID, Source: PatrolRunSourceDemo, - StartedAt: now.Add(-duration), + StartedAt: startedAt, CompletedAt: now, Duration: duration, DurationMs: duration.Milliseconds(), diff --git a/internal/ai/patrol.go b/internal/ai/patrol.go index a3e6c4f8d..3d921ad99 100644 --- a/internal/ai/patrol.go +++ b/internal/ai/patrol.go @@ -88,22 +88,24 @@ const ( ) type PatrolStatus struct { - RuntimeState PatrolRuntimeState `json:"runtime_state"` - Running bool `json:"running"` - Enabled bool `json:"enabled"` - LastPatrolAt *time.Time `json:"last_patrol_at,omitempty"` // Last completed full patrol - LastActivityAt *time.Time `json:"last_activity_at,omitempty"` // Last completed Patrol activity of any kind - TriggerStatus *TriggerStatus `json:"trigger_status,omitempty"` - NextPatrolAt *time.Time `json:"next_patrol_at,omitempty"` - LastDuration time.Duration `json:"last_duration_ms"` - ResourcesChecked int `json:"resources_checked"` - FindingsCount int `json:"findings_count"` - ErrorCount int `json:"error_count"` - Healthy bool `json:"healthy"` - IntervalMs int64 `json:"interval_ms"` // Patrol interval in milliseconds - BlockedReason string `json:"blocked_reason,omitempty"` - BlockedCause PatrolFailureCause `json:"blocked_cause,omitempty"` - BlockedAt *time.Time `json:"blocked_at,omitempty"` + RuntimeState PatrolRuntimeState `json:"runtime_state"` + Running bool `json:"running"` + CurrentRunID string `json:"current_run_id,omitempty"` + CurrentRunStartedAt *time.Time `json:"current_run_started_at,omitempty"` + Enabled bool `json:"enabled"` + LastPatrolAt *time.Time `json:"last_patrol_at,omitempty"` // Last completed full patrol + LastActivityAt *time.Time `json:"last_activity_at,omitempty"` // Last completed Patrol activity of any kind + TriggerStatus *TriggerStatus `json:"trigger_status,omitempty"` + NextPatrolAt *time.Time `json:"next_patrol_at,omitempty"` + LastDuration time.Duration `json:"last_duration_ms"` + ResourcesChecked int `json:"resources_checked"` + FindingsCount int `json:"findings_count"` + ErrorCount int `json:"error_count"` + Healthy bool `json:"healthy"` + IntervalMs int64 `json:"interval_ms"` // Patrol interval in milliseconds + BlockedReason string `json:"blocked_reason,omitempty"` + BlockedCause PatrolFailureCause `json:"blocked_cause,omitempty"` + BlockedAt *time.Time `json:"blocked_at,omitempty"` } // PatrolFindingAssessment is the model's explicit evidence-grounded verdict @@ -549,6 +551,7 @@ type PatrolService struct { // Runtime state running bool runInProgress bool + currentRunID string runStartedAt time.Time stopCh chan struct{} configChanged chan struct{} // Signal when config changes to reset ticker diff --git a/internal/ai/patrol_findings.go b/internal/ai/patrol_findings.go index e69b5cf38..501f81ed5 100644 --- a/internal/ai/patrol_findings.go +++ b/internal/ai/patrol_findings.go @@ -444,14 +444,32 @@ func (p *PatrolService) GetFindingsHistory(startTime *time.Time) []*Finding { return findings } -// ForcePatrol triggers an immediate patrol run. -// Uses context.Background() since this runs async after the HTTP response. -func (p *PatrolService) ForcePatrol(ctx context.Context) { +// PatrolRunAcceptance is the durable acknowledgement returned when a manual +// run has reserved the single Patrol execution slot. +type PatrolRunAcceptance struct { + RunID string + StartedAt time.Time +} + +// ForcePatrol atomically accepts and starts an immediate patrol run. +// The reservation is visible through GetStatus before this method returns, so +// callers never have to infer backend acceptance from the first provider event. +// The detached context lets the accepted run continue after the HTTP request +// that initiated it has completed. +func (p *PatrolService) ForcePatrol(ctx context.Context) (PatrolRunAcceptance, bool) { runCtx := context.Background() if ctx != nil { runCtx = context.WithoutCancel(ctx) } - go p.runPatrolWithTrigger(runCtx, TriggerReasonManual, nil) + runStart, accepted := p.beginRun("full") + if !accepted { + return PatrolRunAcceptance{}, false + } + go p.runPatrolWithTriggerStart(runCtx, TriggerReasonManual, nil, runStart) + return PatrolRunAcceptance{ + RunID: runStart.id, + StartedAt: runStart.startedAt, + }, true } // chatServiceExecutorAccessor is satisfied by *chat.Service, allowing patrol to diff --git a/internal/ai/patrol_manual_acceptance_test.go b/internal/ai/patrol_manual_acceptance_test.go new file mode 100644 index 000000000..a70123d54 --- /dev/null +++ b/internal/ai/patrol_manual_acceptance_test.go @@ -0,0 +1,133 @@ +package ai + +import ( + "context" + "encoding/json" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/ai/tools" + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) + +func TestForcePatrolAcceptanceOwnsStatusAndProducesOneHistoryRecord(t *testing.T) { + persistence := config.NewConfigPersistence(t.TempDir()) + service := NewService(persistence, nil) + service.cfg = &config.AIConfig{Enabled: true, PatrolModel: "mock:model"} + service.provider = &mockProvider{} + + providerRelease := make(chan struct{}) + providerEntered := make(chan struct{}) + var providerCalls atomic.Int32 + executor := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) + service.SetChatService(&mockChatService{ + executor: executor, + executePatrolStreamFunc: func(_ context.Context, _ PatrolExecuteRequest, callback ChatStreamCallback) (*PatrolStreamResponse, error) { + providerCalls.Add(1) + close(providerEntered) + <-providerRelease + content, err := json.Marshal(struct { + Text string `json:"text"` + }{Text: "No actionable issues found."}) + if err != nil { + return nil, err + } + callback(ChatStreamEvent{Type: "content", Data: content}) + return &PatrolStreamResponse{ + Content: "No actionable issues found.", + InputTokens: 12, + OutputTokens: 5, + }, nil + }, + }) + + patrol := NewPatrolService(service, &mockStateProvider{ + state: models.StateSnapshot{ + Nodes: []models.Node{{ID: "node-1", Name: "pve-1", Status: "online"}}, + }, + }) + patrol.SetConfig(PatrolConfig{ + Enabled: true, + AnalyzeNodes: true, + }) + + acceptance, accepted := patrol.ForcePatrol(context.Background()) + if !accepted { + t.Fatal("manual run was not accepted") + } + if acceptance.RunID == "" || acceptance.StartedAt.IsZero() { + t.Fatalf("incomplete acceptance: %+v", acceptance) + } + + status := patrol.GetStatus() + if !status.Running { + t.Fatal("accepted run was not synchronously visible as running") + } + if status.CurrentRunID != acceptance.RunID { + t.Fatalf("status run id = %q, want %q", status.CurrentRunID, acceptance.RunID) + } + + if duplicate, duplicateAccepted := patrol.ForcePatrol(context.Background()); duplicateAccepted { + t.Fatalf("duplicate run was accepted: %+v", duplicate) + } + + select { + case <-providerEntered: + case <-time.After(2 * time.Second): + t.Fatal("accepted run did not reach the controlled provider") + } + if calls := providerCalls.Load(); calls != 1 { + t.Fatalf("provider calls = %d, want 1", calls) + } + + close(providerRelease) + deadline := time.Now().Add(3 * time.Second) + for patrol.GetStatus().Running && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + if patrol.GetStatus().Running { + t.Fatal("manual run did not finish") + } + + history := patrol.GetRunHistory(10) + if len(history) != 1 { + t.Fatalf("history records = %d, want 1: %+v", len(history), history) + } + if history[0].ID != acceptance.RunID { + t.Fatalf("history run id = %q, want %q", history[0].ID, acceptance.RunID) + } + if calls := providerCalls.Load(); calls != 1 { + t.Fatalf("provider calls after completion = %d, want 1", calls) + } +} + +func TestAcceptedManualPatrolRecordsRuntimeStateFailure(t *testing.T) { + patrol := NewPatrolService(nil, nil) + patrol.SetConfig(PatrolConfig{Enabled: true}) + + acceptance, accepted := patrol.ForcePatrol(context.Background()) + if !accepted { + t.Fatal("manual run was not accepted") + } + + deadline := time.Now().Add(2 * time.Second) + for patrol.GetStatus().Running && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + if patrol.GetStatus().Running { + t.Fatal("manual run did not terminate") + } + + history := patrol.GetRunHistory(10) + if len(history) != 1 { + t.Fatalf("history records = %d, want 1: %+v", len(history), history) + } + if history[0].ID != acceptance.RunID { + t.Fatalf("history run id = %q, want %q", history[0].ID, acceptance.RunID) + } + if history[0].Status != "error" || history[0].ErrorCount != 1 { + t.Fatalf("history failure = status %q, errors %d", history[0].Status, history[0].ErrorCount) + } +} diff --git a/internal/ai/patrol_run.go b/internal/ai/patrol_run.go index 664bf81e2..4280a5e02 100644 --- a/internal/ai/patrol_run.go +++ b/internal/ai/patrol_run.go @@ -285,8 +285,51 @@ func (p *PatrolService) runPatrol(ctx context.Context) { p.runPatrolWithTrigger(ctx, TriggerReasonScheduled, nil) } +type patrolRunStart struct { + id string + startedAt time.Time +} + +func (p *PatrolService) recordAcceptedRunFailure(start *patrolRunStart, trigger TriggerReason, summary, detail string) { + if start == nil || p.runHistoryStore == nil { + return + } + completedAt := time.Now() + duration := completedAt.Sub(start.startedAt) + p.runHistoryStore.Add(PatrolRunRecord{ + ID: start.id, + StartedAt: start.startedAt, + CompletedAt: completedAt, + Duration: duration, + DurationMs: duration.Milliseconds(), + Type: "patrol", + TriggerReason: string(trigger), + FindingsSummary: "Analysis incomplete (1 error)", + FindingIDs: []string{}, + ErrorCount: 1, + Status: "error", + ErrorSummary: summary, + ErrorDetail: detail, + }) + p.mu.Lock() + p.lastActivity = completedAt + p.lastFullPatrol = completedAt + p.lastDuration = duration + p.resourcesChecked = 0 + p.errorCount = 1 + p.mu.Unlock() +} + // runPatrolWithTrigger executes a patrol run with trigger context func (p *PatrolService) runPatrolWithTrigger(ctx context.Context, trigger TriggerReason, scope *PatrolScope) { + p.runPatrolWithTriggerStart(ctx, trigger, scope, nil) +} + +// runPatrolWithTriggerStart executes a patrol run, optionally using a start +// reservation that was accepted synchronously by the manual-run API. Keeping +// acceptance and execution on the same reservation removes the interval where +// the API has returned success but status still reports no active run. +func (p *PatrolService) runPatrolWithTriggerStart(ctx context.Context, trigger TriggerReason, scope *PatrolScope, acceptedStart *patrolRunStart) { p.mu.RLock() cfg := p.config breaker := p.circuitBreaker @@ -296,21 +339,42 @@ func (p *PatrolService) runPatrolWithTrigger(ctx context.Context, trigger Trigge // (not from the boot-time config snapshot) because release demo instances // enable mock fixtures only after the license sync authorizes them. if IsDemoMode() { - p.runDemoPatrolCycle(trigger) + if !p.runDemoPatrolCycleWithStart(trigger, acceptedStart) && acceptedStart != nil { + p.recordAcceptedRunFailure(acceptedStart, trigger, + "Patrol demo state unavailable", + "Pulse accepted the run, but demo infrastructure state was not available.") + } return } if !cfg.Enabled { + if acceptedStart != nil { + p.recordAcceptedRunFailure(acceptedStart, trigger, + "Patrol disabled before execution", + "Pulse accepted the run, but Patrol was disabled before execution began.") + p.endRun() + } return } if reason := strings.TrimSpace(cfg.RuntimeBlockedReason); reason != "" { + if acceptedStart != nil { + p.recordAcceptedRunFailure(acceptedStart, trigger, + "Patrol runtime became unavailable", + reason) + p.endRun() + } p.setBlockedReasonWithCause(reason, cfg.RuntimeBlockedCause) log.Info().Str("reason", reason).Str("cause", string(cfg.RuntimeBlockedCause)).Msg("AI Patrol: Skipping run - runtime readiness blocked") return } - if !p.tryStartRun("full") { - return + runStart := acceptedStart + if runStart == nil { + var accepted bool + runStart, accepted = p.beginRun("full") + if !accepted { + return + } } defer p.endRun() @@ -320,8 +384,8 @@ func (p *PatrolService) runPatrolWithTrigger(ctx context.Context, trigger Trigge log.Warn().Msg("AI Patrol: Circuit breaker is open (LLM calls blocked)") } - start := time.Now() - runID := fmt.Sprintf("%d", start.UnixNano()) + start := runStart.startedAt + runID := runStart.id executionID := uuid.NewString() patrolType := "patrol" GetPatrolMetrics().RecordRun(string(trigger), "full") @@ -358,6 +422,11 @@ func (p *PatrolService) runPatrolWithTrigger(ctx context.Context, trigger Trigge // Get current state if !p.hasPatrolRuntimeInputs() { log.Warn().Msg("AI Patrol: No runtime state available") + if acceptedStart != nil { + p.recordAcceptedRunFailure(runStart, trigger, + "Patrol runtime state unavailable", + "Pulse accepted the run, but no infrastructure state was available for analysis.") + } return } @@ -496,10 +565,10 @@ func (p *PatrolService) runPatrolWithTrigger(ctx context.Context, trigger Trigge } p.setBlockedReasonWithCause(reason, cause) log.Info().Str("reason", reason).Str("cause", string(cause)).Msg("AI Patrol: Skipping run - AI unavailable") - return - } - - { + runStats.errors++ + runStats.errorSummary = "Patrol provider unavailable" + runStats.errorDetail = reason + } else { p.clearBlockedReason() // Ensure stream state is clean for this run before the first streamed event. p.resetStreamForRun(runID) @@ -792,7 +861,8 @@ func (p *PatrolService) runScopedPatrol(ctx context.Context, scope PatrolScope) return } - if !p.tryStartRun("scoped") { + runStart, accepted := p.beginRun("scoped") + if !accepted { // Re-queue with backoff if retries remain if scope.RetryCount < scopedPatrolMaxRetries { scope.RetryCount++ @@ -826,8 +896,8 @@ func (p *PatrolService) runScopedPatrol(ctx context.Context, scope PatrolScope) log.Warn().Msg("AI Patrol: Circuit breaker is open for scoped patrol (LLM calls blocked)") } - start := time.Now() - runID := fmt.Sprintf("%d", start.UnixNano()) + start := runStart.startedAt + runID := runStart.id executionID := uuid.NewString() GetPatrolMetrics().RecordRun(string(scope.Reason), "scoped") var runStats struct { @@ -1793,6 +1863,7 @@ func (p *PatrolService) GetStatus() PatrolStatus { status := PatrolStatus{ RuntimeState: PatrolRuntimeStateActive, Running: analysisInProgress, + CurrentRunID: p.currentRunID, Enabled: p.config.Enabled, LastDuration: p.lastDuration, ResourcesChecked: p.resourcesChecked, @@ -1802,6 +1873,10 @@ func (p *PatrolService) GetStatus() PatrolStatus { BlockedReason: p.lastBlockedReason, BlockedCause: p.lastBlockedCause, } + if analysisInProgress && !p.runStartedAt.IsZero() { + startedAt := p.runStartedAt + status.CurrentRunStartedAt = &startedAt + } if p.triggerManager != nil { triggerStatus := p.triggerManager.GetStatus() status.TriggerStatus = &triggerStatus @@ -2730,7 +2805,7 @@ func (p *PatrolService) TriggerPatrolForAlert(alert *alerts.Alert) { } } -func (p *PatrolService) tryStartRun(kind string) bool { +func (p *PatrolService) beginRun(kind string) (*patrolRunStart, bool) { p.mu.Lock() if p.runInProgress { // Detect stuck runs: if the current run has been going for >20 minutes, @@ -2751,18 +2826,28 @@ func (p *PatrolService) tryStartRun(kind string) bool { } else { log.Debug().Str("kind", kind).Msg("AI Patrol: Run already in progress, skipping") } - return false + return nil, false } } + startedAt := time.Now() + runID := fmt.Sprintf("%d", startedAt.UnixNano()) p.runInProgress = true - p.runStartedAt = time.Now() + p.currentRunID = runID + p.runStartedAt = startedAt p.mu.Unlock() - return true + return &patrolRunStart{id: runID, startedAt: startedAt}, true +} + +func (p *PatrolService) tryStartRun(kind string) bool { + _, accepted := p.beginRun(kind) + return accepted } func (p *PatrolService) endRun() { p.mu.Lock() p.runInProgress = false + p.currentRunID = "" + p.runStartedAt = time.Time{} orch := p.investigationOrchestrator p.mu.Unlock() diff --git a/internal/alerts/canonical_override_migration.go b/internal/alerts/canonical_override_migration.go new file mode 100644 index 000000000..69fea985b --- /dev/null +++ b/internal/alerts/canonical_override_migration.go @@ -0,0 +1,69 @@ +package alerts + +import "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + +// MigrateCanonicalOverrideKeys re-homes overrides written under a retired +// canonical resource ID onto the resource's current canonical ID. Only +// provider-declared superseded IDs participate: display aliases, hostnames, +// metric targets, and other lookup conveniences are never persistence keys. +// +// A current-key override wins over an old-key override, and an old ID is left +// untouched when it is still live or maps to more than one current resource. +// The function mutates config only when it can make an unambiguous migration. +func MigrateCanonicalOverrideKeys(config *AlertConfig, resources []unifiedresources.Resource) bool { + if config == nil || len(config.Overrides) == 0 || len(resources) == 0 { + return false + } + + liveIDs := make(map[string]struct{}, len(resources)) + successors := make(map[string]string) + ambiguous := make(map[string]struct{}) + + for _, resource := range resources { + currentID := unifiedresources.CanonicalResourceID(resource.ID) + if currentID == "" { + continue + } + liveIDs[currentID] = struct{}{} + + for _, supersededID := range resource.SupersededCanonicalIDs { + oldID := unifiedresources.CanonicalResourceID(supersededID) + if oldID == "" || oldID == currentID { + continue + } + if existing, ok := successors[oldID]; ok && existing != currentID { + delete(successors, oldID) + ambiguous[oldID] = struct{}{} + continue + } + if _, conflict := ambiguous[oldID]; conflict { + continue + } + successors[oldID] = currentID + } + } + + changed := false + overrides := make(map[string]ThresholdConfig, len(config.Overrides)) + for resourceID, override := range config.Overrides { + overrides[resourceID] = override + } + for oldID, newID := range successors { + if _, stillLive := liveIDs[oldID]; stillLive { + continue + } + override, exists := overrides[oldID] + if !exists { + continue + } + if _, currentExists := overrides[newID]; !currentExists { + overrides[newID] = override + } + delete(overrides, oldID) + changed = true + } + if changed { + config.Overrides = overrides + } + return changed +} diff --git a/internal/alerts/canonical_override_migration_test.go b/internal/alerts/canonical_override_migration_test.go new file mode 100644 index 000000000..fb3e0bc15 --- /dev/null +++ b/internal/alerts/canonical_override_migration_test.go @@ -0,0 +1,68 @@ +package alerts + +import ( + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +func TestMigrateCanonicalOverrideKeysRehomesUnambiguousSupersededIdentity(t *testing.T) { + const ( + oldID = "agent-535886018cb53055" + newID = "agent-b9ed6d0e20e94eaf" + ) + config := AlertConfig{ + Overrides: map[string]ThresholdConfig{ + oldID: { + Memory: &HysteresisThreshold{Trigger: 95, Clear: 90}, + }, + }, + } + resources := []unifiedresources.Resource{{ + ID: newID, + Type: unifiedresources.ResourceTypeAgent, + SupersededCanonicalIDs: []string{oldID}, + }} + + if !MigrateCanonicalOverrideKeys(&config, resources) { + t.Fatal("expected superseded TrueNAS override identity to migrate") + } + if _, exists := config.Overrides[oldID]; exists { + t.Fatalf("override remained under superseded identity %s", oldID) + } + override, exists := config.Overrides[newID] + if !exists || override.Memory == nil { + t.Fatalf("override missing under current canonical identity %s: %+v", newID, config.Overrides) + } + if override.Memory.Trigger != 95 || override.Memory.Clear != 90 { + t.Fatalf("migrated override changed threshold values: %+v", override.Memory) + } +} + +func TestMigrateCanonicalOverrideKeysRefusesAmbiguousOrLiveIdentity(t *testing.T) { + const oldID = "agent-shared" + for name, resources := range map[string][]unifiedresources.Resource{ + "ambiguous successor": { + {ID: "agent-a", SupersededCanonicalIDs: []string{oldID}}, + {ID: "agent-b", SupersededCanonicalIDs: []string{oldID}}, + }, + "still live": { + {ID: oldID}, + {ID: "agent-a", SupersededCanonicalIDs: []string{oldID}}, + }, + } { + t.Run(name, func(t *testing.T) { + config := AlertConfig{ + Overrides: map[string]ThresholdConfig{ + oldID: {Memory: &HysteresisThreshold{Trigger: 95, Clear: 90}}, + }, + } + if MigrateCanonicalOverrideKeys(&config, resources) { + t.Fatal("unsafe succession unexpectedly migrated") + } + if _, exists := config.Overrides[oldID]; !exists { + t.Fatal("unsafe succession removed the original override") + } + }) + } +} diff --git a/internal/api/ai_handlers.go b/internal/api/ai_handlers.go index f44e91f31..e31ec3635 100644 --- a/internal/api/ai_handlers.go +++ b/internal/api/ai_handlers.go @@ -4958,23 +4958,25 @@ func (h *AISettingsHandler) HandleOAuthDisconnect(w http.ResponseWriter, r *http // PatrolStatusResponse is the response for GET /api/ai/patrol/status type PatrolStatusResponse struct { - RuntimeState ai.PatrolRuntimeState `json:"runtime_state"` - Running bool `json:"running"` - Enabled bool `json:"enabled"` - LastPatrolAt *time.Time `json:"last_patrol_at,omitempty"` - LastActivityAt *time.Time `json:"last_activity_at,omitempty"` - TriggerStatus *ai.TriggerStatus `json:"trigger_status,omitempty"` - NextPatrolAt *time.Time `json:"next_patrol_at,omitempty"` - LastDurationMs int64 `json:"last_duration_ms"` - ResourcesChecked int `json:"resources_checked"` - FindingsCount int `json:"findings_count"` - ErrorCount int `json:"error_count"` - Healthy bool `json:"healthy"` - IntervalMs int64 `json:"interval_ms"` // Patrol interval in milliseconds - FixedCount int `json:"fixed_count"` // Number of issues remediated by Patrol - BlockedReason string `json:"blocked_reason,omitempty"` - BlockedCause string `json:"blocked_cause,omitempty"` - BlockedAt *time.Time `json:"blocked_at,omitempty"` + RuntimeState ai.PatrolRuntimeState `json:"runtime_state"` + Running bool `json:"running"` + CurrentRunID string `json:"current_run_id,omitempty"` + CurrentRunStartedAt *time.Time `json:"current_run_started_at,omitempty"` + Enabled bool `json:"enabled"` + LastPatrolAt *time.Time `json:"last_patrol_at,omitempty"` + LastActivityAt *time.Time `json:"last_activity_at,omitempty"` + TriggerStatus *ai.TriggerStatus `json:"trigger_status,omitempty"` + NextPatrolAt *time.Time `json:"next_patrol_at,omitempty"` + LastDurationMs int64 `json:"last_duration_ms"` + ResourcesChecked int `json:"resources_checked"` + FindingsCount int `json:"findings_count"` + ErrorCount int `json:"error_count"` + Healthy bool `json:"healthy"` + IntervalMs int64 `json:"interval_ms"` // Patrol interval in milliseconds + FixedCount int `json:"fixed_count"` // Number of issues remediated by Patrol + BlockedReason string `json:"blocked_reason,omitempty"` + BlockedCause string `json:"blocked_cause,omitempty"` + BlockedAt *time.Time `json:"blocked_at,omitempty"` // License status for Pro feature gating LicenseRequired bool `json:"license_required"` // True if Pro license needed for full features LicenseStatus string `json:"license_status"` // "active", "expired", "grace_period", "none" @@ -5356,26 +5358,28 @@ func (h *AISettingsHandler) HandleGetPatrolStatus(w http.ResponseWriter, r *http } response := PatrolStatusResponse{ - RuntimeState: status.RuntimeState, - Running: status.Running, - Enabled: status.Enabled, - LastPatrolAt: status.LastPatrolAt, - LastActivityAt: status.LastActivityAt, - TriggerStatus: status.TriggerStatus, - NextPatrolAt: status.NextPatrolAt, - LastDurationMs: status.LastDuration.Milliseconds(), - ResourcesChecked: status.ResourcesChecked, - FindingsCount: status.FindingsCount, - ErrorCount: status.ErrorCount, - Healthy: status.Healthy, - IntervalMs: status.IntervalMs, - FixedCount: fixedCount, - BlockedReason: status.BlockedReason, - BlockedCause: patrolFailureCauseResponse(status.BlockedCause), - BlockedAt: status.BlockedAt, - LicenseRequired: !hasAutoFixFeature, - LicenseStatus: licenseStatus, - Readiness: ptrToPatrolReadiness(h.buildPatrolReadiness(r.Context(), aiService, true)), + RuntimeState: status.RuntimeState, + Running: status.Running, + CurrentRunID: status.CurrentRunID, + CurrentRunStartedAt: status.CurrentRunStartedAt, + Enabled: status.Enabled, + LastPatrolAt: status.LastPatrolAt, + LastActivityAt: status.LastActivityAt, + TriggerStatus: status.TriggerStatus, + NextPatrolAt: status.NextPatrolAt, + LastDurationMs: status.LastDuration.Milliseconds(), + ResourcesChecked: status.ResourcesChecked, + FindingsCount: status.FindingsCount, + ErrorCount: status.ErrorCount, + Healthy: status.Healthy, + IntervalMs: status.IntervalMs, + FixedCount: fixedCount, + BlockedReason: status.BlockedReason, + BlockedCause: patrolFailureCauseResponse(status.BlockedCause), + BlockedAt: status.BlockedAt, + LicenseRequired: !hasAutoFixFeature, + LicenseStatus: licenseStatus, + Readiness: ptrToPatrolReadiness(h.buildPatrolReadiness(r.Context(), aiService, true)), } if !hasAutoFixFeature { response.UpgradeURL = upgradeURLForFeatureFromLicensing(featureAIAutoFixValue) @@ -5819,6 +5823,12 @@ func (h *AISettingsHandler) HandleForcePatrol(w http.ResponseWriter, r *http.Req return } + if !patrol.GetStatus().Enabled { + writeErrorResponse(w, http.StatusConflict, "patrol_disabled", + "Patrol is disabled. Enable Patrol before starting a manual run.", nil) + return + } + // Cadence cap: Community tier is limited to 1 patrol run per hour. // Patrol itself is free (ai_patrol), but higher cadence is gated behind Pro/Cloud. if !aiService.HasLicenseFeature(featureAIAutoFixValue) { @@ -5832,12 +5842,27 @@ func (h *AISettingsHandler) HandleForcePatrol(w http.ResponseWriter, r *http.Req } } - // Trigger patrol asynchronously - patrol.ForcePatrol(r.Context()) + // Atomically reserve the execution slot before acknowledging the request. + // This makes backend acceptance independently observable from both provider + // progress events and the timing of the execution goroutine. + acceptance, accepted := patrol.ForcePatrol(r.Context()) + if !accepted { + status := patrol.GetStatus() + details := map[string]string{} + if status.CurrentRunID != "" { + details["current_run_id"] = status.CurrentRunID + } + writeErrorResponse(w, http.StatusConflict, "patrol_already_running", + "Patrol is already running.", details) + return + } response := map[string]interface{}{ - "success": true, - "message": "Triggered patrol run", + "success": true, + "accepted": true, + "message": "Triggered patrol run", + "run_id": acceptance.RunID, + "started_at": acceptance.StartedAt, } if err := utils.WriteJSONResponse(w, response); err != nil { diff --git a/internal/api/ai_handlers_patrol_actions_additional_test.go b/internal/api/ai_handlers_patrol_actions_additional_test.go index 8d1b8278b..01917ca1e 100644 --- a/internal/api/ai_handlers_patrol_actions_additional_test.go +++ b/internal/api/ai_handlers_patrol_actions_additional_test.go @@ -954,11 +954,41 @@ func TestHandleForcePatrol_ConfigDisabled(t *testing.T) { handler.HandleForcePatrol(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("status = %d, want 200", rec.Code) + if rec.Code != http.StatusConflict { + t.Fatalf("status = %d, want 409", rec.Code) } - if !strings.Contains(rec.Body.String(), "Triggered patrol run") { - t.Fatalf("expected success message") + var payload APIError + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode error payload: %v", err) + } + if payload.Code != "patrol_disabled" { + t.Fatalf("code = %q, want patrol_disabled", payload.Code) + } +} + +func TestHandleForcePatrolRejectsDuplicateAfterSynchronousAcceptance(t *testing.T) { + handler, patrol, _, _ := setupAIHandlerWithPatrol(t) + seedReadyAnthropicPatrolRuntime(t, handler) + setUnexportedField(t, patrol, "runInProgress", true) + setUnexportedField(t, patrol, "currentRunID", "run-active") + setUnexportedField(t, patrol, "runStartedAt", time.Now()) + + req := newLoopbackRequest(http.MethodPost, "/api/ai/patrol/run", nil) + rec := httptest.NewRecorder() + handler.HandleForcePatrol(rec, req) + + if rec.Code != http.StatusConflict { + t.Fatalf("status = %d, want 409: %s", rec.Code, rec.Body.String()) + } + var payload APIError + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode error payload: %v", err) + } + if payload.Code != "patrol_already_running" { + t.Fatalf("code = %q, want patrol_already_running", payload.Code) + } + if payload.Details["current_run_id"] != "run-active" { + t.Fatalf("current_run_id = %q, want run-active", payload.Details["current_run_id"]) } } @@ -1019,6 +1049,17 @@ func TestHandleForcePatrol_CommunityTierIgnoresRecentScopedActivityForFullPatrol if strings.Contains(rec.Body.String(), "patrol_rate_limited") { t.Fatalf("expected community force patrol to ignore scoped-only activity, got %s", rec.Body.String()) } + var response struct { + Accepted bool `json:"accepted"` + RunID string `json:"run_id"` + StartedAt string `json:"started_at"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &response); err != nil { + t.Fatalf("decode acceptance response: %v", err) + } + if !response.Accepted || response.RunID == "" || response.StartedAt == "" { + t.Fatalf("incomplete acceptance response: %+v", response) + } } func TestBuildManualScopedPatrolScope(t *testing.T) { diff --git a/internal/hostagent/agent.go b/internal/hostagent/agent.go index 29af183d7..f8aa56991 100644 --- a/internal/hostagent/agent.go +++ b/internal/hostagent/agent.go @@ -29,6 +29,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/sensors" "github.com/rcourtman/pulse-go-rewrite/internal/utils" agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rs/zerolog" gohost "github.com/shirou/gopsutil/v4/host" ) @@ -913,6 +914,10 @@ func (a *Agent) persistReportQueue(queue *utils.Queue[agentshost.Report], fileNa a.logger.Warn().Err(err).Str("dir", a.stateDir).Msg("Failed to create state dir for buffer persistence") return } + if err := os.Chmod(a.stateDir, 0700); err != nil { + a.logger.Warn().Err(err).Str("dir", a.stateDir).Msg("Failed to enforce state dir permissions for buffer persistence") + return + } path := filepath.Join(a.stateDir, fileName) tmpPath := path + ".tmp" @@ -996,6 +1001,7 @@ func (a *Agent) buildReport(ctx context.Context) (agentshost.Report, error) { // Collect S.M.A.R.T. disk data (best effort - don't fail if unavailable) smartData := a.collectSMARTData(collectCtx, runtimeConfig.diskExclude) if len(smartData) > 0 { + annotateSMARTWithDiskIO(smartData, snapshot.DiskIO) sensorData.SMART = smartData } @@ -1210,6 +1216,10 @@ func (a *Agent) persistAgentID(agentID string) { a.logger.Debug().Err(err).Msg("Failed to create state directory for agent-id") return } + if err := a.collector.Chmod(a.stateDir, 0700); err != nil { + a.logger.Debug().Err(err).Msg("Failed to enforce state directory permissions for agent-id") + return + } agentIDPath := filepath.Join(a.stateDir, "agent-id") if err := a.collector.WriteFile(agentIDPath, []byte(agentID), 0600); err != nil { a.logger.Debug().Err(err).Msg("Failed to persist agent-id") @@ -1924,10 +1934,13 @@ func (a *Agent) collectSMARTData(ctx context.Context, diskExclude []string) []ag Serial: disk.Serial, WWN: disk.WWN, Type: disk.Type, + Controller: disk.Controller, + Target: disk.Target, SizeBytes: disk.SizeBytes, Temperature: disk.Temperature, Health: disk.Health, Standby: disk.Standby, + Collection: diskinventory.CloneStatus(disk.Collection), } if disk.Attributes != nil { entry.Attributes = &agentshost.SMARTAttributes{ @@ -1947,9 +1960,20 @@ func (a *Agent) collectSMARTData(ctx context.Context, diskExclude []string) []ag } if pools, err := ZFSDiskPoolMap(ctx); err != nil { + for index := range result { + ensureAgentDiskCollection(&result[index]).Pool = diskinventory.Unavailable( + "zpool", + "ZFS pool membership could not be collected", + ) + } a.logger.Debug().Err(err).Msg("Failed to collect ZFS pool membership for SMART annotation") - } else if len(pools) > 0 { - annotateSMARTWithZFSPools(result, pools) + } else { + if len(pools) > 0 { + annotateSMARTWithZFSPools(result, pools) + } + for index := range result { + ensureAgentDiskCollection(&result[index]).Pool = diskinventory.Available("zpool") + } } a.logger.Debug(). @@ -1959,6 +1983,69 @@ func (a *Agent) collectSMARTData(ctx context.Context, diskExclude []string) []ag return result } +func ensureAgentDiskCollection(disk *agentshost.DiskSMART) *diskinventory.CollectionStatus { + if disk.Collection == nil { + disk.Collection = &diskinventory.CollectionStatus{} + } + return disk.Collection +} + +// annotateSMARTWithDiskIO binds cumulative kernel counters to a SMART +// inventory entry only when the block-device association is unambiguous. +// Controller-member targets share one logical kernel device, so assigning that +// aggregate counter to any member would fabricate per-disk I/O. +func annotateSMARTWithDiskIO(smart []agentshost.DiskSMART, diskIO []agentshost.DiskIO) { + if len(smart) == 0 { + return + } + + ioByDevice := make(map[string]agentshost.DiskIO, len(diskIO)) + for _, entry := range diskIO { + device := diskinventory.DeviceToken(entry.Device) + if device != "" { + ioByDevice[strings.ToLower(device)] = entry + } + } + smartCountByDevice := make(map[string]int, len(smart)) + for _, disk := range smart { + device := strings.ToLower(diskinventory.DeviceToken(disk.Device)) + if device != "" { + smartCountByDevice[device]++ + } + } + + for index := range smart { + disk := &smart[index] + collection := ensureAgentDiskCollection(disk) + device := strings.ToLower(diskinventory.DeviceToken(disk.Device)) + if isMultiplexedDeviceType(disk.Target) || (device != "" && smartCountByDevice[device] > 1) { + collection.IO = diskinventory.Unsupported( + "kernel_diskstats", + "controller target does not expose member-level I/O counters", + ) + continue + } + ioEntry, ok := ioByDevice[device] + if !ok { + if len(diskIO) == 0 { + collection.IO = diskinventory.Unavailable( + "kernel_diskstats", + "disk I/O counters could not be collected", + ) + } else { + collection.IO = diskinventory.Missing( + "kernel_diskstats", + "disk was present but its I/O counters were not reported", + ) + } + continue + } + ioCopy := ioEntry + disk.IO = &ioCopy + collection.IO = diskinventory.Available("kernel_diskstats") + } +} + // runProxmoxSetup performs one-time Proxmox API token setup and node registration. // Supports hosts with multiple Proxmox products (e.g., PVE + PBS on same host). func (a *Agent) runProxmoxSetup(ctx context.Context) { diff --git a/internal/hostagent/enroll.go b/internal/hostagent/enroll.go index b8742a940..98006a551 100644 --- a/internal/hostagent/enroll.go +++ b/internal/hostagent/enroll.go @@ -77,6 +77,8 @@ func (a *Agent) runEnrollmentLoop(ctx context.Context) error { a.persistRuntimeToken(result.RuntimeToken) canonicalID := strings.TrimSpace(result.AgentID) if canonicalID != "" { + a.agentID = canonicalID + a.cfg.AgentID = canonicalID a.persistAgentID(canonicalID) } a.logger.Info(). @@ -204,6 +206,10 @@ func (a *Agent) persistRuntimeToken(token string) { a.logger.Warn().Err(err).Msg("Failed to create state directory for runtime token") return } + if err := a.collector.Chmod(a.stateDir, 0700); err != nil { + a.logger.Warn().Err(err).Msg("Failed to enforce state directory permissions for runtime token") + return + } tokenPath := filepath.Join(a.stateDir, runtimeTokenFile) if err := a.collector.WriteFile(tokenPath, []byte(token), 0600); err != nil { a.logger.Warn().Err(err).Msg("Failed to persist runtime token") diff --git a/internal/hostagent/enroll_test.go b/internal/hostagent/enroll_test.go index 0edd23352..696b548e5 100644 --- a/internal/hostagent/enroll_test.go +++ b/internal/hostagent/enroll_test.go @@ -85,6 +85,9 @@ func TestEnroll_Success(t *testing.T) { if agent.cfg.APIToken != "runtime-tok-123" { t.Errorf("expected token runtime-tok-123, got %s", agent.cfg.APIToken) } + if agent.agentID != "host-test-host" || agent.cfg.AgentID != "host-test-host" { + t.Fatalf("running agent identity was not updated after enrollment: agent=%q config=%q", agent.agentID, agent.cfg.AgentID) + } // Verify runtime token was persisted. tokenPath := filepath.Join(stateDir, runtimeTokenFile) diff --git a/internal/hostagent/issue1595_sas_collection_test.go b/internal/hostagent/issue1595_sas_collection_test.go new file mode 100644 index 000000000..8b8c59227 --- /dev/null +++ b/internal/hostagent/issue1595_sas_collection_test.go @@ -0,0 +1,272 @@ +//go:build !windows + +package hostagent + +import ( + "context" + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" +) + +type issue1595TopologyFixture struct { + Node string `json:"node"` + Instance string `json:"instance"` + AgentID string `json:"agentId"` + Model string `json:"model"` + SizeBytes int64 `json:"sizeBytes"` + Controllers []struct { + ID string `json:"id"` + Pool string `json:"pool"` + Disks []struct { + Device string `json:"device"` + Target string `json:"target"` + Serial string `json:"serial"` + ProviderSerial string `json:"providerSerial"` + Temperature int `json:"temperature"` + ReadBytes uint64 `json:"readBytes"` + WriteBytes uint64 `json:"writeBytes"` + IOTimeMs uint64 `json:"ioTimeMs"` + } `json:"disks"` + } `json:"controllers"` +} + +func loadIssue1595TopologyFixture(t *testing.T) issue1595TopologyFixture { + t.Helper() + path := filepath.Join("..", "..", "testdata", "issue1595_sas_topology.json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read fixture: %v", err) + } + var fixture issue1595TopologyFixture + if err := json.Unmarshal(data, &fixture); err != nil { + t.Fatalf("decode fixture: %v", err) + } + return fixture +} + +func TestIssue1595CollectSMARTLocalPreservesTwentyFourSASDisksAcrossTwoHBAs(t *testing.T) { + fixture := loadIssue1595TopologyFixture(t) + + type diskFixture struct { + controller string + target string + serial string + temperature int + } + byDevice := make(map[string]diskFixture) + var scan strings.Builder + var entries []os.DirEntry + for _, controller := range fixture.Controllers { + for _, disk := range controller.Disks { + byDevice[disk.Device] = diskFixture{ + controller: controller.ID, + target: disk.Target, + serial: disk.Serial, + temperature: disk.Temperature, + } + fmt.Fprintf(&scan, "/dev/%s -d scsi # synthetic issue #1595 SAS disk\n", disk.Device) + entries = append(entries, fakeDirEntry{name: disk.Device}) + } + } + if len(byDevice) != 24 { + t.Fatalf("fixture disk count = %d, want 24", len(byDevice)) + } + + origGOOS := runtimeGOOS + origReadDir := readDir + origReadFile := smartctlReadFile + origEvalSymlinks := smartctlEvalSymlinks + origRun := smartRunCommandOutput + origLookPath := execLookPath + origConcurrency := smartCollectionConcurrency + origThreshold := smartCollectionParallelThreshold + t.Cleanup(func() { + runtimeGOOS = origGOOS + readDir = origReadDir + smartctlReadFile = origReadFile + smartctlEvalSymlinks = origEvalSymlinks + smartRunCommandOutput = origRun + execLookPath = origLookPath + smartCollectionConcurrency = origConcurrency + smartCollectionParallelThreshold = origThreshold + }) + + runtimeGOOS = "linux" + smartCollectionConcurrency = 6 + smartCollectionParallelThreshold = 12 + readDir = func(path string) ([]os.DirEntry, error) { + if path == "/sys/block" { + return entries, nil + } + return nil, fs.ErrNotExist + } + smartctlReadFile = func(path string) ([]byte, error) { + device := issue1595DeviceFromSysfsPath(path) + if device == "" { + return nil, fs.ErrNotExist + } + switch filepath.Base(path) { + case "size": + return []byte(fmt.Sprintf("%d\n", fixture.SizeBytes/512)), nil + case "protocol": + return []byte("SAS\n"), nil + case "rotational": + return []byte("1\n"), nil + case "model": + return []byte(fixture.Model + "\n"), nil + default: + return nil, fs.ErrNotExist + } + } + smartctlEvalSymlinks = func(path string) (string, error) { + device := issue1595DeviceFromSysfsPath(path) + disk, ok := byDevice[device] + if !ok { + return "", fs.ErrNotExist + } + if strings.HasSuffix(path, "/device/subsystem") { + return "/sys/bus/scsi", nil + } + if strings.HasSuffix(path, "/device") || path == "/sys/block/"+device { + host := strings.Split(disk.target, ":")[0] + return fmt.Sprintf( + "/sys/devices/pci0000:00/%s/host%s/target%s/%s/block/%s", + disk.controller, + host, + strings.Join(strings.Split(disk.target, ":")[:3], ":"), + disk.target, + device, + ), nil + } + return "", fs.ErrNotExist + } + execLookPath = func(string) (string, error) { return "smartctl", nil } + + var active atomic.Int32 + var maxActive atomic.Int32 + smartRunCommandOutput = func(ctx context.Context, name string, args ...string) ([]byte, error) { + if len(args) == 1 && args[0] == "--scan-open" { + return []byte(scan.String()), nil + } + device := strings.TrimPrefix(args[len(args)-1], "/dev/") + disk, ok := byDevice[device] + if !ok { + return nil, fmt.Errorf("unexpected SMART target %q", device) + } + current := active.Add(1) + for { + previous := maxActive.Load() + if current <= previous || maxActive.CompareAndSwap(previous, current) { + break + } + } + defer active.Add(-1) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(15 * time.Millisecond): + } + return json.Marshal(map[string]any{ + "device": map[string]any{ + "name": "/dev/" + device, + "type": "scsi", + "protocol": "SAS", + }, + "model_name": fixture.Model, + "serial_number": disk.serial, + "user_capacity": map[string]any{"bytes": fixture.SizeBytes}, + "smart_status": map[string]any{"passed": true}, + "temperature": map[string]any{"current": disk.temperature}, + }) + } + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + results, err := CollectSMARTLocal(ctx, nil) + if err != nil { + t.Fatalf("CollectSMARTLocal() error = %v", err) + } + if len(results) != 24 { + t.Fatalf("collected disk count = %d, want 24", len(results)) + } + if maxActive.Load() < 2 { + t.Fatalf("SMART collection did not use bounded concurrency; max active = %d", maxActive.Load()) + } + for _, result := range results { + want := byDevice[result.Device] + if result.Serial != want.serial || result.Type != "sas" || result.Temperature != want.temperature { + t.Fatalf("disk %s lost SAS SMART identity: %+v", result.Device, result) + } + if result.Controller != want.controller || result.Target != want.target { + t.Fatalf("disk %s topology = %q/%q, want %q/%q", result.Device, result.Controller, result.Target, want.controller, want.target) + } + if result.Collection == nil || + result.Collection.Serial.State != diskinventory.FieldAvailable || + result.Collection.Temperature.State != diskinventory.FieldAvailable || + result.Collection.Controller.State != diskinventory.FieldAvailable { + t.Fatalf("disk %s collection status = %+v", result.Device, result.Collection) + } + } +} + +func TestIssue1595DiskIOAssociationMarksSharedControllerTargetsUnsupported(t *testing.T) { + smart := []agentshost.DiskSMART{ + {Device: "sda [megaraid,0]", Controller: "sda", Target: "megaraid,0"}, + {Device: "sda [megaraid,1]", Controller: "sda", Target: "megaraid,1"}, + } + annotateSMARTWithDiskIO(smart, []agentshost.DiskIO{{Device: "sda", ReadBytes: 100}}) + + for _, disk := range smart { + if disk.IO != nil { + t.Fatalf("controller member %s received aggregate I/O counters", disk.Target) + } + if disk.Collection == nil || disk.Collection.IO.State != diskinventory.FieldUnsupported { + t.Fatalf("controller member %s I/O status = %+v, want unsupported", disk.Target, disk.Collection) + } + } +} + +func TestLinuxBlockDeviceTopologyPreservesDirectNVMeAndSATAPCIController(t *testing.T) { + origEvalSymlinks := smartctlEvalSymlinks + t.Cleanup(func() { smartctlEvalSymlinks = origEvalSymlinks }) + + paths := map[string]string{ + "/sys/block/sda/device": "/sys/devices/pci0000:00/0000:00:17.0/ata1/host0/target0:0:0/0:0:0:0", + "/sys/block/nvme0n1/device": "/sys/devices/pci0000:00/0000:00:1d.0/nvme/nvme0/nvme0n1", + } + smartctlEvalSymlinks = func(path string) (string, error) { + if resolved, ok := paths[path]; ok { + return resolved, nil + } + return "", fs.ErrNotExist + } + + if controller, target := linuxBlockDeviceTopology("sda"); controller != "0000:00:17.0" || target != "0:0:0:0" { + t.Fatalf("SATA topology = %q/%q, want PCI controller and HCTL", controller, target) + } + if controller, target := linuxBlockDeviceTopology("nvme0n1"); controller != "0000:00:1d.0" || target != "" { + t.Fatalf("NVMe topology = %q/%q, want PCI controller without HCTL", controller, target) + } +} + +func issue1595DeviceFromSysfsPath(path string) string { + const prefix = "/sys/block/" + if !strings.HasPrefix(path, prefix) { + return "" + } + remainder := strings.TrimPrefix(path, prefix) + if index := strings.IndexByte(remainder, '/'); index >= 0 { + remainder = remainder[:index] + } + return remainder +} diff --git a/internal/hostagent/smartctl.go b/internal/hostagent/smartctl.go index 75826b443..434f5af43 100644 --- a/internal/hostagent/smartctl.go +++ b/internal/hostagent/smartctl.go @@ -15,10 +15,12 @@ import ( "sort" "strconv" "strings" + "sync" "time" "github.com/rs/zerolog/log" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rcourtman/pulse-go-rewrite/pkg/fsfilters" ) @@ -39,21 +41,27 @@ var ( timeNow = time.Now runtimeGOOS = runtime.GOOS + + smartCollectionConcurrency = 6 + smartCollectionParallelThreshold = 12 ) // DiskSMART represents S.M.A.R.T. data for a single disk. type DiskSMART struct { - Device string `json:"device"` // Block device name (e.g., sda, nvme0n1) - Model string `json:"model,omitempty"` // Disk model - Serial string `json:"serial,omitempty"` // Serial number - WWN string `json:"wwn,omitempty"` // World Wide Name - Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme - SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown) - Temperature int `json:"temperature"` // Temperature in Celsius - Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN - Standby bool `json:"standby,omitempty"` // True if disk was in standby - Attributes *SMARTAttributes `json:"attributes,omitempty"` - LastUpdated time.Time `json:"lastUpdated"` // When this reading was taken + Device string `json:"device"` // Block device name (e.g., sda, nvme0n1) + Model string `json:"model,omitempty"` // Disk model + Serial string `json:"serial,omitempty"` // Serial number + WWN string `json:"wwn,omitempty"` // World Wide Name + Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme + Controller string `json:"controller,omitempty"` // PCI/controller association when reported + Target string `json:"target,omitempty"` // HCTL or smartctl controller-member target + SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown) + Temperature int `json:"temperature"` // Temperature in Celsius + Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN + Standby bool `json:"standby,omitempty"` // True if disk was in standby + Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` + Attributes *SMARTAttributes `json:"attributes,omitempty"` + LastUpdated time.Time `json:"lastUpdated"` // When this reading was taken } // SMARTAttributes holds normalized SMART attributes for both SATA and NVMe disks. @@ -205,6 +213,7 @@ var ( smartTextCurrentTempRE = regexp.MustCompile(`(?i)^current(?: drive)? temperature:\s*(\d{1,3})\b`) smartTextTemperatureRE = regexp.MustCompile(`(?i)^temperature:\s*(\d{1,3})\b`) linuxDirectSATDeviceRE = regexp.MustCompile(`^(sd|hd)[a-z]+$`) + pciControllerAddressRE = regexp.MustCompile(`(?i)^[0-9a-f]{4}:[0-9a-f]{2}:[0-9a-f]{2}\.[0-7]$`) ) type smartctlTarget struct { @@ -236,6 +245,38 @@ func CollectSMARTLocal(ctx context.Context, diskExclude []string) ([]DiskSMART, return nil, nil } + type smartOutcome struct { + smart *DiskSMART + err error + } + outcomes := make([]smartOutcome, len(targets)) + workerCount := smartCollectionConcurrency + if len(targets) < smartCollectionParallelThreshold { + workerCount = 1 + } + if workerCount < 1 { + workerCount = 1 + } + if workerCount > len(targets) { + workerCount = len(targets) + } + jobs := make(chan int) + var workers sync.WaitGroup + workers.Add(workerCount) + for worker := 0; worker < workerCount; worker++ { + go func() { + defer workers.Done() + for index := range jobs { + outcomes[index].smart, outcomes[index].err = collectSMARTTarget(ctx, targets[index]) + } + }() + } + for index := range targets { + jobs <- index + } + close(jobs) + workers.Wait() + var results []DiskSMART var missed []smartctlTarget collected := make(map[string]struct{}, len(targets)) @@ -245,7 +286,10 @@ func CollectSMARTLocal(ctx context.Context, diskExclude []string) ([]DiskSMART, if isMultiplexedDeviceType(target.DeviceType) && block != "" { multiplexed[block] = struct{}{} } - smart, err := collectSMARTTarget(ctx, target) + } + for index, target := range targets { + block := canonicalBlockDeviceForScanPath(target.Path) + smart, err := outcomes[index].smart, outcomes[index].err if err != nil { if errors.Is(err, errSMARTDataUnavailable) { log.Debug(). @@ -801,23 +845,115 @@ func refineLinuxBlockDeviceIdentity(smart *DiskSMART, target smartctlTarget) { if smart == nil || runtimeGOOS != "linux" { return } + block := canonicalBlockDeviceForScanPath(target.Path) + if block == "" { + return + } // Disks addressed behind a multiplexing controller (megaraid,7; cciss,1; // areca,1/1; ...) all share a single /dev path, so the smartctl scan label is // the only thing that disambiguates them and /sys/block describes the array, // not the member. Leave those as-is and trust the smartctl-reported capacity. if isMultiplexedDeviceType(target.DeviceType) { - return - } - block := canonicalBlockDeviceForScanPath(target.Path) - if block == "" { + smart.Controller = block + smart.Target = strings.TrimSpace(target.DeviceType) + ensureControllerCollectionStatus(smart, "smartctl_scan") return } smart.Device = block + smart.Controller, smart.Target = linuxBlockDeviceTopology(block) + ensureControllerCollectionStatus(smart, "sysfs") if size := blockDeviceSizeBytes(block); size > 0 { smart.SizeBytes = size } } +func ensureControllerCollectionStatus(smart *DiskSMART, source string) { + if smart.Collection == nil { + smart.Collection = &diskinventory.CollectionStatus{} + } + if smart.Controller != "" || smart.Target != "" { + smart.Collection.Controller = diskinventory.Available(source) + return + } + smart.Collection.Controller = diskinventory.Missing(source, "controller association was not reported") +} + +// linuxBlockDeviceTopology derives a stable controller association and SCSI +// target from the resolved /sys/block device path. The controller prefers the +// PCI address immediately preceding hostN; the target is the terminal H:C:T:L +// segment. Neither value is fabricated when sysfs does not expose it. +func linuxBlockDeviceTopology(block string) (string, string) { + resolved, err := smartctlEvalSymlinks(filepath.Join("/sys/block", block, "device")) + if err != nil { + return "", "" + } + parts := strings.Split(filepath.Clean(resolved), string(filepath.Separator)) + controller := "" + controllerFallback := "" + target := "" + for index, part := range parts { + if pciControllerAddressRE.MatchString(part) { + controller = part + } + if strings.HasPrefix(part, "host") && hasNumericSuffix(part, "host") && index > 0 { + controllerFallback = parts[index-1] + } + if isSCSITargetAddress(part) { + target = part + } + } + if controller == "" { + controller = controllerFallback + } + return controller, target +} + +func isSCSITargetAddress(value string) bool { + parts := strings.Split(value, ":") + if len(parts) != 4 { + return false + } + for _, part := range parts { + if part == "" || !isAllDigits(part) { + return false + } + } + return true +} + +func linuxBlockDeviceTransport(block string, target smartctlTarget) string { + for _, candidate := range []string{ + readTrimmedFile(filepath.Join("/sys/block", block, "device", "protocol")), + readTrimmedFile(filepath.Join("/sys/block", block, "device", "transport")), + } { + switch normalized := strings.ToLower(strings.TrimSpace(candidate)); { + case strings.Contains(normalized, "nvme"): + return "nvme" + case strings.Contains(normalized, "sas"): + return "sas" + case strings.Contains(normalized, "sata"), strings.Contains(normalized, "ata"): + return "sata" + case strings.Contains(normalized, "usb"): + return "usb" + } + } + if strings.HasPrefix(block, "nvme") { + return "nvme" + } + deviceType := strings.ToLower(strings.TrimSpace(target.DeviceType)) + switch { + case strings.HasPrefix(deviceType, "nvme"): + return "nvme" + case strings.HasPrefix(deviceType, "sat"): + return "sata" + default: + // Preserve the legacy direct sdX fallback when the kernel supplies no + // transport evidence. Successful smartctl probes still override this + // with their protocol field before this fallback is needed. + return "sata" + } +} + // linuxIdentityOnlyDisks builds identity-only entries for physical disks whose // SMART probes produced nothing usable. A real disk that refuses SMART must // still be listed — Proxmox's own disks/list shows it, and a monitoring view @@ -863,17 +999,31 @@ func linuxIdentityOnlyDisks(missed []smartctlTarget, collected, multiplexed map[ continue } - diskType := "sata" - if strings.HasPrefix(block, "nvme") { - diskType = "nvme" + serial := readTrimmedFile(filepath.Join("/sys/block", block, "device", "serial")) + controller, controllerTarget := linuxBlockDeviceTopology(block) + collection := &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data"), + } + if serial != "" { + collection.Serial = diskinventory.Available("sysfs") + } else { + collection.Serial = diskinventory.Missing("sysfs", "disk serial was not reported") + } + if controller != "" || controllerTarget != "" { + collection.Controller = diskinventory.Available("sysfs") + } else { + collection.Controller = diskinventory.Missing("sysfs", "controller association was not reported") } results = append(results, DiskSMART{ Device: block, Model: readTrimmedFile(filepath.Join("/sys/block", block, "device", "model")), - Serial: readTrimmedFile(filepath.Join("/sys/block", block, "device", "serial")), - Type: diskType, + Serial: serial, + Type: linuxBlockDeviceTransport(block, target), + Controller: controller, + Target: controllerTarget, SizeBytes: size, Health: "UNKNOWN", + Collection: collection, LastUpdated: timeNow(), }) log.Debug(). @@ -1056,8 +1206,12 @@ func collectSMARTTarget(ctx context.Context, target smartctlTarget) (*DiskSMART, exitCode := exitErr.ExitCode() if (exitCode == smartctlStandbyExitStatus || exitCode&2 != 0) && len(output) == 0 { standbyResult := &DiskSMART{ - Device: filepath.Base(target.Path), - Standby: true, + Device: filepath.Base(target.Path), + Standby: true, + Collection: &diskinventory.CollectionStatus{ + Serial: diskinventory.Unavailable("smartctl", "disk is in standby"), + Temperature: diskinventory.Unavailable("smartctl", "disk is in standby"), + }, LastUpdated: timeNow(), } if runtimeGOOS == "freebsd" && i < len(attempts)-1 && target.DeviceType == "" { @@ -1338,6 +1492,7 @@ func parseSMARTOutput(output []byte, target smartctlTarget) (*DiskSMART, error) Type: detectDiskType(smartData), Standby: isStandbyPowerMode(smartData.PowerMode), LastUpdated: timeNow(), + Collection: &diskinventory.CollectionStatus{}, } if smartData.WWN.NAA != 0 { @@ -1380,6 +1535,19 @@ func parseSMARTOutput(output []byte, target smartctlTarget) (*DiskSMART, error) } applySMARTTextFallback(result, parseSMARTTextFallback(strings.Join(smartData.Smartctl.Output, "\n"))) + if result.Serial != "" { + result.Collection.Serial = diskinventory.Available("smartctl") + } else { + result.Collection.Serial = diskinventory.Missing("smartctl", "disk serial was not reported") + } + switch { + case result.Temperature > 0: + result.Collection.Temperature = diskinventory.Available("smartctl") + case result.Standby: + result.Collection.Temperature = diskinventory.Unavailable("smartctl", "disk is in standby") + default: + result.Collection.Temperature = diskinventory.Unsupported("smartctl", "device did not expose a temperature reading") + } if result.Health == "" { result.Health = "UNKNOWN" } @@ -1403,6 +1571,7 @@ func parseSMARTTextOutput(text string, target smartctlTarget) (*DiskSMART, error Health: fallback.Health, Standby: fallback.Standby, LastUpdated: timeNow(), + Collection: &diskinventory.CollectionStatus{}, } if result.Type == "" { switch { @@ -1418,6 +1587,19 @@ func parseSMARTTextOutput(text string, target smartctlTarget) (*DiskSMART, error if result.Health == "" { result.Health = "UNKNOWN" } + if result.Serial != "" { + result.Collection.Serial = diskinventory.Available("smartctl_text") + } else { + result.Collection.Serial = diskinventory.Missing("smartctl_text", "disk serial was not reported") + } + switch { + case result.Temperature > 0: + result.Collection.Temperature = diskinventory.Available("smartctl_text") + case result.Standby: + result.Collection.Temperature = diskinventory.Unavailable("smartctl_text", "disk is in standby") + default: + result.Collection.Temperature = diskinventory.Unsupported("smartctl_text", "device did not expose a temperature reading") + } if result.Health == "UNKNOWN" && result.Temperature == 0 && !result.Standby { return nil, errSMARTDataUnavailable } diff --git a/internal/models/converters.go b/internal/models/converters.go index 0b49a4123..5ec9daaf9 100644 --- a/internal/models/converters.go +++ b/internal/models/converters.go @@ -4,6 +4,8 @@ import ( "encoding/json" "strings" "time" + + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) // ToFrontend converts a State to StateFrontend @@ -925,11 +927,15 @@ func hostSensorSummaryToFrontend(src HostSensorSummary) *HostSensorSummaryFronte Serial: disk.Serial, WWN: disk.WWN, Type: disk.Type, + Controller: disk.Controller, + Target: disk.Target, SizeBytes: disk.SizeBytes, Temperature: disk.Temperature, Health: disk.Health, Standby: disk.Standby, - Attributes: disk.Attributes, + IO: cloneDiskIOForFrontend(disk.IO), + Collection: diskinventory.CloneStatus(disk.Collection), + Attributes: cloneSMARTAttributes(disk.Attributes), } } } @@ -937,6 +943,14 @@ func hostSensorSummaryToFrontend(src HostSensorSummary) *HostSensorSummaryFronte return &normalized } +func cloneDiskIOForFrontend(src *DiskIO) *DiskIO { + if src == nil { + return nil + } + dest := *src + return &dest +} + func copyHostThermalState(src *HostThermalState) *HostThermalState { if src == nil { return nil diff --git a/internal/models/deepcopy.go b/internal/models/deepcopy.go index 6535ee789..3b3864734 100644 --- a/internal/models/deepcopy.go +++ b/internal/models/deepcopy.go @@ -3,6 +3,8 @@ package models import ( "reflect" "time" + + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) func cloneBoolPtr(src *bool) *bool { @@ -237,6 +239,11 @@ func cloneHostDiskSMART(src []HostDiskSMART) []HostDiskSMART { for i, disk := range src { diskCopy := disk diskCopy.Attributes = cloneSMARTAttributes(disk.Attributes) + diskCopy.Collection = diskinventory.CloneStatus(disk.Collection) + if disk.IO != nil { + ioCopy := *disk.IO + diskCopy.IO = &ioCopy + } dest[i] = diskCopy } return dest @@ -1102,6 +1109,11 @@ func cloneCephClusters(src []CephCluster) []CephCluster { func clonePhysicalDisk(src PhysicalDisk) PhysicalDisk { dest := src dest.SmartAttributes = cloneSMARTAttributes(src.SmartAttributes) + dest.Collection = diskinventory.CloneStatus(src.Collection) + if src.IO != nil { + ioCopy := *src.IO + dest.IO = &ioCopy + } return dest } diff --git a/internal/models/models.go b/internal/models/models.go index 15d95464d..5af8438b3 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -12,6 +12,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/proxmoxidentity" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) // State represents the current state of all monitored resources @@ -464,17 +465,21 @@ func (s HostSensorSummary) NormalizeCollections() HostSensorSummary { // HostDiskSMART represents S.M.A.R.T. data for a disk from a host agent. type HostDiskSMART struct { - Device string `json:"device"` // Block device name (e.g., sda, nvme0n1) - Model string `json:"model,omitempty"` // Disk model - Serial string `json:"serial,omitempty"` // Serial number - WWN string `json:"wwn,omitempty"` // World Wide Name - Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme - SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown) - Temperature int `json:"temperature"` // Temperature in Celsius - Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN - Standby bool `json:"standby,omitempty"` // True if disk was in standby - Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member) - Attributes *SMARTAttributes `json:"attributes,omitempty"` + Device string `json:"device"` // Block device name (e.g., sda, nvme0n1) + Model string `json:"model,omitempty"` // Disk model + Serial string `json:"serial,omitempty"` // Serial number + WWN string `json:"wwn,omitempty"` // World Wide Name + Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme + Controller string `json:"controller,omitempty"` // Stable controller association when reported + Target string `json:"target,omitempty"` // Controller target/HCTL or smartctl member target + SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown) + Temperature int `json:"temperature"` // Temperature in Celsius + Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN + Standby bool `json:"standby,omitempty"` // True if disk was in standby + Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member) + IO *DiskIO `json:"io,omitempty"` + Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` + Attributes *SMARTAttributes `json:"attributes,omitempty"` } // SMARTAttributes holds normalized SMART attributes for both SATA and NVMe disks. @@ -2395,23 +2400,27 @@ type CephServiceStatus struct { // PhysicalDisk represents a physical disk on a node type PhysicalDisk struct { - ID string `json:"id"` // "{instance}-{node}-{devpath}" - Node string `json:"node"` - Instance string `json:"instance"` - DevPath string `json:"devPath"` // /dev/nvme0n1, /dev/sda - Model string `json:"model"` - Serial string `json:"serial"` - WWN string `json:"wwn"` // World Wide Name - Type string `json:"type"` // nvme, sata, sas - Size int64 `json:"size"` // bytes - Health string `json:"health"` // PASSED, FAILED, UNKNOWN - Wearout int `json:"wearout"` // SSD wear metric from Proxmox (0-100, -1 when unavailable) - Temperature int `json:"temperature"` // Celsius (if available) - RPM int `json:"rpm"` // 0 for SSDs - Used string `json:"used"` // Filesystem or partition usage - StorageGroup string `json:"storageGroup"` // Pool/VG/array this disk belongs to (e.g. ZFS pool name); empty if not matched - SmartAttributes *SMARTAttributes `json:"smartAttributes,omitempty"` - LastChecked time.Time `json:"lastChecked"` + ID string `json:"id"` // "{instance}-{node}-{devpath}" + Node string `json:"node"` + Instance string `json:"instance"` + DevPath string `json:"devPath"` // /dev/nvme0n1, /dev/sda + Model string `json:"model"` + Serial string `json:"serial"` + WWN string `json:"wwn"` // World Wide Name + Type string `json:"type"` // nvme, sata, sas + Controller string `json:"controller,omitempty"` // Controller association when reported + Target string `json:"target,omitempty"` // Controller target/HCTL when reported + Size int64 `json:"size"` // bytes + Health string `json:"health"` // PASSED, FAILED, UNKNOWN + Wearout int `json:"wearout"` // SSD wear metric from Proxmox (0-100, -1 when unavailable) + Temperature int `json:"temperature"` // Celsius (if available) + RPM int `json:"rpm"` // 0 for SSDs + Used string `json:"used"` // Filesystem or partition usage + StorageGroup string `json:"storageGroup"` // Pool/VG/array this disk belongs to (e.g. ZFS pool name); empty if not matched + SmartAttributes *SMARTAttributes `json:"smartAttributes,omitempty"` + IO *DiskIO `json:"io,omitempty"` + Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` + LastChecked time.Time `json:"lastChecked"` } // PBSInstance represents a Proxmox Backup Server instance diff --git a/internal/models/models_frontend.go b/internal/models/models_frontend.go index 8cbb89556..85a80db57 100644 --- a/internal/models/models_frontend.go +++ b/internal/models/models_frontend.go @@ -1,6 +1,10 @@ package models -import "encoding/json" +import ( + "encoding/json" + + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" +) // Frontend-friendly type aliases with proper JSON tags // These extend the base types with additional computed fields @@ -798,16 +802,20 @@ func (s HostSensorSummaryFrontend) NormalizeCollections() HostSensorSummaryFront // HostDiskSMARTFrontend represents S.M.A.R.T. data for a disk from a host agent. type HostDiskSMARTFrontend struct { - Device string `json:"device"` // Device name (e.g., sda) - Model string `json:"model,omitempty"` // Disk model - Serial string `json:"serial,omitempty"` // Serial number - WWN string `json:"wwn,omitempty"` // World Wide Name - Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme - SizeBytes int64 `json:"sizeBytes,omitempty"` - Temperature int `json:"temperature"` // Temperature in Celsius - Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN - Standby bool `json:"standby,omitempty"` // True if disk was in standby - Attributes *SMARTAttributes `json:"attributes,omitempty"` + Device string `json:"device"` // Device name (e.g., sda) + Model string `json:"model,omitempty"` // Disk model + Serial string `json:"serial,omitempty"` // Serial number + WWN string `json:"wwn,omitempty"` // World Wide Name + Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme + Controller string `json:"controller,omitempty"` // Controller association + Target string `json:"target,omitempty"` // Controller target/HCTL + SizeBytes int64 `json:"sizeBytes,omitempty"` + Temperature int `json:"temperature"` // Temperature in Celsius + Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN + Standby bool `json:"standby,omitempty"` // True if disk was in standby + IO *DiskIO `json:"io,omitempty"` + Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` + Attributes *SMARTAttributes `json:"attributes,omitempty"` } // StorageFrontend represents Storage with frontend-friendly field names diff --git a/internal/monitoring/issue1595_collection_trust_test.go b/internal/monitoring/issue1595_collection_trust_test.go new file mode 100644 index 000000000..8c6a79efc --- /dev/null +++ b/internal/monitoring/issue1595_collection_trust_test.go @@ -0,0 +1,314 @@ +package monitoring + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" +) + +type issue1595TopologyFixture struct { + Node string `json:"node"` + Instance string `json:"instance"` + AgentID string `json:"agentId"` + Model string `json:"model"` + SizeBytes int64 `json:"sizeBytes"` + Controllers []struct { + ID string `json:"id"` + Pool string `json:"pool"` + Disks []struct { + Device string `json:"device"` + Target string `json:"target"` + Serial string `json:"serial"` + ProviderSerial string `json:"providerSerial"` + Temperature int `json:"temperature"` + ReadBytes uint64 `json:"readBytes"` + WriteBytes uint64 `json:"writeBytes"` + IOTimeMs uint64 `json:"ioTimeMs"` + } `json:"disks"` + } `json:"controllers"` +} + +func loadIssue1595TopologyFixture(t *testing.T) issue1595TopologyFixture { + t.Helper() + data, err := os.ReadFile(filepath.Join("..", "..", "testdata", "issue1595_sas_topology.json")) + if err != nil { + t.Fatalf("read issue #1595 fixture: %v", err) + } + var fixture issue1595TopologyFixture + if err := json.Unmarshal(data, &fixture); err != nil { + t.Fatalf("decode issue #1595 fixture: %v", err) + } + return fixture +} + +func TestIssue1595SASTopologySurvivesMergeRegistryAndReadState(t *testing.T) { + fixture := loadIssue1595TopologyFixture(t) + now := time.Date(2026, 7, 20, 12, 0, 0, 0, time.UTC) + nodeID := fixture.Instance + "-" + fixture.Node + + host := models.Host{ + ID: fixture.AgentID, + Hostname: fixture.Node, + Status: "online", + LastSeen: now, + LinkedNodeID: nodeID, + } + node := models.Node{ + ID: nodeID, + Name: fixture.Node, + Instance: fixture.Instance, + Status: "online", + LastSeen: now, + LinkedAgentID: fixture.AgentID, + } + + var providerDisks []models.PhysicalDisk + expectedBySerial := make(map[string]models.PhysicalDisk) + for _, controller := range fixture.Controllers { + for index, disk := range controller.Disks { + device := "/dev/" + disk.Device + io := &models.DiskIO{ + Device: disk.Device, + ReadBytes: disk.ReadBytes, + WriteBytes: disk.WriteBytes, + ReadOps: uint64(100 + index), + WriteOps: uint64(200 + index), + ReadTime: uint64(300 + index), + WriteTime: uint64(400 + index), + IOTime: disk.IOTimeMs, + } + host.DiskIO = append(host.DiskIO, *io) + host.Sensors.SMART = append(host.Sensors.SMART, models.HostDiskSMART{ + Device: device, + Model: fixture.Model, + Serial: disk.Serial, + Type: "sas", + Controller: controller.ID, + Target: disk.Target, + SizeBytes: fixture.SizeBytes, + Temperature: disk.Temperature, + Health: "PASSED", + Pool: controller.Pool, + IO: io, + Collection: &diskinventory.CollectionStatus{ + Serial: diskinventory.Available("smartctl"), + Temperature: diskinventory.Available("smartctl"), + IO: diskinventory.Available("linux-diskstats"), + Controller: diskinventory.Available("linux-sysfs"), + Pool: diskinventory.Available("zpool-status"), + }, + }) + + providerDisk := models.PhysicalDisk{ + ID: fixture.Instance + "-" + fixture.Node + "-" + disk.Device, + Node: fixture.Node, + Instance: fixture.Instance, + DevPath: device, + Model: fixture.Model, + Serial: disk.ProviderSerial, + Type: "unknown", + Size: fixture.SizeBytes, + Health: "UNKNOWN", + Wearout: -1, + Used: "ZFS", + StorageGroup: controller.Pool, + Collection: &diskinventory.CollectionStatus{ + Serial: diskinventory.Available("pve-api"), + Temperature: diskinventory.Unsupported("pve-api", "provider does not report disk temperature"), + IO: diskinventory.Unsupported("pve-api", "provider does not report per-disk I/O"), + Controller: diskinventory.Missing("pve-api", "controller association absent"), + Pool: diskinventory.Available("pve-storage"), + }, + LastChecked: now, + } + providerDisks = append(providerDisks, providerDisk) + + want := providerDisk + want.Serial = disk.Serial + want.Type = "sas" + want.Controller = controller.ID + want.Target = disk.Target + want.Temperature = disk.Temperature + want.Health = "PASSED" + want.IO = io + expectedBySerial[disk.Serial] = want + } + } + + merged := mergeHostAgentSMARTIntoDisks(providerDisks, []models.Node{node}, []models.Host{host}) + if len(merged) != 24 { + t.Fatalf("merged disk count = %d, want 24", len(merged)) + } + for _, disk := range merged { + want, ok := expectedBySerial[disk.Serial] + if !ok { + t.Fatalf("provider SAS address survived as stable serial for %s: %q", disk.DevPath, disk.Serial) + } + assertIssue1595PhysicalDisk(t, disk, want) + } + + registry := unifiedresources.NewRegistry(nil) + registry.IngestSnapshot(models.StateSnapshot{ + Nodes: []models.Node{node}, + Hosts: []models.Host{host}, + PhysicalDisks: providerDisks, + }) + + resources := registry.ListByType(unifiedresources.ResourceTypePhysicalDisk) + if len(resources) != 24 { + t.Fatalf("registry physical disk count = %d, want 24 distinct disks", len(resources)) + } + for _, resource := range resources { + if resource.PhysicalDisk == nil { + t.Fatalf("physical disk resource %q has no physicalDisk metadata", resource.ID) + } + _, ok := expectedBySerial[resource.PhysicalDisk.Serial] + if !ok { + t.Fatalf("registry serial = %q, want a smartctl serial", resource.PhysicalDisk.Serial) + } + if !hasIssue1595Source(resource.Sources, unifiedresources.SourceAgent) || + !hasIssue1595Source(resource.Sources, unifiedresources.SourceProxmox) { + t.Fatalf("disk %q sources = %v, want agent and proxmox", resource.PhysicalDisk.Serial, resource.Sources) + } + } + + views := registry.PhysicalDisks() + if len(views) != 24 { + t.Fatalf("read-state physical disk count = %d, want 24", len(views)) + } + for _, view := range views { + want, ok := expectedBySerial[view.Serial()] + if !ok { + t.Fatalf("read-state serial = %q, want smartctl serial", view.Serial()) + } + if view.MetricResourceID() != want.Serial { + t.Fatalf("disk %q metrics target = %q, want serial-stable target", want.Serial, view.MetricResourceID()) + } + readBack := physicalDiskFromReadStateView(view) + assertIssue1595PhysicalDisk(t, readBack, want) + if readBack.Collection == nil || + readBack.Collection.Serial.State != diskinventory.FieldAvailable || + readBack.Collection.Temperature.State != diskinventory.FieldAvailable || + readBack.Collection.IO.State != diskinventory.FieldAvailable || + readBack.Collection.Controller.State != diskinventory.FieldAvailable || + readBack.Collection.Pool.State != diskinventory.FieldAvailable { + t.Fatalf("disk %q collection status = %+v", readBack.Serial, readBack.Collection) + } + } +} + +func TestPhysicalDiskUnavailableEvidenceIsRetainedWithoutPretendingItWasCollected(t *testing.T) { + previous := models.PhysicalDisk{ + Serial: "ZR5TESTA0001", + Controller: "0000:03:00.0", + Target: "6:0:0:0", + Temperature: 30, + StorageGroup: "tank-a", + IO: &models.DiskIO{Device: "sda", ReadBytes: 1000}, + } + current := models.PhysicalDisk{ + Collection: &diskinventory.CollectionStatus{ + Serial: diskinventory.Missing("smartctl", "serial absent from successful response"), + Temperature: diskinventory.Unavailable("smartctl", "collection deadline exceeded"), + IO: diskinventory.Unsupported("controller", "per-member counters unavailable"), + Controller: diskinventory.Unavailable("linux-sysfs", "topology lookup failed"), + Pool: diskinventory.Unavailable("zpool-status", "command failed"), + }, + } + + got := preserveUnavailablePhysicalDiskEvidence(current, previous) + if got.Serial != previous.Serial || + got.Controller != previous.Controller || + got.Target != previous.Target || + got.Temperature != previous.Temperature || + got.StorageGroup != previous.StorageGroup || + got.IO == nil || + got.IO.ReadBytes != previous.IO.ReadBytes { + t.Fatalf("unavailable evidence was discarded: %+v", got) + } + if got.Collection.Temperature.State != diskinventory.FieldUnavailable || + got.Collection.IO.State != diskinventory.FieldUnsupported || + got.Collection.Serial.State != diskinventory.FieldMissing { + t.Fatalf("retained values concealed current collection state: %+v", got.Collection) + } + + got.IO.ReadBytes = 2000 + if previous.IO.ReadBytes != 1000 { + t.Fatal("retained I/O evidence aliases the previous snapshot") + } +} + +func TestTrustedSMARTSerialPromotionDoesNotRewriteSATAOrNVMeIdentity(t *testing.T) { + for _, diskType := range []string{"sata", "nvme"} { + t.Run(diskType, func(t *testing.T) { + device := "/dev/sda" + if diskType == "nvme" { + device = "/dev/nvme0n1" + } + disks := []models.PhysicalDisk{{ + ID: "provider-disk", + Node: "node", + DevPath: device, + Serial: "PROVIDER-SERIAL", + Type: diskType, + }} + hosts := []models.Host{{ + ID: "agent", + Sensors: models.HostSensorSummary{ + SMART: []models.HostDiskSMART{{ + Device: device, + Serial: "SMARTCTL-SERIAL", + Type: diskType, + Collection: &diskinventory.CollectionStatus{ + Serial: diskinventory.Available("smartctl"), + }, + }}, + }, + }} + + got := mergeHostAgentSMARTIntoDisks( + disks, + []models.Node{{Name: "node", LinkedAgentID: "agent"}}, + hosts, + )[0] + if got.Serial != "PROVIDER-SERIAL" || got.Type != diskType { + t.Fatalf("%s identity changed during SAS remediation: %+v", diskType, got) + } + }) + } +} + +func assertIssue1595PhysicalDisk(t *testing.T, got, want models.PhysicalDisk) { + t.Helper() + if got.Serial != want.Serial || + got.Type != "sas" || + got.Controller != want.Controller || + got.Target != want.Target || + got.Temperature != want.Temperature || + got.StorageGroup != want.StorageGroup || + got.IO == nil || + got.IO.ReadBytes != want.IO.ReadBytes || + got.IO.WriteBytes != want.IO.WriteBytes || + got.IO.ReadOps != want.IO.ReadOps || + got.IO.WriteOps != want.IO.WriteOps || + got.IO.ReadTime != want.IO.ReadTime || + got.IO.WriteTime != want.IO.WriteTime || + got.IO.IOTime != want.IO.IOTime { + t.Fatalf("disk %s lost trusted inventory data:\n got %+v\n want %+v", got.DevPath, got, want) + } +} + +func hasIssue1595Source(sources []unifiedresources.DataSource, want unifiedresources.DataSource) bool { + for _, source := range sources { + if source == want { + return true + } + } + return false +} diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 4f7e9715f..db7ac2da3 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -29,6 +29,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/system" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/internal/websocket" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" "github.com/rcourtman/pulse-go-rewrite/pkg/pbs" "github.com/rcourtman/pulse-go-rewrite/pkg/pmg" @@ -390,7 +391,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [ for _, temp := range smartTemps { if temp.WWN != "" && strings.EqualFold(temp.WWN, updated[i].WWN) { if temp.Temperature > 0 && !temp.StandbySkipped { - updated[i].Temperature = temp.Temperature + setPhysicalDiskTemperature(&updated[i], temp.Temperature, "proxmox_node_smart") log.Debug(). Str("disk", updated[i].DevPath). Str("wwn", updated[i].WWN). @@ -407,7 +408,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [ for _, temp := range smartTemps { if temp.Serial != "" && strings.EqualFold(temp.Serial, updated[i].Serial) { if temp.Temperature > 0 && !temp.StandbySkipped { - updated[i].Temperature = temp.Temperature + setPhysicalDiskTemperature(&updated[i], temp.Temperature, "proxmox_node_smart") log.Debug(). Str("disk", updated[i].DevPath). Str("serial", updated[i].Serial). @@ -426,7 +427,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [ normalizedTempDev := normalizeSMARTDeviceIdentifier(temp.Device) if normalizedTempDev != "" && normalizedTempDev == normalizedDevPath { if temp.Temperature > 0 && !temp.StandbySkipped { - updated[i].Temperature = temp.Temperature + setPhysicalDiskTemperature(&updated[i], temp.Temperature, "proxmox_node_smart") log.Debug(). Str("disk", updated[i].DevPath). Int("temp", temp.Temperature). @@ -466,7 +467,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [ continue } - updated[diskIdx].Temperature = int(math.Round(tempVal)) + setPhysicalDiskTemperature(&updated[diskIdx], int(math.Round(tempVal)), "proxmox_node_nvme") log.Debug(). Str("disk", updated[diskIdx].DevPath). Int("temp", updated[diskIdx].Temperature). @@ -477,6 +478,17 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [ return updated } +func setPhysicalDiskTemperature(disk *models.PhysicalDisk, temperature int, source string) { + if disk == nil || temperature <= 0 { + return + } + disk.Temperature = temperature + if disk.Collection == nil { + disk.Collection = &diskinventory.CollectionStatus{} + } + disk.Collection.Temperature = diskinventory.Available(source) +} + // mergeHostAgentSMARTIntoDisks merges SMART temperature data from linked host agents // into physical disks for Proxmox nodes. This allows disk temps collected by the // pulse-agent running on a PVE node to populate the Physical Disks view. @@ -522,7 +534,7 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No continue } - // Find matching SMART entry by WWN, serial, or device path + // Find matching SMART entry by WWN, serial, or topology-scoped device path. var matched *models.HostDiskSMART // Try to match by WWN (most reliable) @@ -550,29 +562,38 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No normalizedDevPath := normalizeSMARTDeviceIdentifier(updated[i].DevPath) for j := range smartData { normalizedDiskDev := normalizeSMARTDeviceIdentifier(smartData[j].Device) - if normalizedDiskDev != "" && normalizedDiskDev == normalizedDevPath { + if normalizedDiskDev != "" && + normalizedDiskDev == normalizedDevPath && + diskTopologyCompatible(updated[i].Controller, updated[i].Target, smartData[j].Controller, smartData[j].Target) { matched = &smartData[j] break } } } - if matched == nil || matched.Standby { + if matched == nil { continue } if strings.TrimSpace(updated[i].Model) == "" && strings.TrimSpace(matched.Model) != "" { updated[i].Model = strings.TrimSpace(matched.Model) } - if strings.TrimSpace(updated[i].Serial) == "" && strings.TrimSpace(matched.Serial) != "" { + if strings.TrimSpace(matched.Serial) != "" && + (strings.TrimSpace(updated[i].Serial) == "" || shouldPromoteHostAgentSerial(updated[i], *matched)) { updated[i].Serial = strings.TrimSpace(matched.Serial) } if strings.TrimSpace(updated[i].WWN) == "" && strings.TrimSpace(matched.WWN) != "" { updated[i].WWN = strings.TrimSpace(matched.WWN) } - if strings.TrimSpace(updated[i].Type) == "" && strings.TrimSpace(matched.Type) != "" { + if shouldPromoteHostAgentDiskType(updated[i].Type, matched.Type) { updated[i].Type = strings.TrimSpace(matched.Type) } + if strings.TrimSpace(updated[i].Controller) == "" && strings.TrimSpace(matched.Controller) != "" { + updated[i].Controller = strings.TrimSpace(matched.Controller) + } + if strings.TrimSpace(updated[i].Target) == "" && strings.TrimSpace(matched.Target) != "" { + updated[i].Target = strings.TrimSpace(matched.Target) + } if updated[i].Size <= 0 && matched.SizeBytes > 0 { updated[i].Size = matched.SizeBytes } @@ -581,7 +602,7 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No } // Merge temperature if not already set - if updated[i].Temperature == 0 && matched.Temperature > 0 { + if !matched.Standby && updated[i].Temperature == 0 && matched.Temperature > 0 { updated[i].Temperature = matched.Temperature log.Debug(). Str("device", updated[i].DevPath). @@ -598,6 +619,11 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No } } } + if matched.IO != nil { + ioCopy := *matched.IO + updated[i].IO = &ioCopy + } + updated[i].Collection = diskinventory.MergeStatus(updated[i].Collection, matched.Collection) if (strings.TrimSpace(updated[i].Health) == "" || strings.EqualFold(updated[i].Health, "unknown")) && strings.TrimSpace(matched.Health) != "" { updated[i].Health = matched.Health @@ -607,6 +633,61 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No return updated } +func diskTopologyCompatible(leftController, leftTarget, rightController, rightTarget string) bool { + leftController = strings.TrimSpace(leftController) + leftTarget = strings.TrimSpace(leftTarget) + rightController = strings.TrimSpace(rightController) + rightTarget = strings.TrimSpace(rightTarget) + if leftController != "" && rightController != "" && !strings.EqualFold(leftController, rightController) { + return false + } + if leftTarget != "" && rightTarget != "" && !strings.EqualFold(leftTarget, rightTarget) { + return false + } + return true +} + +func shouldPromoteHostAgentSerial(disk models.PhysicalDisk, smart models.HostDiskSMART) bool { + if smart.Collection != nil && + strings.EqualFold(strings.TrimSpace(smart.Type), "sas") && + smart.Collection.Serial.State == diskinventory.FieldAvailable && + strings.HasPrefix(strings.ToLower(strings.TrimSpace(smart.Collection.Serial.Source)), "smartctl") { + return true + } + // Older agents did not carry collection provenance. Proxmox commonly puts + // a 64-bit SAS address in its serial field; a non-address smartctl serial is + // the actual drive identity and must replace that transport address. + return strings.EqualFold(strings.TrimSpace(disk.Type), "sas") && + strings.EqualFold(strings.TrimSpace(smart.Type), "sas") && + looksLikeSASAddress(disk.Serial) && + !looksLikeSASAddress(smart.Serial) +} + +func looksLikeSASAddress(value string) bool { + value = strings.TrimPrefix(strings.ToLower(strings.TrimSpace(value)), "0x") + if len(value) != 16 { + return false + } + for _, char := range value { + if (char < '0' || char > '9') && (char < 'a' || char > 'f') { + return false + } + } + return true +} + +func shouldPromoteHostAgentDiskType(existing, incoming string) bool { + incoming = strings.ToLower(strings.TrimSpace(incoming)) + if incoming == "" { + return false + } + existing = strings.ToLower(strings.TrimSpace(existing)) + if existing == "" || existing == "unknown" || existing == "scsi" { + return true + } + return false +} + func deriveWearoutFromSMARTAttributes(attrs *models.SMARTAttributes) int { if attrs == nil || attrs.PercentageUsed == nil { return -1 @@ -636,17 +717,38 @@ func physicalDiskFromReadStateView(view *unifiedresources.PhysicalDiskView) mode Serial: view.Serial(), WWN: view.WWN(), Type: view.DiskType(), + Controller: view.Controller(), + Target: view.Target(), Size: view.SizeBytes(), Health: view.Health(), Wearout: view.Wearout(), Temperature: view.Temperature(), RPM: view.RPM(), Used: view.Used(), + StorageGroup: view.StorageGroup(), SmartAttributes: smartAttributesFromUnifiedMeta(view.SMART()), + IO: physicalDiskIOFromUnifiedMeta(view.IO()), + Collection: diskinventory.CloneStatus(view.Collection()), LastChecked: view.LastSeen(), } } +func physicalDiskIOFromUnifiedMeta(in *unifiedresources.PhysicalDiskIOMeta) *models.DiskIO { + if in == nil { + return nil + } + return &models.DiskIO{ + Device: in.Device, + ReadBytes: in.ReadBytes, + WriteBytes: in.WriteBytes, + ReadOps: in.ReadOps, + WriteOps: in.WriteOps, + ReadTime: in.ReadTimeMs, + WriteTime: in.WriteTimeMs, + IOTime: in.IOTimeMs, + } +} + func smartAttributesFromUnifiedMeta(in *unifiedresources.SMARTMeta) *models.SMARTAttributes { if in == nil { return nil @@ -3443,11 +3545,15 @@ func hostSensorsFromReadStateView(sensors *unifiedresources.HostSensorMeta) mode Serial: smart.Serial, WWN: smart.WWN, Type: smart.Type, + Controller: smart.Controller, + Target: smart.Target, SizeBytes: smart.SizeBytes, Temperature: smart.Temperature, Health: smart.Health, Standby: smart.Standby, Pool: smart.Pool, + IO: physicalDiskIOFromUnifiedMeta(smart.IO), + Collection: diskinventory.CloneStatus(smart.Collection), Attributes: smartAttributesCopy(smart.Attributes), }) } diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index 14087bf8c..c46462bfa 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -2678,14 +2678,24 @@ func hostDiskIOMetricResourceID(host models.Host, io models.DiskIO, proxmoxDisks return "" } + smartMetricID := "" for _, disk := range host.Sensors.SMART { if disk.Standby { continue } if strings.EqualFold(normalizeHostDiskDevice(disk.Device), device) { - return unifiedresources.HostSMARTDiskSourceID(host, disk) + candidate := unifiedresources.HostSMARTDiskSourceID(host, disk) + if smartMetricID != "" && smartMetricID != candidate { + // Multiple controller members share this kernel block path. + // The counter belongs to the aggregate device, not any member. + return "" + } + smartMetricID = candidate } } + if smartMetricID != "" { + return smartMetricID + } for _, pd := range proxmoxDisks { if pd.device == "" || pd.metricID == "" { diff --git a/internal/monitoring/monitor_alert_override_migration_test.go b/internal/monitoring/monitor_alert_override_migration_test.go new file mode 100644 index 000000000..8724fd83e --- /dev/null +++ b/internal/monitoring/monitor_alert_override_migration_test.go @@ -0,0 +1,64 @@ +package monitoring + +import ( + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +func TestSyncUnifiedResourceAlertsPersistsCanonicalOverrideSuccession(t *testing.T) { + const ( + oldID = "agent-535886018cb53055" + newID = "agent-b9ed6d0e20e94eaf" + ) + + dataDir := t.TempDir() + persistence := config.NewConfigPersistence(dataDir) + manager := alerts.NewManagerWithDataDir(dataDir) + t.Cleanup(manager.Stop) + + alertConfig := manager.GetConfig() + alertConfig.Enabled = false + alertConfig.Overrides = map[string]alerts.ThresholdConfig{ + oldID: { + Memory: &alerts.HysteresisThreshold{Trigger: 95, Clear: 90}, + }, + } + manager.UpdateConfig(alertConfig) + if err := persistence.SaveAlertConfig(manager.GetConfig()); err != nil { + t.Fatalf("seed legacy alert config: %v", err) + } + + monitor := &Monitor{ + alertManager: manager, + configPersist: persistence, + state: models.NewState(), + } + monitor.syncUnifiedResourceAlertsToState([]unifiedresources.Resource{{ + ID: newID, + Type: unifiedresources.ResourceTypeAgent, + SupersededCanonicalIDs: []string{oldID}, + }}) + + inMemory := manager.GetConfig() + if _, exists := inMemory.Overrides[oldID]; exists { + t.Fatalf("in-memory override remained under superseded identity %s", oldID) + } + if override := inMemory.Overrides[newID]; override.Memory == nil || override.Memory.Trigger != 95 { + t.Fatalf("in-memory override missing under canonical identity %s: %+v", newID, override) + } + + reloaded, err := config.NewConfigPersistence(dataDir).LoadAlertConfig() + if err != nil { + t.Fatalf("reload migrated alert config: %v", err) + } + if _, exists := reloaded.Overrides[oldID]; exists { + t.Fatalf("persisted override remained under superseded identity %s", oldID) + } + if override := reloaded.Overrides[newID]; override.Memory == nil || override.Memory.Trigger != 95 { + t.Fatalf("reloaded override missing under canonical identity %s: %+v", newID, override) + } +} diff --git a/internal/monitoring/monitor_alert_sync.go b/internal/monitoring/monitor_alert_sync.go index 88fc1d8fc..b2415b385 100644 --- a/internal/monitoring/monitor_alert_sync.go +++ b/internal/monitoring/monitor_alert_sync.go @@ -3,6 +3,7 @@ package monitoring import ( "strings" + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" "github.com/rcourtman/pulse-go-rewrite/internal/logging" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" @@ -95,6 +96,17 @@ func (m *Monitor) syncUnifiedResourceAlertsToState(resources []unifiedresources. return } + config := m.alertManager.GetConfig() + if alerts.MigrateCanonicalOverrideKeys(&config, resources) { + if m.configPersist == nil { + log.Warn().Msg("cannot persist canonical alert override migration without config persistence") + } else if err := m.configPersist.SaveAlertConfig(config); err != nil { + log.Error().Err(err).Msg("failed to persist canonical alert override migration") + } else { + m.alertManager.UpdateConfig(config) + } + } + m.alertManager.CheckUnifiedResourceMetrics(resources) m.alertManager.SyncUnifiedResourceIncidents(resources) m.syncAlertsToState() diff --git a/internal/monitoring/monitor_helpers.go b/internal/monitoring/monitor_helpers.go index 7461d0917..3216e8b98 100644 --- a/internal/monitoring/monitor_helpers.go +++ b/internal/monitoring/monitor_helpers.go @@ -11,6 +11,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/models" agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rs/zerolog/log" ) @@ -616,11 +617,27 @@ func convertAgentSMARTToModels(smart []agentshost.DiskSMART) []models.HostDiskSM Serial: disk.Serial, WWN: disk.WWN, Type: disk.Type, + Controller: disk.Controller, + Target: disk.Target, SizeBytes: disk.SizeBytes, Temperature: disk.Temperature, Health: disk.Health, Standby: disk.Standby, Pool: disk.Pool, + Collection: diskinventory.CloneStatus(disk.Collection), + } + if disk.IO != nil { + ioCopy := models.DiskIO{ + Device: disk.IO.Device, + ReadBytes: disk.IO.ReadBytes, + WriteBytes: disk.IO.WriteBytes, + ReadOps: disk.IO.ReadOps, + WriteOps: disk.IO.WriteOps, + ReadTime: disk.IO.ReadTime, + WriteTime: disk.IO.WriteTime, + IOTime: disk.IO.IOTime, + } + entry.IO = &ioCopy } if disk.Attributes != nil { entry.Attributes = convertAgentSMARTAttributes(disk.Attributes) diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index b72c72259..44965dd76 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -14,6 +14,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/logging" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring/errors" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rcourtman/pulse-go-rewrite/pkg/fsfilters" "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" "github.com/rs/zerolog" @@ -1003,10 +1004,13 @@ func (m *Monitor) maybePollPhysicalDisksAsync( // knows which ZFS pool (if any) it belongs to. Errors are // non-fatal; we simply leave StorageGroup empty. var poolAssignment *diskPoolAssignment + poolStatus := diskinventory.Unavailable("proxmox_zfs", "ZFS pool monitoring is disabled") if zfsPoolingEnabled { if pools, pErr := pveClient.GetZFSPoolsWithDetails(nodeCtx, node.Node); pErr == nil { poolAssignment = buildDiskPoolAssignment(pools) + poolStatus = diskinventory.Available("proxmox_zfs") } else { + poolStatus = diskinventory.Unavailable("proxmox_zfs", "ZFS pool membership query failed") log.Debug(). Err(pErr). Str("node", node.Node). @@ -1020,21 +1024,32 @@ func (m *Monitor) maybePollPhysicalDisksAsync( for _, disk := range disks { diskID := fmt.Sprintf("%s-%s-%s", inst, node.Node, strings.ReplaceAll(disk.DevPath, "/", "-")) physicalDisk := models.PhysicalDisk{ - ID: diskID, - Node: node.Node, - Instance: inst, - DevPath: disk.DevPath, - Model: disk.Model, - Serial: disk.Serial, - WWN: disk.WWN, - Type: disk.Type, - Size: disk.Size, - Health: disk.Health, - Wearout: disk.Wearout, - RPM: disk.RPM, - Used: disk.Used, + ID: diskID, + Node: node.Node, + Instance: inst, + DevPath: disk.DevPath, + Model: disk.Model, + Serial: disk.Serial, + WWN: disk.WWN, + Type: disk.Type, + Size: disk.Size, + Health: disk.Health, + Wearout: disk.Wearout, + RPM: disk.RPM, + Used: disk.Used, + Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unsupported("proxmox_disks", "Proxmox disk inventory does not expose temperature"), + IO: diskinventory.Unsupported("proxmox_disks", "Proxmox disk inventory does not expose physical-disk I/O counters"), + Controller: diskinventory.Missing("proxmox_disks", "controller association was not reported"), + Pool: poolStatus, + }, LastChecked: time.Now(), } + if strings.TrimSpace(disk.Serial) != "" { + physicalDisk.Collection.Serial = diskinventory.Available("proxmox_disks") + } else { + physicalDisk.Collection.Serial = diskinventory.Missing("proxmox_disks", "disk serial was not reported") + } if poolAssignment != nil { physicalDisk.StorageGroup = poolAssignment.lookup(physicalDisk) } @@ -1058,6 +1073,11 @@ func (m *Monitor) maybePollPhysicalDisksAsync( allDisks = mergeNVMeTempsIntoDisks(allDisks, nodesFromState) allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts) + for index := range allDisks { + if previous, ok := existingDisksMap[allDisks[index].ID]; ok { + allDisks[index] = preserveUnavailablePhysicalDiskEvidence(allDisks[index], previous) + } + } for _, disk := range allDisks { if !polledNodes[disk.Node] { continue @@ -1134,11 +1154,15 @@ func physicalDisksFromHostAgentSMART(inst, nodeName string, smartEntries []model Serial: strings.TrimSpace(smart.Serial), WWN: strings.TrimSpace(smart.WWN), Type: strings.TrimSpace(smart.Type), + Controller: strings.TrimSpace(smart.Controller), + Target: strings.TrimSpace(smart.Target), Size: smart.SizeBytes, Health: health, Wearout: deriveWearoutFromSMARTAttributes(smart.Attributes), Temperature: smart.Temperature, StorageGroup: strings.TrimSpace(smart.Pool), + IO: cloneDiskIO(smart.IO), + Collection: diskinventory.CloneStatus(smart.Collection), SmartAttributes: smartAttributesCopy(smart.Attributes), LastChecked: now, }) @@ -1146,6 +1170,44 @@ func physicalDisksFromHostAgentSMART(inst, nodeName string, smartEntries []model return disks } +func cloneDiskIO(in *models.DiskIO) *models.DiskIO { + if in == nil { + return nil + } + out := *in + return &out +} + +func preserveUnavailablePhysicalDiskEvidence(current, previous models.PhysicalDisk) models.PhysicalDisk { + if current.Collection == nil { + return current + } + if current.Collection.Serial.State != diskinventory.FieldAvailable && + strings.TrimSpace(current.Serial) == "" { + current.Serial = previous.Serial + } + if current.Collection.Temperature.State != diskinventory.FieldAvailable && + current.Temperature <= 0 { + current.Temperature = previous.Temperature + } + if current.Collection.Controller.State != diskinventory.FieldAvailable { + if current.Controller == "" { + current.Controller = previous.Controller + } + if current.Target == "" { + current.Target = previous.Target + } + } + if current.Collection.Pool.State != diskinventory.FieldAvailable && + strings.TrimSpace(current.StorageGroup) == "" { + current.StorageGroup = previous.StorageGroup + } + if current.Collection.IO.State != diskinventory.FieldAvailable && current.IO == nil { + current.IO = cloneDiskIO(previous.IO) + } + return current +} + func proxmoxDiskFromPhysicalDisk(disk models.PhysicalDisk) proxmox.Disk { return proxmox.Disk{ DevPath: disk.DevPath, diff --git a/internal/truenas/contract_test.go b/internal/truenas/contract_test.go index ef0c668ff..710f16255 100644 --- a/internal/truenas/contract_test.go +++ b/internal/truenas/contract_test.go @@ -3,6 +3,7 @@ package truenas import ( "context" "math" + "slices" "strings" "testing" "time" @@ -681,6 +682,46 @@ func newConnectionProvider(t *testing.T, fixtures FixtureSnapshot, connectionID return provider } +func TestConnectionBackedSystemCanonicalIDSurvivesReconnectWithChangedReportedIdentity(t *testing.T) { + previous := IsFeatureEnabled() + SetFeatureEnabled(true) + t.Cleanup(func() { + SetFeatureEnabled(previous) + }) + + const connectionID = "conn-stable" + first := DefaultFixtures() + first.System.Hostname = "truenas-before-reconnect" + first.System.MachineID = "serial-before-reconnect" + second := DefaultFixtures() + second.System.Hostname = "truenas-after-reconnect" + second.System.MachineID = "serial-after-reconnect" + + canonicalIDs := make([]string, 0, 2) + for _, fixtures := range []FixtureSnapshot{first, second} { + registry := unifiedresources.NewRegistry(unifiedresources.NewMemoryStore()) + registry.IngestRecords( + unifiedresources.SourceTrueNAS, + newConnectionProvider(t, fixtures, connectionID).Records(), + ) + systemID := unifiedresources.SourceSpecificID( + unifiedresources.ResourceTypeAgent, + unifiedresources.SourceTrueNAS, + "system:"+connectionID, + ) + system := mustResourceByID(t, registry.List(), systemID) + canonicalIDs = append(canonicalIDs, system.ID) + } + + if canonicalIDs[0] != canonicalIDs[1] { + t.Fatalf( + "connection-backed canonical ID drifted across reconnect/refetch: before=%s after=%s", + canonicalIDs[0], + canonicalIDs[1], + ) + } +} + func TestRegistryIngestRecordsKeepsSameHostnameSystemsDistinct(t *testing.T) { previous := IsFeatureEnabled() SetFeatureEnabled(true) @@ -800,6 +841,22 @@ func TestIngestRecordsSucceedLegacyHostnameScopedCanonicalIDs(t *testing.T) { if system.Agent == nil || system.Agent.AgentID != "conn-a" { t.Fatalf("expected connection-scoped system record, got %+v", system.Agent) } + if system.Canonical == nil || !slices.Contains(system.Canonical.Aliases, legacySystemID) { + t.Fatalf( + "expected connection-backed system %s to expose superseded override identity %s, got %+v", + newSystemID, + legacySystemID, + system.Canonical, + ) + } + if !slices.Contains(system.Canonical.SupersededIDs, legacySystemID) { + t.Fatalf( + "expected connection-backed system %s to publish explicit superseded identity %s, got %+v", + newSystemID, + legacySystemID, + system.Canonical.SupersededIDs, + ) + } for oldID, newID := range map[string]string{legacySystemID: newSystemID, legacyPoolID: newPoolID} { if _, found, err := store.GetResourceOperatorState(oldID); err != nil || found { diff --git a/internal/unifiedresources/adapters.go b/internal/unifiedresources/adapters.go index 6a1929274..1fd98ce4f 100644 --- a/internal/unifiedresources/adapters.go +++ b/internal/unifiedresources/adapters.go @@ -12,6 +12,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/operationreceipt" "github.com/rcourtman/pulse-go-rewrite/internal/platformsupport" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) func resourceFromProxmoxNode(node models.Node, linkedHost *models.Host) (Resource, ResourceIdentity) { @@ -267,11 +268,15 @@ func resourceFromHost(host models.Host) (Resource, ResourceIdentity) { Serial: s.Serial, WWN: s.WWN, Type: s.Type, + Controller: s.Controller, + Target: s.Target, SizeBytes: s.SizeBytes, Temperature: s.Temperature, Health: s.Health, Standby: s.Standby, Pool: s.Pool, + IO: physicalDiskIOToMeta(s.IO), + Collection: diskinventory.CloneStatus(s.Collection), Attributes: cloneSMARTAttributes(s.Attributes), } } @@ -839,6 +844,8 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res Serial: serial, WWN: strings.TrimSpace(disk.WWN), DiskType: diskType, + Controller: strings.TrimSpace(disk.Controller), + Target: strings.TrimSpace(disk.Target), SizeBytes: sizeBytes, Health: health, Wearout: -1, @@ -851,6 +858,8 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res ReadCount: unraidDiskCounter(unraidDisk, "read"), WriteCount: unraidDiskCounter(unraidDisk, "write"), ErrorCount: unraidDiskCounter(unraidDisk, "error"), + IO: physicalDiskIOToMeta(disk.IO), + Collection: diskinventory.CloneStatus(disk.Collection), SMART: convertSMARTAttributes(disk.Attributes), Risk: physicalDiskRiskFromAssessment(assessment), }, @@ -2005,6 +2014,8 @@ func resourceFromPhysicalDisk(disk models.PhysicalDisk) (Resource, ResourceIdent Serial: disk.Serial, WWN: disk.WWN, DiskType: disk.Type, + Controller: disk.Controller, + Target: disk.Target, SizeBytes: disk.Size, Health: disk.Health, Wearout: disk.Wearout, @@ -2012,6 +2023,8 @@ func resourceFromPhysicalDisk(disk models.PhysicalDisk) (Resource, ResourceIdent RPM: disk.RPM, Used: disk.Used, StorageGroup: disk.StorageGroup, + IO: physicalDiskIOToMeta(disk.IO), + Collection: diskinventory.CloneStatus(disk.Collection), Risk: physicalDiskRiskFromAssessment(assessment), } @@ -2047,6 +2060,22 @@ func resourceFromPhysicalDisk(disk models.PhysicalDisk) (Resource, ResourceIdent return resource, identity } +func physicalDiskIOToMeta(in *models.DiskIO) *PhysicalDiskIOMeta { + if in == nil { + return nil + } + return &PhysicalDiskIOMeta{ + Device: strings.TrimSpace(in.Device), + ReadBytes: in.ReadBytes, + WriteBytes: in.WriteBytes, + ReadOps: in.ReadOps, + WriteOps: in.WriteOps, + ReadTimeMs: in.ReadTime, + WriteTimeMs: in.WriteTime, + IOTimeMs: in.IOTime, + } +} + func convertSMARTAttributes(attrs *models.SMARTAttributes) *SMARTMeta { if attrs == nil { return nil diff --git a/internal/unifiedresources/canonical_identity.go b/internal/unifiedresources/canonical_identity.go index 1b8d62671..ada14f278 100644 --- a/internal/unifiedresources/canonical_identity.go +++ b/internal/unifiedresources/canonical_identity.go @@ -19,11 +19,12 @@ func RefreshCanonicalIdentity(resource *Resource) { } resource.Canonical = &CanonicalIdentity{ - DisplayName: displayName, - Hostname: hostname, - PlatformID: platformID, - PrimaryID: primaryID, - Aliases: aliases, + DisplayName: displayName, + Hostname: hostname, + PlatformID: platformID, + PrimaryID: primaryID, + Aliases: aliases, + SupersededIDs: uniqueTrimmed(resource.SupersededCanonicalIDs...), } } @@ -79,7 +80,8 @@ func canonicalProxmoxNodePrimaryID(resource Resource) string { } func canonicalAliases(resource Resource, primaryID, platformID, hostname string) []string { - values := []string{ + values := append([]string{}, resource.SupersededCanonicalIDs...) + values = append(values, primaryID, targetResourceID(resource.MetricsTarget), targetAgentID(resource.DiscoveryTarget), @@ -99,7 +101,7 @@ func canonicalAliases(resource Resource, primaryID, platformID, hostname string) hostname, strings.TrimSpace(resource.Identity.MachineID), strings.TrimSpace(resource.ID), - } + ) return uniqueTrimmed(values...) } diff --git a/internal/unifiedresources/clone.go b/internal/unifiedresources/clone.go index b7332fbe2..58be2da01 100644 --- a/internal/unifiedresources/clone.go +++ b/internal/unifiedresources/clone.go @@ -4,6 +4,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) func cloneResourcePtr(in *Resource) *Resource { @@ -23,6 +24,7 @@ func cloneResource(in *Resource) Resource { out.DiscoveryTarget = cloneDiscoveryTarget(in.DiscoveryTarget) out.DiscoveryReadiness = cloneResourceDiscoveryReadiness(in.DiscoveryReadiness) out.MetricsTarget = cloneMetricsTarget(in.MetricsTarget) + out.SupersededCanonicalIDs = cloneStringSlice(in.SupersededCanonicalIDs) out.PlatformScopes = cloneStringSlice(in.PlatformScopes) out.Sources = cloneDataSourceSlice(in.Sources) out.SourceStatus = cloneSourceStatusMap(in.SourceStatus) @@ -554,11 +556,21 @@ func clonePhysicalDiskMeta(in *PhysicalDiskMeta) *PhysicalDiskMeta { } out := *in out.TemperatureAggregate = cloneTemperatureAggregateMeta(in.TemperatureAggregate) + out.IO = clonePhysicalDiskIOMeta(in.IO) + out.Collection = diskinventory.CloneStatus(in.Collection) out.SMART = cloneSMARTMeta(in.SMART) out.Risk = clonePhysicalDiskRisk(in.Risk) return &out } +func clonePhysicalDiskIOMeta(in *PhysicalDiskIOMeta) *PhysicalDiskIOMeta { + if in == nil { + return nil + } + out := *in + return &out +} + func cloneTemperatureAggregateMeta(in *TemperatureAggregateMeta) *TemperatureAggregateMeta { if in == nil { return nil diff --git a/internal/unifiedresources/code_standards_test.go b/internal/unifiedresources/code_standards_test.go index ac09abf55..f000de24e 100644 --- a/internal/unifiedresources/code_standards_test.go +++ b/internal/unifiedresources/code_standards_test.go @@ -1696,8 +1696,9 @@ func TestCanonicalIdentityIsCanonicalResourceField(t *testing.T) { requiredSnippets := []string{ "json:\"canonicalIdentity,omitempty\"", "type CanonicalIdentity struct {", - "DisplayName string `json:\"displayName,omitempty\"`", - "Aliases []string `json:\"aliases,omitempty\"`", + "DisplayName string `json:\"displayName,omitempty\"`", + "Aliases []string `json:\"aliases,omitempty\"`", + "SupersededIDs []string `json:\"supersededIds,omitempty\"`", } for _, snippet := range requiredSnippets { if !strings.Contains(typesSource, snippet) { diff --git a/internal/unifiedresources/physical_disk_ids.go b/internal/unifiedresources/physical_disk_ids.go index 1d38eefe5..1c85ba2a3 100644 --- a/internal/unifiedresources/physical_disk_ids.go +++ b/internal/unifiedresources/physical_disk_ids.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) // PreferredPhysicalDiskMetricID returns the canonical history key used for @@ -22,11 +23,14 @@ func PreferredPhysicalDiskMetricID(serial, wwn, fallback string) string { func HostSMARTDiskSourceID(host models.Host, disk models.HostDiskSMART) string { device := normalizePhysicalDiskDeviceToken(disk.Device) - fallback := "" - if device != "" { - fallback = fmt.Sprintf("%s:%s", strings.TrimSpace(host.ID), device) - } - return PreferredPhysicalDiskMetricID(disk.Serial, disk.WWN, fallback) + return diskinventory.PreferredID( + disk.Serial, + disk.WWN, + strings.TrimSpace(host.ID), + device, + disk.Controller, + disk.Target, + ) } func HostUnraidDiskSourceID(host models.Host, disk models.HostUnraidDisk) string { @@ -41,6 +45,9 @@ func HostUnraidDiskSourceID(host models.Host, disk models.HostUnraidDisk) string } func PhysicalDiskMetricID(disk models.PhysicalDisk) string { + if strings.TrimSpace(disk.Serial) != "" || strings.TrimSpace(disk.WWN) != "" { + return PreferredPhysicalDiskMetricID(disk.Serial, disk.WWN, "") + } fallback := strings.TrimSpace(disk.ID) if fallback == "" && strings.TrimSpace(disk.DevPath) != "" { fallback = fmt.Sprintf( @@ -50,20 +57,42 @@ func PhysicalDiskMetricID(disk models.PhysicalDisk) string { strings.ReplaceAll(strings.TrimSpace(disk.DevPath), "/", "-"), ) } - return PreferredPhysicalDiskMetricID(disk.Serial, disk.WWN, fallback) + if diskinventory.IsControllerMemberTarget(disk.Target) { + return diskinventory.PreferredID( + "", + "", + fallback, + disk.DevPath, + disk.Controller, + disk.Target, + ) + } + return strings.TrimSpace(fallback) } func PhysicalDiskMetaMetricID(disk *PhysicalDiskMeta, fallback string) string { if disk == nil { return strings.TrimSpace(fallback) } + if serial := strings.TrimSpace(disk.Serial); serial != "" { + return serial + } + if wwn := strings.TrimSpace(disk.WWN); wwn != "" { + return wwn + } + if diskinventory.IsControllerMemberTarget(disk.Target) && strings.TrimSpace(fallback) != "" { + return diskinventory.PreferredID( + "", + "", + strings.TrimSpace(fallback), + disk.DevPath, + disk.Controller, + disk.Target, + ) + } return PreferredPhysicalDiskMetricID(disk.Serial, disk.WWN, fallback) } func normalizePhysicalDiskDeviceToken(device string) string { - device = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(device), "/dev/")) - if fields := strings.Fields(device); len(fields) > 0 { - device = fields[0] - } - return strings.TrimSpace(device) + return diskinventory.DeviceToken(device) } diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 34ae5d611..9e88edc95 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -13,6 +13,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/operationaltrust" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rcourtman/pulse-go-rewrite/pkg/fsfilters" ) @@ -525,6 +526,7 @@ func (rr *ResourceRegistry) IngestRecords(source DataSource, records []IngestRec } } newID := rr.ingest(source, sourceID, resource, record.Identity) + rr.retainSupersededCanonicalIDs(newID, record.SupersededCanonicalIDs) for _, superseded := range record.SupersededCanonicalIDs { superseded = CanonicalResourceID(superseded) if superseded == "" || newID == "" || superseded == newID { @@ -552,6 +554,35 @@ func (rr *ResourceRegistry) IngestRecords(source DataSource, records []IngestRec rr.mu.Unlock() } +// retainSupersededCanonicalIDs keeps record-declared identity eras on the +// live resource. Canonical metadata exposes them as aliases, and alert +// configuration migration consumes the explicit field so display aliases +// such as hostnames are never mistaken for persistence keys. +func (rr *ResourceRegistry) retainSupersededCanonicalIDs(resourceID string, supersededIDs []string) { + resourceID = CanonicalResourceID(resourceID) + if resourceID == "" || len(supersededIDs) == 0 { + return + } + + rr.mu.Lock() + defer rr.mu.Unlock() + + resource := rr.resources[resourceID] + if resource == nil { + return + } + + ids := append([]string(nil), resource.SupersededCanonicalIDs...) + for _, supersededID := range supersededIDs { + supersededID = CanonicalResourceID(supersededID) + if supersededID == "" || supersededID == resourceID { + continue + } + ids = append(ids, supersededID) + } + resource.SupersededCanonicalIDs = uniqueTrimmed(ids...) +} + // applyRecordSuccessions re-keys operator-owned store rows from canonical IDs // that ingested records declared superseded (IngestRecord.SupersededCanonicalIDs) // onto the records' current canonical IDs. Mirrors the guards of pin-driven @@ -2524,6 +2555,13 @@ func (rr *ResourceRegistry) findMatch(identity ResourceIdentity, resourceType Re } func (rr *ResourceRegistry) resolveLinkedResource(source DataSource, sourceID string, resource Resource) string { + if resource.Type == ResourceTypePhysicalDisk && + (source == SourceAgent || source == SourceProxmox) { + if linked := rr.resolveLinkedPhysicalDisk(source, resource); linked != "" { + return linked + } + } + switch source { case SourceProxmox: if resource.Proxmox != nil && resource.Proxmox.LinkedAgentID != "" { @@ -2562,6 +2600,76 @@ func (rr *ResourceRegistry) resolveLinkedResource(source DataSource, sourceID st return "" } +// resolveLinkedPhysicalDisk correlates agent and Proxmox observations inside +// one already-linked host boundary. This is the durable join for cases where +// Proxmox reports a SAS address in its serial field while smartctl reports the +// drive's real serial. Device paths are safe only inside the common parent and +// only when the topology match is unique. +func (rr *ResourceRegistry) resolveLinkedPhysicalDisk(source DataSource, incoming Resource) string { + if incoming.PhysicalDisk == nil || incoming.ParentID == nil { + return "" + } + parentID := strings.TrimSpace(*incoming.ParentID) + if parentID == "" { + return "" + } + + incomingDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(incoming.PhysicalDisk.DevPath)) + incomingSerial := strings.TrimSpace(incoming.PhysicalDisk.Serial) + incomingWWN := strings.TrimSpace(incoming.PhysicalDisk.WWN) + matchID := "" + for resourceID, existing := range rr.resources { + if existing == nil || + existing.Type != ResourceTypePhysicalDisk || + existing.PhysicalDisk == nil || + existing.ParentID == nil || + strings.TrimSpace(*existing.ParentID) != parentID || + hasDataSource(existing.Sources, source) { + continue + } + if source == SourceProxmox && !hasDataSource(existing.Sources, SourceAgent) { + continue + } + if source == SourceAgent && !hasDataSource(existing.Sources, SourceProxmox) { + continue + } + + identityMatch := + (incomingSerial != "" && strings.EqualFold(incomingSerial, existing.PhysicalDisk.Serial)) || + (incomingWWN != "" && strings.EqualFold(incomingWWN, existing.PhysicalDisk.WWN)) + existingDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(existing.PhysicalDisk.DevPath)) + agentReportsSAS := (source == SourceProxmox && strings.EqualFold(existing.PhysicalDisk.DiskType, "sas")) || + (source == SourceAgent && strings.EqualFold(incoming.PhysicalDisk.DiskType, "sas")) + deviceMatch := agentReportsSAS && + incomingDevice != "" && + incomingDevice == existingDevice && + physicalDiskTopologyCompatible(incoming.PhysicalDisk, existing.PhysicalDisk) + if !identityMatch && !deviceMatch { + continue + } + if matchID != "" && matchID != resourceID { + return "" + } + matchID = resourceID + } + return matchID +} + +func physicalDiskTopologyCompatible(left, right *PhysicalDiskMeta) bool { + if left == nil || right == nil { + return false + } + if left.Controller != "" && right.Controller != "" && + !strings.EqualFold(strings.TrimSpace(left.Controller), strings.TrimSpace(right.Controller)) { + return false + } + if left.Target != "" && right.Target != "" && + !strings.EqualFold(strings.TrimSpace(left.Target), strings.TrimSpace(right.Target)) { + return false + } + return true +} + type availabilityLinkResolution struct { ResourceID string State AvailabilityCorrelationState @@ -2939,6 +3047,24 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou previous := existing.PhysicalDisk existing.PhysicalDisk = mergePhysicalDiskData(existing.PhysicalDisk, incoming.PhysicalDisk) if source == SourceProxmox && previous != nil && hasDataSource(existing.Sources, SourceAgent) { + if previous.Serial != "" && + (previous.Collection == nil || + (previous.Collection.Serial.State == diskinventory.FieldAvailable && + strings.HasPrefix(strings.ToLower(previous.Collection.Serial.Source), "smartctl"))) { + existing.PhysicalDisk.Serial = previous.Serial + } + if previous.WWN != "" { + existing.PhysicalDisk.WWN = previous.WWN + } + if previous.DiskType != "" { + existing.PhysicalDisk.DiskType = previous.DiskType + } + if previous.Controller != "" { + existing.PhysicalDisk.Controller = previous.Controller + } + if previous.Target != "" { + existing.PhysicalDisk.Target = previous.Target + } if previous.Temperature > 0 { existing.PhysicalDisk.Temperature = previous.Temperature } @@ -2949,6 +3075,13 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou smart := *previous.SMART existing.PhysicalDisk.SMART = &smart } + if previous.IO != nil { + existing.PhysicalDisk.IO = clonePhysicalDiskIOMeta(previous.IO) + } + existing.PhysicalDisk.Collection = diskinventory.MergeStatus( + incoming.PhysicalDisk.Collection, + previous.Collection, + ) } } if existing.PhysicalDisk != nil && (mergedPhysicalDisk || len(incoming.Incidents) > 0) { @@ -3181,6 +3314,12 @@ func mergePhysicalDiskData(existing *PhysicalDiskMeta, incoming *PhysicalDiskMet if incoming.DiskType != "" { merged.DiskType = incoming.DiskType } + if incoming.Controller != "" { + merged.Controller = incoming.Controller + } + if incoming.Target != "" { + merged.Target = incoming.Target + } if incoming.SizeBytes > 0 { merged.SizeBytes = incoming.SizeBytes } @@ -3223,6 +3362,10 @@ func mergePhysicalDiskData(existing *PhysicalDiskMeta, incoming *PhysicalDiskMet if incoming.ErrorCount > 0 { merged.ErrorCount = incoming.ErrorCount } + if incoming.IO != nil { + merged.IO = clonePhysicalDiskIOMeta(incoming.IO) + } + merged.Collection = diskinventory.MergeStatus(merged.Collection, incoming.Collection) if incoming.SMART != nil { smart := *incoming.SMART merged.SMART = &smart diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index 21ce4aa8c..6a579fc3b 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -7,6 +7,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/operationaltrust" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) // Resource represents a unified resource aggregated across multiple data sources. @@ -23,9 +24,15 @@ type Resource struct { DiscoveryReadiness *ResourceDiscoveryReadiness `json:"discoveryReadiness,omitempty"` MetricsTarget *MetricsTarget `json:"metricsTarget,omitempty"` Canonical *CanonicalIdentity `json:"canonicalIdentity,omitempty"` - Policy *ResourcePolicy `json:"policy,omitempty"` - AISafeSummary string `json:"aiSafeSummary,omitempty"` - PlatformScopes []string `json:"platformScopes,omitempty"` + // SupersededCanonicalIDs carries retired canonical IDs that are known to + // identify this same resource. Providers declare them on IngestRecord when + // an identity derivation changes; the registry retains them so alert + // overrides and other operator-authored configuration can migrate onto the + // current canonical ID instead of becoming orphaned. + SupersededCanonicalIDs []string `json:"-"` + Policy *ResourcePolicy `json:"policy,omitempty"` + AISafeSummary string `json:"aiSafeSummary,omitempty"` + PlatformScopes []string `json:"platformScopes,omitempty"` Sources []DataSource `json:"sources"` SourceStatus map[DataSource]SourceStatus `json:"sourceStatus,omitempty"` @@ -151,11 +158,12 @@ type MetricsTarget struct { // unified resource so frontend surfaces do not need to reconstruct labels and // host hints from source-specific facets. type CanonicalIdentity struct { - DisplayName string `json:"displayName,omitempty"` - Hostname string `json:"hostname,omitempty"` - PlatformID string `json:"platformId,omitempty"` - PrimaryID string `json:"primaryId,omitempty"` - Aliases []string `json:"aliases,omitempty"` + DisplayName string `json:"displayName,omitempty"` + Hostname string `json:"hostname,omitempty"` + PlatformID string `json:"platformId,omitempty"` + PrimaryID string `json:"primaryId,omitempty"` + Aliases []string `json:"aliases,omitempty"` + SupersededIDs []string `json:"supersededIds,omitempty"` } // ResourceType represents the kind of resource. @@ -438,27 +446,44 @@ type ResourceIncident struct { // PhysicalDiskMeta contains physical disk-specific metadata. type PhysicalDiskMeta struct { - DevPath string `json:"devPath"` - Model string `json:"model,omitempty"` - Serial string `json:"serial,omitempty"` - WWN string `json:"wwn,omitempty"` - DiskType string `json:"diskType"` // nvme, sata, sas - SizeBytes int64 `json:"sizeBytes"` - Health string `json:"health"` // PASSED, FAILED, UNKNOWN - Wearout int `json:"wearout"` // 0-100, -1 unavailable - Temperature int `json:"temperature"` // Celsius - TemperatureAggregate *TemperatureAggregateMeta `json:"temperatureAggregate,omitempty"` - RPM int `json:"rpm"` - Used string `json:"used,omitempty"` - StorageRole string `json:"storageRole,omitempty"` - StorageGroup string `json:"storageGroup,omitempty"` - StorageState string `json:"storageState,omitempty"` - SpunDown bool `json:"spunDown,omitempty"` - ReadCount int64 `json:"readCount,omitempty"` - WriteCount int64 `json:"writeCount,omitempty"` - ErrorCount int64 `json:"errorCount,omitempty"` - SMART *SMARTMeta `json:"smart,omitempty"` - Risk *PhysicalDiskRisk `json:"risk,omitempty"` + DevPath string `json:"devPath"` + Model string `json:"model,omitempty"` + Serial string `json:"serial,omitempty"` + WWN string `json:"wwn,omitempty"` + DiskType string `json:"diskType"` // nvme, sata, sas + Controller string `json:"controller,omitempty"` + Target string `json:"target,omitempty"` + SizeBytes int64 `json:"sizeBytes"` + Health string `json:"health"` // PASSED, FAILED, UNKNOWN + Wearout int `json:"wearout"` // 0-100, -1 unavailable + Temperature int `json:"temperature"` // Celsius + TemperatureAggregate *TemperatureAggregateMeta `json:"temperatureAggregate,omitempty"` + RPM int `json:"rpm"` + Used string `json:"used,omitempty"` + StorageRole string `json:"storageRole,omitempty"` + StorageGroup string `json:"storageGroup,omitempty"` + StorageState string `json:"storageState,omitempty"` + SpunDown bool `json:"spunDown,omitempty"` + ReadCount int64 `json:"readCount,omitempty"` + WriteCount int64 `json:"writeCount,omitempty"` + ErrorCount int64 `json:"errorCount,omitempty"` + IO *PhysicalDiskIOMeta `json:"io,omitempty"` + Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` + SMART *SMARTMeta `json:"smart,omitempty"` + Risk *PhysicalDiskRisk `json:"risk,omitempty"` +} + +// PhysicalDiskIOMeta preserves the cumulative kernel counters attributed to a +// physical disk. Rates are derived separately by the monitoring layer. +type PhysicalDiskIOMeta struct { + Device string `json:"device,omitempty"` + ReadBytes uint64 `json:"readBytes,omitempty"` + WriteBytes uint64 `json:"writeBytes,omitempty"` + ReadOps uint64 `json:"readOps,omitempty"` + WriteOps uint64 `json:"writeOps,omitempty"` + ReadTimeMs uint64 `json:"readTimeMs,omitempty"` + WriteTimeMs uint64 `json:"writeTimeMs,omitempty"` + IOTimeMs uint64 `json:"ioTimeMs,omitempty"` } // TemperatureAggregateMeta stores recent aggregate temperature history for a @@ -568,17 +593,21 @@ type HostThermalState struct { // HostSMARTMeta describes a disk's SMART data. type HostSMARTMeta struct { - Device string `json:"device"` - Model string `json:"model,omitempty"` - Serial string `json:"serial,omitempty"` - WWN string `json:"wwn,omitempty"` - Type string `json:"type,omitempty"` - SizeBytes int64 `json:"sizeBytes,omitempty"` - Temperature int `json:"temperature"` - Health string `json:"health"` - Standby bool `json:"standby,omitempty"` - Pool string `json:"pool,omitempty"` - Attributes *models.SMARTAttributes `json:"attributes,omitempty"` + Device string `json:"device"` + Model string `json:"model,omitempty"` + Serial string `json:"serial,omitempty"` + WWN string `json:"wwn,omitempty"` + Type string `json:"type,omitempty"` + Controller string `json:"controller,omitempty"` + Target string `json:"target,omitempty"` + SizeBytes int64 `json:"sizeBytes,omitempty"` + Temperature int `json:"temperature"` + Health string `json:"health"` + Standby bool `json:"standby,omitempty"` + Pool string `json:"pool,omitempty"` + IO *PhysicalDiskIOMeta `json:"io,omitempty"` + Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` + Attributes *models.SMARTAttributes `json:"attributes,omitempty"` } // HostRAIDDeviceMeta describes a device in a RAID array. diff --git a/internal/unifiedresources/views.go b/internal/unifiedresources/views.go index 251423cc3..e4ee316ec 100644 --- a/internal/unifiedresources/views.go +++ b/internal/unifiedresources/views.go @@ -7,6 +7,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" ) // Metric helpers (nil-safe). @@ -2049,6 +2050,20 @@ func (v PhysicalDiskView) DiskType() string { return v.r.PhysicalDisk.DiskType } +func (v PhysicalDiskView) Controller() string { + if v.r == nil || v.r.PhysicalDisk == nil { + return "" + } + return strings.TrimSpace(v.r.PhysicalDisk.Controller) +} + +func (v PhysicalDiskView) Target() string { + if v.r == nil || v.r.PhysicalDisk == nil { + return "" + } + return strings.TrimSpace(v.r.PhysicalDisk.Target) +} + func (v PhysicalDiskView) SizeBytes() int64 { if v.r == nil || v.r.PhysicalDisk == nil { return 0 @@ -2113,6 +2128,13 @@ func (v PhysicalDiskView) Used() string { return v.r.PhysicalDisk.Used } +func (v PhysicalDiskView) StorageGroup() string { + if v.r == nil || v.r.PhysicalDisk == nil { + return "" + } + return strings.TrimSpace(v.r.PhysicalDisk.StorageGroup) +} + func (v PhysicalDiskView) SMART() *SMARTMeta { if v.r == nil || v.r.PhysicalDisk == nil || v.r.PhysicalDisk.SMART == nil { return nil @@ -2120,6 +2142,20 @@ func (v PhysicalDiskView) SMART() *SMARTMeta { return cloneSMARTMeta(v.r.PhysicalDisk.SMART) } +func (v PhysicalDiskView) IO() *PhysicalDiskIOMeta { + if v.r == nil || v.r.PhysicalDisk == nil { + return nil + } + return clonePhysicalDiskIOMeta(v.r.PhysicalDisk.IO) +} + +func (v PhysicalDiskView) Collection() *diskinventory.CollectionStatus { + if v.r == nil || v.r.PhysicalDisk == nil { + return nil + } + return diskinventory.CloneStatus(v.r.PhysicalDisk.Collection) +} + func (v PhysicalDiskView) MetricResourceID() string { if v.r == nil { return "" diff --git a/pkg/agents/host/report.go b/pkg/agents/host/report.go index 5469000f9..2cc81f50a 100644 --- a/pkg/agents/host/report.go +++ b/pkg/agents/host/report.go @@ -1,6 +1,10 @@ package host -import "time" +import ( + "time" + + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" +) // Report represents the payload sent by the host module of pulse-agent. type Report struct { @@ -227,17 +231,21 @@ const ( // DiskSMART represents S.M.A.R.T. data for a single disk. type DiskSMART struct { - Device string `json:"device"` // Block device name (e.g., sda, nvme0n1) - Model string `json:"model,omitempty"` // Disk model - Serial string `json:"serial,omitempty"` // Serial number - WWN string `json:"wwn,omitempty"` // World Wide Name - Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme - SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown) - Temperature int `json:"temperature"` // Temperature in Celsius - Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN - Standby bool `json:"standby,omitempty"` // True if disk was in standby - Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member) - Attributes *SMARTAttributes `json:"attributes,omitempty"` + Device string `json:"device"` // Block device name (e.g., sda, nvme0n1) + Model string `json:"model,omitempty"` // Disk model + Serial string `json:"serial,omitempty"` // Serial number + WWN string `json:"wwn,omitempty"` // World Wide Name + Type string `json:"type,omitempty"` // Transport type: sata, sas, nvme + Controller string `json:"controller,omitempty"` // Stable controller association when the OS reports one + Target string `json:"target,omitempty"` // Controller target/HCTL or smartctl member target + SizeBytes int64 `json:"sizeBytes,omitempty"` // Capacity in bytes (0 when unknown) + Temperature int `json:"temperature"` // Temperature in Celsius + Health string `json:"health,omitempty"` // PASSED, FAILED, UNKNOWN + Standby bool `json:"standby,omitempty"` // True if disk was in standby + Pool string `json:"pool,omitempty"` // ZFS pool this disk belongs to (empty if not a ZFS member) + IO *DiskIO `json:"io,omitempty"` // Cumulative kernel counters when attributable to this disk + Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` + Attributes *SMARTAttributes `json:"attributes,omitempty"` } // SMARTAttributes holds normalized SMART attributes for both SATA and NVMe disks. diff --git a/pkg/diskinventory/identity.go b/pkg/diskinventory/identity.go new file mode 100644 index 000000000..b2c8dc618 --- /dev/null +++ b/pkg/diskinventory/identity.go @@ -0,0 +1,56 @@ +package diskinventory + +import ( + "fmt" + "strings" +) + +// DeviceToken returns the kernel block-device token from either a canonical +// /dev path or a legacy smartctl display label such as "sda [scsi]". +func DeviceToken(device string) string { + device = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(device), "/dev/")) + if fields := strings.Fields(device); len(fields) > 0 { + device = fields[0] + } + return strings.TrimSpace(device) +} + +// PreferredID selects stable hardware identity first and scopes topology +// fallbacks to the reporting host. Controller and target are included only +// when present, preserving legacy direct-device IDs for SATA/NVMe disks. +func PreferredID(serial, wwn, scope, device, controller, target string) string { + if serial = strings.TrimSpace(serial); serial != "" { + return serial + } + if wwn = strings.TrimSpace(wwn); wwn != "" { + return wwn + } + + scope = strings.TrimSpace(scope) + device = DeviceToken(device) + controller = strings.TrimSpace(controller) + target = strings.TrimSpace(target) + if scope == "" || device == "" { + return "" + } + if !IsControllerMemberTarget(target) { + return fmt.Sprintf("%s:%s", scope, device) + } + return fmt.Sprintf("%s:%s@%s/%s", scope, device, controller, target) +} + +// IsControllerMemberTarget reports whether target addresses one member behind +// a shared controller block path (for example megaraid,7 or cciss,1). +func IsControllerMemberTarget(target string) bool { + target = strings.TrimSpace(target) + index := strings.IndexByte(target, ',') + if index < 0 || index+1 >= len(target) { + return false + } + for _, char := range target[index+1:] { + if char < '0' || char > '9' { + return false + } + } + return true +} diff --git a/pkg/diskinventory/identity_test.go b/pkg/diskinventory/identity_test.go new file mode 100644 index 000000000..60c010321 --- /dev/null +++ b/pkg/diskinventory/identity_test.go @@ -0,0 +1,34 @@ +package diskinventory + +import "testing" + +func TestPreferredIDPreservesDirectDeviceFallbacks(t *testing.T) { + for _, test := range []struct { + name string + device string + controller string + target string + want string + }{ + {name: "sata", device: "/dev/sda", want: "host:sda"}, + {name: "nvme", device: "nvme0n1", want: "host:nvme0n1"}, + {name: "direct sas hctl", device: "sdb", controller: "0000:03:00.0", target: "6:0:0:0", want: "host:sdb"}, + {name: "controller member", device: "sdc [megaraid,7]", controller: "sdc", target: "megaraid,7", want: "host:sdc@sdc/megaraid,7"}, + } { + t.Run(test.name, func(t *testing.T) { + got := PreferredID("", "", "host", test.device, test.controller, test.target) + if got != test.want { + t.Fatalf("PreferredID() = %q, want %q", got, test.want) + } + }) + } +} + +func TestPreferredIDKeepsExistingHardwareIdentityPriority(t *testing.T) { + if got := PreferredID(" SERIAL ", "WWN", "host", "sda", "controller", "megaraid,7"); got != "SERIAL" { + t.Fatalf("serial identity = %q, want SERIAL", got) + } + if got := PreferredID("", " WWN ", "host", "sda", "controller", "megaraid,7"); got != "WWN" { + t.Fatalf("WWN identity = %q, want WWN", got) + } +} diff --git a/pkg/diskinventory/status.go b/pkg/diskinventory/status.go new file mode 100644 index 000000000..68e98b483 --- /dev/null +++ b/pkg/diskinventory/status.go @@ -0,0 +1,100 @@ +package diskinventory + +import "strings" + +// FieldState describes whether a physical-disk field was observed during the +// current collection pass. It deliberately separates provider limitations +// from transient collection failures and from fields that should have been +// present but were not. +type FieldState string + +const ( + FieldAvailable FieldState = "available" + FieldUnavailable FieldState = "unavailable" + FieldUnsupported FieldState = "unsupported" + FieldMissing FieldState = "missing" +) + +// FieldStatus carries collection state and provenance for one disk signal. +type FieldStatus struct { + State FieldState `json:"state"` + Source string `json:"source,omitempty"` + Reason string `json:"reason,omitempty"` +} + +// CollectionStatus is the field-level payload carried by collection evidence; +// it is not a separate trust posture or mutation lifecycle. Operational trust +// may wrap this payload in its shared EvidenceEnvelope, while Collection Trust +// remains responsible for evaluating the observation. Empty means the report +// predates this contract. +type CollectionStatus struct { + Serial FieldStatus `json:"serial,omitempty"` + Temperature FieldStatus `json:"temperature,omitempty"` + IO FieldStatus `json:"io,omitempty"` + Controller FieldStatus `json:"controller,omitempty"` + Pool FieldStatus `json:"pool,omitempty"` +} + +func Available(source string) FieldStatus { + return FieldStatus{State: FieldAvailable, Source: strings.TrimSpace(source)} +} + +func Unavailable(source, reason string) FieldStatus { + return FieldStatus{ + State: FieldUnavailable, + Source: strings.TrimSpace(source), + Reason: strings.TrimSpace(reason), + } +} + +func Unsupported(source, reason string) FieldStatus { + return FieldStatus{ + State: FieldUnsupported, + Source: strings.TrimSpace(source), + Reason: strings.TrimSpace(reason), + } +} + +func Missing(source, reason string) FieldStatus { + return FieldStatus{ + State: FieldMissing, + Source: strings.TrimSpace(source), + Reason: strings.TrimSpace(reason), + } +} + +func CloneStatus(status *CollectionStatus) *CollectionStatus { + if status == nil { + return nil + } + clone := *status + return &clone +} + +// MergeStatus keeps an available observation over a weaker state while still +// allowing a current available observation to replace older provenance. +func MergeStatus(existing, incoming *CollectionStatus) *CollectionStatus { + if existing == nil { + return CloneStatus(incoming) + } + if incoming == nil { + return CloneStatus(existing) + } + merged := *existing + merged.Serial = mergeFieldStatus(merged.Serial, incoming.Serial) + merged.Temperature = mergeFieldStatus(merged.Temperature, incoming.Temperature) + merged.IO = mergeFieldStatus(merged.IO, incoming.IO) + merged.Controller = mergeFieldStatus(merged.Controller, incoming.Controller) + merged.Pool = mergeFieldStatus(merged.Pool, incoming.Pool) + return &merged +} + +func mergeFieldStatus(existing, incoming FieldStatus) FieldStatus { + if incoming.State == "" { + return existing + } + if existing.State == FieldAvailable && incoming.State != FieldAvailable { + return existing + } + return incoming +} diff --git a/pkg/diskinventory/status_test.go b/pkg/diskinventory/status_test.go new file mode 100644 index 000000000..4c0bba288 --- /dev/null +++ b/pkg/diskinventory/status_test.go @@ -0,0 +1,26 @@ +package diskinventory + +import "testing" + +func TestMergeStatusPrefersAvailableEvidencePerField(t *testing.T) { + existing := &CollectionStatus{ + Serial: Available("smartctl"), + Temperature: Unavailable("smartctl", "deadline exceeded"), + } + incoming := &CollectionStatus{ + Serial: Unsupported("provider", "not exposed"), + Temperature: Available("provider"), + IO: Missing("kernel", "counter absent"), + } + + got := MergeStatus(existing, incoming) + if got.Serial.State != FieldAvailable || got.Serial.Source != "smartctl" { + t.Fatalf("available serial evidence was downgraded: %+v", got.Serial) + } + if got.Temperature.State != FieldAvailable || got.Temperature.Source != "provider" { + t.Fatalf("available temperature evidence did not win: %+v", got.Temperature) + } + if got.IO.State != FieldMissing { + t.Fatalf("missing I/O state was not retained: %+v", got.IO) + } +} diff --git a/scripts/install.sh b/scripts/install.sh index 9ca019d62..319bd6c20 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -94,11 +94,14 @@ KUBECONFIG_PATH="" # Path to kubeconfig file for Kubernetes monitoring KUBE_INCLUDE_ALL_PODS="false" KUBE_INCLUDE_ALL_DEPLOYMENTS="false" DISK_EXCLUDES=() # Array for multiple --disk-exclude values -STATE_DIR="/var/lib/pulse-agent" # Persistent state directory (overridden per platform) +DEFAULT_STATE_DIR="/var/lib/pulse-agent" +STATE_DIR="$DEFAULT_STATE_DIR" # Persistent state directory (overridden per platform) +STATE_DIR_SOURCE="default" # default, explicit, recovered, or platform CURL_CA_BUNDLE="" # Path to CA bundle for curl and agent TLS (sets SSL_CERT_FILE) NON_INTERACTIVE="false" TOKEN_FILE_PATH="" # Path to file containing the token RUNTIME_TOKEN_FILE="" # Secure token file passed to the installed service +RUNTIME_TOKEN_CHANGED="false" OUTPUT_FORMAT="text" # "text" (default) or "json" PREFLIGHT_ONLY="false" INSTALL_SIGNATURE_NAMESPACE="pulse-install" @@ -147,6 +150,33 @@ log_error() { printf "[ERROR] %s\n" "$1" fi } + +# Feed the API token to curl through a private config file. Passing a header +# value with -H would expose the token in the transient curl process argv. +curl_with_pulse_token() { + local config_file="" + local curl_rc=0 + + if [[ -z "$PULSE_TOKEN" ]]; then + curl "$@" + return $? + fi + case "$PULSE_TOKEN" in + *$'\r'*|*$'\n'*) return 2 ;; + esac + + config_file=$(mktemp) + TMP_FILES+=("$config_file") + chmod 600 "$config_file" + printf 'header = "X-API-Token: %s"\n' "$PULSE_TOKEN" > "$config_file" + if curl --config "$config_file" "$@"; then + curl_rc=0 + else + curl_rc=$? + fi + rm -f "$config_file" + return "$curl_rc" +} url_encode() { local input="$1" local output="" @@ -385,11 +415,10 @@ verify_agent_server_registration() { return 1 fi - if [[ -n "$PULSE_TOKEN" ]]; then lookup_args+=(-H "X-API-Token: ${PULSE_TOKEN}"); fi if [[ "$INSECURE" == "true" ]]; then lookup_args+=(-k); fi if [[ -n "$CURL_CA_BUNDLE" ]]; then lookup_args+=(--cacert "$CURL_CA_BUNDLE"); fi - lookup_out=$(curl "${lookup_args[@]}" "${PULSE_URL}/api/agents/agent/lookup?hostname=$(url_encode "$lookup_hostname")" 2>/dev/null || true) + lookup_out=$(curl_with_pulse_token "${lookup_args[@]}" "${PULSE_URL}/api/agents/agent/lookup?hostname=$(url_encode "$lookup_hostname")" 2>/dev/null || true) lookup_status="${lookup_out##*$'\n'}" lookup_body="${lookup_out%$'\n'*}" @@ -1055,6 +1084,42 @@ portable_sed_in_place() { sed -i '' "$expr" "$target" 2>/dev/null || sed -i "$expr" "$target" 2>/dev/null || true } +select_platform_state_dir() { + local platform_default="$1" + + if [[ "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + STATE_DIR="$platform_default" + STATE_DIR_SOURCE="platform" + fi +} + +discover_state_dir_from_saved_installer() { + local script_path="${1:-$0}" + local script_dir="" + + if [[ "${STATE_DIR_SOURCE:-default}" != "default" || ! -f "$script_path" ]]; then + return 1 + fi + script_dir=$(cd "$(dirname "$script_path")" 2>/dev/null && pwd -P) || return 1 + if [[ -f "$script_dir/connection.env" ]]; then + STATE_DIR="$script_dir" + STATE_DIR_SOURCE="recovered" + return 0 + fi + return 1 +} + +remove_agent_state_dir() { + local state_dir="${1:-$STATE_DIR}" + + if [[ -z "$state_dir" || "$state_dir" != /* || "$state_dir" == "/" || + "$state_dir" == *$'\r'* || "$state_dir" == *$'\n'* ]]; then + log_warn "Refusing to remove invalid agent state directory: ${state_dir:-}" + return 1 + fi + rm -rf -- "$state_dir" +} + detect_qnap_data_volume() { local qnap_vol="" local candidate="" @@ -1081,11 +1146,14 @@ detect_qnap_data_volume() { find_qnap_state_dir() { local candidate="" - if [[ -n "$STATE_DIR" ]] && [[ "$STATE_DIR" != "/var/lib/pulse-agent" ]]; then - if [[ -d "$STATE_DIR" ]] || [[ -f "$STATE_DIR/connection.env" ]] || [[ -f "$STATE_DIR/agent-id" ]]; then - printf '%s\n' "$STATE_DIR" - return 0 - fi + if [[ -n "$STATE_DIR" && "${STATE_DIR_SOURCE:-default}" != "default" ]]; then + printf '%s\n' "$STATE_DIR" + return 0 + fi + if [[ -n "$STATE_DIR" ]] && [[ "$STATE_DIR" != "/var/lib/pulse-agent" ]] && \ + { [[ -d "$STATE_DIR" ]] || [[ -f "$STATE_DIR/connection.env" ]] || [[ -f "$STATE_DIR/agent-id" ]]; }; then + printf '%s\n' "$STATE_DIR" + return 0 fi candidate=$(detect_qnap_data_volume || true) @@ -1533,28 +1601,50 @@ build_plist_program_arguments() { ensure_runtime_token_file() { local state_dir="${1:-$STATE_DIR}" local token_file="${state_dir}/token" + local previous_token="" + local old_umask="" + local token_tmp="" RUNTIME_TOKEN_FILE="" + RUNTIME_TOKEN_CHANGED="false" if [[ -z "$PULSE_TOKEN" ]]; then rm -f "$token_file" 2>/dev/null || true log_info "No API token provided; installer will configure token-optional agent runtime." return 0 fi - mkdir -p "$state_dir" - local old_umask="" old_umask=$(umask) umask 077 - if ! printf '%s' "$PULSE_TOKEN" > "$token_file"; then + mkdir -p "$state_dir" + chmod 700 "$state_dir" + if [[ -f "$token_file" && ! -L "$token_file" ]]; then + previous_token=$(cat "$token_file" 2>/dev/null || true) + fi + if [[ "$previous_token" != "$PULSE_TOKEN" ]]; then + RUNTIME_TOKEN_CHANGED="true" + fi + token_tmp=$(mktemp "${state_dir}/.token.XXXXXX") + TMP_FILES+=("$token_tmp") + if ! printf '%s' "$PULSE_TOKEN" > "$token_tmp"; then umask "$old_umask" fail "Failed to write runtime token file: $token_file" "$EXIT_GENERAL" fi - umask "$old_umask" - chmod 600 "$token_file" + chmod 600 "$token_tmp" if [[ "$(id -u 2>/dev/null || echo 1)" == "0" ]]; then - chown root:root "$token_file" 2>/dev/null || true + chown root:root "$token_tmp" 2>/dev/null || true fi + if ! mv -f "$token_tmp" "$token_file"; then + umask "$old_umask" + fail "Failed to install runtime token file: $token_file" "$EXIT_GENERAL" + fi + umask "$old_umask" RUNTIME_TOKEN_FILE="$token_file" + # A changed bootstrap token is explicit re-enrollment intent. Preserve the + # runtime token across ordinary restarts and tokenless updates, but do not + # let a stale runtime token shadow fresh enrollment credentials. + if [[ "${ENROLL:-false}" == "true" && "$RUNTIME_TOKEN_CHANGED" == "true" ]]; then + rm -f "${state_dir}/runtime.token" 2>/dev/null || true + fi log_info "Token stored securely at $token_file (mode 600)" } @@ -1609,7 +1699,11 @@ recover_token_from_default_agent_token_file() { # v5.1.x Linux services could omit --token and --token-file because the # Go agent read this default file itself. - for token_path in "${STATE_DIR%/}/token" "/var/lib/pulse-agent/token" "$TRUENAS_STATE_DIR/token"; do + local token_paths=("${STATE_DIR%/}/token") + if [[ "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + token_paths+=("${DEFAULT_STATE_DIR:-/var/lib/pulse-agent}/token" "$TRUENAS_STATE_DIR/token") + fi + for token_path in "${token_paths[@]}"; do [[ -n "$token_path" && -f "$token_path" ]] || continue recovered_token=$(cat "$token_path" 2>/dev/null || true) if [[ -n "$recovered_token" ]]; then @@ -1623,6 +1717,15 @@ recover_token_from_default_agent_token_file() { recover_connection_state() { local file="$1" + local saved_state_dir="" + + saved_state_dir=$(read_connection_state_value "$file" "PULSE_STATE_DIR") + if [[ -n "$saved_state_dir" && "$saved_state_dir" == /* && "$saved_state_dir" != "/" && + "$saved_state_dir" != *$'\r'* && "$saved_state_dir" != *$'\n'* && + "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + STATE_DIR="$saved_state_dir" + STATE_DIR_SOURCE="recovered" + fi if [[ -z "$PULSE_URL" ]]; then PULSE_URL=$(read_connection_state_value "$file" "PULSE_URL") @@ -1734,7 +1837,12 @@ apply_recovered_agent_arg_value() { RECOVERED_AGENT_ARG_STATE="true" ;; state-dir) - if [[ -n "$value" && "$STATE_DIR" == "/var/lib/pulse-agent" ]]; then STATE_DIR="$value"; fi + if [[ -n "$value" && "$value" == /* && "$value" != "/" && + "$value" != *$'\r'* && "$value" != *$'\n'* && + "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + STATE_DIR="$value" + STATE_DIR_SOURCE="recovered" + fi RECOVERED_AGENT_ARG_STATE="true" ;; kubeconfig) @@ -1893,6 +2001,16 @@ recover_connection_state_from_env_stream() { fi RECOVERED_AGENT_ENV_STATE="true" ;; + PULSE_STATE_DIR=*) + value="${env_line#*=}" + if [[ -n "$value" && "$value" == /* && "$value" != "/" && + "$value" != *$'\r'* && "$value" != *$'\n'* && + "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + STATE_DIR="$value" + STATE_DIR_SOURCE="recovered" + fi + RECOVERED_AGENT_ENV_STATE="true" + ;; PULSE_AGENT_ID=*) value="${env_line#*=}" if [[ -z "$AGENT_ID" ]]; then AGENT_ID="$value"; fi @@ -2109,6 +2227,36 @@ recover_connection_state_from_systemd_unit() { return 1 } +launchd_agent_arg_stream() { + local plist_path="${1:-/Library/LaunchDaemons/com.pulse.agent.plist}" + + [[ -f "$plist_path" ]] || return 1 + awk ' + /ProgramArguments<\/key>/ { in_program_args = 1; next } + in_program_args && /<\/array>/ { exit } + in_program_args && // { + value = $0 + sub(/^.*/, "", value) + sub(/<\/string>.*$/, "", value) + gsub(/&/, "\\&", value) + gsub(/</, "<", value) + gsub(/>/, ">", value) + gsub(/"/, "\"", value) + gsub(/'/, "\047", value) + print value + } + ' "$plist_path" +} + +recover_connection_state_from_launchd_plist() { + local plist_path="${1:-/Library/LaunchDaemons/com.pulse.agent.plist}" + + if recover_connection_state_from_arg_stream < <(launchd_agent_arg_stream "$plist_path"); then + return 0 + fi + return 1 +} + recover_connection_state_from_service_scripts() { local candidate="" local line="" @@ -2176,6 +2324,10 @@ recover_connection_state_from_existing_agent() { log_info "Recovered connection details from the existing Pulse Agent service." return 0 fi + if recover_connection_state_from_launchd_plist; then + log_info "Recovered connection details from the existing Pulse Agent launchd service." + return 0 + fi if recover_connection_state_from_service_scripts; then log_info "Recovered connection details from the existing Pulse Agent service script." return 0 @@ -2187,18 +2339,24 @@ recover_connection_state_from_existing_agent() { find_connection_state_file() { local conn_env="" local qnap_state_dir="" + local conn_paths=("${STATE_DIR%/}/connection.env") - for conn_env in /var/lib/pulse-agent/connection.env /boot/config/plugins/pulse-agent/connection.env "$TRUENAS_STATE_DIR/connection.env"; do + if [[ "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + conn_paths+=("${DEFAULT_STATE_DIR:-/var/lib/pulse-agent}/connection.env" /boot/config/plugins/pulse-agent/connection.env "$TRUENAS_STATE_DIR/connection.env") + fi + for conn_env in "${conn_paths[@]}"; do if [[ -f "$conn_env" ]]; then printf '%s\n' "$conn_env" return 0 fi done - qnap_state_dir=$(find_qnap_state_dir || true) - if [[ -n "$qnap_state_dir" ]] && [[ -f "$qnap_state_dir/connection.env" ]]; then - printf '%s\n' "$qnap_state_dir/connection.env" - return 0 + if [[ "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + qnap_state_dir=$(find_qnap_state_dir || true) + if [[ -n "$qnap_state_dir" ]] && [[ -f "$qnap_state_dir/connection.env" ]]; then + printf '%s\n' "$qnap_state_dir/connection.env" + return 0 + fi fi return 1 @@ -2207,11 +2365,17 @@ find_connection_state_file() { recover_agent_id_from_state_file() { local aid_path="" local qnap_state_dir="" - local aid_paths=(/var/lib/pulse-agent/agent-id /boot/config/plugins/pulse-agent/agent-id "$TRUENAS_STATE_DIR/agent-id") + local aid_paths=("${STATE_DIR%/}/agent-id") - qnap_state_dir=$(find_qnap_state_dir || true) - if [[ -n "$qnap_state_dir" ]]; then - aid_paths+=("$qnap_state_dir/agent-id") + if [[ "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + aid_paths+=("${DEFAULT_STATE_DIR:-/var/lib/pulse-agent}/agent-id" /boot/config/plugins/pulse-agent/agent-id "$TRUENAS_STATE_DIR/agent-id") + fi + + if [[ "${STATE_DIR_SOURCE:-default}" == "default" ]]; then + qnap_state_dir=$(find_qnap_state_dir || true) + if [[ -n "$qnap_state_dir" ]]; then + aid_paths+=("$qnap_state_dir/agent-id") + fi fi for aid_path in "${aid_paths[@]}"; do @@ -2228,22 +2392,31 @@ recover_agent_id_from_state_file() { save_connection_info() { local state_dir="$1" local conn_env="${state_dir}/connection.env" + local conn_tmp="" + local old_umask="" + old_umask=$(umask) + umask 077 mkdir -p "$state_dir" + chmod 700 "$state_dir" # Save connection details so uninstall can deregister without --url/--token. # Single-quote values to prevent shell interpretation on read-back. # Legacy connection files may contain PULSE_TOKEN, but new installs persist # only the protected token file path. - : > "$conn_env" - write_connection_state_value "$conn_env" "PULSE_URL" "$PULSE_URL" - write_connection_state_value "$conn_env" "PULSE_TOKEN_FILE" "$RUNTIME_TOKEN_FILE" - write_connection_state_value "$conn_env" "PULSE_AGENT_ID" "$AGENT_ID" - write_connection_state_value "$conn_env" "PULSE_HOSTNAME" "$HOSTNAME_OVERRIDE" + conn_tmp=$(mktemp "${state_dir}/.connection.env.XXXXXX") + TMP_FILES+=("$conn_tmp") + write_connection_state_value "$conn_tmp" "PULSE_STATE_DIR" "$state_dir" + write_connection_state_value "$conn_tmp" "PULSE_URL" "$PULSE_URL" + write_connection_state_value "$conn_tmp" "PULSE_TOKEN_FILE" "$RUNTIME_TOKEN_FILE" + write_connection_state_value "$conn_tmp" "PULSE_AGENT_ID" "$AGENT_ID" + write_connection_state_value "$conn_tmp" "PULSE_HOSTNAME" "$HOSTNAME_OVERRIDE" if [[ "$INSECURE" == "true" ]]; then - write_connection_state_value "$conn_env" "PULSE_INSECURE_SKIP_VERIFY" "true" + write_connection_state_value "$conn_tmp" "PULSE_INSECURE_SKIP_VERIFY" "true" fi - write_connection_state_value "$conn_env" "PULSE_SERVER_FINGERPRINT" "$SERVER_FINGERPRINT" - write_connection_state_value "$conn_env" "PULSE_CACERT" "$CURL_CA_BUNDLE" - chmod 600 "$conn_env" + write_connection_state_value "$conn_tmp" "PULSE_SERVER_FINGERPRINT" "$SERVER_FINGERPRINT" + write_connection_state_value "$conn_tmp" "PULSE_CACERT" "$CURL_CA_BUNDLE" + chmod 600 "$conn_tmp" + mv -f "$conn_tmp" "$conn_env" + umask "$old_umask" # Save a copy of this install script for offline uninstall. # When run via "curl | bash", $0 is /dev/stdin — not a usable file. # Try local copy first, then download a fresh copy from the server. @@ -2304,7 +2477,7 @@ while [[ $# -gt 0 ]]; do --uninstall) UNINSTALL="true"; shift ;; --agent-id) AGENT_ID="$2"; shift 2 ;; --hostname) HOSTNAME_OVERRIDE="$2"; shift 2 ;; - --state-dir) STATE_DIR="$2"; shift 2 ;; + --state-dir) STATE_DIR="$2"; STATE_DIR_SOURCE="explicit"; shift 2 ;; --kube-include-all-pods) KUBE_INCLUDE_ALL_PODS="true"; shift ;; --kube-include-all-deployments) KUBE_INCLUDE_ALL_DEPLOYMENTS="true"; shift ;; --disk-exclude) DISK_EXCLUDES+=("$2"); shift 2 ;; @@ -2317,6 +2490,13 @@ while [[ $# -gt 0 ]]; do esac done +discover_state_dir_from_saved_installer "$0" || true + +if [[ -z "$STATE_DIR" || "$STATE_DIR" != /* || "$STATE_DIR" == "/" || + "$STATE_DIR" == *$'\r'* || "$STATE_DIR" == *$'\n'* ]]; then + fail "--state-dir must be an absolute, non-root path." "$EXIT_MISSING_ARGS" +fi + if [[ -n "$OBSERVERS_FILE" ]]; then if [[ "$OBSERVERS_FILE" != /* ]]; then fail "Observer config path must be absolute: ${OBSERVERS_FILE}" "$EXIT_MISSING_ARGS" @@ -2358,33 +2538,48 @@ if [[ -n "$PULSE_URL" ]]; then PULSE_URL="${PULSE_URL%/}" fi -# --- Update State Recovery --- -# Update commands are intentionally tokenless for already-installed agents. The -# installer reuses the canonical saved connection state instead of asking the -# operator to mint a new install token for a host Pulse already knows about. -if [[ "$UPDATE_ONLY" == "true" ]]; then +# --- Installed Lifecycle State Recovery --- +# An explicit state directory is authoritative. Without one, inspect the active +# process/service first so a custom installation wins over stale default-path +# artifacts, then merge its canonical connection.env and agent-id state. +if [[ "$UPDATE_ONLY" == "true" || "$UNINSTALL" == "true" ]]; then + if [[ "$STATE_DIR_SOURCE" != "explicit" ]]; then + recover_connection_state_from_existing_agent || true + fi + + local lifecycle_conn_env="" + lifecycle_conn_env=$(find_connection_state_file || true) + if [[ -n "$lifecycle_conn_env" ]]; then + log_info "Recovering connection details from ${lifecycle_conn_env}..." + recover_connection_state "$lifecycle_conn_env" + fi + if update_connection_state_incomplete; then - local update_conn_env="" - update_conn_env=$(find_connection_state_file || true) - if [[ -n "$update_conn_env" ]]; then - log_info "Recovering connection details from ${update_conn_env}..." - recover_connection_state "$update_conn_env" - fi - if update_connection_state_incomplete; then - recover_connection_state_from_existing_agent || true - fi - if [[ -n "$PULSE_URL" && -n "$PULSE_TOKEN" ]]; then - : - elif [[ -z "$PULSE_URL" || -z "$PULSE_TOKEN" ]]; then - fail "No existing Pulse Agent connection state found. Use the install command instead." "$EXIT_MISSING_ARGS" - fi - if [[ -z "$AGENT_ID" ]]; then - AGENT_ID=$(recover_agent_id_from_state_file || true) - if [[ -n "$AGENT_ID" ]]; then - log_info "Recovered agent ID from persisted agent-id state." - fi + recover_connection_state_from_existing_agent || true + fi + + if [[ -z "$AGENT_ID" ]]; then + AGENT_ID=$(recover_agent_id_from_state_file || true) + if [[ -n "$AGENT_ID" ]]; then + log_info "Recovered agent ID from persisted agent-id state." fi fi + + if [[ "$UPDATE_ONLY" == "true" && ( -z "$PULSE_URL" || -z "$PULSE_TOKEN" ) ]]; then + fail "No existing Pulse Agent connection state found. Use the install command instead." "$EXIT_MISSING_ARGS" + fi +fi + +if [[ -z "$STATE_DIR" || "$STATE_DIR" != /* || "$STATE_DIR" == "/" || + "$STATE_DIR" == *$'\r'* || "$STATE_DIR" == *$'\n'* ]]; then + fail "Recovered Pulse Agent state directory is invalid." "$EXIT_MISSING_ARGS" +fi + +if [[ "$STATE_DIR_SOURCE" == "explicit" || "$STATE_DIR_SOURCE" == "recovered" ]]; then + TRUENAS_STATE_DIR="$STATE_DIR" + TRUENAS_LOG_DIR="$TRUENAS_STATE_DIR/logs" + TRUENAS_BOOTSTRAP_SCRIPT="$TRUENAS_STATE_DIR/bootstrap-pulse-agent.sh" + TRUENAS_ENV_FILE="$TRUENAS_STATE_DIR/pulse-agent.env" fi if [[ -n "$PULSE_URL" ]]; then @@ -2553,18 +2748,6 @@ if [[ "$UNINSTALL" == "true" ]]; then log_info "Uninstalling ${AGENT_NAME} and cleaning up legacy agents..." local qnap_state_dir="" - # Recover connection details from the canonical installer-owned state artifact - # if command line input is only partial. Read keys explicitly instead of - # sourcing the file so uninstall cannot execute persisted shell content. - if [[ -z "$PULSE_URL" || -z "$PULSE_TOKEN" || -z "$AGENT_ID" || -z "$HOSTNAME_OVERRIDE" || -z "$CURL_CA_BUNDLE" || "$INSECURE" != "true" ]]; then - local conn_env="" - conn_env=$(find_connection_state_file || true) - if [[ -n "$conn_env" ]]; then - log_info "Recovering connection details from ${conn_env}..." - recover_connection_state "$conn_env" - fi - fi - # Try to notify the Pulse server about uninstallation if we have connection details # This ensures the agent record is removed and any linked PVE nodes are updated immediately. if [[ -n "$PULSE_URL" ]]; then @@ -2572,7 +2755,10 @@ if [[ "$UNINSTALL" == "true" ]]; then # Priority: agent-id file (canonical) > hostname API lookup (fallback) if [[ -z "$AGENT_ID" ]]; then local aid_path="" - local aid_paths=(/var/lib/pulse-agent/agent-id /boot/config/plugins/pulse-agent/agent-id "$TRUENAS_STATE_DIR/agent-id") + local aid_paths=("${STATE_DIR%/}/agent-id") + if [[ "$STATE_DIR_SOURCE" == "default" ]]; then + aid_paths+=("$DEFAULT_STATE_DIR/agent-id" /boot/config/plugins/pulse-agent/agent-id "$TRUENAS_STATE_DIR/agent-id") + fi qnap_state_dir=$(find_qnap_state_dir || true) if [[ -n "$qnap_state_dir" ]]; then aid_paths+=("$qnap_state_dir/agent-id") @@ -2596,11 +2782,10 @@ if [[ "$UNINSTALL" == "true" ]]; then fi if [[ -n "$LOOKUP_HOSTNAME" ]]; then LOOKUP_ARGS=(-fsSL --connect-timeout 5) - if [[ -n "$PULSE_TOKEN" ]]; then LOOKUP_ARGS+=(-H "X-API-Token: ${PULSE_TOKEN}"); fi if [[ "$INSECURE" == "true" ]]; then LOOKUP_ARGS+=(-k); fi if [[ -n "$CURL_CA_BUNDLE" ]]; then LOOKUP_ARGS+=(--cacert "$CURL_CA_BUNDLE"); fi LOOKUP_HOSTNAME_ESCAPED=$(url_encode "$LOOKUP_HOSTNAME") - LOOKUP_RESP=$(curl "${LOOKUP_ARGS[@]}" "${PULSE_URL}/api/agents/agent/lookup?hostname=${LOOKUP_HOSTNAME_ESCAPED}" 2>/dev/null || true) + LOOKUP_RESP=$(curl_with_pulse_token "${LOOKUP_ARGS[@]}" "${PULSE_URL}/api/agents/agent/lookup?hostname=${LOOKUP_HOSTNAME_ESCAPED}" 2>/dev/null || true) if [[ -n "$LOOKUP_RESP" ]]; then # Extract .agent.id from JSON (portable, no jq dependency) AGENT_ID=$(echo "$LOOKUP_RESP" | grep -o '"id"[[:space:]]*:[[:space:]]*"[^"]*"' | head -1 | sed 's/.*"id"[[:space:]]*:[[:space:]]*"//; s/"$//' || true) @@ -2614,12 +2799,11 @@ if [[ "$UNINSTALL" == "true" ]]; then if [[ -n "$AGENT_ID" ]]; then log_info "Notifying Pulse server to unregister agent ID: ${AGENT_ID}..." CURL_ARGS=(-fsSL --connect-timeout 5 -X POST -H "Content-Type: application/json") - if [[ -n "$PULSE_TOKEN" ]]; then CURL_ARGS+=(-H "X-API-Token: ${PULSE_TOKEN}"); fi if [[ "$INSECURE" == "true" ]]; then CURL_ARGS+=(-k); fi if [[ -n "$CURL_CA_BUNDLE" ]]; then CURL_ARGS+=(--cacert "$CURL_CA_BUNDLE"); fi # Send unregistration request (ignore errors as we are uninstalling anyway) - curl "${CURL_ARGS[@]}" -d "{\"agentId\": \"${AGENT_ID}\"}" "${PULSE_URL}/api/agents/agent/uninstall" >/dev/null 2>&1 || true + curl_with_pulse_token "${CURL_ARGS[@]}" -d "{\"agentId\": \"${AGENT_ID}\"}" "${PULSE_URL}/api/agents/agent/uninstall" >/dev/null 2>&1 || true fi fi @@ -2640,7 +2824,7 @@ if [[ "$UNINSTALL" == "true" ]]; then # Remove legacy binaries # Remove agent state directory (contains agent ID, proxmox registration state, etc.) - rm -rf /var/lib/pulse-agent + remove_agent_state_dir "$STATE_DIR" # Remove log files rm -f /var/log/pulse-agent.log @@ -3171,12 +3355,12 @@ if [[ -f /etc/unraid-version ]]; then # Unraid's /boot is FAT32 (no execute permission), so we store the binary there # for persistence but copy it to RAM disk (/usr/local/bin) for execution - UNRAID_STORAGE_DIR="/boot/config/plugins/pulse-agent" + select_platform_state_dir "/boot/config/plugins/pulse-agent" + UNRAID_STORAGE_DIR="$STATE_DIR" UNRAID_STORED_BINARY="${UNRAID_STORAGE_DIR}/${BINARY_NAME}" RUNTIME_BINARY="${INSTALL_DIR}/${BINARY_NAME}" GO_SCRIPT="/boot/config/go" - STATE_DIR="$UNRAID_STORAGE_DIR" mkdir -p "$UNRAID_STORAGE_DIR" # Copy binary to persistent storage (for survival across reboots) @@ -3285,7 +3469,7 @@ if [[ -f /sbin/getcfg ]] || [[ -f /etc/config/qpkg.conf ]]; then fail "Could not find a writable QNAP data volume. Is a storage volume configured?" fi - STATE_DIR="${QNAP_VOL}/.pulse-agent" + select_platform_state_dir "${QNAP_VOL}/.pulse-agent" QNAP_STORED_BINARY="${STATE_DIR}/${BINARY_NAME}" RUNTIME_BINARY="${INSTALL_DIR}/${BINARY_NAME}" WRAPPER_SCRIPT="${STATE_DIR}/start-pulse-agent.sh" @@ -3351,7 +3535,11 @@ fi # Note: /data may have exec=off on some TrueNAS systems. We try multiple runtime locations. if [[ "$TRUENAS" == true ]]; then log_info "Configuring TrueNAS SCALE/CORE installation..." - STATE_DIR="$TRUENAS_STATE_DIR" + select_platform_state_dir "$TRUENAS_STATE_DIR" + TRUENAS_STATE_DIR="$STATE_DIR" + TRUENAS_LOG_DIR="$TRUENAS_STATE_DIR/logs" + TRUENAS_BOOTSTRAP_SCRIPT="$TRUENAS_STATE_DIR/bootstrap-pulse-agent.sh" + TRUENAS_ENV_FILE="$TRUENAS_STATE_DIR/pulse-agent.env" # Stop any existing agent before we modify binaries # The runtime binary may be in /root/bin or /var/tmp, not just INSTALL_DIR @@ -3454,6 +3642,7 @@ if [[ "$TRUENAS" == true ]]; then # Store environment/config for reference cat > "$TRUENAS_ENV_FILE" <&2; return 99; } + curl() { return 1; } +` + extractInstallShellFunction(t, "write_connection_state_value") + ` +` + extractInstallShellFunction(t, "read_connection_state_value") + ` +` + extractInstallShellFunction(t, "recover_token_from_default_agent_token_file") + ` +` + extractInstallShellFunction(t, "recover_connection_state") + ` +` + extractInstallShellFunction(t, "ensure_runtime_token_file") + ` +` + extractInstallShellFunction(t, "build_exec_arg_items") + ` +` + extractInstallShellFunction(t, "join_exec_arg_items") + ` +` + extractInstallShellFunction(t, "build_exec_args") + ` +` + extractInstallShellFunction(t, "systemd_agent_requires_lxc_attach") + ` +` + extractInstallShellFunction(t, "render_systemd_agent_unit") + ` +` + extractInstallShellFunction(t, "save_connection_info") + recovery + ` + ensure_runtime_token_file "$STATE_DIR" + build_exec_args + render_systemd_agent_unit "` + unitPath + `" "/test/pulse-agent" "$EXEC_ARGS" "network-online.target" "network-online.target" "root" "" + save_connection_info "$STATE_DIR" + ` + cmd := exec.Command("bash", "-c", script) + cmd.Stdin = strings.NewReader(token + "\n") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("render lifecycle service: %v\n%s", err, out) + } +} + +type runningLifecycleAgent struct { + cmd *exec.Cmd + logs *bytes.Buffer +} + +func startLifecycleAgent(t *testing.T, binaryPath, pulseURL, stateDir string, commandsEnabled bool) *runningLifecycleAgent { + t.Helper() + args := []string{ + "--url", pulseURL, + "--token-file", filepath.Join(stateDir, "token"), + "--state-dir", stateDir, + "--interval", "1s", + "--hostname", "state-lifecycle-host", + "--enable-host", + "--enable-docker=false", + "--disable-auto-update", + "--health-addr", "", + "--enroll", + } + if commandsEnabled { + args = append(args, "--enable-commands") + } + logs := &bytes.Buffer{} + cmd := exec.Command(binaryPath, args...) + cmd.Env = append(os.Environ(), "PULSE_AGENT_CONFIG_SIGNATURE_REQUIRED=false") + cmd.Stdout = logs + cmd.Stderr = logs + if err := cmd.Start(); err != nil { + t.Fatalf("start pulse-agent: %v", err) + } + t.Cleanup(func() { + if cmd.Process != nil { + _ = cmd.Process.Kill() + _, _ = cmd.Process.Wait() + } + }) + for _, arg := range cmd.Args { + if strings.Contains(arg, "bootstrap-") || strings.Contains(arg, "runtime-") { + t.Fatalf("agent argv leaked token: %q", cmd.Args) + } + } + return &runningLifecycleAgent{cmd: cmd, logs: logs} +} + +func (p *runningLifecycleAgent) stop(t *testing.T) string { + t.Helper() + if p.cmd.Process == nil { + return p.logs.String() + } + if err := p.cmd.Process.Signal(syscall.SIGTERM); err != nil { + t.Fatalf("signal pulse-agent: %v", err) + } + done := make(chan error, 1) + go func() { done <- p.cmd.Wait() }() + select { + case err := <-done: + if err != nil { + t.Fatalf("pulse-agent shutdown: %v\n%s", err, p.logs.String()) + } + case <-time.After(10 * time.Second): + _ = p.cmd.Process.Kill() + t.Fatalf("pulse-agent did not stop\n%s", p.logs.String()) + } + p.cmd.Process = nil + return p.logs.String() +} + +func assertPrivateLifecycleFile(t *testing.T, path string, want os.FileMode) { + t.Helper() + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat %s: %v", path, err) + } + if runtime.GOOS != "windows" && info.Mode().Perm() != want { + t.Fatalf("%s mode = %o, want %o", path, info.Mode().Perm(), want) + } +} + +func TestPulseAgentStateDirLifecycleIntegration(t *testing.T) { + if testing.Short() { + t.Skip("real agent lifecycle integration") + } + binaryPath := buildLifecycleAgent(t) + + for _, tc := range []struct { + name string + commandsEnabled bool + customState bool + }{ + {name: "default_state_commands_disabled", commandsEnabled: false, customState: false}, + {name: "custom_state_commands_enabled", commandsEnabled: true, customState: true}, + } { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + stateName := "default-state" + if tc.customState { + stateName = "custom-state" + } + stateDir := filepath.Join(root, stateName) + unitPath := filepath.Join(root, "pulse-agent.service") + + controlPlane := &agentLifecycleControlPlane{online: true} + controlPlane.setCredentials("bootstrap-one", "runtime-one", "agent-one") + server := httptest.NewServer(http.HandlerFunc(controlPlane.serveHTTP)) + defer server.Close() + + stateSource := "default" + if tc.customState { + stateSource = "explicit" + } + renderLifecycleService(t, stateDir, stateSource, unitPath, server.URL, "bootstrap-one", tc.commandsEnabled, false) + unit, err := os.ReadFile(unitPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(unit), "--state-dir "+stateDir) || + !strings.Contains(string(unit), "--token-file "+filepath.Join(stateDir, "token")) { + t.Fatalf("generated service does not own canonical state paths:\n%s", unit) + } + if strings.Contains(string(unit), "bootstrap-one") || strings.Contains(string(unit), "runtime-one") { + t.Fatalf("generated service leaked a token:\n%s", unit) + } + if tc.commandsEnabled != strings.Contains(string(unit), "--enable-commands") { + t.Fatalf("generated service command mode mismatch:\n%s", unit) + } + + proc := startLifecycleAgent(t, binaryPath, server.URL, stateDir, tc.commandsEnabled) + waitForLifecycleState(t, 20*time.Second, "initial enrollment and report", func(state agentLifecycleSnapshot) bool { + return state.enrollmentCount == 1 && state.reportCount >= 1 && + state.lastReportToken == "runtime-one" && state.lastReportID == "agent-one" && + state.lastCommands == tc.commandsEnabled + }, controlPlane) + logOutput := proc.stop(t) + + for path, mode := range map[string]os.FileMode{ + stateDir: 0700, + filepath.Join(stateDir, "token"): 0600, + filepath.Join(stateDir, "runtime.token"): 0600, + filepath.Join(stateDir, "agent-id"): 0600, + filepath.Join(stateDir, "connection.env"): 0600, + } { + assertPrivateLifecycleFile(t, path, mode) + } + + beforeRestart := controlPlane.snapshot() + proc = startLifecycleAgent(t, binaryPath, server.URL, stateDir, tc.commandsEnabled) + waitForLifecycleState(t, 20*time.Second, "restart report with persisted identity", func(state agentLifecycleSnapshot) bool { + return state.reportCount > beforeRestart.reportCount && + state.lastReportToken == "runtime-one" && state.lastReportID == "agent-one" + }, controlPlane) + if got := controlPlane.snapshot().enrollmentCount; got != 1 { + t.Fatalf("ordinary restart re-enrolled unexpectedly: enrollments=%d", got) + } + + controlPlane.setOnline(false) + reportsBeforeOutage := controlPlane.snapshot().reportCount + time.Sleep(1500 * time.Millisecond) + controlPlane.setOnline(true) + waitForLifecycleState(t, 20*time.Second, "report recovery after server restart", func(state agentLifecycleSnapshot) bool { + return state.reportCount > reportsBeforeOutage && state.lastReportToken == "runtime-one" + }, controlPlane) + logOutput += proc.stop(t) + + renderLifecycleService(t, stateDir, stateSource, unitPath, server.URL, "bootstrap-one", tc.commandsEnabled, true) + if _, err := os.Stat(filepath.Join(stateDir, "runtime.token")); err != nil { + t.Fatalf("update did not preserve runtime enrollment token: %v", err) + } + + controlPlane.setCredentials("bootstrap-two", "runtime-two", "agent-two") + renderLifecycleService(t, stateDir, stateSource, unitPath, server.URL, "bootstrap-two", tc.commandsEnabled, false) + if _, err := os.Stat(filepath.Join(stateDir, "runtime.token")); !os.IsNotExist(err) { + t.Fatalf("fresh bootstrap did not clear stale runtime token: %v", err) + } + proc = startLifecycleAgent(t, binaryPath, server.URL, stateDir, tc.commandsEnabled) + waitForLifecycleState(t, 20*time.Second, "re-enrollment and canonical report", func(state agentLifecycleSnapshot) bool { + return state.enrollmentCount == 2 && state.lastReportToken == "runtime-two" && + state.lastReportID == "agent-two" && state.lastCommands == tc.commandsEnabled + }, controlPlane) + logOutput += proc.stop(t) + + for _, secret := range []string{"bootstrap-one", "runtime-one", "bootstrap-two", "runtime-two"} { + if strings.Contains(logOutput, secret) { + t.Fatalf("agent logs leaked %q:\n%s", secret, logOutput) + } + } + + removeScript := ` + set -euo pipefail + STATE_DIR="` + stateDir + `" + log_warn() { :; } +` + extractInstallShellFunction(t, "remove_agent_state_dir") + ` + remove_agent_state_dir "$STATE_DIR" + ` + if out, err := exec.Command("bash", "-c", removeScript).CombinedOutput(); err != nil { + t.Fatalf("uninstall state cleanup: %v\n%s", err, out) + } + if _, err := os.Stat(stateDir); !os.IsNotExist(err) { + t.Fatalf("uninstall did not remove canonical state directory: %v", err) + } + + connectionState, err := os.ReadFile(filepath.Join(root, stateName, "connection.env")) + if err == nil { + t.Fatalf("connection state survived uninstall: %s", connectionState) + } + }) + } +} diff --git a/scripts/installtests/install_sh_test.go b/scripts/installtests/install_sh_test.go index c06121436..03af150aa 100644 --- a/scripts/installtests/install_sh_test.go +++ b/scripts/installtests/install_sh_test.go @@ -54,7 +54,7 @@ func TestInstallSHPersistsAndVerifiesServerFingerprint(t *testing.T) { `verify_pinned_server_certificate() {`, `openssl s_client -connect "$target" -servername "$host"`, `EXEC_ARG_ITEMS+=(--server-fingerprint "$SERVER_FINGERPRINT")`, - `write_connection_state_value "$conn_env" "PULSE_SERVER_FINGERPRINT" "$SERVER_FINGERPRINT"`, + `write_connection_state_value "$conn_tmp" "PULSE_SERVER_FINGERPRINT" "$SERVER_FINGERPRINT"`, `SERVER_FINGERPRINT=$(read_connection_state_value "$file" "PULSE_SERVER_FINGERPRINT")`, `if [[ "$OS" == "linux" || "$OS" == "freebsd" ]]; then`, } @@ -373,6 +373,8 @@ func TestBuildPlistProgramArgumentsUsesSharedExecArgs(t *testing.T) { `--health-addr`, `--hostname`, `Richard's Mac & Mini`, + `--state-dir`, + `/var/lib/pulse-agent`, `--disk-exclude`, `Time Machine`, } @@ -655,12 +657,13 @@ func TestInstallSHPersistsIdentityInConnectionEnv(t *testing.T) { `read_connection_state_value() {`, `recover_connection_state() {`, `find_connection_state_file() {`, - `write_connection_state_value "$conn_env" "PULSE_TOKEN_FILE" "$RUNTIME_TOKEN_FILE"`, - `write_connection_state_value "$conn_env" "PULSE_AGENT_ID" "$AGENT_ID"`, - `write_connection_state_value "$conn_env" "PULSE_HOSTNAME" "$HOSTNAME_OVERRIDE"`, - `write_connection_state_value "$conn_env" "PULSE_INSECURE_SKIP_VERIFY" "true"`, - `write_connection_state_value "$conn_env" "PULSE_CACERT" "$CURL_CA_BUNDLE"`, - `recover_connection_state "$conn_env"`, + `write_connection_state_value "$conn_tmp" "PULSE_STATE_DIR" "$state_dir"`, + `write_connection_state_value "$conn_tmp" "PULSE_TOKEN_FILE" "$RUNTIME_TOKEN_FILE"`, + `write_connection_state_value "$conn_tmp" "PULSE_AGENT_ID" "$AGENT_ID"`, + `write_connection_state_value "$conn_tmp" "PULSE_HOSTNAME" "$HOSTNAME_OVERRIDE"`, + `write_connection_state_value "$conn_tmp" "PULSE_INSECURE_SKIP_VERIFY" "true"`, + `write_connection_state_value "$conn_tmp" "PULSE_CACERT" "$CURL_CA_BUNDLE"`, + `recover_connection_state "$lifecycle_conn_env"`, } for _, needle := range required { if !strings.Contains(script, needle) { @@ -677,9 +680,10 @@ func TestInstallSHRecoversSavedStateForPartialUninstallContext(t *testing.T) { script := string(content) needles := []string{ - `if [[ -z "$PULSE_URL" || -z "$PULSE_TOKEN" || -z "$AGENT_ID" || -z "$HOSTNAME_OVERRIDE" || -z "$CURL_CA_BUNDLE" || "$INSECURE" != "true" ]]; then`, - `# Recover connection details from the canonical installer-owned state artifact`, - `conn_env=$(find_connection_state_file || true)`, + `if [[ "$UPDATE_ONLY" == "true" || "$UNINSTALL" == "true" ]]; then`, + `# An explicit state directory is authoritative.`, + `lifecycle_conn_env=$(find_connection_state_file || true)`, + `recover_connection_state "$lifecycle_conn_env"`, } for _, needle := range needles { if !strings.Contains(script, needle) { @@ -699,12 +703,13 @@ func TestInstallSHSupportsSavedStateUpdateMode(t *testing.T) { `--update Update an existing agent using saved connection state`, `UPDATE_ONLY="false"`, `--update) UPDATE_ONLY="true"; shift ;;`, - `if [[ "$UPDATE_ONLY" == "true" ]]; then`, - `update_conn_env=$(find_connection_state_file || true)`, - `recover_connection_state "$update_conn_env"`, + `if [[ "$UPDATE_ONLY" == "true" || "$UNINSTALL" == "true" ]]; then`, + `lifecycle_conn_env=$(find_connection_state_file || true)`, + `recover_connection_state "$lifecycle_conn_env"`, `recover_connection_state_from_existing_agent() {`, `recover_connection_state_from_running_agent`, `recover_connection_state_from_systemd_unit`, + `recover_connection_state_from_launchd_plist`, `recover_connection_state_from_service_scripts`, `running_agent_arg_stream() {`, `running_agent_env_stream() {`, @@ -720,7 +725,7 @@ func TestInstallSHSupportsSavedStateUpdateMode(t *testing.T) { `[[ "$RECOVERED_AGENT_ENV_STATE" == "true" ]] && recovered_connection_state_ready`, `if update_connection_state_incomplete; then`, `recover_connection_state_from_existing_agent || true`, - `if [[ -n "$PULSE_URL" && -n "$PULSE_TOKEN" ]]; then`, + `if [[ "$UPDATE_ONLY" == "true" && ( -z "$PULSE_URL" || -z "$PULSE_TOKEN" ) ]]; then`, `recover_agent_id_from_state_file() {`, `AGENT_ID=$(recover_agent_id_from_state_file || true)`, `No existing Pulse Agent connection state found. Use the install command instead.`, @@ -1430,7 +1435,7 @@ func TestInstallSHUsesCanonicalQNAPBootstrapRenderer(t *testing.T) { `remove_qnap_autorun_block() {`, `write_qnap_wrapper_script() {`, `append_qnap_autorun_block() {`, - `STATE_DIR="${QNAP_VOL}/.pulse-agent"`, + `select_platform_state_dir "${QNAP_VOL}/.pulse-agent"`, `write_qnap_wrapper_script "$WRAPPER_SCRIPT" "$RUNTIME_BINARY" "$QNAP_STORED_BINARY"`, `append_qnap_autorun_block "$AUTORUN_PATH" "$WRAPPER_SCRIPT" "$STATE_DIR"`, `complete_installation_flow "$STATE_DIR" "Installation complete! Agent is running." "Upgrade complete! Agent is running." "tail -f /var/log/${AGENT_NAME}.log"`, @@ -2228,6 +2233,501 @@ func TestStateDirFlagIsAcceptedByInstallerParser(t *testing.T) { } } +func TestInstallSHCustomStateDirOwnsTokenAndEnrollmentContinuity(t *testing.T) { + stateDir := filepath.Join(t.TempDir(), "custom-state") + if err := os.MkdirAll(stateDir, 0755); err != nil { + t.Fatal(err) + } + tokenPath := filepath.Join(stateDir, "token") + runtimePath := filepath.Join(stateDir, "runtime.token") + if err := os.WriteFile(tokenPath, []byte("aaa111"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(runtimePath, []byte("runtime-one"), 0600); err != nil { + t.Fatal(err) + } + + ensureToken := extractInstallShellFunction(t, "ensure_runtime_token_file") + run := func(token string) string { + t.Helper() + script := ` + set -euo pipefail + PULSE_TOKEN="` + token + `" + STATE_DIR="` + stateDir + `" + ENROLL="true" + RUNTIME_TOKEN_FILE="" + RUNTIME_TOKEN_CHANGED="false" + log_info() { :; } +` + ensureToken + ` + ensure_runtime_token_file "$STATE_DIR" + printf 'changed=%s runtime=%s token_file=%s\n' \ + "$RUNTIME_TOKEN_CHANGED" "$([[ -f "$STATE_DIR/runtime.token" ]] && echo present || echo missing)" "$RUNTIME_TOKEN_FILE" + ` + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + return string(out) + } + + if got := run("aaa111"); !strings.Contains(got, "changed=false runtime=present") { + t.Fatalf("unchanged bootstrap token did not preserve enrollment runtime state:\n%s", got) + } + if got := run("bbb222"); !strings.Contains(got, "changed=true runtime=missing") { + t.Fatalf("changed bootstrap token did not force re-enrollment:\n%s", got) + } + + for path, want := range map[string]os.FileMode{ + stateDir: 0700, + tokenPath: 0600, + } { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != want { + t.Fatalf("%s mode = %o, want %o", path, got, want) + } + } +} + +func TestInstallSHExplicitCustomStateNeverFallsBackToDefaultInstance(t *testing.T) { + root := t.TempDir() + customState := filepath.Join(root, "custom") + defaultState := filepath.Join(root, "default") + for _, dir := range []string{customState, defaultState} { + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + } + customConn := filepath.Join(customState, "connection.env") + if err := os.WriteFile(customConn, []byte("PULSE_URL='https://custom.example'\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(defaultState, "connection.env"), []byte("PULSE_URL='https://default.example'\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(defaultState, "agent-id"), []byte("default-agent"), 0600); err != nil { + t.Fatal(err) + } + + script := ` + set -euo pipefail + STATE_DIR="` + customState + `" + STATE_DIR_SOURCE="explicit" + DEFAULT_STATE_DIR="` + defaultState + `" + TRUENAS_STATE_DIR="` + filepath.Join(root, "truenas") + `" +` + extractInstallShellFunction(t, "find_connection_state_file") + ` +` + extractInstallShellFunction(t, "recover_agent_id_from_state_file") + ` + printf 'connection=%s\n' "$(find_connection_state_file)" + rm -f "$STATE_DIR/connection.env" + printf 'fallback_connection=%s\n' "$(find_connection_state_file || true)" + printf 'fallback_agent=%s\n' "$(recover_agent_id_from_state_file || true)" + ` + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + got := string(out) + if !strings.Contains(got, "connection="+customConn) { + t.Fatalf("custom connection state was not preferred:\n%s", got) + } + if !strings.Contains(got, "fallback_connection=\n") || !strings.Contains(got, "fallback_agent=\n") { + t.Fatalf("explicit custom state borrowed default instance state:\n%s", got) + } +} + +func TestInstallSHSavedInstallerDiscoversItsCustomStateDir(t *testing.T) { + stateDir := filepath.Join(t.TempDir(), "saved custom state") + if err := os.MkdirAll(stateDir, 0700); err != nil { + t.Fatal(err) + } + installerPath := filepath.Join(stateDir, "install.sh") + if err := os.WriteFile(installerPath, []byte("#!/usr/bin/env bash\n"), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(stateDir, "connection.env"), []byte("PULSE_URL='https://pulse.example'\n"), 0600); err != nil { + t.Fatal(err) + } + script := ` + set -euo pipefail + STATE_DIR="/var/lib/pulse-agent" + STATE_DIR_SOURCE="default" +` + extractInstallShellFunction(t, "discover_state_dir_from_saved_installer") + ` + discover_state_dir_from_saved_installer "` + installerPath + `" + printf 'state=%s source=%s\n' "$STATE_DIR" "$STATE_DIR_SOURCE" + ` + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + got := string(out) + resolvedStateDir, err := filepath.EvalSymlinks(stateDir) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(got, "state="+resolvedStateDir) || !strings.Contains(got, "source=recovered") { + t.Fatalf("saved installer did not discover adjacent custom state:\n%s", got) + } +} + +func TestInstallSHDiscoversCustomStateDirFromGeneratedSystemdUnit(t *testing.T) { + for _, commandsEnabled := range []bool{false, true} { + t.Run(map[bool]string{false: "commands_disabled", true: "commands_enabled"}[commandsEnabled], func(t *testing.T) { + root := t.TempDir() + customState := filepath.Join(root, "custom state") + defaultState := filepath.Join(root, "default") + if err := os.MkdirAll(customState, 0700); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(defaultState, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(customState, "token"), []byte("custom123"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(defaultState, "token"), []byte("default999"), 0600); err != nil { + t.Fatal(err) + } + commandArg := "" + if commandsEnabled { + commandArg = " --enable-commands" + } + unitPath := filepath.Join(root, "pulse-agent.service") + unit := `[Service] +ExecStart=/usr/local/bin/pulse-agent --url https://custom.example --token-file ` + + strings.ReplaceAll(filepath.Join(customState, "token"), " ", `\ `) + + ` --state-dir ` + strings.ReplaceAll(customState, " ", `\ `) + commandArg + "\n" + if err := os.WriteFile(unitPath, []byte(unit), 0644); err != nil { + t.Fatal(err) + } + + script := ` + set -euo pipefail + AGENT_NAME="pulse-agent" + PULSE_URL="" + PULSE_TOKEN="" + INTERVAL="30s" + INTERVAL_EXPLICIT="false" + ENABLE_HOST="true" + HOST_EXPLICIT="false" + ENABLE_DOCKER="" + DOCKER_EXPLICIT="false" + ENABLE_KUBERNETES="" + KUBERNETES_EXPLICIT="false" + KUBECONFIG_PATH="" + ENABLE_PROXMOX="" + PROXMOX_EXPLICIT="false" + PROXMOX_TYPE="" + INSECURE="false" + ENABLE_COMMANDS="false" + ENROLL="false" + HEALTH_ADDR="" + HEALTH_ADDR_SET="false" + AGENT_ID="" + HOSTNAME_OVERRIDE="" + STATE_DIR="` + defaultState + `" + STATE_DIR_SOURCE="default" + DEFAULT_STATE_DIR="` + defaultState + `" + TRUENAS_STATE_DIR="` + filepath.Join(root, "truenas") + `" + CURL_CA_BUNDLE="" + SERVER_FINGERPRINT="" + OBSERVERS_FILE="" + KUBE_INCLUDE_ALL_PODS="false" + KUBE_INCLUDE_ALL_DEPLOYMENTS="false" + DISK_EXCLUDES=() + log_warn() { :; } + systemctl() { printf '%s\n' "` + unitPath + `"; } +` + extractInstallShellFunction(t, "strip_recovered_arg_quotes") + ` +` + extractInstallShellFunction(t, "normalize_recovered_agent_arg_key") + ` +` + extractInstallShellFunction(t, "apply_recovered_agent_arg_value") + ` +` + extractInstallShellFunction(t, "recovered_connection_state_ready") + ` +` + extractInstallShellFunction(t, "recover_token_from_default_agent_token_file") + ` +` + extractInstallShellFunction(t, "recover_connection_state_from_arg_stream") + ` +` + extractInstallShellFunction(t, "recover_connection_state_from_env_stream") + ` +` + extractInstallShellFunction(t, "split_recovered_shell_words") + ` +` + extractInstallShellFunction(t, "recover_connection_state_from_systemd_unit") + ` +` + extractInstallShellFunction(t, "remove_agent_state_dir") + ` + recover_connection_state_from_systemd_unit + printf 'state=%s source=%s url=%s token=%s commands=%s\n' \ + "$STATE_DIR" "$STATE_DIR_SOURCE" "$PULSE_URL" "$PULSE_TOKEN" "$ENABLE_COMMANDS" + remove_agent_state_dir "$STATE_DIR" + ` + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + got := string(out) + for _, want := range []string{ + "state=" + customState, + "source=recovered", + "url=https://custom.example", + "token=custom123", + "commands=" + map[bool]string{false: "false", true: "true"}[commandsEnabled], + } { + if !strings.Contains(got, want) { + t.Fatalf("systemd discovery missing %q:\n%s", want, got) + } + } + if strings.Contains(got, "default999") { + t.Fatalf("systemd discovery borrowed default token:\n%s", got) + } + if _, err := os.Stat(customState); !os.IsNotExist(err) { + t.Fatalf("uninstall did not remove discovered custom state: %v", err) + } + if _, err := os.Stat(defaultState); err != nil { + t.Fatalf("uninstall removed the default instance instead of discovered custom state: %v", err) + } + }) + } +} + +func TestInstallSHDiscoversCustomStateDirFromGeneratedLaunchdPlist(t *testing.T) { + root := t.TempDir() + customState := filepath.Join(root, "custom & state") + if err := os.MkdirAll(customState, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(customState, "token"), []byte("launch123"), 0600); err != nil { + t.Fatal(err) + } + plistPath := filepath.Join(root, "com.pulse.agent.plist") + escapedState := strings.ReplaceAll(customState, "&", "&") + plist := ` + +ProgramArguments + +/usr/local/bin/pulse-agent +--url +https://launch.example +--token-file +` + escapedState + `/token +--state-dir +` + escapedState + ` +--enable-commands + +` + if err := os.WriteFile(plistPath, []byte(plist), 0644); err != nil { + t.Fatal(err) + } + + script := ` + set -euo pipefail + PULSE_URL="" + PULSE_TOKEN="" + INTERVAL="30s" + INTERVAL_EXPLICIT="false" + ENABLE_HOST="true" + HOST_EXPLICIT="false" + ENABLE_DOCKER="" + DOCKER_EXPLICIT="false" + ENABLE_KUBERNETES="" + KUBERNETES_EXPLICIT="false" + KUBECONFIG_PATH="" + ENABLE_PROXMOX="" + PROXMOX_EXPLICIT="false" + PROXMOX_TYPE="" + INSECURE="false" + ENABLE_COMMANDS="false" + ENROLL="false" + HEALTH_ADDR="" + HEALTH_ADDR_SET="false" + AGENT_ID="" + HOSTNAME_OVERRIDE="" + STATE_DIR="/var/lib/pulse-agent" + STATE_DIR_SOURCE="default" + DEFAULT_STATE_DIR="/var/lib/pulse-agent" + TRUENAS_STATE_DIR="/data/pulse-agent" + CURL_CA_BUNDLE="" + SERVER_FINGERPRINT="" + OBSERVERS_FILE="" + KUBE_INCLUDE_ALL_PODS="false" + KUBE_INCLUDE_ALL_DEPLOYMENTS="false" + DISK_EXCLUDES=() +` + extractInstallShellFunction(t, "strip_recovered_arg_quotes") + ` +` + extractInstallShellFunction(t, "normalize_recovered_agent_arg_key") + ` +` + extractInstallShellFunction(t, "apply_recovered_agent_arg_value") + ` +` + extractInstallShellFunction(t, "recovered_connection_state_ready") + ` +` + extractInstallShellFunction(t, "recover_token_from_default_agent_token_file") + ` +` + extractInstallShellFunction(t, "recover_connection_state_from_arg_stream") + ` +` + extractInstallShellFunction(t, "launchd_agent_arg_stream") + ` +` + extractInstallShellFunction(t, "recover_connection_state_from_launchd_plist") + ` + recover_connection_state_from_launchd_plist "` + plistPath + `" + printf 'state=%s url=%s token=%s commands=%s\n' "$STATE_DIR" "$PULSE_URL" "$PULSE_TOKEN" "$ENABLE_COMMANDS" + ` + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + got := string(out) + for _, want := range []string{ + "state=" + customState, + "url=https://launch.example", + "token=launch123", + "commands=true", + } { + if !strings.Contains(got, want) { + t.Fatalf("launchd discovery missing %q:\n%s", want, got) + } + } +} + +func TestInstallSHConnectionEnvPersistsCanonicalStateDirWithoutTokenValue(t *testing.T) { + stateDir := filepath.Join(t.TempDir(), "state") + script := ` + set -euo pipefail + STATE_DIR="` + stateDir + `" + PULSE_URL="https://pulse.example.com" + PULSE_TOKEN="deadbeef" + RUNTIME_TOKEN_FILE="$STATE_DIR/token" + AGENT_ID="agent-custom" + HOSTNAME_OVERRIDE="host-custom" + INSECURE="false" + SERVER_FINGERPRINT="" + CURL_CA_BUNDLE="" + SAVED_INSTALL_SCRIPT="" +` + extractInstallShellFunction(t, "write_connection_state_value") + ` +` + extractInstallShellFunction(t, "save_connection_info") + ` + curl() { return 1; } + save_connection_info "$STATE_DIR" + cat "$STATE_DIR/connection.env" + ` + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + got := string(out) + if !strings.Contains(got, "PULSE_STATE_DIR='"+stateDir+"'") || + !strings.Contains(got, "PULSE_TOKEN_FILE='"+filepath.Join(stateDir, "token")+"'") { + t.Fatalf("connection.env did not persist canonical state paths:\n%s", got) + } + if strings.Contains(got, "PULSE_TOKEN='") || strings.Contains(got, "deadbeef") { + t.Fatalf("connection.env leaked the token value:\n%s", got) + } + info, err := os.Stat(filepath.Join(stateDir, "connection.env")) + if err != nil { + t.Fatal(err) + } + if gotMode := info.Mode().Perm(); gotMode != 0600 { + t.Fatalf("connection.env mode = %o, want 600", gotMode) + } +} + +func TestInstallSHStateWritesReplaceSymlinksAtomically(t *testing.T) { + stateDir := filepath.Join(t.TempDir(), "state") + if err := os.MkdirAll(stateDir, 0700); err != nil { + t.Fatal(err) + } + victimToken := filepath.Join(t.TempDir(), "victim-token") + victimConnection := filepath.Join(t.TempDir(), "victim-connection") + if err := os.WriteFile(victimToken, []byte("victim-token-content"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(victimConnection, []byte("victim-connection-content"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(victimToken, filepath.Join(stateDir, "token")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(victimConnection, filepath.Join(stateDir, "connection.env")); err != nil { + t.Fatal(err) + } + + script := ` + set -euo pipefail + STATE_DIR="` + stateDir + `" + PULSE_URL="https://pulse.example.com" + IFS= read -r PULSE_TOKEN + RUNTIME_TOKEN_FILE="" + RUNTIME_TOKEN_CHANGED="false" + ENROLL="false" + AGENT_ID="agent-one" + HOSTNAME_OVERRIDE="host-one" + INSECURE="false" + SERVER_FINGERPRINT="" + CURL_CA_BUNDLE="" + SAVED_INSTALL_SCRIPT="" + NON_INTERACTIVE="true" + TMP_FILES=() + log_info() { :; } + curl() { return 1; } +` + extractInstallShellFunction(t, "write_connection_state_value") + ` +` + extractInstallShellFunction(t, "ensure_runtime_token_file") + ` +` + extractInstallShellFunction(t, "save_connection_info") + ` + ensure_runtime_token_file "$STATE_DIR" + save_connection_info "$STATE_DIR" + ` + cmd := exec.Command("bash", "-c", script) + cmd.Stdin = strings.NewReader("new-token-content\n") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + + for path, want := range map[string]string{ + victimToken: "victim-token-content", + victimConnection: "victim-connection-content", + } { + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != want { + t.Fatalf("state write followed symlink and modified %s: %q", path, got) + } + } + for _, path := range []string{filepath.Join(stateDir, "token"), filepath.Join(stateDir, "connection.env")} { + info, err := os.Lstat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode()&os.ModeSymlink != 0 { + t.Fatalf("%s remained a symlink after atomic replacement", path) + } + } +} + +func TestInstallSHCurlTokenTransportKeepsSecretOutOfArgv(t *testing.T) { + recordDir := t.TempDir() + argsPath := filepath.Join(recordDir, "args") + configPath := filepath.Join(recordDir, "config") + script := ` + set -euo pipefail + PULSE_TOKEN="feedface" + curl() { + printf '%s\n' "$@" > "` + argsPath + `" + while [[ $# -gt 0 ]]; do + if [[ "$1" == "--config" ]]; then + cp "$2" "` + configPath + `" + shift 2 + continue + fi + shift + done + } +` + extractInstallShellFunction(t, "curl_with_pulse_token") + ` + curl_with_pulse_token -sS https://pulse.example.com/api/agents/agent/lookup + ` + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + args, err := os.ReadFile(argsPath) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(args), "feedface") || strings.Contains(string(args), "X-API-Token") { + t.Fatalf("curl argv leaked token material:\n%s", args) + } + config, err := os.ReadFile(configPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(config), "X-API-Token: feedface") { + t.Fatalf("private curl config did not carry auth header:\n%s", config) + } +} + func TestSetupUpdateCommandHonorsRCChannelAndCustomPaths(t *testing.T) { tmpDir := t.TempDir() updatePath := filepath.Join(tmpDir, "update") @@ -3783,6 +4283,7 @@ func TestSetupAutoUpdatesPreservesRCChannelWhenUpdatingExistingConfig(t *testing // actionable error instead of leaving a silent 401 loop behind (issue #1515). func TestInstallSHVerifyAgentServerRegistrationDetectsRejectedToken(t *testing.T) { urlEncode := extractInstallShellFunction(t, "url_encode") + curlWithPulseToken := extractInstallShellFunction(t, "curl_with_pulse_token") verifyFn := extractInstallShellFunction(t, "verify_agent_server_registration") cases := []struct { @@ -3816,6 +4317,7 @@ func TestInstallSHVerifyAgentServerRegistrationDetectsRejectedToken(t *testing.T HOSTNAME_OVERRIDE="omv" INSECURE="false" CURL_CA_BUNDLE="" +` + curlWithPulseToken + ` ` + urlEncode + ` ` + verifyFn + ` verify_agent_server_registration diff --git a/testdata/issue1595_sas_topology.json b/testdata/issue1595_sas_topology.json new file mode 100644 index 000000000..84ed45e93 --- /dev/null +++ b/testdata/issue1595_sas_topology.json @@ -0,0 +1,47 @@ +{ + "issue": 1595, + "description": "Synthetic public-evidence topology: 24 same-model SAS disks, two HBAs, two ZFS pools. Serial values and counters are synthetic.", + "node": "nuclear", + "instance": "pve-lab", + "agentId": "agent-nuclear", + "model": "ST18000NM019J", + "sizeBytes": 18000207937536, + "controllers": [ + { + "id": "0000:03:00.0", + "pool": "tank-a", + "disks": [ + {"device":"sda","target":"6:0:0:0","serial":"ZR5TESTA0001","providerSerial":"5000c500a0000001","temperature":30,"readBytes":1000000,"writeBytes":2000000,"ioTimeMs":100}, + {"device":"sdb","target":"6:0:1:0","serial":"ZR5TESTA0002","providerSerial":"5000c500a0000002","temperature":31,"readBytes":1100000,"writeBytes":2100000,"ioTimeMs":110}, + {"device":"sdc","target":"6:0:2:0","serial":"ZR5TESTA0003","providerSerial":"5000c500a0000003","temperature":32,"readBytes":1200000,"writeBytes":2200000,"ioTimeMs":120}, + {"device":"sdd","target":"6:0:3:0","serial":"ZR5TESTA0004","providerSerial":"5000c500a0000004","temperature":33,"readBytes":1300000,"writeBytes":2300000,"ioTimeMs":130}, + {"device":"sde","target":"6:0:4:0","serial":"ZR5TESTA0005","providerSerial":"5000c500a0000005","temperature":34,"readBytes":1400000,"writeBytes":2400000,"ioTimeMs":140}, + {"device":"sdf","target":"6:0:5:0","serial":"ZR5TESTA0006","providerSerial":"5000c500a0000006","temperature":35,"readBytes":1500000,"writeBytes":2500000,"ioTimeMs":150}, + {"device":"sdg","target":"6:0:6:0","serial":"ZR5TESTA0007","providerSerial":"5000c500a0000007","temperature":36,"readBytes":1600000,"writeBytes":2600000,"ioTimeMs":160}, + {"device":"sdh","target":"6:0:7:0","serial":"ZR5TESTA0008","providerSerial":"5000c500a0000008","temperature":37,"readBytes":1700000,"writeBytes":2700000,"ioTimeMs":170}, + {"device":"sdi","target":"6:0:8:0","serial":"ZR5TESTA0009","providerSerial":"5000c500a0000009","temperature":38,"readBytes":1800000,"writeBytes":2800000,"ioTimeMs":180}, + {"device":"sdj","target":"6:0:9:0","serial":"ZR5TESTA0010","providerSerial":"5000c500a0000010","temperature":39,"readBytes":1900000,"writeBytes":2900000,"ioTimeMs":190}, + {"device":"sdk","target":"6:0:10:0","serial":"ZR5TESTA0011","providerSerial":"5000c500a0000011","temperature":40,"readBytes":2000000,"writeBytes":3000000,"ioTimeMs":200}, + {"device":"sdl","target":"6:0:11:0","serial":"ZR5TESTA0012","providerSerial":"5000c500a0000012","temperature":41,"readBytes":2100000,"writeBytes":3100000,"ioTimeMs":210} + ] + }, + { + "id": "0000:04:00.0", + "pool": "tank-b", + "disks": [ + {"device":"sdm","target":"7:0:0:0","serial":"ZR5TESTB0001","providerSerial":"5000c500b0000001","temperature":30,"readBytes":3000000,"writeBytes":4000000,"ioTimeMs":300}, + {"device":"sdn","target":"7:0:1:0","serial":"ZR5TESTB0002","providerSerial":"5000c500b0000002","temperature":31,"readBytes":3100000,"writeBytes":4100000,"ioTimeMs":310}, + {"device":"sdo","target":"7:0:2:0","serial":"ZR5TESTB0003","providerSerial":"5000c500b0000003","temperature":32,"readBytes":3200000,"writeBytes":4200000,"ioTimeMs":320}, + {"device":"sdp","target":"7:0:3:0","serial":"ZR5TESTB0004","providerSerial":"5000c500b0000004","temperature":33,"readBytes":3300000,"writeBytes":4300000,"ioTimeMs":330}, + {"device":"sdq","target":"7:0:4:0","serial":"ZR5TESTB0005","providerSerial":"5000c500b0000005","temperature":34,"readBytes":3400000,"writeBytes":4400000,"ioTimeMs":340}, + {"device":"sdr","target":"7:0:5:0","serial":"ZR5TESTB0006","providerSerial":"5000c500b0000006","temperature":35,"readBytes":3500000,"writeBytes":4500000,"ioTimeMs":350}, + {"device":"sds","target":"7:0:6:0","serial":"ZR5TESTB0007","providerSerial":"5000c500b0000007","temperature":36,"readBytes":3600000,"writeBytes":4600000,"ioTimeMs":360}, + {"device":"sdt","target":"7:0:7:0","serial":"ZR5TESTB0008","providerSerial":"5000c500b0000008","temperature":37,"readBytes":3700000,"writeBytes":4700000,"ioTimeMs":370}, + {"device":"sdu","target":"7:0:8:0","serial":"ZR5TESTB0009","providerSerial":"5000c500b0000009","temperature":38,"readBytes":3800000,"writeBytes":4800000,"ioTimeMs":380}, + {"device":"sdv","target":"7:0:9:0","serial":"ZR5TESTB0010","providerSerial":"5000c500b0000010","temperature":39,"readBytes":3900000,"writeBytes":4900000,"ioTimeMs":390}, + {"device":"sdw","target":"7:0:10:0","serial":"ZR5TESTB0011","providerSerial":"5000c500b0000011","temperature":40,"readBytes":4000000,"writeBytes":5000000,"ioTimeMs":400}, + {"device":"sdx","target":"7:0:11:0","serial":"ZR5TESTB0012","providerSerial":"5000c500b0000012","temperature":41,"readBytes":4100000,"writeBytes":5100000,"ioTimeMs":410} + ] + } + ] +}