From ab978b6cfc4a27749e653f975d729609c8d53f52 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 23:57:05 +0100 Subject: [PATCH 1/4] Keep Podman CPU interval evidence when one-shot percentages are zero Retain a baseline on every response and fall back to interval counters for zero, absent or invalid compatibility percentages. Preserve positive native units and Docker calculations, rebaseline on restarts and reject out-of-order intervals. Add connected collector controls and the matching monitoring and agent contracts. Addresses the source gap reported in #2447; native payload and reporter acceptance remain unverified. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 15 ++ .../v6/internal/subsystems/monitoring.md | 14 ++ internal/dockeragent/agent.go | 1 + internal/dockeragent/agent_cpu_test.go | 24 ++-- internal/dockeragent/agent_internal_test.go | 134 ++++++++++++++++++ internal/dockeragent/collect.go | 29 +++- 6 files changed, 200 insertions(+), 17 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 4754a53d0d..f7aba26ab8 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,21 @@ ## Purpose +### Podman interval CPU evidence — issue #2447 + +Every one-shot container stats response retains a cumulative CPU baseline, +even when a positive Podman compatibility percentage takes precedence. Zero, +absent or invalid percentages use the valid nanosecond/wall-clock interval, +without Docker's host-core multiplier. First samples, counter resets and changes +of container start time rebaseline rather than presenting lifetime CPU as current +usage. Repeated or older read timestamps cannot move the baseline backwards. +Positive native percentages keep the units established by #1391; a genuine idle +counter interval remains zero. Report shapes, enrollment, runtime transport trust +and command authority are unchanged. `TestCollectContainerPodmanCPUIntervals` +in `internal/dockeragent/agent_internal_test.go` exercises decoding through the +real collector, idle/advancing counters, start changes and Docker isolation. +Synthetic controls do not establish a native Podman payload or reporter recovery. + ### MD RAID required members and spares — issue #2369 Host RAID reports carry optional `requiredDevices`: the configured member count diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 26ce03a228..38bca29d81 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -17,6 +17,20 @@ ## Purpose +### Podman zero-percentage fallback — issue #2447 + +One-shot compatibility stats with `cpu:0` do not suppress advancing cumulative +CPU samples. The collector records every interval and uses Podman's CPU-time +nanoseconds over elapsed wall-clock nanoseconds, independent of host core count. +A positive finite reported percentage remains authoritative, preserving #1391. +Counter resets or a changed container start establish a new baseline; duplicate +and out-of-order reads cannot create subsequent spikes. No timestamp means no +measurable counter interval. The existing report CPU field, graph/alert units, +Docker system-counter calculation, memory and permission scopes are unchanged. +`TestCollectContainerPodmanCPUIntervals` in `agent_internal_test.go` covers the +actual JSON/collector boundary, including changing percentage availability. +Native Podman shape and graph/alert acceptance remain separate evidence. + ### Guest-agent coordination around backups — issue #2439 All PVE QEMU guest-agent reads, including disk diagnostics and legacy meminfo diff --git a/internal/dockeragent/agent.go b/internal/dockeragent/agent.go index 0c268a91a3..4872bdc4fe 100644 --- a/internal/dockeragent/agent.go +++ b/internal/dockeragent/agent.go @@ -229,6 +229,7 @@ type cpuSample struct { systemUsage uint64 onlineCPUs uint32 read time.Time + startedAt time.Time } // New creates a new Docker / Podman module instance. diff --git a/internal/dockeragent/agent_cpu_test.go b/internal/dockeragent/agent_cpu_test.go index f719bff24c..3bbfefee98 100644 --- a/internal/dockeragent/agent_cpu_test.go +++ b/internal/dockeragent/agent_cpu_test.go @@ -28,7 +28,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { OnlineCPUs: 2, }, } - got := agent.calculateContainerCPUPercent("container-123456", stats1) + got := agent.calculateContainerCPUPercent("container-123456", stats1, time.Time{}) if got != 0 { t.Fatalf("first call: expected 0, got %f", got) } @@ -45,7 +45,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { OnlineCPUs: 2, }, } - got = agent.calculateContainerCPUPercent("container-123456", stats2) + got = agent.calculateContainerCPUPercent("container-123456", stats2, time.Time{}) if got <= 0 { t.Fatalf("second call: expected percent > 0, got %f", got) } @@ -68,7 +68,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - got := agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}) if got != 0 { t.Fatalf("expected 0, got %f", got) } @@ -103,7 +103,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - got := agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}) if got <= 0 { t.Fatalf("expected percent > 0, got %f", got) } @@ -136,7 +136,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - got := agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}) if got <= 0 { t.Fatalf("expected percent > 0, got %f", got) } @@ -168,7 +168,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - got := agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}) if got <= 0 { t.Fatalf("expected percent > 0, got %f", got) } @@ -200,7 +200,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - if got := agent.calculateContainerCPUPercent("container-123456", stats); got != 0 { + if got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}); got != 0 { t.Fatalf("expected 0, got %f", got) } }) @@ -232,7 +232,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - if got := agent.calculateContainerCPUPercent("container-123456", stats); got != 0 { + if got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}); got != 0 { t.Fatalf("expected 0, got %f", got) } }) @@ -263,7 +263,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - if got := agent.calculateContainerCPUPercent("container-123456", stats); got != 0 { + if got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}); got != 0 { t.Fatalf("expected 0, got %f", got) } }) @@ -291,7 +291,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { PreCPUStats: containertypes.CPUStats{}, } - got := agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}) if got != 0 { t.Fatalf("expected 0, got %f", got) } @@ -319,7 +319,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - got := agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}) if got != 0 { t.Fatalf("expected 0 on first call (manual tracking), got %f", got) } @@ -349,7 +349,7 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - got := agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats, time.Time{}) if math.Abs(got-25.0) > 0.01 { t.Fatalf("expected podman cpu percent near 25.0, got %f", got) } diff --git a/internal/dockeragent/agent_internal_test.go b/internal/dockeragent/agent_internal_test.go index 193caaf5bb..c4bffdc700 100644 --- a/internal/dockeragent/agent_internal_test.go +++ b/internal/dockeragent/agent_internal_test.go @@ -1840,3 +1840,137 @@ func TestCollectorRuntimeBoundaryLossIsDaemonUnavailable(t *testing.T) { } } } + +// Exercise the compatibility JSON through the actual container collector, not +// just the delta helper. These are synthetic inputs, not a native Podman trace. +func TestCollectContainerPodmanCPUIntervals(t *testing.T) { + type sample struct { + second int + total uint64 + percent string // Empty means absent; other values are literal JSON. + start int + stopped bool + want float64 + } + tests := []struct { + name string + runtime RuntimeKind + cpus int + samples []sample + }{ + {"zero with advancing and idle counters", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "0"}, + {second: 1, total: 130_000_000, percent: "0", want: 3}, + {second: 2, total: 130_000_000, percent: "0"}, + {second: 3, total: 1_630_000_000, percent: "0", want: 150}, + }}, + {"absent percentage uses nanosecond wall clock units", RuntimePodman, 1, []sample{ + {second: 0, total: 100_000_000}, + {second: 1, total: 130_000_000, want: 3}, + }}, + {"null percentage", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "null"}, + {second: 1, total: 130_000_000, percent: "null", want: 3}, + }}, + {"positive percentages keep units and retain the baseline", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "0.32", want: 0.32}, + {second: 1, total: 130_000_000, percent: "0.64", want: 0.64}, + {second: 2, total: 160_000_000, percent: "0", want: 3}, + {second: 3, total: 190_000_000, want: 3}, + }}, + {"absent then reported then zero", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000}, + {second: 1, total: 130_000_000, percent: "0.32", want: 0.32}, + {second: 2, total: 160_000_000, percent: "0", want: 3}, + }}, + {"invalid negative percentage", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "-1"}, + {second: 1, total: 130_000_000, percent: "-1", want: 3}, + }}, + {"unparseable optional percentage", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: `"unknown"`}, + {second: 1, total: 130_000_000, percent: `"unknown"`, want: 3}, + }}, + {"counter reset rebaselines rather than using new lifetime", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "0"}, + {second: 1, total: 10_000_000, percent: "0"}, + {second: 2, total: 40_000_000, percent: "0", want: 3}, + }}, + {"restart with an already larger counter", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "0"}, + {second: 1, total: 260_000_000, percent: "0", start: 1}, + {second: 2, total: 290_000_000, percent: "0", start: 1, want: 3}, + }}, + {"repeated and old timestamps do not move the baseline", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "0"}, + {second: 1, total: 130_000_000, percent: "0", want: 3}, + {second: 1, total: 150_000_000, percent: "0"}, + {second: 0, total: 170_000_000, percent: "0"}, + {second: 2, total: 160_000_000, percent: "0", want: 3}, + }}, + {"missing timestamp cannot invent an interval", RuntimePodman, 16, []sample{ + {second: -1, total: 100_000_000, percent: "0"}, + {second: 1, total: 130_000_000, percent: "0"}, + {second: 2, total: 160_000_000, percent: "0", want: 3}, + }}, + {"stop clears the baseline", RuntimePodman, 16, []sample{ + {second: 0, total: 100_000_000, percent: "0.32", want: 0.32}, + {second: 1, stopped: true}, + {second: 2, total: 260_000_000, percent: "0"}, + {second: 3, total: 290_000_000, percent: "0", want: 3}, + }}, + {"docker ignores compat percentage and retains system delta", RuntimeDocker, 16, []sample{ + {second: 0, total: 100_000_000, percent: "0.32"}, + {second: 1, total: 130_000_000, percent: "0.32", want: 48}, + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + epoch := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC) + var current sample + agent := &Agent{ + runtime: tt.runtime, + logger: zerolog.Nop(), + cpuCount: tt.cpus, + docker: &fakeDockerClient{ + containerInspectWithRawFn: func(context.Context, string, bool) (containertypes.InspectResponse, []byte, error) { + inspect := baseInspect() + inspect.State = &containertypes.State{ + Running: !current.stopped, + StartedAt: epoch.Add(time.Duration(current.start-60) * time.Second).Format(time.RFC3339Nano), + } + return inspect, nil, nil + }, + containerStatsOneShotFn: func(context.Context, string) (dockerStatsResponseReader, error) { + read := "null" + if current.second >= 0 { + read = fmt.Sprintf("%q", epoch.Add(time.Duration(current.second)*time.Second).Format(time.RFC3339Nano)) + } + percent := "" + if current.percent != "" { + percent = `,"cpu":` + current.percent + } + payload := fmt.Sprintf(`{"read":%s,"cpu_stats":{"cpu_usage":{"total_usage":%d},"system_cpu_usage":%d,"online_cpus":%d%s},"memory_stats":{"usage":1000000,"limit":4000000}}`, + read, current.total, uint64(current.second+2)*1_000_000_000, tt.cpus, percent) + return dockerStatsResponseReader{Body: io.NopCloser(strings.NewReader(payload))}, nil + }, + }, + } + for i, s := range tt.samples { + current = s + container, err := agent.collectContainer(context.Background(), containertypes.Summary{ + ID: "container-123456", Names: []string{"/cpu-fixture"}, Image: "fixture:local", + }) + if err != nil { + t.Fatalf("sample %d: %v", i, err) + } + if math.Abs(container.CPUPercent-s.want) > 1e-9 { + t.Errorf("sample %d: CPU = %g%%, want %g%%", i, container.CPUPercent, s.want) + } + if !s.stopped && (container.MemoryUsageBytes != 1_000_000 || container.MemoryLimitBytes != 4_000_000) { + t.Fatalf("sample %d: memory collection changed: %+v", i, container) + } + } + }) + } +} diff --git a/internal/dockeragent/collect.go b/internal/dockeragent/collect.go index 28c8b92874..9ca434c9f8 100644 --- a/internal/dockeragent/collect.go +++ b/internal/dockeragent/collect.go @@ -601,10 +601,14 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum return agentsdocker.Container{}, fmt.Errorf("decode stats: %w", err) } - if a.runtime == RuntimePodman && podmanCPUPercent != nil { - cpuPercent = safeFloat(*podmanCPUPercent) - } else { - cpuPercent = a.calculateContainerCPUPercent(summary.ID, stats) + // Keep a baseline even when Podman supplies a percentage: one-shot + // compatibility responses can report cpu:0 while total_usage advances. + // Preserve positive native percentages (and their units, #1391), but + // use our interval delta for an absent, zero or invalid percentage. + cpuPercent = a.calculateContainerCPUPercent(summary.ID, stats, parseTime(inspect.State.StartedAt)) + if a.runtime == RuntimePodman && podmanCPUPercent != nil && + *podmanCPUPercent > 0 && !math.IsInf(*podmanCPUPercent, 0) { + cpuPercent = *podmanCPUPercent } memUsage, memLimit, memPercent = calculateMemoryUsage(stats) blockIO = summarizeBlockIO(stats) @@ -1138,7 +1142,7 @@ func decodeContainerStatsPayload(payload []byte) (containertypes.StatsResponse, return stats, probe.CPUStats.CPU, nil } -func (a *Agent) calculateContainerCPUPercent(containerID string, stats containertypes.StatsResponse) float64 { +func (a *Agent) calculateContainerCPUPercent(containerID string, stats containertypes.StatsResponse, startedAt time.Time) float64 { a.cpuMu.Lock() defer a.cpuMu.Unlock() @@ -1147,6 +1151,7 @@ func (a *Agent) calculateContainerCPUPercent(containerID string, stats container systemUsage: stats.CPUStats.SystemUsage, onlineCPUs: stats.CPUStats.OnlineCPUs, read: stats.Read, + startedAt: startedAt, } // Always use manual delta tracking. Docker's PreCPUStats is unreliable for @@ -1155,6 +1160,14 @@ func (a *Agent) calculateContainerCPUPercent(containerID string, stats container // (from container start) and producing a constant lifetime-average CPU% // instead of a current value. prev, ok := a.prevContainerCPU[containerID] + if a.runtime == RuntimePodman && ok && !prev.read.IsZero() && !current.read.IsZero() && !current.read.After(prev.read) { + // Repeated or out-of-order responses must not move the baseline back + // and manufacture usage in the next interval. + return 0 + } + if a.prevContainerCPU == nil { + a.prevContainerCPU = make(map[string]cpuSample) + } if !ok { // First time seeing this container - store current sample and return 0 // On next collection cycle we'll have a previous sample to compare against @@ -1169,6 +1182,12 @@ func (a *Agent) calculateContainerCPUPercent(containerID string, stats container // We have a previous sample - update it after calculation a.prevContainerCPU[containerID] = current + if a.runtime == RuntimePodman && (current.totalUsage < prev.totalUsage || + (!prev.startedAt.IsZero() && !current.startedAt.IsZero() && !current.startedAt.Equal(prev.startedAt))) { + // A restart is not an interval sample, even if the new lifetime counter + // has already exceeded the old one. Start again from this baseline. + return 0 + } var totalDelta float64 if current.totalUsage >= prev.totalUsage { From 9677b5cd60758f01fd9f3dd9eeeeb58e82ae3ed7 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 23:58:11 +0100 Subject: [PATCH 2/4] Separate TrueNAS probes from runtime poll and readiness evidence A saved connection test reads system.info on a separate client. Keep both probe results out of runtime timestamps, failures, retry cadence and inventory readiness. Preserve stored-secret and tenant boundaries, and validate the existing API evidence through real local client/poller/handler controls. Update the matching API, monitoring, lifecycle and recovery contracts together. This repairs the diagnostic gap in #2382; its native polling/session cause remains unverified. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 11 +++ .../v6/internal/subsystems/api-contracts.md | 15 ++++ .../v6/internal/subsystems/monitoring.md | 12 ++- .../internal/subsystems/storage-recovery.md | 11 +++ .../connections_handlers_continuity_test.go | 28 ++----- internal/api/contract_test.go | 64 +++++++++++++++ internal/api/truenas_handlers.go | 14 +--- internal/api/truenas_handlers_test.go | 10 +-- internal/api/truenas_poll_evidence_test.go | 81 +++++++++++++++++++ internal/monitoring/truenas_poller.go | 49 ----------- internal/monitoring/truenas_poller_test.go | 24 +++--- 11 files changed, 223 insertions(+), 96 deletions(-) create mode 100644 internal/api/truenas_poll_evidence_test.go diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index f7aba26ab8..4ed5d3ff47 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,17 @@ ## Purpose +### TrueNAS probes are not fleet or inventory liveness + +A separate settings connection probe does not establish runtime provider +freshness, verified fleet credentials, observed resources or readiness for +monitored-system accounting. These remain tied to actual scheduled refreshes. +Stored-secret selection, tenant isolation and authentication responses remain +unchanged. The connections continuity test seeds its success from a real local +runtime poll rather than a probe-shaped synthetic success. The API contract +control retains a runtime inventory error despite a subsequent successful +system-information probe; no synthetic success establishes installed acceptance. + ### Podman interval CPU evidence — issue #2447 Every one-shot container stats response retains a cumulative CPU baseline, diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index deffbfb875..32da5c32ad 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -20,6 +20,21 @@ ## Purpose +### TrueNAS saved probes do not rewrite runtime evidence — issue #2382 + +The saved connection test uses stored server-owned secrets, or merges an edited +payload under the existing validation and tenant boundary. It reads system +information on a separate client and reports the probe's result in its existing +response. Neither success nor failure rewrites the runtime provider's `poll` +timestamps, last error, failure count, retry schedule, `observed` contribution +or supplemental-inventory readiness. A successful probe can coexist with failed +required inventory collection; only a completed runtime refresh can clear that +runtime failure. No new route, response field or permission is added. +`TestTrueNASSavedProbePreservesRuntimeEvidence` in `internal/api/contract_test.go` +uses an HTTP appliance analogue through the actual client, poller and handlers, +including prior inventory followed by required-method failure and no initial poll. +This is diagnostic truthfulness, not native appliance/session recovery. + ### Resource maintenance API reference The canonical and shipped API references document the existing authenticated diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 38bca29d81..c2e2bb42c7 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -273,8 +273,11 @@ or appliance health, and sending keepalives is not proactive pong-timeout detect Successful TrueNAS refreshes target start-to-start cadence, bounded below by completion plus min(five seconds, configured interval), preventing back-to-back load on slow appliances. Failed refreshes retain a full completion-based retry -interval. Manual connection tests retain completion-based scheduling. Observed -last-attempt and last-success timestamps remain completion timestamps; resource +interval. Manual connection probes use a separate client and do not alter +scheduling or poll evidence: success must not clear a runtime error or advance +last-attempt, last-success, observed collection or inventory readiness. Failure +remains in the probe response, not a fabricated runtime attempt. Observed +last-attempt and last-success timestamps remain runtime completion timestamps; resource freshness thresholds must not be extended to hide genuinely stale devices. `TestAuthenticatedRPCSurvivesIdleTransportTimeout` and @@ -285,6 +288,11 @@ server, opening-context cancellation, concurrent controls/RPCs and sender dispos `internal/monitoring/truenas_poller_test.go` verifies due boundaries, short/slow cycles, completion timestamps and unchanged failure backoff. These are synthetic runtime proofs, not native firmware timeout or reporter-resolution evidence. +`TestTrueNASSavedProbePreservesRuntimeEvidence` in `internal/api/contract_test.go` +checks the actual saved probe and list handlers against never-polled and +previously observed-but-failing runtime states. A synthetic successful login +with a required pool-method failure is not attribution of the native #2382 +regression; the failing appliance/session method remains field evidence. ### TrueNAS empty app inventories and session-disposal evidence diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 0d7f50f36c..d451abd1df 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -21,6 +21,17 @@ ## Purpose +### TrueNAS connection probes do not settle recovery inventory + +A successful settings probe reads system information only, on a separate client. +It cannot advance the runtime poll watermark, manufacture an initial settled +inventory or update the last observed recovery/resource contribution. A probe +failure likewise does not impersonate a completed inventory refresh. Actual +runtime auth/permission failures and existing cache withdrawal remain effective; +a later probe success must not erase them. Source controls cover never-polled +and previously observed-but-failing states through the real API boundary. +No restore, guest mutation or native appliance acceptance is implied. + ### Partial backup inventory is not empty recovery evidence The existing Proxmox Backups view treats PVE and PBS inventories as independent diff --git a/internal/api/connections_handlers_continuity_test.go b/internal/api/connections_handlers_continuity_test.go index d74d9cd322..e1b78cb1e9 100644 --- a/internal/api/connections_handlers_continuity_test.go +++ b/internal/api/connections_handlers_continuity_test.go @@ -164,26 +164,12 @@ func TestConnectionsLedgerSeparatesTelemetryHealthFromCommandAdmission(t *testin } func TestConnectionsHandleListUsesTrueNASPollerRuntimeSummary(t *testing.T) { - persistence := config.NewConfigPersistence(t.TempDir()) - connection := config.TrueNASInstance{ - ID: "tn1", - Name: "TrueNAS", - Host: "truenas.lan", - APIKey: "secret", - UseHTTPS: true, - Enabled: true, - PollIntervalSecs: 60, - MonitorDatasets: true, - MonitorPools: true, - MonitorReplication: true, - } - if err := persistence.SaveTrueNASConfig([]config.TrueNASInstance{connection}); err != nil { - t.Fatalf("SaveTrueNASConfig: %v", err) - } - - poller := monitoring.NewTrueNASPoller(nil, time.Minute, nil) - successAt := time.Now().UTC().Add(-30 * time.Second) - poller.RecordConnectionTestSuccess("default", connection.ID, connection, successAt) + f := newTrueNASPollEvidenceFixture(t) + persistence, connection, poller := f.persistence, f.connection, f.poller + poller.Start(context.Background()) + waitForTrueNASPollEvidence(t, f, func(s monitoring.TrueNASConnectionSummary) bool { return s.Poll.LastSuccessAt != nil }) + poller.Stop() + successAt := *f.summary().Poll.LastSuccessAt handler := NewConnectionsHandlers( func(context.Context) *config.Config { return nil }, @@ -209,7 +195,7 @@ func TestConnectionsHandleListUsesTrueNASPollerRuntimeSummary(t *testing.T) { var tn *Connection for i := range resp.Connections { - if resp.Connections[i].ID == "truenas:tn1" { + if resp.Connections[i].ID == "truenas:"+connection.ID { tn = &resp.Connections[i] break } diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index e02f948562..666e92f1d1 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -25131,3 +25131,67 @@ func TestContract_HostedRuntimeAgentInstallCommandCarriesToken(t *testing.T) { } } } + +func TestTrueNASSavedProbePreservesRuntimeEvidence(t *testing.T) { + for _, state := range []string{"never polled", "failed after observed inventory"} { + t.Run(state, func(t *testing.T) { + f := newTrueNASPollEvidenceFixture(t) + if state != "never polled" { + f.poller.Start(context.Background()) + waitForTrueNASPollEvidence(t, f, func(s monitoring.TrueNASConnectionSummary) bool { + return s.Poll.LastSuccessAt != nil && s.Observed != nil + }) + f.failPools.Store(true) + waitForTrueNASPollEvidence(t, f, func(s monitoring.TrueNASConnectionSummary) bool { return s.Poll.LastError != nil }) + f.poller.Stop() + if !strings.Contains(f.summary().Poll.LastError.Message, "pools") { + t.Fatalf("expected required inventory failure: %+v", f.summary().Poll) + } + } + before, err := json.Marshal(f.summary()) + if err != nil { + t.Fatal(err) + } + readyBefore, settledBefore := f.poller.SupplementalInventoryReadyAt(nil, "default") + for _, failProbe := range []bool{false, true, false} { + f.failLogin.Store(failProbe) + rec := httptest.NewRecorder() + f.handler.HandleTestSavedConnection(rec, httptest.NewRequest(http.MethodPost, "/api/truenas/connections/"+f.connection.ID+"/test", nil)) + wantStatus := http.StatusOK + if failProbe { + wantStatus = http.StatusBadRequest + } + if rec.Code != wantStatus { + t.Fatalf("probe status = %d, want %d: %s", rec.Code, wantStatus, rec.Body.String()) + } + after, err := json.Marshal(f.summary()) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Errorf("probe changed runtime polling evidence: before=%s after=%s", before, after) + } + readyAfter, settledAfter := f.poller.SupplementalInventoryReadyAt(nil, "default") + if settledAfter != settledBefore || !readyAfter.Equal(readyBefore) { + t.Errorf("probe changed inventory readiness: (%v,%t) -> (%v,%t)", readyBefore, settledBefore, readyAfter, settledAfter) + } + } + // Verify the same runtime truth is exposed through the actual list + // handler, not just the poller's internal summary accessor. + rec := httptest.NewRecorder() + f.handler.HandleList(rec, httptest.NewRequest(http.MethodGet, "/api/truenas/connections", nil)) + var rows []trueNASConnectionResponse + if rec.Code != http.StatusOK || json.Unmarshal(rec.Body.Bytes(), &rows) != nil || len(rows) != 1 { + t.Fatalf("list failed: %d %s", rec.Code, rec.Body.String()) + } + exposed, _ := json.Marshal(rows[0].Poll) + poll, _ := json.Marshal(f.summary().Poll) + if string(exposed) != string(poll) { + t.Fatalf("list poll evidence = %s, want %s", exposed, poll) + } + if strings.Contains(rec.Body.String(), f.connection.APIKey) { + t.Fatal("list exposed the stored key") + } + }) + } +} diff --git a/internal/api/truenas_handlers.go b/internal/api/truenas_handlers.go index 62ad0dc002..c6f1f0266d 100644 --- a/internal/api/truenas_handlers.go +++ b/internal/api/truenas_handlers.go @@ -412,11 +412,6 @@ func (h *TrueNASHandlers) HandleTestSavedConnection(w http.ResponseWriter, r *ht } invalidConfig, err := h.testConnectionInstance(r, instance) if err != nil { - if !hasPayload && h != nil && h.getPoller != nil { - if poller := h.getPoller(r.Context()); poller != nil { - poller.RecordConnectionTestFailure(resolveTenantOrgID(r), connectionID, instance, err, time.Now().UTC()) - } - } if invalidConfig { writeErrorResponse(w, http.StatusBadRequest, "truenas_invalid_config", "Invalid TrueNAS connection configuration", map[string]string{"error": err.Error()}) return @@ -424,11 +419,10 @@ func (h *TrueNASHandlers) HandleTestSavedConnection(w http.ResponseWriter, r *ht writeErrorResponse(w, http.StatusBadRequest, "truenas_connection_failed", "Failed to connect to TrueNAS", map[string]string{"error": err.Error()}) return } - if !hasPayload && h != nil && h.getPoller != nil { - if poller := h.getPoller(r.Context()); poller != nil { - poller.RecordConnectionTestSuccess(resolveTenantOrgID(r), connectionID, instance, time.Now().UTC()) - } - } + // This probe reads system information through a separate client. It + // neither refreshes the runtime provider nor contributes inventory, so + // it must not clear poll errors, advance freshness/readiness or defer + // the next scheduled poll. Its result belongs to this response only. writeJSON(w, http.StatusOK, map[string]any{"success": true}) return } diff --git a/internal/api/truenas_handlers_test.go b/internal/api/truenas_handlers_test.go index 7f700ec045..79086ec57d 100644 --- a/internal/api/truenas_handlers_test.go +++ b/internal/api/truenas_handlers_test.go @@ -1381,7 +1381,7 @@ func TestTrueNASHandlers_HandlePreviewSavedConnection_ReturnsUnavailableWhenSupp assertMonitoredSystemUsageUnavailableReason(t, rec, monitoring.MonitoredSystemUsageUnavailableSupplementalInventoryRebuildPending) } -func TestTrueNASHandlers_HandleTestSavedConnection_UpdatesPollSummaryFailure(t *testing.T) { +func TestTrueNASHandlers_HandleTestSavedConnection_FailureDoesNotInventPollAttempt(t *testing.T) { setTrueNASFeatureForTest(t, true) connection := config.TrueNASInstance{ @@ -1415,11 +1415,11 @@ func TestTrueNASHandlers_HandleTestSavedConnection_UpdatesPollSummaryFailure(t * t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String()) } summary := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID] - if summary.Poll == nil || summary.Poll.LastError == nil { - t.Fatalf("expected saved retest failure to update poll summary, got %+v", summary.Poll) + if summary.Poll == nil || summary.Poll.LastError != nil || summary.Poll.LastAttemptAt != nil || summary.Poll.LastSuccessAt != nil { + t.Fatalf("a separate failed probe must not invent a runtime poll attempt, got %+v", summary.Poll) } - if summary.Poll.LastError.Message != "authentication failed" { - t.Fatalf("expected failure message preserved, got %+v", summary.Poll.LastError) + if !strings.Contains(rec.Body.String(), "authentication failed") { + t.Fatalf("expected probe failure in its own response, got %s", rec.Body.String()) } } diff --git a/internal/api/truenas_poll_evidence_test.go b/internal/api/truenas_poll_evidence_test.go new file mode 100644 index 0000000000..c276929256 --- /dev/null +++ b/internal/api/truenas_poll_evidence_test.go @@ -0,0 +1,81 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" +) + +type trueNASPollEvidenceFixture struct { + handler *TrueNASHandlers + persistence *config.ConfigPersistence + poller *monitoring.TrueNASPoller + connection config.TrueNASInstance + failPools atomic.Bool + failLogin atomic.Bool +} + +// A deterministic REST appliance analogue: system.info can succeed while a +// required inventory method fails. This proves the source distinction, not +// the cause or transport of either #2382 operator's native failure. +func newTrueNASPollEvidenceFixture(t *testing.T) *trueNASPollEvidenceFixture { + t.Helper() + setTrueNASFeatureForTest(t, true) + f := &trueNASPollEvidenceFixture{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/v2.0/system/info": + if f.failLogin.Load() { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"probe login denied"}`)) + return + } + _, _ = w.Write([]byte(`{"hostname":"poll-fixture","version":"TrueNAS-SCALE-24.10.2","uptime_seconds":100}`)) + case "/api/v2.0/pool": + if f.failPools.Load() { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"inventory unavailable"}`)) + return + } + _, _ = w.Write([]byte(`[{"id":1,"name":"tank","status":"ONLINE","size":1000,"allocated":400,"free":600}]`)) + case "/api/v2.0/pool/dataset", "/api/v2.0/disk", "/api/v2.0/alert/list": + _, _ = w.Write([]byte(`[]`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + cfg := &config.Config{DataPath: t.TempDir()} + f.handler, f.persistence, _ = newTrueNASHandlersForTest(t, cfg) + f.connection = trueNASInstanceFromRawURL(t, "poll-fixture", server.URL, true) + if err := f.persistence.SaveTrueNASConfig([]config.TrueNASInstance{f.connection}); err != nil { + t.Fatal(err) + } + f.poller = monitoring.NewTrueNASPoller(config.NewMultiTenantPersistence(cfg.DataPath), 50*time.Millisecond, nil) + f.handler.getPoller = func(context.Context) *monitoring.TrueNASPoller { return f.poller } + t.Cleanup(f.poller.Stop) + return f +} + +func (f *trueNASPollEvidenceFixture) summary() monitoring.TrueNASConnectionSummary { + return f.poller.ConnectionSummaries("default", []config.TrueNASInstance{f.connection})[f.connection.ID] +} + +func waitForTrueNASPollEvidence(t *testing.T, f *trueNASPollEvidenceFixture, ready func(monitoring.TrueNASConnectionSummary) bool) { + t.Helper() + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) { + if ready(f.summary()) { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("missing runtime poll evidence: %+v", f.summary()) +} diff --git a/internal/monitoring/truenas_poller.go b/internal/monitoring/truenas_poller.go index 51bf22ffc9..debcdc8b7a 100644 --- a/internal/monitoring/truenas_poller.go +++ b/internal/monitoring/truenas_poller.go @@ -861,55 +861,6 @@ func (p *TrueNASPoller) recordConnectionFailureLocked( } } -// RecordConnectionTestSuccess updates one saved TrueNAS connection summary after -// a manual row-level test without clearing the last observed contribution -// summary. -func (p *TrueNASPoller) RecordConnectionTestSuccess( - orgID string, - connID string, - instance config.TrueNASInstance, - at time.Time, -) { - if p == nil { - return - } - connID = strings.TrimSpace(connID) - if connID == "" { - return - } - orgID = normalizeTrueNASOrgID(orgID) - instance.ApplyDefaults() - - p.mu.Lock() - defer p.mu.Unlock() - p.recordConnectionSuccessLocked(orgID, connID, instance, at, at, nil) -} - -// RecordConnectionTestFailure updates one saved TrueNAS connection summary after -// a manual row-level test failure while preserving any previously observed -// resource contribution summary. -func (p *TrueNASPoller) RecordConnectionTestFailure( - orgID string, - connID string, - instance config.TrueNASInstance, - err error, - at time.Time, -) { - if p == nil || err == nil { - return - } - connID = strings.TrimSpace(connID) - if connID == "" { - return - } - orgID = normalizeTrueNASOrgID(orgID) - instance.ApplyDefaults() - - p.mu.Lock() - defer p.mu.Unlock() - p.recordConnectionFailureLocked(orgID, connID, instance, err, at) -} - func buildTrueNASObservedSummary(snapshot *truenas.FixtureSnapshot) *TrueNASConnectionObservedSummary { if snapshot == nil { return nil diff --git a/internal/monitoring/truenas_poller_test.go b/internal/monitoring/truenas_poller_test.go index fcf3a34c68..290814828e 100644 --- a/internal/monitoring/truenas_poller_test.go +++ b/internal/monitoring/truenas_poller_test.go @@ -396,7 +396,7 @@ func TestTrueNASPollerConnectionSummariesCaptureFailures(t *testing.T) { } } -func TestTrueNASPollerManualConnectionTestsUpdateSummariesWithoutClearingObservedCounts(t *testing.T) { +func TestTrueNASPollerRuntimeRecoveryUpdatesSummariesAndObservedCounts(t *testing.T) { poller := NewTrueNASPoller(nil, time.Minute, nil) connection := config.TrueNASInstance{ ID: "manual-test-conn", @@ -416,24 +416,28 @@ func TestTrueNASPollerManualConnectionTestsUpdateSummariesWithoutClearingObserve } firstSuccess := time.Date(2026, time.March, 30, 10, 0, 0, 0, time.UTC) failureAt := firstSuccess.Add(2 * time.Minute) - manualSuccessAt := failureAt.Add(2 * time.Minute) + recoveryAt := failureAt.Add(2 * time.Minute) poller.mu.Lock() poller.recordConnectionSuccessLocked("default", connection.ID, connection, firstSuccess, firstSuccess, snapshot) poller.recordConnectionFailureLocked("default", connection.ID, connection, errors.New("manual auth failed"), failureAt) poller.mu.Unlock() - poller.RecordConnectionTestSuccess("default", connection.ID, connection, manualSuccessAt) + snapshot.CollectedAt = recoveryAt + poller.mu.Lock() + poller.recordConnectionSuccessLocked("default", connection.ID, connection, recoveryAt, recoveryAt, snapshot) + poller.mu.Unlock() summary := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID] if summary.Poll == nil || summary.Poll.LastSuccessAt == nil { - t.Fatalf("expected manual success to update poll summary, got %+v", summary.Poll) + t.Fatalf("expected runtime recovery to update poll summary, got %+v", summary.Poll) } - if summary.Poll.LastError != nil { - t.Fatalf("expected manual success to clear previous error, got %+v", summary.Poll.LastError) + if summary.Poll.LastError != nil || summary.Poll.ConsecutiveFailures != 0 || !summary.Poll.LastSuccessAt.Equal(recoveryAt) { + t.Fatalf("expected runtime recovery to clear previous error, got %+v", summary.Poll) } - if summary.Observed == nil || summary.Observed.Host != "manual-test" || summary.Observed.StoragePools != 1 { - t.Fatalf("expected observed summary to be preserved after manual success, got %+v", summary.Observed) + if summary.Observed == nil || summary.Observed.Host != "manual-test" || summary.Observed.StoragePools != 1 || + summary.Observed.CollectedAt == nil || !summary.Observed.CollectedAt.Equal(recoveryAt) { + t.Fatalf("expected observed summary to advance after runtime recovery, got %+v", summary.Observed) } } @@ -1653,7 +1657,9 @@ func TestTrueNASPollerSupplementalInventoryReadyAtUsesPersistedActiveConnections } attemptedAt := time.Now().UTC() - poller.RecordConnectionTestSuccess("default", connection.ID, connection, attemptedAt) + poller.mu.Lock() + poller.recordConnectionSuccessLocked("default", connection.ID, connection, attemptedAt, attemptedAt, &truenas.FixtureSnapshot{CollectedAt: attemptedAt}) + poller.mu.Unlock() readyAt, settled := poller.SupplementalInventoryReadyAt(nil, "default") if !settled { From afd6cf975744836a2b5e461890563e2e1c3ce33f Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:32:36 +0100 Subject: [PATCH 3/4] Keep legacy-key probe controls independent of poll success The connected proof passed both probe/poll evidence cases but found one older fixture still expecting a successful stored-key probe to manufacture poll success. Keep its stored-secret, HTTPS and missing-ID controls while requiring untouched runtime timestamps and observations. Retain the adverse proof separately. Change-source: pulse-maintainer --- internal/api/truenas_handlers_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/api/truenas_handlers_test.go b/internal/api/truenas_handlers_test.go index 79086ec57d..f1ba5961ed 100644 --- a/internal/api/truenas_handlers_test.go +++ b/internal/api/truenas_handlers_test.go @@ -1005,8 +1005,8 @@ func TestTrueNASHandlers_HandleTestSavedConnection_UsesStoredLegacyAPIKey(t *tes t.Fatalf("unexpected saved client config: %+v", gotConfig) } summary := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID] - if summary.Poll == nil || summary.Poll.LastSuccessAt == nil { - t.Fatalf("expected saved retest to update poll summary success state, got %+v", summary.Poll) + if summary.Poll == nil || summary.Poll.LastSuccessAt != nil || summary.Poll.LastAttemptAt != nil || summary.Observed != nil { + t.Fatalf("a successful legacy-key probe must not invent runtime polling evidence, got %+v", summary) } missingReq := httptest.NewRequest(http.MethodPost, "/api/truenas/connections/missing/test", nil) From ac51a2d37847d2bdc639ce81908d66161ae62a03 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:57:19 +0100 Subject: [PATCH 4/4] Keep saved TrueNAS probe contract independent of runtime polling Update the legacy contract assertion exposed by the full API suite. A successful system-info-only probe remains successful but must not invent a runtime attempt, success or observed inventory. The real poll and failure-preservation controls remain unchanged. Change-source: pulse-maintainer --- internal/api/contract_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 666e92f1d1..fdf0d3e677 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -2903,7 +2903,7 @@ func TestContract_TrueNASConnectionsDisabledMessageIsExplicit(t *testing.T) { } } -func TestContract_TrueNASSavedConnectionTestsUpdateRuntimeSummary(t *testing.T) { +func TestContract_TrueNASSavedConnectionTestsPreserveRuntimeSummary(t *testing.T) { setTrueNASFeatureForTest(t, true) connection := config.TrueNASInstance{ @@ -2934,8 +2934,8 @@ func TestContract_TrueNASSavedConnectionTestsUpdateRuntimeSummary(t *testing.T) t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) } summary := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID] - if summary.Poll == nil || summary.Poll.LastSuccessAt == nil { - t.Fatalf("expected saved manual test to refresh poll summary, got %+v", summary.Poll) + if summary.Poll == nil || summary.Poll.LastAttemptAt != nil || summary.Poll.LastSuccessAt != nil || summary.Observed != nil { + t.Fatalf("a saved probe must not invent runtime polling or inventory, got %+v", summary) } }