From 94e10d65d8330c35f275fe4b693193c64ae736e7 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:17:29 +0100 Subject: [PATCH 1/4] Give organization deletion time for a busy tenant loop to stop 244d322dce made organization deletion refuse, and keep the data, when the tenant's monitoring loop has not exited. That is the right guard, but it reused tenantMonitorShutdownTimeout, a two-second budget that had only ever been a best-effort wait before logging and carrying on. A loop that is still starting up or finishing a tick can take longer to see its cancelled context. In E2E run 37329726585 a just-used organization's loop took 8-10 seconds, so both attempts of 13-api-token-scope's org cleanup got 503 tenant_shutdown_incomplete. Any real delete shortly after a tenant was busy hits the same refusal until a retry. Deletion now waits on its own tenantDeletionShutdownTimeout (30s) before it refuses. Process shutdown and RemoveTenant keep the two-second budget, and a loop that never exits still refuses and retains the data. Both budgets are variables so tests can shorten them; the never-stopping-loop test no longer waits on the real budget. --- .../v6/internal/subsystems/monitoring.md | 8 +++ .../guest_metadata_shutdown_test.go | 1 + .../monitoring/monitor_additional_test.go | 54 +++++++++++++++++++ internal/monitoring/multi_tenant_monitor.go | 26 +++++++-- 4 files changed, 84 insertions(+), 5 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 0e47f9e058..c0ac04025a 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1403,6 +1403,14 @@ reporting source of truth. `MultiTenantMonitor.ListOrganizationIDs` may expose persisted organization IDs to API-owned background workers, but it must not initialize monitors, start pollers, or reinterpret tenant IDs as monitored resource health. +Organization deletion waits up to `tenantDeletionShutdownTimeout` (30 seconds) +for the tenant monitoring loop to exit before it refuses with +`tenant_shutdown_incomplete` and keeps the data. Process shutdown and +`RemoveTenant` keep the two-second `tenantMonitorShutdownTimeout`. Deletion +gates the data on that wait, so the budget must cover a loop that is still +starting up or finishing a tick: under CI load a just-used organization's loop +took 8-10 seconds to see its cancelled context, and the two-second budget turned +an ordinary delete into a refusal. A loop that never exits still refuses. Proxmox physical-disk polling is also a continuity boundary. A failed or permission-denied `disks/list` call must remain an error so the monitor can use linked host-agent inventory or retain same-instance, same-node prior evidence; diff --git a/internal/monitoring/guest_metadata_shutdown_test.go b/internal/monitoring/guest_metadata_shutdown_test.go index 8c9ad302ee..074518a4b9 100644 --- a/internal/monitoring/guest_metadata_shutdown_test.go +++ b/internal/monitoring/guest_metadata_shutdown_test.go @@ -79,6 +79,7 @@ func TestTenantDeletionRetainsBlockedGuestWriterOwnership(t *testing.T) { } } func TestTenantDeletionRetainsUnfinishedMonitorLoop(t *testing.T) { + shortenTenantShutdownBudgets(t, 50*time.Millisecond, 100*time.Millisecond) mtm := NewMultiTenantMonitor(&config.Config{}, nil, nil) _, cancel := context.WithCancel(context.Background()) done := make(chan struct{}) diff --git a/internal/monitoring/monitor_additional_test.go b/internal/monitoring/monitor_additional_test.go index 15a9875f7b..eaaa70230a 100644 --- a/internal/monitoring/monitor_additional_test.go +++ b/internal/monitoring/monitor_additional_test.go @@ -1102,3 +1102,57 @@ func TestAutomaticGuestMemoryLinkNextPoll(t *testing.T) { t.Fatalf("ambiguous hostname selected %q/%q/%q", node, linkedVM, ct) } } + +// shortenTenantShutdownBudgets makes the shutdown waits fast enough for tests +// while keeping deletion's budget longer than the plain one, as in production. +func shortenTenantShutdownBudgets(t *testing.T, plain, deletion time.Duration) { + t.Helper() + previousPlain, previousDeletion := tenantMonitorShutdownTimeout, tenantDeletionShutdownTimeout + tenantMonitorShutdownTimeout, tenantDeletionShutdownTimeout = plain, deletion + t.Cleanup(func() { + tenantMonitorShutdownTimeout, tenantDeletionShutdownTimeout = previousPlain, previousDeletion + }) +} + +// A tenant monitor that is still starting up or finishing a tick can take +// longer than the plain shutdown budget to see its cancelled context. In CI a +// just-used organization's loop took 8-10s, so deletion refused with +// tenant_shutdown_incomplete. Deletion waits on its own, longer budget instead. +func TestTenantDeletionWaitsForSlowMonitoringLoop(t *testing.T) { + shortenTenantShutdownBudgets(t, 50*time.Millisecond, 2*time.Second) + + slowLoop := func(mtm *MultiTenantMonitor, orgID string) *Monitor { + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + <-ctx.Done() + // Exits well after the plain budget but inside deletion's. + time.Sleep(300 * time.Millisecond) + close(done) + }() + monitor := &Monitor{} + mtm.monitors[orgID] = monitor + mtm.tenantCancel[orgID] = cancel + mtm.tenantDone[orgID] = done + return monitor + } + + mtm := NewMultiTenantMonitor(&config.Config{}, nil, nil) + t.Cleanup(mtm.Stop) + + // RemoveTenant keeps the plain budget, gives up and retains the owner. + retained := slowLoop(mtm, "removal") + mtm.RemoveTenant("removal") + if got, ok := mtm.PeekMonitor("removal"); !ok || got != retained { + t.Fatal("plain removal discarded a monitor whose loop had not stopped") + } + + slowLoop(mtm, "deletion") + if err := mtm.BeginTenantDeletion("deletion"); err != nil { + t.Fatalf("deletion refused a loop that stopped inside its budget: %v", err) + } + if _, ok := mtm.PeekMonitor("deletion"); ok { + t.Fatal("completed deletion left the monitor registered") + } + mtm.FinishTenantDeletion("deletion") +} diff --git a/internal/monitoring/multi_tenant_monitor.go b/internal/monitoring/multi_tenant_monitor.go index 248b842239..a11950ad1f 100644 --- a/internal/monitoring/multi_tenant_monitor.go +++ b/internal/monitoring/multi_tenant_monitor.go @@ -47,7 +47,19 @@ func NewMultiTenantMonitor(baseCfg *config.Config, persistence *config.MultiTena } } -const tenantMonitorShutdownTimeout = 2 * time.Second +// Both shutdown budgets are variables so tests can shorten them. +var ( + // tenantMonitorShutdownTimeout bounds how long process shutdown and + // RemoveTenant wait for a tenant's monitoring loop to exit. + tenantMonitorShutdownTimeout = 2 * time.Second + + // tenantDeletionShutdownTimeout bounds how long organization deletion + // waits for the loop. Deletion refuses, and keeps the data, while the loop + // still runs, so this is a correctness gate rather than a best-effort + // wait: a loop that is still starting up or finishing a tick can take + // several seconds to see its cancelled context. + tenantDeletionShutdownTimeout = 30 * time.Second +) // SetRecoveryManager wires a recovery store manager into all existing and future tenant monitors. func (mtm *MultiTenantMonitor) SetRecoveryManager(manager *recoverymanager.Manager) { @@ -377,7 +389,11 @@ func (mtm *MultiTenantMonitor) removeTenant(orgID string, deleting bool) error { if cancel != nil { cancel() } - if !waitForTenantMonitorShutdown(orgID, done) { + shutdownTimeout := tenantMonitorShutdownTimeout + if deleting { + shutdownTimeout = tenantDeletionShutdownTimeout + } + if !waitForTenantMonitorShutdown(orgID, done, shutdownTimeout) { return fmt.Errorf("organization %q monitoring loop has not stopped", orgID) } if monitor != nil { @@ -397,19 +413,19 @@ func (mtm *MultiTenantMonitor) removeTenant(orgID string, deleting bool) error { return nil } -func waitForTenantMonitorShutdown(orgID string, done <-chan struct{}) bool { +func waitForTenantMonitorShutdown(orgID string, done <-chan struct{}, timeout time.Duration) bool { if done == nil { return true } - timer := time.NewTimer(tenantMonitorShutdownTimeout) + timer := time.NewTimer(timeout) defer timer.Stop() select { case <-done: return true case <-timer.C: - logger := log.Warn().Dur("timeout", tenantMonitorShutdownTimeout) + logger := log.Warn().Dur("timeout", timeout) if orgID != "" { logger = logger.Str("org_id", orgID) } From 74a195daaf75b1620630f8f69054e64b81d99939 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:35:39 +0100 Subject: [PATCH 2/4] Flag an outdated agent once on the Infrastructure page Every outdated Pulse Agent row showed an amber Agent update badge and, on a second line, Version behind, and the band counted each of them as a system that needs attention ("11 systems needs attention"), although an agent waiting for an update is maintenance rather than an incident. Version behind and Update available are now maintenance signals: the badge, which opens Agent Doctor, carries them, so they never become the row's problem line and do not count toward needs attention. The band says how many agent updates are available in a neutral tone instead. A failed update and auto-update switched off stay problems, since each needs a different action, and a row's problem is picked before the three-highlight cap so a second agent's failure cannot hide behind another agent's version lag. The band's counts now agree in number, and the Actions column is wide only when a row shows an install button beside Manage, so the Health cell keeps the badge and the last-seen time on one line. --- .../v6/internal/subsystems/agent-lifecycle.md | 14 ++++ .../v6/internal/subsystems/api-contracts.md | 5 ++ .../subsystems/frontend-primitives.md | 4 +- .../8743aba58b02bde2.json | 53 +++++++++++++++ .../Settings/InfrastructureSourceManager.tsx | 38 +++++++++-- .../InfrastructureSourceManager.test.tsx | 46 +++++++++++++ .../__tests__/connectionsTableModel.test.ts | 65 +++++++++++++++++++ .../__tests__/settingsArchitecture.test.ts | 8 ++- .../__tests__/useConnectionsLedger.test.ts | 2 + .../Settings/connectionsTableModel.ts | 24 +++++-- .../Settings/useConnectionsLedger.ts | 5 +- 11 files changed, 249 insertions(+), 15 deletions(-) create mode 100644 frontend-modern/browser-verification/8743aba58b02bde2.json diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index e64451bebf..4891c82049 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -5127,6 +5127,20 @@ API and Pulse Agent`) from the shared ledger contract. The compact landing may render its API / Agent / API + Agent badge beside the system name, with the full phrase available through accessible metadata and the detail flow, instead of spending a separate table column on collection method. +An agent that is behind the Pulse Agent target or has an update waiting is +maintenance, not a problem: the row's `Agent update` badge says so once and +opens Agent Doctor, so `primaryRowProblem` skips signals marked `maintenance` +(Version behind, Update available) and the row adds no `Version behind` line +beneath itself. Those rows therefore do not count toward the band's `needs +attention` total, which counts them separately and neutrally as agent updates +available. A failed update and auto-update switched off remain row problems, +because each asks for a different action, and the Manage drawer still lists +every fleet signal. A row's problem is picked from all of its visible signals +before the three-highlight cap, and problems rank ahead of maintenance in that +list, so a second attached agent's failed update cannot fall behind another +agent's version lag. The Actions column is only as wide as an install button +beside Manage needs when a row in view shows one, so Health keeps its state, +the update badge and the last-seen time on one line. Source badge class selection may use semantic gray treatment for API-only rows and typed non-gray tones for agent, probe, or combined sources, but the source identity remains the API/Agent/Probe label and subtitle from the shared ledger diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index e459897ac5..12452c53b1 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -7343,6 +7343,11 @@ agent-applied setting, `fleet.configDrift` must remain `not-applicable` and diagnostics, but must not count that passive state as source setup attention. Only a real managed desired config fingerprint can create pending rollout attention when an applied fingerprint is absent or mismatched. +Likewise `fleet.versionDrift: behind` and `fleet.updateStatus: +update-available` are maintenance, not source setup attention: +`useConnectionsLedger.ts` keeps them out of a row's problem and the band's +needs-attention count, which the row's agent-update count and badge carry +instead, while `updateStatus: failed` and `disabled` remain row problems. The `fleet.commandPolicy` object is the canonical desired/applied convergence contract for remote command enablement. It must carry the desired server policy, the applied agent-reported truth when available, the effective diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 1fbee03224..21e66618d5 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -2631,7 +2631,9 @@ Agent`), with the plain-language source phrase available through accessible always-on version column for Pulse Agent; exact version text belongs in the edit/detail surfaces, while the landing table only surfaces a compact warning badge when an attached or standalone agent actually has an update - available. That same table boundary must reuse the `System` cell for compact + available. That badge is the row's only version flag: the row adds no + `Version behind` problem line beneath it, and the source band counts those + rows as agent updates available rather than as needing attention. That same table boundary must reuse the `System` cell for compact standalone-agent identity such as `Unraid 7.1.0`; raw reported addresses belong in the governed Manage detail or an explicitly expanded cluster-member row, not an always-on diagnostics column. diff --git a/frontend-modern/browser-verification/8743aba58b02bde2.json b/frontend-modern/browser-verification/8743aba58b02bde2.json new file mode 100644 index 0000000000..29c1b5cf97 --- /dev/null +++ b/frontend-modern/browser-verification/8743aba58b02bde2.json @@ -0,0 +1,53 @@ +{ + "version": 1, + "base_sha": "2218ab6c18cecde801a1a4e920071089032d46a8", + "verified_at": "2026-10-05T16:35:39Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx", + "frontend-modern/src/components/Settings/connectionsTableModel.ts", + "frontend-modern/src/components/Settings/useConnectionsLedger.ts" + ], + "content_sha256": { + "frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx": "0e631a2eaf7ba72e2a70e99ceca0639c82372b8700b831f48d5b93f9dce35ab1", + "frontend-modern/src/components/Settings/connectionsTableModel.ts": "1ee02ad9c25795e44f766d7d4dff615cd2323e3691c30e844a3d5dfa4973c64d", + "frontend-modern/src/components/Settings/useConnectionsLedger.ts": "4761b1a14a6af04225af07036ecf800334a12ab3b1aa418e400ddf4e2bb841ec" + }, + "routes": [ + "/settings/infrastructure" + ], + "viewports": [ + { + "width": 390, + "height": 1600 + }, + { + "width": 1100, + "height": 1000 + }, + { + "width": 1280, + "height": 1000 + }, + { + "width": 1440, + "height": 1100 + }, + { + "width": 1920, + "height": 1000 + } + ], + "states": [ + "Mock estate with 13 connected systems, 11 of them Pulse Agent hosts behind the agent target with an update available", + "Each outdated agent row shows Active, one Agent update badge (opens Agent Doctor) and its last-seen time on one line; main added a second italic Version behind line under every one of the 11 rows", + "The band reads 13 systems connected \u00b7 All active \u00b7 11 agent updates available in a neutral tone and Open Agent Doctor is a secondary button; main read 11 systems needs attention in amber with a primary button although every one was only version lag", + "Health column at 1280 / 1440 / 1920: 228 / 267 / 390px with no clipped cell (main 176 / 216 / 332px with 11 and 6 clipped cells, the last-seen time cut to 18s a...); no cell wraps", + "Phone card layout at 390: neutral band, no Version behind line on any card, 11 Agent update badges; below 1100 the page uses the card layout as before", + "Re-measured after rebasing onto 2218ab6c18 (shared Table row-click change from #2524): same widths, no clipped cells, phone layout unchanged" + ], + "interactions": [ + "Measured column widths and clipped cells on main (reverse-applied patch) and on the change at 1100, 1280, 1440 and 1920", + "Counted Version behind lines and Agent update badges in the live phone layout" + ] +} diff --git a/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx b/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx index c6f8bdc6f3..cf9f66f608 100644 --- a/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx +++ b/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx @@ -502,6 +502,15 @@ export const InfrastructureSourceManager: Component !props.readOnly; + // Actions holds Manage (about 96px with padding) and, on a row missing agent + // coverage, an install button beside it. Reserving the wide column only when + // a row shows that button leaves Health room for its state, the Agent update + // badge and the last-seen time on one line. + const actionColumnShowsInstall = createMemo( + () => + Boolean(props.onAddSourceStep || props.onAddSource) && + infrastructureRows().some((row) => rowNeedsAgentCoverage(row)), + ); const lastDiscoveryResultText = createMemo(() => formatRelativeTimestamp(props.discoveryScanStatus().lastResultAt), ); @@ -543,6 +552,11 @@ export const InfrastructureSourceManager: Component Boolean(row.problem) || row.members.some((member) => member.problem), ).length, ); + // Outdated agents are maintenance rather than attention (see + // FleetGovernanceSignal.maintenance); the band still counts them once. + const agentUpdateTotal = createMemo(() => + infrastructureRows().reduce((total, row) => total + row.agentUpdateCount, 0), + ); const discoveryMonitorTitle = createMemo(() => { const status = props.discoveryScanStatus(); if (status.scanning) return 'Scanning'; @@ -815,7 +829,16 @@ export const InfrastructureSourceManager: Component - {formatCount(fleetAttentionSystemCount(), 'system')} needs attention + {formatCount(fleetAttentionSystemCount(), 'system')}{' '} + {fleetAttentionSystemCount() === 1 ? 'needs' : 'need'} attention + + + 0}> + + + {formatCount(agentUpdateTotal(), 'agent update')} available 0}> @@ -823,7 +846,8 @@ export const InfrastructureSourceManager: Component - {formatCount(uncoveredAgentTargetCount(), 'host')} has limited coverage + {formatCount(uncoveredAgentTargetCount(), 'host')}{' '} + {uncoveredAgentTargetCount() === 1 ? 'has' : 'have'} limited coverage @@ -897,17 +921,19 @@ export const InfrastructureSourceManager: Component - + System - + Coverage - + Health - + Actions diff --git a/frontend-modern/src/components/Settings/__tests__/InfrastructureSourceManager.test.tsx b/frontend-modern/src/components/Settings/__tests__/InfrastructureSourceManager.test.tsx index 89b27335d3..16e8b825ae 100644 --- a/frontend-modern/src/components/Settings/__tests__/InfrastructureSourceManager.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/InfrastructureSourceManager.test.tsx @@ -6,7 +6,9 @@ import { agentConnectionIDsForInfrastructureRow, } from '../InfrastructureSourceManager'; import { + fleetGovernanceSignalsForConnection, primaryRowProblem, + visibleFleetGovernanceSignals, type FleetGovernanceSignal, type InfrastructureSystemMemberRow, type InfrastructureSystemRow, @@ -398,6 +400,50 @@ describe('InfrastructureSourceManager setup summary', () => { expect(onOpenAgentDoctor).toHaveBeenCalledWith(['agent:behind-host']); }); + it('flags an outdated agent once, as an update rather than as attention', () => { + const outdated = connectionFixture({ + id: 'agent:outdated-host', + name: 'outdated-host', + agentUpdateAvailable: true, + fleet: { + enrollmentState: 'enrolled', + livenessState: 'active', + versionDrift: 'behind', + adapterHealth: 'healthy', + configRollout: 'reported', + credentialStatus: 'verified', + updateStatus: 'update-available', + remoteControl: 'disabled', + }, + }); + const fleetSignals = fleetGovernanceSignalsForConnection(outdated); + const outdatedRow = row({ + connection: outdated, + agentUpdateCount: 1, + fleetSignals, + fleetHighlights: visibleFleetGovernanceSignals(fleetSignals), + problem: primaryRowProblem(visibleFleetGovernanceSignals(fleetSignals, Infinity)), + }); + + render(() => ( + [outdatedRow]} + discoveredNodes={() => []} + discoveryEnabled={false} + discoveryScanStatus={() => ({ scanning: false })} + readOnly={false} + onOpenAgentDoctor={vi.fn()} + /> + )); + + expect( + screen.getByRole('button', { name: 'Open Agent Doctor for outdated-host' }), + ).toHaveTextContent('Agent update'); + expect(screen.queryByText('Version behind')).toBeNull(); + expect(screen.queryByText(/needs? attention/i)).toBeNull(); + expect(screen.getByText('1 agent update available')).toBeInTheDocument(); + }); + it('still counts actionable member posture when the cluster parent is healthy', () => { render(() => ( { }); }); +describe('primaryRowProblem', () => { + const outdatedFleet = (updateStatus: 'update-available' | 'failed' | 'disabled') => + connectionFixture({ + agentUpdate: + updateStatus === 'failed' + ? { state: 'error', autoUpdate: true, lastError: 'signature verification failed' } + : undefined, + fleet: { + enrollmentState: 'enrolled', + livenessState: 'active', + versionDrift: 'behind', + adapterHealth: 'healthy', + configRollout: 'reported', + credentialStatus: 'verified', + updateStatus, + remoteControl: 'disabled', + }, + }); + + it('leaves an outdated agent to its Agent update badge instead of naming a problem', () => { + const highlights = visibleFleetGovernanceSignals( + fleetGovernanceSignalsForConnection(outdatedFleet('update-available')), + ); + // The Manage drawer keeps the structured signals. + expect(highlights.map((signal) => signal.label)).toEqual([ + 'Version behind', + 'Update available', + ]); + expect(primaryRowProblem(highlights)).toBeUndefined(); + }); + + it("finds a failed update on a second attached agent behind another agent's version lag", () => { + const signals = [ + ...fleetGovernanceSignalsForConnection(outdatedFleet('update-available')), + ...fleetGovernanceSignalsForConnection({ + ...outdatedFleet('failed'), + id: 'agent:host-2', + name: 'host-2', + }), + ]; + expect(primaryRowProblem(visibleFleetGovernanceSignals(signals, Infinity))).toMatchObject({ + label: 'Agent update failed', + tone: 'critical', + }); + // The capped highlight list leads with the problem, not the maintenance. + expect(visibleFleetGovernanceSignals(signals)[0]?.label).toBe('Agent update failed'); + }); + + it('still names a failed update or disabled auto-update, which need a different action', () => { + expect( + primaryRowProblem( + visibleFleetGovernanceSignals(fleetGovernanceSignalsForConnection(outdatedFleet('failed'))), + ), + ).toMatchObject({ label: 'Agent update failed', tone: 'critical' }); + expect( + primaryRowProblem( + visibleFleetGovernanceSignals( + fleetGovernanceSignalsForConnection(outdatedFleet('disabled')), + ), + ), + ).toMatchObject({ label: 'Auto-update off', tone: 'warning' }); + }); +}); + describe('fleetGovernanceSignalsForConnection connection-type gating', () => { it('omits agent-fleet governance signals for pull-based API sources', () => { const signals = fleetGovernanceSignalsForConnection( diff --git a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts index 5ed24fe4ae..768cf83f60 100644 --- a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts @@ -1665,8 +1665,12 @@ describe('settings architecture guardrails', () => { expect(infrastructureSourceManagerSource).toContain('Connection posture'); expect(infrastructureSourceManagerSource).toContain("'system')} connected"); expect(infrastructureSourceManagerSource).toContain('All active'); - expect(infrastructureSourceManagerSource).toContain('needs attention'); - expect(infrastructureSourceManagerSource).toContain('has limited coverage'); + expect(infrastructureSourceManagerSource).toContain( + "{fleetAttentionSystemCount() === 1 ? 'needs' : 'need'} attention", + ); + expect(infrastructureSourceManagerSource).toContain( + "{uncoveredAgentTargetCount() === 1 ? 'has' : 'have'} limited coverage", + ); expect(infrastructureSourceManagerSource).toContain('setupConfidenceAction'); expect(infrastructureSourceManagerSource).not.toContain('Infrastructure coverage'); expect(infrastructureSourceManagerSource).not.toContain('Fleet governance'); diff --git a/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts b/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts index b72056ef39..74821b59e1 100644 --- a/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts @@ -552,6 +552,8 @@ describe('useConnectionsLedger', () => { 'Rollout pending', 'Version behind', ]); + // Version lag rides on the Agent update badge; the problem line names the drift. + expect(byID.get('agent:drifted')?.problem?.label).toBe('Config drift'); // Pull-based API sources (PVE/PBS/etc.) have no Pulse Agent, so agent-fleet // governance (rollout/config/version/command-policy) must not surface on // them. Only source-agnostic posture like credential health applies; an diff --git a/frontend-modern/src/components/Settings/connectionsTableModel.ts b/frontend-modern/src/components/Settings/connectionsTableModel.ts index 5c1c874426..7bbc204974 100644 --- a/frontend-modern/src/components/Settings/connectionsTableModel.ts +++ b/frontend-modern/src/components/Settings/connectionsTableModel.ts @@ -256,6 +256,11 @@ export interface FleetGovernanceSignal { label: string; detail: string; tone: FleetGovernanceSignalTone; + // An agent that is behind or has an update waiting is maintenance, not a + // problem: the row's Agent update badge already says so and opens Agent + // Doctor, so the row does not repeat it as its problem line or count it as + // needing attention. The Manage drawer still lists the signal. + maintenance?: boolean; } const DEFAULT_FLEET_GOVERNANCE: ConnectionFleetGovernance = { @@ -462,6 +467,7 @@ const versionSignal = (state: ConnectionFleetVersionDrift): FleetGovernanceSigna label: 'Version behind', detail: 'This agent is behind the current Pulse Agent target.', tone: 'warning', + maintenance: true, }; case 'current': return { @@ -686,6 +692,7 @@ const updateSignal = ( label: 'Update available', detail: 'A newer Pulse Agent binary is available for this system.', tone: 'warning', + maintenance: true, }; case 'checking': return { @@ -906,8 +913,12 @@ export const fleetGovernanceSignalsForConnection = ( return signals; }; +// The row shows at most three highlights. A row's problem is picked from the +// whole visible list (limit Infinity) so a critical signal from a second +// attached agent cannot fall behind the cap. export const visibleFleetGovernanceSignals = ( signals: readonly FleetGovernanceSignal[], + limit = 3, ): FleetGovernanceSignal[] => { const hasPassiveAgentConfigConfirmation = signals.some(isPassiveAgentConfigConfirmationSignal); const visibleSignals = signals.filter((signal) => { @@ -929,11 +940,14 @@ export const visibleFleetGovernanceSignals = ( if (signal.key === 'command-policy' && signal.tone === 'info') return false; return true; }); + // Problems lead; maintenance (an agent behind or with an update waiting) + // follows them, since the Agent update badge already carries it. const attention = visibleSignals.filter( - (signal) => signal.tone === 'critical' || signal.tone === 'warning', + (signal) => !signal.maintenance && (signal.tone === 'critical' || signal.tone === 'warning'), ); - const control = visibleSignals.filter((signal) => signal.tone === 'info'); - return [...attention, ...control].slice(0, 3); + const maintenance = visibleSignals.filter((signal) => signal.maintenance); + const control = visibleSignals.filter((signal) => !signal.maintenance && signal.tone === 'info'); + return [...attention, ...maintenance, ...control].slice(0, limit); }; const isPassiveAgentConfigConfirmationSignal = (signal: FleetGovernanceSignal): boolean => { @@ -1027,9 +1041,11 @@ export interface InfrastructureSystemRow { // Pick the single most important problem to surface in the row's status // column. Critical beats warning; otherwise first wins — the upstream // builder has already prioritised highlights (attention before info). +// Maintenance signals never become the problem (see FleetGovernanceSignal). export const primaryRowProblem = ( - signals: readonly FleetGovernanceSignal[], + allSignals: readonly FleetGovernanceSignal[], ): InfrastructureRowProblem | undefined => { + const signals = allSignals.filter((signal) => !signal.maintenance); const critical = signals.find((signal) => signal.tone === 'critical'); if (critical) { return { label: critical.label, detail: critical.detail, tone: 'critical' }; diff --git a/frontend-modern/src/components/Settings/useConnectionsLedger.ts b/frontend-modern/src/components/Settings/useConnectionsLedger.ts index 2ab8538b99..5a54535676 100644 --- a/frontend-modern/src/components/Settings/useConnectionsLedger.ts +++ b/frontend-modern/src/components/Settings/useConnectionsLedger.ts @@ -307,7 +307,7 @@ const buildMemberRow = ( // still rolls up from the prioritised highlight list. const problem = (agentConnection && agentAttachmentProblem(agentConnection)) ?? - primaryRowProblem(fleetHighlights); + primaryRowProblem(visibleFleetGovernanceSignals(fleetSignals, Infinity)); return { id: member.id, nodeIdentity: member.nodeIdentity?.trim() || undefined, @@ -423,7 +423,8 @@ const buildRow = ( lastErrorMessage, fleetSignals, fleetHighlights, - problem: attachmentProblem ?? primaryRowProblem(fleetHighlights), + problem: + attachmentProblem ?? primaryRowProblem(visibleFleetGovernanceSignals(fleetSignals, Infinity)), enabled: primaryConnection.enabled, canEdit: EDITABLE_CONNECTION_TYPES.includes(primaryConnection.type), canPause: primaryConnection.capabilities.supportsPause, From 574b4dd35005cf395ad7359db9a93353ae5b7093 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:48:05 +0100 Subject: [PATCH 3/4] Name mock history seeding as the observed slow tenant stop The deletion-budget note and its test said only that a loop still starting up can be slow to stop. The CI failures have a specific cause: in mock mode Monitor.Start runs startMockMetricsSampler synchronously before its select loop, and prepareMockMetricsHistory seeds seven days of fixture history without a context, so the loop saw its cancelled context only once seeding finished ("Mock metrics history sampler started" and "monitoring loop stopped" log in the same second in runs 37329726585 and 37331626618). Say so in the contract and the test comment. --- .../release-control/v6/internal/subsystems/monitoring.md | 9 ++++++--- internal/monitoring/monitor_additional_test.go | 8 +++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index c0ac04025a..5e249dedf6 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1408,9 +1408,12 @@ for the tenant monitoring loop to exit before it refuses with `tenant_shutdown_incomplete` and keeps the data. Process shutdown and `RemoveTenant` keep the two-second `tenantMonitorShutdownTimeout`. Deletion gates the data on that wait, so the budget must cover a loop that is still -starting up or finishing a tick: under CI load a just-used organization's loop -took 8-10 seconds to see its cancelled context, and the two-second budget turned -an ordinary delete into a refusal. A loop that never exits still refuses. +starting up or finishing a tick. The observed case is mock mode: +`Monitor.Start` runs `startMockMetricsSampler` synchronously before its select +loop, and `prepareMockMetricsHistory` seeds seven days of fixture history +without a context, so a just-used organization's loop took 8-10 seconds under CI +load to see its cancelled context and the two-second budget turned an ordinary +delete into a refusal. A loop that never exits still refuses. Proxmox physical-disk polling is also a continuity boundary. A failed or permission-denied `disks/list` call must remain an error so the monitor can use linked host-agent inventory or retain same-instance, same-node prior evidence; diff --git a/internal/monitoring/monitor_additional_test.go b/internal/monitoring/monitor_additional_test.go index eaaa70230a..bc771e5d42 100644 --- a/internal/monitoring/monitor_additional_test.go +++ b/internal/monitoring/monitor_additional_test.go @@ -1115,9 +1115,11 @@ func shortenTenantShutdownBudgets(t *testing.T, plain, deletion time.Duration) { } // A tenant monitor that is still starting up or finishing a tick can take -// longer than the plain shutdown budget to see its cancelled context. In CI a -// just-used organization's loop took 8-10s, so deletion refused with -// tenant_shutdown_incomplete. Deletion waits on its own, longer budget instead. +// longer than the plain shutdown budget to see its cancelled context. In CI's +// mock mode, Start seeds mock metrics history synchronously before its select +// loop, so a just-used organization's loop took 8-10s to stop and deletion +// refused with tenant_shutdown_incomplete. Deletion waits on its own, longer +// budget instead. func TestTenantDeletionWaitsForSlowMonitoringLoop(t *testing.T) { shortenTenantShutdownBudgets(t, 50*time.Millisecond, 2*time.Second) From d086054b7ea419f98efa95c99e2aa378eea40dc9 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:53:13 +0100 Subject: [PATCH 4/4] Stop mock metrics seeding when its monitor is cancelled A tenant monitor's loop only sees its cancelled context after startMockMetricsSampler returns, and seeding the 50-node demo estate took 4-11s on CI runners (about 2.7s on an M-series Mac). Organization deletion cancels the loop and waits for it, so in mock mode every deletion of a just-used organization was held for the whole seed, and under the 2s budget #2518 introduced it was refused with tenant_shutdown_incomplete (Core E2E 13-api-token-scope since 15:03 UTC on 2026-10-05). Seeding now checks the context before each resource and returns ctx.Err() once cancelled, so the loop exits within a millisecond of the cancel. Series already sampled are recorded whole. A cancelled seed is never stored as the shared template other tenants reuse, and a monitor cancelled while it waited on the template lock returns without seeding. startMockMetricsSampler then skips the chart-cache warm-up and the sampler goroutine. #2529 separately gives deletion a longer bounded wait for any slow loop; this removes the mock-mode cause of the wait. Contract-Neutral: Mock-mode seeding stops early on monitor cancellation; no route, field, schema, persistence format or provider behaviour changes. --- internal/monitoring/mock_metrics_history.go | 69 ++++++++-- .../mock_metrics_history_cancel_test.go | 126 ++++++++++++++++++ .../monitoring/mock_metrics_history_test.go | 42 +++--- .../mock_metrics_seed_cache_test.go | 12 +- 4 files changed, 211 insertions(+), 38 deletions(-) create mode 100644 internal/monitoring/mock_metrics_history_cancel_test.go diff --git a/internal/monitoring/mock_metrics_history.go b/internal/monitoring/mock_metrics_history.go index 6b10b55612..487fb7d1c4 100644 --- a/internal/monitoring/mock_metrics_history.go +++ b/internal/monitoring/mock_metrics_history.go @@ -540,13 +540,18 @@ func canonicalMetricSeriesWithSampler(sampler mock.MetricSampler, resourceType, return sampler.SampleMetricSeries(resourceType, resourceID, metric, timestamps) } -func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.FixtureGraph, now time.Time, seedDuration, interval time.Duration) { +// seedMockMetricsHistory returns ctx.Err() when ctx is cancelled partway. A +// large demo estate takes several seconds to seed, and a tenant monitor's loop +// only sees its cancelled context once seeding returns, so organization +// deletion would otherwise wait for the whole estate. Each series that was +// sampled is recorded whole; the rest are skipped. +func seedMockMetricsHistory(ctx context.Context, mh *MetricsHistory, ms *metrics.Store, graph mock.FixtureGraph, now time.Time, seedDuration, interval time.Duration) error { if mh == nil { - return + return nil } state := graph.State if seedDuration <= 0 || interval <= 0 { - return + return nil } sampler := mock.NewMetricSampler(graph) now = normalizeMockMetricTimestamp(now, interval) @@ -669,6 +674,9 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi } } recordStorageTimeline := func(storageID string, currentTotal float64) { + if ctx.Err() != nil { + return + } if strings.TrimSpace(storageID) == "" || currentTotal <= 0 || numPoints == 0 { return } @@ -693,6 +701,9 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi } recordNode := func(node models.Node) { + if ctx.Err() != nil { + return + } if node.ID == "" { return } @@ -719,7 +730,7 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi includeDiskIO bool, includeNetwork bool, ) { - if len(metricIDs) == 0 || storeID == "" { + if ctx.Err() != nil || len(metricIDs) == 0 || storeID == "" { return } uniqueMetricIDs := make([]string, 0, len(metricIDs)) @@ -784,7 +795,7 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi seedStoreSeries(storeType, storeID, storeMetrics...) } recordGuestTemperature := func(metricIDs []string, storeType, storeID string) { - if len(metricIDs) == 0 || strings.TrimSpace(storeID) == "" { + if ctx.Err() != nil || len(metricIDs) == 0 || strings.TrimSpace(storeID) == "" { return } temperatureSeries := canonicalMetricSeriesWithSampler( @@ -802,7 +813,7 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi seedStoreSeries(storeType, storeID, "temperature") } recordGuestMemoryUsed := func(metricIDs []string, storeType, storeID string, memoryTotal float64) { - if len(metricIDs) == 0 || strings.TrimSpace(storeID) == "" || memoryTotal <= 0 { + if ctx.Err() != nil || len(metricIDs) == 0 || strings.TrimSpace(storeID) == "" || memoryTotal <= 0 { return } percentSeries := canonicalMetricSeriesWithSampler(sampler, storeType, storeID, "memory", seedTimestamps) @@ -915,6 +926,9 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi // series for one physical disk, shared by the native and TrueNAS fixture // disk loops below. seedDiskTelemetry := func(resourceID string) { + if ctx.Err() != nil { + return + } tempSeries := canonicalMetricSeriesWithSampler(sampler, "disk", resourceID, "smart_temp", seedTimestamps) busySeries := canonicalMetricSeriesWithSampler(sampler, "disk", resourceID, "disk", seedTimestamps) diskReadSeries := canonicalMetricSeriesWithSampler(sampler, "disk", resourceID, "diskread", seedTimestamps) @@ -1025,6 +1039,9 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi recordGuestTemperature(systemMetricIDs, "agent", trueNASFixtures.System.Hostname) for _, pool := range trueNASFixtures.Pools { + if ctx.Err() != nil { + break + } poolKey := mock.TrueNASPoolMetricID(trueNASFixtures.System.Hostname, pool.Name) diskSeries := canonicalMetricSeriesWithSampler(sampler, "storage", poolKey, "usage", seedTimestamps) mh.addGuestMetricSeries(poolKey, "disk", diskSeries, seedTimestamps) @@ -1032,6 +1049,9 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi } for _, dataset := range trueNASFixtures.Datasets { + if ctx.Err() != nil { + break + } dsKey := mock.TrueNASDatasetMetricID(trueNASFixtures.System.Hostname, dataset.Name) totalBytes := dataset.UsedBytes + dataset.AvailBytes diskSeries := canonicalMetricSeriesWithSampler(sampler, "storage", dsKey, "usage", seedTimestamps) @@ -1115,6 +1135,9 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi } flushStoreBatch() + if err := ctx.Err(); err != nil { + return err + } if ms != nil { log.Info(). Int("rows", storeRows). @@ -1122,9 +1145,11 @@ func seedMockMetricsHistory(mh *MetricsHistory, ms *metrics.Store, graph mock.Fi Msg("mock seeding: metrics store backfill completed") } log.Debug().Msg("mock seeding: completed") + return nil } func prepareMockMetricsHistory( + ctx context.Context, graph mock.FixtureGraph, fixtureRevision uint64, now time.Time, @@ -1132,7 +1157,7 @@ func prepareMockMetricsHistory( interval time.Duration, maxDataPoints int, storeSink *metrics.Store, -) (*MetricsHistory, bool) { +) (*MetricsHistory, bool, error) { // Enforce the eager-history cardinality limit at the allocation/cache // boundary. Callers may retain the complete fixture graph for inventory and // realtime updates, but neither the active history nor the reusable seed @@ -1141,8 +1166,10 @@ func prepareMockMetricsHistory( seedAt := normalizeMockMetricTimestamp(now, interval) if storeSink != nil { history := NewMetricsHistory(maxDataPoints, seedDuration) - seedMockMetricsHistory(history, storeSink, graph, seedAt, seedDuration, interval) - return history, false + if err := seedMockMetricsHistory(ctx, history, storeSink, graph, seedAt, seedDuration, interval); err != nil { + return nil, false, err + } + return history, false, nil } key := mockMetricsSeedCacheKey{ @@ -1155,12 +1182,20 @@ func prepareMockMetricsHistory( mockMetricsSeedCache.Lock() defer mockMetricsSeedCache.Unlock() + // The lock is held for a whole seed, so a monitor may have been cancelled + // while it waited behind another tenant's. + if err := ctx.Err(); err != nil { + return nil, false, err + } if mockMetricsSeedCache.history != nil && mockMetricsSeedCache.key == key { - return mockMetricsSeedCache.history.clone(), true + return mockMetricsSeedCache.history.clone(), true, nil } template := NewMetricsHistory(maxDataPoints, seedDuration) - seedMockMetricsHistory(template, nil, graph, seedAt, seedDuration, interval) + if err := seedMockMetricsHistory(ctx, template, nil, graph, seedAt, seedDuration, interval); err != nil { + // A partial template must never be reused by another tenant. + return nil, false, err + } mockMetricsSeedCache.key = key mockMetricsSeedCache.history = template mockMetricsSeedCache.generation++ @@ -1168,7 +1203,7 @@ func prepareMockMetricsHistory( time.AfterFunc(mockMetricsSeedCacheTTL, func() { expireMockMetricsSeedCache(generation) }) - return template.clone(), false + return template.clone(), false, nil } // recordTrueNASFixturesMetrics records live fixture ticks for TrueNAS host, @@ -1911,7 +1946,8 @@ func (m *Monitor) startMockMetricsSampler(ctx context.Context) { log.Warn().Msg("PULSE_MOCK_SEED_METRICS_STORE is set but the metrics store is unavailable; mock report history will stay empty") } } - history, cacheHit := prepareMockMetricsHistory( + history, cacheHit, err := prepareMockMetricsHistory( + samplerCtx, historyGraph, fixtureRevision, time.Now(), @@ -1920,6 +1956,13 @@ func (m *Monitor) startMockMetricsSampler(ctx context.Context) { maxPoints, storeSink, ) + if err != nil { + // The monitor is stopping. Return before warming chart caches or + // starting the sampler so its loop sees the cancellation now; the + // caller's stopMockMetricsSampler releases mockMetricsCancel. + log.Info().Err(err).Msg("Mock metrics sampler: seeding cancelled") + return + } m.mu.Lock() m.metricsHistory = history m.mu.Unlock() diff --git a/internal/monitoring/mock_metrics_history_cancel_test.go b/internal/monitoring/mock_metrics_history_cancel_test.go new file mode 100644 index 0000000000..3794848884 --- /dev/null +++ b/internal/monitoring/mock_metrics_history_cancel_test.go @@ -0,0 +1,126 @@ +package monitoring + +import ( + "context" + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/mock" + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) + +// cancelAfterChecks reports cancellation once Err has been consulted more +// than allowed times, so a test can cancel seeding partway through without +// racing a timer. +type cancelAfterChecks struct { + context.Context + remaining atomic.Int32 +} + +func newCancelAfterChecks(allowed int32) *cancelAfterChecks { + ctx := &cancelAfterChecks{Context: context.Background()} + ctx.remaining.Store(allowed) + return ctx +} + +func (c *cancelAfterChecks) Err() error { + if c.remaining.Add(-1) < 0 { + return context.Canceled + } + return nil +} + +func threeNodeMockGraph() mock.FixtureGraph { + return fixtureGraphWithState(models.StateSnapshot{ + Nodes: []models.Node{ + {ID: "node-a", Name: "node-a", Status: "online", CPU: 0.2, Memory: models.Memory{Usage: 40, Total: 1024}}, + {ID: "node-b", Name: "node-b", Status: "online", CPU: 0.3, Memory: models.Memory{Usage: 50, Total: 1024}}, + {ID: "node-c", Name: "node-c", Status: "online", CPU: 0.4, Memory: models.Memory{Usage: 60, Total: 1024}}, + }, + }) +} + +// Org deletion refused with tenant_shutdown_incomplete because a tenant +// monitor's loop only sees its cancelled context after mock seeding returns, +// and seeding a large demo estate took 4-11s in CI. Seeding must stop at the +// next resource instead of finishing the estate. +func TestSeedMockMetricsHistoryStopsPartwayWhenCancelled(t *testing.T) { + now := time.Now().UTC().Truncate(time.Minute) + mh := NewMetricsHistory(3500, time.Hour) + + err := seedMockMetricsHistory(newCancelAfterChecks(1), mh, nil, threeNodeMockGraph(), now, time.Hour, time.Minute) + if !errors.Is(err, context.Canceled) { + t.Fatalf("seedMockMetricsHistory error = %v, want context.Canceled", err) + } + if len(mh.GetNodeMetrics("node-a", "cpu", time.Hour)) == 0 { + t.Fatal("node seeded before cancellation has no history") + } + for _, nodeID := range []string{"node-b", "node-c"} { + if got := len(mh.GetNodeMetrics(nodeID, "cpu", time.Hour)); got != 0 { + t.Fatalf("node %s seeded %d points after cancellation", nodeID, got) + } + } +} + +func TestPrepareMockMetricsHistoryDoesNotCacheCancelledSeed(t *testing.T) { + resetMockMetricsSeedCacheForTest(t) + now := time.Now().UTC().Truncate(time.Minute) + graph := threeNodeMockGraph() + + // One check passes the cache lock, one seeds node-a, then cancellation. + history, cacheHit, err := prepareMockMetricsHistory(newCancelAfterChecks(2), graph, 7, now, time.Hour, time.Minute, 3500, nil) + if !errors.Is(err, context.Canceled) || history != nil || cacheHit { + t.Fatalf("cancelled prepare = (%v, %v, %v), want (nil, false, context.Canceled)", history, cacheHit, err) + } + mockMetricsSeedCache.Lock() + cached := mockMetricsSeedCache.history + mockMetricsSeedCache.Unlock() + if cached != nil { + t.Fatal("cancelled seed left a partial template for other tenants to reuse") + } + + history, cacheHit, err = prepareMockMetricsHistory(context.Background(), graph, 7, now, time.Hour, time.Minute, 3500, nil) + if err != nil || cacheHit { + t.Fatalf("seed after cancellation = (cacheHit %v, err %v), want a fresh seed", cacheHit, err) + } + if len(history.GetNodeMetrics("node-c", "cpu", time.Hour)) == 0 { + t.Fatal("fresh seed after cancellation is incomplete") + } +} + +func TestStartMockMetricsSamplerSkipsSeedingWhenMonitorStopping(t *testing.T) { + setMockSamplerTestEnv(t, time.Hour, time.Minute) + resetMockMetricsSeedCacheForTest(t) + previousEnabled := mock.IsMockEnabled() + previousConfig := mock.GetConfig() + t.Cleanup(func() { + mustSetMockEnabled(t, false) + mock.SetMockConfig(previousConfig) + if previousEnabled { + mustSetMockEnabled(t, true) + } + }) + mustSetMockEnabled(t, false) + mock.SetMockConfig(compactMockFixtureConfig()) + mustSetMockEnabled(t, true) + + nodes := mock.CurrentFixtureGraph().State.Nodes + if len(nodes) == 0 { + t.Fatal("expected mock fixture nodes") + } + monitor := &Monitor{ + metricsHistory: NewMetricsHistory(1000, time.Hour), + state: models.NewState(), + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + monitor.startMockMetricsSampler(ctx) + t.Cleanup(monitor.stopMockMetricsSampler) + + if got := len(monitor.metricsHistory.GetNodeMetrics(nodes[0].ID, "cpu", time.Hour)); got != 0 { + t.Fatalf("stopping monitor seeded %d points for node %s", got, nodes[0].ID) + } +} diff --git a/internal/monitoring/mock_metrics_history_test.go b/internal/monitoring/mock_metrics_history_test.go index 15a7cae996..d9defb1050 100644 --- a/internal/monitoring/mock_metrics_history_test.go +++ b/internal/monitoring/mock_metrics_history_test.go @@ -176,7 +176,7 @@ func TestSeedMockMetricsHistory_PopulatesSeries(t *testing.T) { } mh := NewMetricsHistory(1000, 24*time.Hour) - seedMockMetricsHistory(mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) + seedMockMetricsHistory(context.Background(), mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) nodeCPU := mh.GetNodeMetrics("node-1", "cpu", time.Hour) if len(nodeCPU) < 10 { @@ -285,7 +285,7 @@ func TestSeedMockMetricsHistory_AppendsSingleTerminalNowPoint(t *testing.T) { } mh := NewMetricsHistory(1000, 24*time.Hour) - seedMockMetricsHistory(mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) + seedMockMetricsHistory(context.Background(), mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) memorySeries := mh.GetGuestMetrics("docker:cont-1", "memory", time.Hour) if len(memorySeries) < 2 { @@ -334,7 +334,7 @@ func TestSeedMockMetricsHistory_PopulatesKubernetesPodSeries(t *testing.T) { } mh := NewMetricsHistory(1000, 24*time.Hour) - seedMockMetricsHistory(mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) + seedMockMetricsHistory(context.Background(), mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) metricID := kubernetesPodMetricID(state.KubernetesClusters[0], state.KubernetesClusters[0].Pods[0]) if metricID == "" { @@ -387,7 +387,7 @@ func TestSeedMockMetricsHistory_PopulatesKubernetesClusterNodeAndDeploymentSerie } mh := NewMetricsHistory(1000, 24*time.Hour) - seedMockMetricsHistory(mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) + seedMockMetricsHistory(context.Background(), mh, nil, fixtureGraphWithState(state), now, time.Hour, interval) clusterMetricID := kubernetesClusterMetricID(state.KubernetesClusters[0]) nodeMetricID := kubernetesNodeMetricID(state.KubernetesClusters[0], state.KubernetesClusters[0].Nodes[0]) @@ -468,7 +468,7 @@ func TestSeedMockMetricsHistory_SeedsMetricsStore(t *testing.T) { defer store.Close() mh := NewMetricsHistory(1000, seedDuration) - seedMockMetricsHistory(mh, store, fixtureGraphWithState(state), now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), mh, store, fixtureGraphWithState(state), now, seedDuration, time.Minute) points, err := store.Query("vm", "vm-100", "cpu", now.Add(-seedDuration), now, 3600) if err != nil { @@ -531,7 +531,7 @@ func TestSeedMockMetricsHistory_StoreSeedGapFillsWithoutDuplicates(t *testing.T) return len(points) } - seedMockMetricsHistory(NewMetricsHistory(1000, seedDuration), store, fixtureGraphWithState(state), now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), NewMetricsHistory(1000, seedDuration), store, fixtureGraphWithState(state), now, seedDuration, time.Minute) first := countPoints(now) if first == 0 { t.Fatal("expected initial store seed to write minute-tier points") @@ -539,7 +539,7 @@ func TestSeedMockMetricsHistory_StoreSeedGapFillsWithoutDuplicates(t *testing.T) // Re-seeding at the same instant must not add rows: timestamps sit on the // tier spacing grids and existing coverage short-circuits the backfill. - seedMockMetricsHistory(NewMetricsHistory(1000, seedDuration), store, fixtureGraphWithState(state), now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), NewMetricsHistory(1000, seedDuration), store, fixtureGraphWithState(state), now, seedDuration, time.Minute) if got := countPoints(now); got != first { t.Fatalf("expected idempotent reseed to keep %d points, got %d", first, got) } @@ -560,7 +560,7 @@ func TestSeedMockMetricsHistory_StoreSeedGapFillsWithoutDuplicates(t *testing.T) if gapPoints == 0 { t.Fatal("expected the advanced clock to expose new minute-tier grid points") } - seedMockMetricsHistory(NewMetricsHistory(1000, seedDuration), store, fixtureGraphWithState(state), later, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), NewMetricsHistory(1000, seedDuration), store, fixtureGraphWithState(state), later, seedDuration, time.Minute) if got, want := countPoints(later), first+gapPoints; got != want { t.Fatalf("expected gap fill to append exactly the missing grid points (%d), got %d", want, got) } @@ -597,7 +597,7 @@ func TestSeedMockMetricsHistory_SeedsDiskTemperatureMetricsStore(t *testing.T) { defer store.Close() mh := NewMetricsHistory(1000, seedDuration) - seedMockMetricsHistory(mh, store, fixtureGraphWithState(state), now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), mh, store, fixtureGraphWithState(state), now, seedDuration, time.Minute) points, err := store.Query("disk", "SERIAL-001", "smart_temp", now.Add(-seedDuration), now, 3600) if err != nil { @@ -634,7 +634,7 @@ func TestSeedMockMetricsHistory_SeedsVMwareMetricsStore(t *testing.T) { }, } sampler := mock.NewMetricSampler(graph) - seedMockMetricsHistory(mh, store, graph, now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), mh, store, graph, now, seedDuration, time.Minute) hostPoints, err := store.Query("agent", "vc-mock-1:host:host-101", "cpu", now.Add(-seedDuration), now, 3600) if err != nil { @@ -707,7 +707,7 @@ func TestSeedMockMetricsHistory_SeedsTrueNASMetricsStore(t *testing.T) { }, } sampler := mock.NewMetricSampler(graph) - seedMockMetricsHistory(mh, store, graph, now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), mh, store, graph, now, seedDuration, time.Minute) systemPoints, err := store.Query("agent", fixtures.System.Hostname, "disk", now.Add(-seedDuration), now, 3600) if err != nil { @@ -814,7 +814,7 @@ func TestSeedMockMetricsHistory_UsesCanonicalMockFixtureGraphForLegacyAndProvide defer store.Close() mh := NewMetricsHistory(1000, historyRetention) - seedMockMetricsHistory(mh, store, graph, now, seedDuration, sampleInterval) + seedMockMetricsHistory(context.Background(), mh, store, graph, now, seedDuration, sampleInterval) nodePoints, err := store.Query("node", graph.State.Nodes[0].ID, "cpu", now.Add(-seedDuration), now, 3600) if err != nil { @@ -1455,7 +1455,7 @@ func TestSeedMockMetricsHistory_StaysContinuousWithSubsequentLiveMockTicks(t *te } mh := NewMetricsHistory(5000, seedDuration) - seedMockMetricsHistory(mh, nil, fixtureGraphWithState(seedState), now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), mh, nil, fixtureGraphWithState(seedState), now, seedDuration, time.Minute) vmCPUSeeded := mh.GetGuestMetrics("vm-tail", "cpu", seedDuration) if len(vmCPUSeeded) == 0 { @@ -1626,7 +1626,7 @@ func TestRecordMockStateToMetricsHistory_ContinuesCanonicalKubernetesClusterNode deploymentMetricID := kubernetesDeploymentMetricID(seedState.KubernetesClusters[0], seedState.KubernetesClusters[0].Deployments[0]) mh := NewMetricsHistory(5000, seedDuration) - seedMockMetricsHistory(mh, nil, fixtureGraphWithState(seedState), now, seedDuration, time.Minute) + seedMockMetricsHistory(context.Background(), mh, nil, fixtureGraphWithState(seedState), now, seedDuration, time.Minute) recordMockStateToMetricsHistory(mh, nil, fixtureGraphWithState(seedState), next) for name, metricID := range map[string]string{ @@ -1664,7 +1664,7 @@ func TestMockMetricsHistoryKeepsGraphPersonaAcrossSeedAndLiveBoundary(t *testing sampler := mock.NewMetricSampler(graph) mh := NewMetricsHistory(5000, boundedMockHistoryProofWindow) - seedMockMetricsHistory(mh, nil, graph, now, boundedMockHistoryProofWindow, time.Minute) + seedMockMetricsHistory(context.Background(), mh, nil, graph, now, boundedMockHistoryProofWindow, time.Minute) recordMockStateToMetricsHistory(mh, nil, graph, next) series := mh.GetGuestMetrics(resourceID, "memory", boundedMockHistoryProofWindow) @@ -1699,7 +1699,7 @@ func TestSeedMockMetricsHistory_UsesCanonicalMetricModelForPlatformFixtures(t *t } mh := NewMetricsHistory(20_000, 4*time.Hour) - seedMockMetricsHistory(mh, nil, graph, ts, 4*time.Hour, time.Minute) + seedMockMetricsHistory(context.Background(), mh, nil, graph, ts, 4*time.Hour, time.Minute) assertCanonicalSeries := func(series []MetricPoint, resourceType, resourceID, metric string) { t.Helper() @@ -1765,7 +1765,7 @@ func TestSeedMockMetricsHistory_NormalizesTerminalTimestampToCanonicalGrid(t *te }) mh := NewMetricsHistory(4_000, 2*time.Hour) - seedMockMetricsHistory(mh, nil, graph, now, 2*time.Hour, time.Minute) + seedMockMetricsHistory(context.Background(), mh, nil, graph, now, 2*time.Hour, time.Minute) points := mh.GetGuestMetrics("vm-grid", "cpu", 2*time.Hour) if len(points) == 0 { @@ -2030,7 +2030,7 @@ func TestSeedMockMetricsHistory_DiskTelemetryParityAcrossNativeAndTrueNAS(t *tes } mh := NewMetricsHistory(1000, seedDuration) - seedMockMetricsHistory(mh, nil, mock.FixtureGraph{ + seedMockMetricsHistory(context.Background(), mh, nil, mock.FixtureGraph{ State: state, PlatformFixtures: mock.PlatformFixtures{ TrueNAS: fixtures, @@ -2064,7 +2064,7 @@ func TestMockDockerHostSeedCoversTheSameSeriesAsTheSyntheticGenerator(t *testing } now := time.Now().UTC() - seedMockMetricsHistory(history, nil, graph, now, 2*time.Hour, time.Minute) + seedMockMetricsHistory(context.Background(), history, nil, graph, now, 2*time.Hour, time.Minute) hostID := graph.State.DockerHosts[0].ID for _, metricType := range mockHostChartMetricTypes { @@ -2087,7 +2087,7 @@ func TestMockDockerContainerSeedCoversDrawerHistorySeries(t *testing.T) { } now := time.Now().UTC() - seedMockMetricsHistory(history, nil, graph, now, 2*time.Hour, time.Minute) + seedMockMetricsHistory(context.Background(), history, nil, graph, now, 2*time.Hour, time.Minute) containerID := graph.State.DockerHosts[0].Containers[0].ID for _, metricType := range mockGuestChartMetricTypes { @@ -2110,7 +2110,7 @@ func TestMockNodeSeedCoversDrawerHistorySeries(t *testing.T) { } now := time.Now().UTC() - seedMockMetricsHistory(history, nil, graph, now, 2*time.Hour, time.Minute) + seedMockMetricsHistory(context.Background(), history, nil, graph, now, 2*time.Hour, time.Minute) nodeID := graph.State.Nodes[0].ID for _, metricType := range mockNodeChartMetricTypes { diff --git a/internal/monitoring/mock_metrics_seed_cache_test.go b/internal/monitoring/mock_metrics_seed_cache_test.go index 3f2bddf583..7fee313de5 100644 --- a/internal/monitoring/mock_metrics_seed_cache_test.go +++ b/internal/monitoring/mock_metrics_seed_cache_test.go @@ -1,6 +1,7 @@ package monitoring import ( + "context" "testing" "time" @@ -52,7 +53,8 @@ func TestPrepareMockMetricsHistoryReusesRevisionSeedWithIndependentHistories(t * }, }) - first, firstCacheHit := prepareMockMetricsHistory( + first, firstCacheHit, _ := prepareMockMetricsHistory( + context.Background(), graph, fixtureRevision, now, @@ -64,7 +66,8 @@ func TestPrepareMockMetricsHistoryReusesRevisionSeedWithIndependentHistories(t * if firstCacheHit { t.Fatal("first seed unexpectedly reported a cache hit") } - second, secondCacheHit := prepareMockMetricsHistory( + second, secondCacheHit, _ := prepareMockMetricsHistory( + context.Background(), graph, fixtureRevision, now, @@ -84,7 +87,8 @@ func TestPrepareMockMetricsHistoryReusesRevisionSeedWithIndependentHistories(t * firstGeneration := mockMetricsSeedCache.generation mockMetricsSeedCache.Unlock() - _, revisionCacheHit := prepareMockMetricsHistory( + _, revisionCacheHit, _ := prepareMockMetricsHistory( + context.Background(), graph, fixtureRevision+1, now, @@ -163,7 +167,7 @@ func TestPrepareMockMetricsHistoryBoundsFullDemoEstateBeforeCaching(t *testing.T } now := time.Now().UTC().Truncate(time.Minute) - history, cacheHit := prepareMockMetricsHistory(full, 99, now, time.Hour, time.Minute, 3500, nil) + history, cacheHit, _ := prepareMockMetricsHistory(context.Background(), full, 99, now, time.Hour, time.Minute, 3500, nil) if cacheHit { t.Fatal("first full-estate seed unexpectedly reported a cache hit") }