diff --git a/.github/workflows/unified-agent-native.yml b/.github/workflows/unified-agent-native.yml index ca020f2c39..ec5fa82dc2 100644 --- a/.github/workflows/unified-agent-native.yml +++ b/.github/workflows/unified-agent-native.yml @@ -84,6 +84,26 @@ jobs: go-version-file: go.mod cache: true + - name: Supply GNU timeout for macOS guidance checks + if: runner.os == 'macOS' + shell: bash + run: | + set -euo pipefail + brew install coreutils + gnu_timeout="$(brew --prefix coreutils)/bin/gtimeout" + test -x "$gnu_timeout" + "$gnu_timeout" --version | grep -q 'GNU coreutils' + timeout_bin="$(mktemp -d "$RUNNER_TEMP/pulse-guidance-tools.XXXXXX")" + ln -s "$gnu_timeout" "$timeout_bin/timeout" + printf '%s\n' "$timeout_bin" >> "$GITHUB_PATH" + + - name: Verify native Unix timeout prerequisite + if: matrix.unix + shell: bash + run: | + set -euo pipefail + timeout --version | grep -q 'GNU coreutils' + - name: Test native agent runtime run: >- go test diff --git a/docs/DOCKER.md b/docs/DOCKER.md index ac46ce3f9e..e35f9f2cf1 100644 --- a/docs/DOCKER.md +++ b/docs/DOCKER.md @@ -123,6 +123,57 @@ services: --- +## Container CPU readings + +Pulse shows container CPU as a share of the **Docker/Podman host's total CPU +capacity**: 100% means all its logical CPUs. `docker stats` and `podman stats` +normally use **100% for one logical CPU**, so their readings can exceed 100%. +For that scale, divide the stats percentage by the CPU count reported for the +runtime host to compare it with Pulse. + +If the engine runs inside a VM, use that VM's logical CPUs, not the physical +Proxmox host's CPUs. Pulse's percentage is not utilisation of the container's +own CPU quota or cpuset. If the reported CPU count differs from your configured +VM size, retain both in the comparison rather than guessing a divisor. + +| Example host logical CPUs | Stats CPU | Pulse CPU before display rounding | +| --- | --- | --- | +| 4 | 240% | 60% | +| 6 | 16.52% | About 2.75% | +| 6 | 1.04% | About 0.17% | + +These are examples, not an assumption about your host. A smaller Pulse value +alone does not establish a collection fault. Whole-percent table labels, +including those in **v6.5.0-rc.1**, can display a fractional reading below 0.5% +as 0%. Use **CPU History** rather than the rounded label; display rounding does +not change stored readings or the values used by CPU alert evaluation. A dash +or missing History is unavailable data, not a measured zero. + +For a useful comparison: + +1. Match the same container on the same runtime host. Compare a similar time + interval in CPU History with `docker stats` or `podman stats`; separate + sampling intervals can legitimately differ. +2. Apply the reported host CPU count only to a stats reading on the per-CPU + scale. Do not divide Pulse History again, or compare with an average since + container start as though it were a recent interval. +3. Check that observation times and History points advance during ordinary + monitoring. A connection marked Active does not prove fresh CPU samples. +4. If the normalised readings still disagree, or History stops advancing, + report the server and agent versions, reported CPU count, sample times and + bounded readings. Do not restart workloads, create CPU load, lower alert + thresholds or export diagnostics just to make this comparison. + +CPU alerts use the same host-capacity scale, with the configured thresholds, +duration and suppression rules. Non-zero History proves a collected reading, +not that an alert should fire or that its notification was delivered. If a +normal workload already demonstrates an alert problem, retain the reading, +threshold, duration and observed result; no forced alert test is needed. See +[safe performance measurements](TROUBLESHOOTING.md#excessive-cpu-writes-or-database-growth) +when measuring the Pulse server itself rather than a monitored container. + +--- + ## 🔄 Updates These steps update the Pulse server, not the containers it monitors. Record the diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index dd9e0a54ef..213b11b17d 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -251,6 +251,20 @@ repair an older generated unit rather than adding a JSON-parsing wrapper. follow the [platform-specific guide](TEMPERATURE_MONITORING.md) for any needed setup change during a maintenance window. +#### Container CPU differs from Docker or Podman stats + +Pulse uses the runtime host's total CPU capacity; `docker stats` and +`podman stats` normally use 100% per logical CPU. Divide a per-CPU stats reading +by the **reported runtime host CPU count**, not the physical Proxmox host's +count when the engine runs inside a VM. Compare the same container and time +interval in **CPU History**, not just a rounded table label. A dash or missing +History is unavailable, not zero; an Active connection alone is not fresh CPU +evidence. + +See [container CPU readings](DOCKER.md#container-cpu-readings) for worked +examples, rounding and alert limits, and a passive comparison. Do not restart +workloads, create CPU load or lower alert thresholds to test this. + #### Docker hosts appearing/disappearing Cloned hosts can share a **saved Pulse agent ID**, not just an OS machine ID. diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 469e7d2737..0e9db6c03d 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -7520,13 +7520,26 @@ coarse activity class (`list`, `export`, `verify`, `summary`); the handler runs unchanged whether or not recording succeeds, and a router without persistence serves the request rather than failing it (`TestWithAuditReadActivity_NilPersistenceIsSafe`). -### Monitor shutdown drains queued guest metadata writes +### Monitor shutdown seals guest metadata admission and retains incomplete teardown + +`Monitor.Stop` seals guest identity/metadata mutations and waits under its +unchanged two-second metadata budget. One store-owned snapshot writer coalesces +changes admitted during filesystem I/O; an older queued identity cannot overwrite +an operator edit or reverse a newer accepted identity. `Close` reports both an +incomplete drain and a final persistence error, while the public void Stop logs +failure rather than claiming a successful stop. + +Tenant removal must first observe its monitoring loop exit and then a successful +metadata close. Failure retains the monitor, cancellation/done handles and +lifecycle guard: lazy recreation cannot start a second writer in the same +directory. `BeginTenantDeletion` returns that failure to the organisation +handler, which keeps persistence and offboarding callbacks untouched. Only +successful shutdown allows directory deletion. A later explicit shutdown attempt +may persist retained metadata after a completed write failure is repaired; +admission stays sealed and no in-flight attempt is replayed. Other tenants keep their existing +independent runtime. Store drain does not establish independent guest thaw, +workload liveness or containment in installed software. -`Monitor.Stop` waits for in-flight `GuestMetadataStore` writes before closing -the metrics store, so a tenant monitor that has been stopped is guaranteed not -to write into its data directory afterwards. Tenant offboarding and any caller -that removes a tenant directory can rely on `Stop` having quiesced disk writes, -rather than racing a detached goroutine. ### Per-tenant resource stores are released on offboarding and shutdown `ResourceHandlers.getStore` opens a SQLite handle per org and caches it for the diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 4cc867961f..1b1ee43136 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -20,6 +20,20 @@ ## Purpose +### Organisation deletion retains data after incomplete monitoring shutdown + +Authenticated organisation-owner deletion waits for tenant-loop exit and sealed +guest metadata persistence before removing the directory or calling offboarding. +An incomplete drain or final write failure returns HTTP 503 with the existing +error envelope and `tenant_shutdown_incomplete`; the message says the data was +retained and exposes no filesystem path or underlying error. The runtime owner +and recreation guard stay retained. Default-organisation immutability, session +and membership checks, successful HTTP 204 and other tenants' access are +unchanged. `TestOrgDeleteRetainsDataWhenGuestMetadataPersistenceFails` exercises +a real confined rename failure through the ordinary handler and metadata store; +existing successful deletion and authorization controls remain required. + + ### Optional guest-memory observation reaches existing frontend consumers Frontend Memory and ResourceMetric model the existing optional server-owned @@ -11207,3 +11221,20 @@ Pinning the Patrol attention detail header on phones layout-only change. It reads no new attention fields, sends no new request, and leaves the attention projection, lifecycle, and queue ordering contracts above unchanged. + +### Router-owned worker shutdown + +Each Router owns its five route-specific limiters, eight endpoint-category +limiters and, in hosted mode, its tenant limiter. `ShutdownBackgroundWorkers` +seals lifecycle-worker admission, cancels and joins the constructor-owned +workers exactly once; the server's existing `ShutdownAIIntelligence` calls +that same lifecycle. HTTP admission must stop before shutdown. Resource and +RBAC stores retain their separate shutdown owners. Integration fixtures use +this lifecycle before stopping the monitor and removing temporary data. + +`RateLimiter.Stop` is concurrent-safe and joins its cleanup worker. Stopping +cleanup does not clear request history, bypass rate enforcement or stop another +Router's or the package-global limiters. Limits, windows, tenant selection, +revocation and response payloads are unchanged. The connected lifetime and +ownership controls are in `router_limiter_lifecycle_test.go`; the integration +fixture's cleanup also runs on a failed listener/request assertion. diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 48bbc86ac5..0e47f9e058 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -17,6 +17,36 @@ ## Purpose +### Fresh broadcast projection owns one resource/target capture + +Canonical live, fallback and standalone-host continuity broadcast reads prefer +the adapter's detached bulk resource/metrics-target capture. After host +coalescing selects presentation identity, missing history targets are attached +from that same capture, including authoritative absence. Stores without this +optional capability retain the previous point-resolver fallback. Only the owned +outer coalesced slice is decorated in place; nested source payloads stay read-only. + +Every read still observes complete current resource data and derives live alert +health, metadata URL edits/clears and connected/ignored infrastructure. No +timestamp-only cache, telemetry omission, policy change or new wire field is +introduced. All existing catalogs, discovery/history coordinates, original +observation times, LastSeen, identity and tenant/revocation boundaries remain. + +`broadcast_metrics_snapshot_test.go` compares the complete served JSON with an +independent earlier projection across the metadata-heavy nine-provider demo, +typed-view and same-time dirty cases; it also covers captured target replacement +and continuity/metadata/live-alert boundaries. `monitor_host_agents_test.go` +retains its one-materialization and live-change regression. Benchmarks include +the exact parent's complete conversion and final JSON encoding, with cold, +list-only, typed-clean and dirty inputs; they do not establish installed CPU/RSS +relief, native identity refresh throughput or completion of #2199. + +Complete-content comparisons use one health-evaluation instant through a +private clock seam. Production still samples real UTC at the same post-read +boundary. The minute-boundary control requires age labels to advance without +renewing LastSeen; comparisons retain every health field instead of stripping +the reproduced `1m`/`2m` clock difference from JSON. + ### Report identity lookups do not rebuild unrelated Kubernetes metadata The producer's two temporary host/Docker-host admission registries use a @@ -4581,27 +4611,45 @@ host dataset evidence is copied onto the matching provider-owned ZFS pool; provider health, scan, device, and error fields remain authoritative. When the provider cannot return pool detail, monitoring may synthesize only a minimal `UNKNOWN` pool so valid dataset evidence is still inspectable. -### Guest metadata writes are owned by the store and drained on shutdown +### Guest identity admission coalesces owned JSON snapshots -`persistGuestIdentity` no longer detaches its own goroutine per changed guest. -It calls `GuestMetadataStore.SetAsync`, which tracks the write on a WaitGroup -so `GuestMetadataStore.WaitForPendingWrites` can drain it. `Monitor.Stop` drains -before closing the metrics store, under a bounded timeout matching -`tenantMonitorShutdownTimeout` so a wedged store cannot hold up tenant teardown. +`persistGuestIdentity` calls `GuestMetadataStore.RememberIdentity`, which merges +only the last-known name/type under the store lock, preserves operator URLs, +descriptions, tags and notes, and never downgrades OCI. Admission is visible +before return. `SetAsync` deep-copies its full replacement at admission as well; +caller mutation and goroutine scheduling cannot reverse accepted update order. -Untracked writes were observable, not theoretical: a queued write could land -after the monitor stopped and after a tenant directory was being removed, -leaving a stray `guest_metadata.json.tmp` from the interrupted atomic write. -That is what made `TestHostedTenantAgentInstallTokenCannotReportToOtherTenant` -fail its `t.TempDir` cleanup with "directory not empty". -`TestGuestMetadataStore_WaitForPendingWritesDrainsQueuedWrites` and -`TestGuestMetadataStore_DataDirIsRemovableAfterDrain` pin the drain and fail if -`SetAsync` stops tracking its goroutine. +One lazy store-owned writer snapshots the complete metadata map. Filesystem I/O +does not hold the memory lock, so changes admitted during that I/O are coalesced +into the next complete snapshot. A separate write mutex serializes it with +synchronous Set/Delete/ReplaceAll/UpdateAll and legacy migration; an older +snapshot cannot resurrect a deleted or migrated entry or overwrite an operator +edit. The existing tenant-local JSON shape, private file permissions and atomic +temporary-file/rename path stay unchanged. A failed write is reported and not +spin-retried; a later ordinary identity poll can retry the unpersisted state. -Known and deliberately unchanged: each changed guest still triggers a full-file -save, so one poll cycle over N changed guests performs N marshals and N atomic -writes that serialize on the store mutex. Coalescing them is a behavioural -change beyond the shutdown defect. +`WaitForPendingWrites` reports quiescence, not durable success. `Close` seals +mutation admission and observes the background writer and any admitted +synchronous write within the unchanged caller budget. Timeout or final +persistence failure remains an error. Monitor Stop logs that failure instead of +claiming a clean stop; tenant removal retains the runtime owner and lifecycle +guard, and organisation deletion refuses to remove its directory. An unfinished +monitoring loop likewise prevents deletion. Shutdown does not cancel an admitted +filesystem write or silently discard metadata to obtain a timely result. A later +explicit close may retry a completed failed snapshot after filesystem repair; +admission stays sealed, successful close is idempotent, and no close retries +itself or overlaps a still-running attempt. + +`internal/config/guest_writer_control_test.go` exercises blocked snapshots at +25/100 updates and caller-copy ownership. `guest_metadata_writer_test.go` covers +ordered replacement, synchronous mutations, field-scoped identity, failure and +explicit recovery, bounded close with blocked synchronous/asynchronous I/O, +closed admission, real filesystem permissions and tenant separation. Existing +ten-second drain and cleanup controls remain unchanged. Monitoring shutdown +controls retain blocked writer/loop ownership; the real organisation-delete +handler returns its safe failure before offboarding or directory removal. +These are source controls, not an attribution of an earlier drain timeout, +native CPU/RSS relief or all-path guest-agent recovery. ### Monitoring projects canonical resource policy into alert evaluation diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index e2c9654529..8610516f3c 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -3744,3 +3744,26 @@ only explicit matching refreshes; the full `GuestDrawer.test.tsx` pins the unchanged cap and no live-recovery read. `guestHistorySegments.test.ts` pins complete point preservation at the existing request bound. Neither claims a timing benchmark, fleet CPU attribution or native incident relief. + + +### Accessible Workloads backup evidence + +Both the Backup column badge and its hidden-column name indicator are native +read-only disclosure buttons. Keyboard activation and an ordinary first touch +open the same named-guest completion evidence that fine-pointer hover shows. +Exact time and age use readable theme text rather than colour alone. The shared +Dialog owns focus, Escape/backdrop dismissal and background isolation, and +returns focus to the originating disclosure. Nested evidence activation must +not trigger the enclosing guest row. Touch targets are at least 44px; compact +desktop shields and the existing status colours remain. + +The evidence content is reactive to the same guest identity: running is not +completion, malformed/future times are unknown, and only the existing absence +sentinels mean no backup recorded. A removed row unmounts its disclosure and +restores dialog isolation. Opening details adds no API/history read, collector, +permission, backup/guest operation or polling timer. Dialog machinery and +content are mounted only when opened; existing freshness thresholds and source +projection are unchanged. `GuestRow.test.tsx` covers both nested row paths and +`BackupStatusAccess.test.tsx` covers semantics, focus and observation changes. +Browser proof exercises production rows with synthetic data, not native backup +or guest-recovery acceptance. diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index f16a46ca9c..15c3f338e7 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -23,6 +23,29 @@ and sort the complete canonical change table while startup and ingestion wait. ## Purpose +### Broadcast bulk reads capture resources and history coordinates together + +`ListWithMetricsTargets` and the monitor adapter's matching optional read return +detached resources and a detached target map from one locked registry generation. +The existing source-mapping inversion runs once for this read when typed views +are dirty; it does not build all typed views or add a retained generation cache. +Clean typed views may supply their already-current inverse mapping. Legacy List, +GetAll and individual target reads preserve their shapes and semantics. + +The target map stays separate from resources until presentation host coalescing +selects the row identity. Missing coordinates remain missing in that capture; +they never fall through to a later live registry. Mapping changes with unchanged +LastSeen take effect on the next read. Returned resources, policy/identity blocks +and target values remain caller-owned across mutations and adapter replacement. +Projection cannot pair an earlier resource row with a later history identity. + +`registry_test.go`, `registry_projection_test.go` and +`monitor_adapter_read_state_test.go` exercise full resource/point-target parity, +same-time edits, concurrent mapping writes, detachments, empty/nil reads and +tenant separation. Connected monitoring proof covers all nine demo providers, +continuity overlays and complete served JSON. Paired cold/clean/dirty benchmarks +measure whole materialization, not a new point-lookup envelope or native relief. + Canonical resource History honours selected memory observation provenance and Proxmox VM filesystem deferrals instead of treating numeric carriers as fresh measurements. ### Canonical resource History cannot promote retained guest readings diff --git a/frontend-modern/browser-tests/backup-details-access.cjs b/frontend-modern/browser-tests/backup-details-access.cjs new file mode 100644 index 0000000000..0fef9ecc26 --- /dev/null +++ b/frontend-modern/browser-tests/backup-details-access.cjs @@ -0,0 +1,359 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const crypto = require('node:crypto'); +const { chromium, webkit } = require('playwright'); +const root = '/workspace'; +const frontend = path.join(root, 'frontend-modern'); +const output = path.join(root, 'tmp/backup-details-access/browser'); +const origin = 'http://127.0.0.1:5298'; +const runtime = [ + 'frontend-modern/src/components/Workloads/GuestRowCells.tsx', + 'frontend-modern/src/components/Workloads/GuestRow.tsx', +]; +const hash = (p) => crypto.createHash('sha256').update(fs.readFileSync(p)).digest('hex'); +(async () => { + fs.mkdirSync(output, { recursive: true }); + const version = require('playwright/package.json').version; + assert.equal( + version, + JSON.parse(fs.readFileSync(path.join(root, 'tests/integration/package-lock.json'))).packages[ + 'node_modules/@playwright/test' + ].version, + ); + const hashes = Object.fromEntries( + runtime.map((source) => [source, hash(path.join(root, source))]), + ); + process.chdir(frontend); + const { createServer } = await import( + path.join(frontend, 'node_modules/vite/dist/node/index.js') + ); + const server = await createServer({ + cacheDir: path.join(output, 'vite-cache'), + server: { host: '127.0.0.1', port: 5298, strictPort: true }, + }); + const results = []; + const captures = []; + const phases = []; + const started = Date.now(); + const mark = (stage) => { + const fact = { + stage, + elapsedMs: Date.now() - started, + memory: process.memoryUsage(), + resources: process.resourceUsage(), + }; + phases.push(fact); + console.log(JSON.stringify({ browserPhase: fact })); + }; + let browser; + let phase = 'server'; + try { + mark('listen-start'); + await server.listen(); + mark('listen-complete'); + // Supported Vite transform warmup, not a production build or app execution. + // Keep page navigation and the tool deadline unchanged; collect phases. + await server.warmupRequest('/browser-tests/backup-details-access.tsx'); + await server.warmupRequest('/src/index.css'); + mark('entry-css-warmup-complete'); + for (const [name, engine, phone, dark] of [ + ['desktop-light', chromium, false, false], + ['desktop-dark', chromium, false, true], + ['phone-light', webkit, true, false], + ['phone-dark', webkit, true, true], + ]) { + phase = name; + browser = await engine.launch( + engine === chromium + ? { headless: true, channel: 'chromium', args: ['--no-sandbox'] } + : { headless: true }, + ); + const page = await browser.newPage({ + viewport: { width: phone ? 390 : 1365, height: phone ? 844 : 900 }, + isMobile: phone, + hasTouch: phone, + locale: 'en-GB', + timezoneId: 'UTC', + }); + page.setDefaultTimeout(12000); + page.setDefaultNavigationTimeout(45000); + await page.clock.setFixedTime(new Date('2026-10-05T08:00:00Z')); + const errors = []; + const operations = []; + const reads = []; + const checks = []; + page.on('pageerror', (e) => errors.push(e.message)); + await page.route('**/*', (route) => { + const request = route.request(); + const url = new URL(request.url()); + assert.equal(url.origin, origin, 'no off-origin requests'); + if (!url.pathname.startsWith('/api/')) return route.continue(); + if (request.method() !== 'GET') operations.push(`${request.method()} ${url.pathname}`); + reads.push(url.pathname); + return route.fulfill({ + json: { success: true, data: [], alerts: [], config: null, anomalies: [] }, + }); + }); + if (dark) + await page.addInitScript(() => + document.addEventListener('DOMContentLoaded', () => + document.documentElement.classList.add('dark'), + ), + ); + mark(`${name}:navigate-start`); + await page.goto(`${origin}/browser-tests/backup-details-access.html`, { + waitUntil: 'domcontentloaded', + }); + mark(`${name}:domcontentloaded`); + const row = page.getByRole('region', { name: 'backup-vm row' }); + const badge = row.getByRole('button', { name: /^Backup status:/ }); + await badge.waitFor(); + const identity = await page.evaluate(() => window.__backupDetails.identity()); + const rowActionCount = async () => + Number(await page.getByRole('status', { name: 'Row action count' }).innerText()); + const activate = async (el) => (phone ? el.tap() : el.click()); + const capture = async (suffix) => { + const file = path.join(output, `${name}-${suffix}.png`); + await page.screenshot({ path: file, fullPage: true }); + captures.push({ + path: path.relative(root, file), + bytes: fs.statSync(file).size, + sha256: hash(file), + }); + }; + const dialog = page.getByRole('dialog', { name: /^Backup status details for / }); + const close = page.getByRole('button', { name: 'Close backup details' }); + const checkOpen = async (trigger) => { + await dialog.waitFor({ state: 'visible' }); + assert.equal(await trigger.getAttribute('aria-expanded'), 'true'); + assert.equal(await rowActionCount(), 0, 'backup action must not open the guest row'); + await close.waitFor(); + await page.waitForFunction( + () => document.activeElement?.getAttribute('aria-label') === 'Close backup details', + ); + // DOM visibility can precede the CSS entrance animation. Verify and + // capture the final user-visible panel, not its initial off-screen frame. + await dialog.evaluate(async (el) => { + await Promise.all( + el + .getAnimations({ subtree: true }) + .filter((a) => Number.isFinite(a.effect?.getTiming().iterations)) + .map((a) => a.finished.catch(() => {})), + ); + }); + const rect = await dialog.boundingBox(); + assert( + rect.x >= 0 && rect.x + rect.width <= (phone ? 390 : 1365), + 'dialog stays within viewport', + ); + assert( + rect.y >= 0 && rect.y + rect.height <= (phone ? 844 : 900), + 'settled panel stays vertically in view', + ); + assert( + await page.locator('main').evaluate((el) => el.closest('body > *').hasAttribute('inert')), + 'background isolated', + ); + }; + const checkClosed = async (trigger) => { + await dialog.waitFor({ state: 'hidden' }); + assert.equal(await trigger.getAttribute('aria-expanded'), 'false'); + assert( + await trigger.evaluate((el) => document.activeElement === el), + 'focus returns to source evidence', + ); + assert.equal( + await page.locator('main').evaluate((el) => el.closest('body > *').hasAttribute('inert')), + false, + ); + }; + assert.equal(await badge.getAttribute('aria-haspopup'), 'dialog'); + assert.equal((await badge.innerText()).trim(), '', 'fresh shield remains compact'); + if (!phone) { + await badge.hover(); + const tooltip = page.locator('[data-tooltip-portal="true"]'); + await tooltip.waitFor({ state: 'visible' }); + assert((await tooltip.innerText()).includes('Last completed backup')); + await page.getByRole('heading').hover(); + // Genuine sequential keyboard traversal, not synthetic click/focus. + await page.keyboard.press('Tab'); // column control + await page.keyboard.press('Tab'); // guest disclosure + await page.keyboard.press('Tab'); // backup disclosure + assert(await badge.evaluate((el) => document.activeElement === el)); + await page.keyboard.press('Enter'); + checks.push('desktop hover preserved and sequential Tab/Enter opens actual badge'); + } else { + const rect = await badge.boundingBox(); + assert(rect.width >= 44 && rect.height >= 44, 'touch disclosure is at least 44px'); + await activate(badge); + assert.equal( + await page.locator('[data-tooltip-portal="true"]').count(), + 0, + 'no synthetic hover overlay', + ); + checks.push('ordinary first tap opens 44px target, without hover UI'); + } + await checkOpen(badge); + assert((await dialog.innerText()).includes('backup-vm')); + assert((await dialog.innerText()).includes('Mon, 5 Oct 2026')); + assert((await dialog.innerText()).includes('06:00:00')); + assert((await dialog.innerText()).includes('2 hours ago')); + await capture('fresh-completed'); + await page.keyboard.press('Tab'); + assert( + await close.evaluate((el) => document.activeElement === el), + 'one-action dialog retains focus', + ); + await page.evaluate(() => + window.__backupDetails.update({ timestamp: '2026-10-05T06:00:00Z', running: true }), + ); + assert((await dialog.innerText()).includes('Backup running now')); + assert((await dialog.innerText()).includes('Last completed backup')); + assert.equal(await page.evaluate(() => window.__backupDetails.identity()), identity); + checks.push('same identity updates running while preserving prior completed observation'); + await capture('running-completed'); + await page.evaluate(() => + window.__backupDetails.update({ timestamp: '2026-10-05T08:01:00Z', running: true }), + ); + await dialog.getByText(/timestamp is in the future/).waitFor(); + assert(!(await dialog.innerText()).includes('Last completed backup')); + assert(!(await dialog.innerText()).includes('No backup has ever')); + await capture('future-running'); + await page.keyboard.press('Escape'); + await checkClosed(badge); + if (!phone) { + await page.keyboard.press('Space'); + await checkOpen(badge); + assert((await dialog.innerText()).includes('timestamp is in the future')); + await page.keyboard.press('Escape'); + await checkClosed(badge); + checks.push('native Space reopens current evidence and Escape restores focus'); + } + await page.getByRole('button', { name: 'Hide Backup column' }).click(); + const indicator = row.getByRole('button', { name: /^Backup running now/ }); + await activate(indicator); + await checkOpen(indicator); + assert((await dialog.innerText()).includes('timestamp is in the future')); + checks.push('hidden-column indicator exposes exactly the same uncertainty'); + await page.evaluate(() => + window.__backupDetails.update({ timestamp: 'bad-time', running: false }), + ); + await dialog.getByText(/invalid timestamp/).waitFor(); + await page.evaluate(() => window.__backupDetails.update({ timestamp: '', running: false })); + await dialog + .getByText('No backup has ever been recorded for this guest.', { exact: true }) + .waitFor(); + assert(!(await dialog.innerText()).includes('Invalid Date')); + assert(!(await dialog.innerText()).includes('NaN')); + checks.push('malformed then absence retain distinct truthful states'); + await activate(close); + await checkClosed(row.getByRole('button', { name: 'No backup found', exact: true })); + const ct = page + .getByRole('region', { name: 'backup-ct row' }) + .getByRole('button', { name: 'No backup found', exact: true }); + await activate(ct); + await checkOpen(ct); + assert((await dialog.innerText()).includes('backup-ct')); + assert( + !(await dialog.innerText()).includes('2 hours ago'), + 'adjacent guest never inherits another observation', + ); + await capture('ct-missing'); + await page.locator('[data-dialog-backdrop]').click({ position: { x: 3, y: 3 } }); + await checkClosed(ct); + await page.evaluate(() => + window.__backupDetails.update({ timestamp: '2026-10-05T06:00:00Z', running: false }), + ); + const recovered = row.getByRole('button', { name: /^Last backup:/ }); + await activate(recovered); + await checkOpen(recovered); + assert((await dialog.innerText()).includes('2 hours ago')); + await page.evaluate(() => window.__backupDetails.remove()); + await dialog.waitFor({ state: 'hidden' }); + assert.equal(await page.locator('[data-dialog-layer]').count(), 0); + assert.equal( + await page.locator('main').evaluate((el) => el.closest('body > *').hasAttribute('inert')), + false, + ); + checks.push( + 'recovery and actual row removal release dialog/inertness without extra operations', + ); + assert.equal(await rowActionCount(), 0); + await activate( + page.getByRole('region', { name: 'backup-ct row' }).getByText('backup-ct', { exact: true }), + ); + assert.equal(await rowActionCount(), 1, 'ordinary row action still works'); + assert.deepEqual(errors, []); + assert.deepEqual(operations, []); + assert( + !reads.some((p) => /backup|metrics|guest|resources/.test(p)), + 'opening evidence adds no collector/history read', + ); + results.push({ + name, + browser: browser.version(), + checks, + apiReads: reads, + operations, + errors, + finalRowActions: await rowActionCount(), + }); + mark(`${name}:checks-complete`); + await page.close(); + await browser.close(); + browser = undefined; + } + fs.writeFileSync( + path.join(output, 'result.json'), + JSON.stringify( + { + result: 'passed', + playwrightVersion: version, + runtime: hashes, + phases, + results, + captures, + }, + null, + 2, + ) + '\n', + ); + console.log( + JSON.stringify({ + result: 'passed', + cases: results.length, + checks: results.reduce((n, r) => n + r.checks.length, 0), + captures: captures.length, + hashes, + playwrightVersion: version, + }), + ); + } catch (e) { + fs.writeFileSync( + path.join(output, 'failure.json'), + JSON.stringify( + { + result: 'failed', + phase, + error: String(e), + runtime: hashes, + phases, + playwrightVersion: version, + captures, + results, + }, + null, + 2, + ) + '\n', + ); + throw e; + } finally { + if (browser) await browser.close(); + await server.close(); + mark('browser-server-closed'); + } +})().catch((e) => { + console.error(e); + process.exitCode = 1; +}); diff --git a/frontend-modern/browser-tests/backup-details-access.html b/frontend-modern/browser-tests/backup-details-access.html new file mode 100644 index 0000000000..488f6afbdc --- /dev/null +++ b/frontend-modern/browser-tests/backup-details-access.html @@ -0,0 +1,12 @@ + + + + + + Backup evidence accessibility verification + + +
+ + + diff --git a/frontend-modern/browser-tests/backup-details-access.tsx b/frontend-modern/browser-tests/backup-details-access.tsx new file mode 100644 index 0000000000..1f7c7030c9 --- /dev/null +++ b/frontend-modern/browser-tests/backup-details-access.tsx @@ -0,0 +1,110 @@ +// Production Workloads rows/shared dialog, with explicit guest observations. +// Mapping/collection are unchanged. No backup, guest, thaw or credential operation. +import { createSignal, Show } from 'solid-js'; +import { render } from 'solid-js/web'; +import { Router, Route } from '@solidjs/router'; +import { GuestRow } from '../src/components/Workloads/GuestRow'; +import { DarkModeContext } from '../src/contexts/appRuntime'; +import type { WorkloadGuest } from '../src/types/workloads'; +import '../src/index.css'; + +type Observation = { timestamp: string; running: boolean }; +const guest = (vmid: number, value: Observation): WorkloadGuest => ({ + id: `fixture-pve1-${vmid}`, + vmid, + name: vmid === 101 ? 'backup-vm' : 'backup-ct', + node: 'pve1', + instance: 'fixture', + status: 'running', + type: vmid === 101 ? 'qemu' : 'lxc', + workloadType: vmid === 101 ? 'vm' : 'system-container', + platformType: 'proxmox-pve', + cpu: 0.1, + cpus: 2, + memory: { total: 1024 ** 3, used: 1024 ** 3 / 4, free: (3 * 1024 ** 3) / 4, usage: 25 }, + disk: { total: 10 * 1024 ** 3, used: 0, free: 10 * 1024 ** 3, usage: 0 }, + networkIn: 0, + networkOut: 0, + diskRead: 0, + diskWrite: 0, + uptime: 3600, + template: false, + lastBackup: value.timestamp === '' ? 0 : Date.parse(value.timestamp), + backupInProgress: value.running, + tags: null, + lock: value.running ? 'backup' : '', + lastSeen: '2026-10-05T08:00:00Z', +}); +const initial: Observation = { timestamp: '2026-10-05T06:00:00Z', running: false }; +function Fixture() { + const [vm, setVM] = createSignal(guest(101, initial)); + const ct = guest(102, { timestamp: '', running: false }); + const [vmPresent, setVMPresent] = createSignal(true); + const [actions, setActions] = createSignal(0); + const [showBackup, setShowBackup] = createSignal(true); + (window as any).__backupDetails = { + update: (value: Observation) => setVM(guest(101, value)), + remove: () => setVMPresent(false), + identity: () => vm().id, + }; + return ( +
+

Backup evidence accessibility

+

+ Synthetic guest observations, no backup commands or collector reads. +

+ + +
+ + + setActions((n) => n + 1)} + /> + +
+
+
+
+ + + setActions((n) => n + 1)} + /> + +
+
+

+ {actions()} +

+
+ ); +} +render( + () => ( + + ( + document.documentElement.classList.contains('dark')} + > + + + )} + /> + + ), + document.getElementById('root')!, +); diff --git a/frontend-modern/browser-tests/container-cpu-help.cjs b/frontend-modern/browser-tests/container-cpu-help.cjs new file mode 100644 index 0000000000..dbaa300a2d --- /dev/null +++ b/frontend-modern/browser-tests/container-cpu-help.cjs @@ -0,0 +1,117 @@ +// The real Docs renderer, router and shipped help. No runtime socket, workload, +// credentials, alert test or diagnostics collection is exercised. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const crypto = require('node:crypto'); +const { chromium, webkit } = require('playwright'); + +(async () => { + const root = path.resolve('frontend-modern'); + const artifacts = path.join(root, 'node_modules', 'container-cpu-help-browser'); + fs.mkdirSync(artifacts, { recursive: true }); + process.chdir(root); + const { createServer } = await import(path.join(root, 'node_modules/vite/dist/node/index.js')); + const server = await createServer({ + root, configFile: path.join(root, 'vite.config.ts'), + cacheDir: path.join(root, 'node_modules', '.vite-container-cpu-help'), + server: { host: '127.0.0.1', port: 5265, strictPort: true }, + optimizeDeps: { noDiscovery: true, include: [] }, + }); + const results = []; + let browser; + try { + await server.listen(); + for (const [engine, width, tone] of [ + ['chromium', 1280, 'light'], ['chromium', 1280, 'dark'], + ['webkit', 390, 'light'], ['webkit', 390, 'dark'], + ]) { + browser = await { chromium, webkit }[engine].launch(engine === 'chromium' + ? { headless: true, channel: 'chromium', args: ['--no-sandbox'] } + : { headless: true }); + const page = await browser.newPage({ viewport: { width, height: 900 }, + ...(engine === 'webkit' ? { isMobile: true, hasTouch: true } : {}) }); + const errors = []; + const operations = []; + page.on('pageerror', (error) => errors.push(error.message)); + page.on('console', (message) => { + if (message.type() === 'error') errors.push(message.text()); + }); + page.on('request', (request) => { + if (request.url().includes('/api/') || request.method() !== 'GET') + operations.push({ method: request.method(), url: request.url() }); + }); + await page.goto('http://127.0.0.1:5265/browser-tests/docs-fragment-navigation.html?scenario=troubleshooting', + { waitUntil: 'domcontentloaded', timeout: 60_000 }); + if (tone === 'dark') await page.evaluate(async () => { + document.documentElement.classList.add('dark'); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + }); + const entry = page.getByRole('heading', { + name: 'Container CPU differs from Docker or Podman stats', exact: true, + }); + await entry.waitFor(); + await entry.evaluate((heading) => heading.scrollIntoView({ block: 'start' })); + await page.screenshot({ path: path.join(artifacts, `${engine}-${tone}-entry.png`) }); + const guideLink = page.getByRole('link', { name: 'container CPU readings', exact: true }); + if (engine === 'webkit') await guideLink.tap(); + else { + await guideLink.focus(); + await page.keyboard.press('Enter'); + } + const heading = page.getByRole('heading', { name: 'Container CPU readings', exact: true }); + await heading.waitFor(); + await page.waitForFunction(() => { + const h = document.getElementById('container-cpu-readings'); + return h && h.getBoundingClientRect().top >= 0 && h.getBoundingClientRect().top < innerHeight; + }); + assert.ok(page.url().endsWith('/docs/DOCKER#container-cpu-readings')); + const article = (await page.locator('article').innerText()).replace(/\s+/g, ' '); + for (const phrase of ["Docker/Podman host's total CPU capacity", '100% for one logical CPU', + 'not an assumption about your host', 'v6.5.0-rc.1', 'unavailable data, not a measured zero', + 'Do not divide Pulse History again', 'Active does not prove fresh CPU samples', + 'not that an alert should fire or that its notification was delivered', + 'Do not restart workloads, create CPU load, lower alert thresholds']) + assert.ok(article.includes(phrase), phrase); + const table = page.locator('article table').filter({ hasText: 'Example host logical CPUs' }); + assert.equal(await table.getByRole('row').count(), 4); + assert.equal(await table.getByRole('columnheader').count(), 3); + const rows = await table.getByRole('row').allTextContents(); + assert.ok(rows[1].includes('240%') && rows[1].includes('60%')); + assert.ok(rows[2].includes('16.52%') && rows[2].includes('2.75%')); + assert.ok(rows[3].includes('1.04%') && rows[3].includes('0.17%')); + assert.ok(await page.evaluate(() => document.documentElement.scrollWidth <= innerWidth + 1)); + await table.scrollIntoViewIfNeeded(); + await page.screenshot({ path: path.join(artifacts, `${engine}-${tone}-examples.png`) }); + const performance = page.getByRole('link', { name: 'safe performance measurements', exact: true }); + if (engine === 'webkit') await performance.tap(); + else { + await performance.focus(); + await page.keyboard.press('Enter'); + } + await page.getByRole('heading', { name: 'Excessive CPU, writes or database growth', exact: true }).waitFor(); + await page.waitForFunction(() => { + const h = document.getElementById('excessive-cpu-writes-or-database-growth'); + return h && h.getBoundingClientRect().top >= 0 && h.getBoundingClientRect().top < innerHeight; + }); + assert.ok(page.url().endsWith('/docs/TROUBLESHOOTING#excessive-cpu-writes-or-database-growth')); + assert.deepEqual(errors, []); + assert.deepEqual(operations, []); + results.push({ engine, version: browser.version(), width, tone, exampleRows: rows, + entryLink: engine === 'webkit' ? 'touch' : 'keyboard', performanceLink: true, + noDocumentOverflow: true, errors, operations }); + await browser.close(); + browser = undefined; + } + const docs = ['DOCKER.md', 'TROUBLESHOOTING.md'].map((name) => ({ name, + sha256: crypto.createHash('sha256').update(fs.readFileSync(path.join(root, 'public', 'docs', name))).digest('hex'), + })); + const result = { result: 'passed', playwright: require('playwright/package.json').version, + scope: 'shipped CPU help rendering and navigation, not collector accuracy or alert delivery', docs, results }; + fs.writeFileSync(path.join(artifacts, 'result.json'), JSON.stringify(result, null, 2) + '\n'); + console.log(JSON.stringify(result)); + } finally { + if (browser) await browser.close(); + await server.close(); + } +})().catch((error) => { console.error(error); process.exitCode = 1; }); diff --git a/frontend-modern/browser-verification/5dc87cd447b452f0.json b/frontend-modern/browser-verification/5dc87cd447b452f0.json new file mode 100644 index 0000000000..7e21a7e09a --- /dev/null +++ b/frontend-modern/browser-verification/5dc87cd447b452f0.json @@ -0,0 +1,37 @@ +{ + "version": 1, + "base_sha": "7387703d23ca855539e67855389712d780d2a868", + "verified_at": "2026-10-05T10:40:50.589686Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Workloads/GuestRow.tsx", + "frontend-modern/src/components/Workloads/GuestRowCells.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Workloads/GuestRowCells.tsx": "90cc2bac8ab59556148aac49026aef6d18615a4d85ef39891f8d51ddac804681", + "frontend-modern/src/components/Workloads/GuestRow.tsx": "6668d1f0f6aba4d79e006adc80ac2b323445e624f3e462de4373cdd4a9d5c6e3" + }, + "routes": ["/browser-tests/backup-details-access.html"], + "viewports": [ + { + "width": 1365, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Production Workloads GuestRow, both backup disclosures, shared Dialog and CSS with explicit synthetic VM/CT observations. Desktop Chromium and touch WebKit, each light/dark. Playwright 1.56.1 matches the integration lock.", + "Completed date/time and age remain distinct from a running task. Same-ID running, future/malformed uncertainty, absence and recovery update open evidence without inventing completion.", + "Sixteen final screenshots opened: named VM/CT evidence and Close controls remain readable and within desktop/phone bounds after the finite entrance animation.", + "Final fixture does not exercise canonical projection or native backup correctness, PBS/source agreement, guest recovery, physical devices or published availability. Earlier timeouts and superseded source/capture failures are retained in the lane outcome." + ], + "interactions": [ + "Existing hover is preserved. Sequential Tab/Enter and native Space open the desktop badge; ordinary first touch opens a minimum-44px phone target without a hover tooltip.", + "Close focus, background inertness, focus containment and Escape/Close/backdrop dismissal return focus to the originating disclosure. Nested activation does not open the enclosing guest row.", + "Hidden-column indicator presents the same uncertainty and named guest as the Backup column. Adjacent CT has no VM observation. Recovery and actual row removal release dialog isolation; an ordinary CT row activation still works.", + "Four cases, 22 grouped checks, no page errors or operation requests. Only the existing anomaly fixture GET occurs. No collector/history/resource read is added. Browser/server closure completed under unchanged deadlines." + ] +} diff --git a/frontend-modern/public/docs/DOCKER.md b/frontend-modern/public/docs/DOCKER.md index ac46ce3f9e..e35f9f2cf1 100644 --- a/frontend-modern/public/docs/DOCKER.md +++ b/frontend-modern/public/docs/DOCKER.md @@ -123,6 +123,57 @@ services: --- +## Container CPU readings + +Pulse shows container CPU as a share of the **Docker/Podman host's total CPU +capacity**: 100% means all its logical CPUs. `docker stats` and `podman stats` +normally use **100% for one logical CPU**, so their readings can exceed 100%. +For that scale, divide the stats percentage by the CPU count reported for the +runtime host to compare it with Pulse. + +If the engine runs inside a VM, use that VM's logical CPUs, not the physical +Proxmox host's CPUs. Pulse's percentage is not utilisation of the container's +own CPU quota or cpuset. If the reported CPU count differs from your configured +VM size, retain both in the comparison rather than guessing a divisor. + +| Example host logical CPUs | Stats CPU | Pulse CPU before display rounding | +| --- | --- | --- | +| 4 | 240% | 60% | +| 6 | 16.52% | About 2.75% | +| 6 | 1.04% | About 0.17% | + +These are examples, not an assumption about your host. A smaller Pulse value +alone does not establish a collection fault. Whole-percent table labels, +including those in **v6.5.0-rc.1**, can display a fractional reading below 0.5% +as 0%. Use **CPU History** rather than the rounded label; display rounding does +not change stored readings or the values used by CPU alert evaluation. A dash +or missing History is unavailable data, not a measured zero. + +For a useful comparison: + +1. Match the same container on the same runtime host. Compare a similar time + interval in CPU History with `docker stats` or `podman stats`; separate + sampling intervals can legitimately differ. +2. Apply the reported host CPU count only to a stats reading on the per-CPU + scale. Do not divide Pulse History again, or compare with an average since + container start as though it were a recent interval. +3. Check that observation times and History points advance during ordinary + monitoring. A connection marked Active does not prove fresh CPU samples. +4. If the normalised readings still disagree, or History stops advancing, + report the server and agent versions, reported CPU count, sample times and + bounded readings. Do not restart workloads, create CPU load, lower alert + thresholds or export diagnostics just to make this comparison. + +CPU alerts use the same host-capacity scale, with the configured thresholds, +duration and suppression rules. Non-zero History proves a collected reading, +not that an alert should fire or that its notification was delivered. If a +normal workload already demonstrates an alert problem, retain the reading, +threshold, duration and observed result; no forced alert test is needed. See +[safe performance measurements](TROUBLESHOOTING.md#excessive-cpu-writes-or-database-growth) +when measuring the Pulse server itself rather than a monitored container. + +--- + ## 🔄 Updates These steps update the Pulse server, not the containers it monitors. Record the diff --git a/frontend-modern/public/docs/TROUBLESHOOTING.md b/frontend-modern/public/docs/TROUBLESHOOTING.md index dd9e0a54ef..213b11b17d 100644 --- a/frontend-modern/public/docs/TROUBLESHOOTING.md +++ b/frontend-modern/public/docs/TROUBLESHOOTING.md @@ -251,6 +251,20 @@ repair an older generated unit rather than adding a JSON-parsing wrapper. follow the [platform-specific guide](TEMPERATURE_MONITORING.md) for any needed setup change during a maintenance window. +#### Container CPU differs from Docker or Podman stats + +Pulse uses the runtime host's total CPU capacity; `docker stats` and +`podman stats` normally use 100% per logical CPU. Divide a per-CPU stats reading +by the **reported runtime host CPU count**, not the physical Proxmox host's +count when the engine runs inside a VM. Compare the same container and time +interval in **CPU History**, not just a rounded table label. A dash or missing +History is unavailable, not zero; an Active connection alone is not fresh CPU +evidence. + +See [container CPU readings](DOCKER.md#container-cpu-readings) for worked +examples, rounding and alert limits, and a passive comparison. Do not restart +workloads, create CPU load or lower alert thresholds to test this. + #### Docker hosts appearing/disappearing Cloned hosts can share a **saved Pulse agent ID**, not just an OS machine ID. diff --git a/frontend-modern/src/components/Workloads/GuestRow.tsx b/frontend-modern/src/components/Workloads/GuestRow.tsx index e9e573c69b..45b126ebd6 100644 --- a/frontend-modern/src/components/Workloads/GuestRow.tsx +++ b/frontend-modern/src/components/Workloads/GuestRow.tsx @@ -323,6 +323,7 @@ export function GuestRow(props: GuestRowProps) { {/* Show backup indicator in name cell only if backup column is hidden */} diff --git a/frontend-modern/src/components/Workloads/GuestRowCells.tsx b/frontend-modern/src/components/Workloads/GuestRowCells.tsx index 2ace665fbe..5a8f8f23a7 100644 --- a/frontend-modern/src/components/Workloads/GuestRowCells.tsx +++ b/frontend-modern/src/components/Workloads/GuestRowCells.tsx @@ -1,4 +1,6 @@ -import { createMemo, For, Show } from 'solid-js'; +import { createMemo, createSignal, For, Show } from 'solid-js'; + +import { Dialog } from '@/components/shared/Dialog'; import { TooltipPortal } from '@/components/shared/TooltipPortal'; import { useTooltip } from '@/hooks/useTooltip'; @@ -46,28 +48,18 @@ function BackupShieldIcon(props: { icon: 'check' | 'warning' | 'x' | 'running'; function BackupIndicator(props: { lastBackup: string | number | null | undefined; isTemplate: boolean; + guestName?: string; backupRunning?: boolean; }) { if (props.isTemplate) return null; - const alertsActivation = useAlertsActivation(); - const backupInfo = createMemo(() => - getBackupInfo(props.lastBackup, alertsActivation.getBackupThresholds()), - ); - const displayStatus = createMemo(() => - props.backupRunning ? 'running' : backupInfo().status, - ); - const config = createMemo(() => getWorkloadsGuestBackupStatusPresentation(displayStatus())); - - const tooltipText = createMemo(() => { - const info = backupInfo(); - return getWorkloadsGuestBackupTooltip(info.status, info.ageFormatted, props.backupRunning); - }); - return ( - - - + ); } @@ -316,8 +308,12 @@ function OSInfoCell(props: { osName: string; osVersion: string; agentVersion: st function BackupStatusCell(props: { lastBackup: string | number | null | undefined; backupRunning?: boolean; + indicator?: boolean; + guestName?: string; }) { const tip = useTooltip(); + const [detailsOpen, setDetailsOpen] = createSignal(false); + let trigger: HTMLButtonElement | undefined; const alertsActivation = useAlertsActivation(); const info = createMemo(() => @@ -337,6 +333,13 @@ function BackupStatusCell(props: { const hasCompletedBackup = createMemo(() => info().ageMs !== null); const ariaLabel = createMemo(() => { const currentInfo = info(); + if (props.indicator) { + return getWorkloadsGuestBackupTooltip( + currentInfo.status, + currentInfo.ageFormatted, + props.backupRunning, + ); + } if (currentInfo.status === 'unknown') { return `Backup status: ${props.backupRunning ? 'backup running now, ' : ''}${currentInfo.ageFormatted}`; } @@ -349,61 +352,111 @@ function BackupStatusCell(props: { return `Backup status: ${currentInfo.status}, last backup ${currentInfo.ageFormatted}`; }); + // Both entry points reveal the same retained completion evidence. Hover + // stays a shortcut on a fine pointer; the native button opens a read-only + // dialog for keyboard/touch without stealing the enclosing row action. + const Evidence = () => ( +
+ +

{props.guestName}

+
+ +

Backup running now…

+
+ +
+
Last completed backup
+ +
{info().ageFormatted}
+
+
+ +

{info().ageFormatted}

+
+ +

+ {props.backupRunning + ? 'No completed backup yet - the first backup is running now.' + : 'No backup has ever been recorded for this guest.'} +

+
+
+ ); + const focusClass = + 'focus-visible:outline-hidden focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-1 focus-visible:ring-offset-surface'; + return ( <> - { + event.stopPropagation(); + tip.onMouseLeave(); + setDetailsOpen(true); + }} aria-label={ariaLabel()} + title={props.indicator ? ariaLabel() : undefined} + aria-haspopup="dialog" + aria-expanded={detailsOpen()} > - + {badgeLabel()} - + -
-
- Backup Status +
+
+ Backup status
- -
Backup running now…
-
- -
-
- {props.backupRunning ? 'Last completed backup' : 'Last backup'} -
-
- {new Date(props.lastBackup!).toLocaleDateString(undefined, { - weekday: 'short', - year: 'numeric', - month: 'short', - day: 'numeric', - })} -
-
{new Date(props.lastBackup!).toLocaleTimeString()}
-
-
- - {info().ageFormatted} - -
-
- -
{info().ageFormatted}
-
- -
- {props.backupRunning - ? 'No completed backup yet - the first backup is running now.' - : 'No backup has ever been recorded for this guest.'} -
-
+
+ + setDetailsOpen(false)} + returnFocus={() => trigger} + panelClass="max-w-sm" + > +
+

Backup status

+ +
+
+ +
+
+
); } diff --git a/frontend-modern/src/components/Workloads/__tests__/BackupStatusAccess.test.tsx b/frontend-modern/src/components/Workloads/__tests__/BackupStatusAccess.test.tsx new file mode 100644 index 0000000000..e0986f7944 --- /dev/null +++ b/frontend-modern/src/components/Workloads/__tests__/BackupStatusAccess.test.tsx @@ -0,0 +1,106 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createSignal } from 'solid-js'; +import { cleanup, fireEvent, render, screen } from '@solidjs/testing-library'; +import { BackupIndicator, BackupStatusCell } from '../GuestRowCells'; + +vi.mock('@/stores/alertsActivation', () => ({ + useAlertsActivation: () => ({ getBackupThresholds: () => undefined }), +})); + +const completed = Date.parse('2026-10-05T06:00:00Z'); +const now = Date.parse('2026-10-05T08:00:00Z'); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +describe('backup evidence access', () => { + it.each(['badge', 'indicator'] as const)( + 'regression: %s exposes completed-backup details through a native button', + async (variant) => { + vi.useFakeTimers(); + vi.setSystemTime(now); + render(() => + variant === 'badge' ? ( + + ) : ( + + ), + ); + const trigger = screen.getByRole('button', { + name: variant === 'badge' ? /^Backup status:/ : /^Last backup:/, + }); + expect(trigger).toHaveAttribute('type', 'button'); + expect(trigger).toHaveAttribute('aria-haspopup', 'dialog'); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + fireEvent.click(trigger); + const dialog = screen.getByRole('dialog', { name: 'Backup status details' }); + expect(dialog).toHaveTextContent('Last completed backup'); + expect(dialog).toHaveTextContent( + new Date(completed).toLocaleDateString(undefined, { + weekday: 'short', + year: 'numeric', + month: 'short', + day: 'numeric', + }), + ); + expect(dialog).toHaveTextContent('2 hours ago'); + expect(trigger).toHaveAttribute('aria-expanded', 'true'); + await Promise.resolve(); + expect(screen.getByRole('button', { name: 'Close backup details' })).toHaveFocus(); + fireEvent.keyDown(document, { key: 'Escape' }); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(trigger).toHaveFocus(); + }, + ); + + it('keeps open details reactive without interpreting running or uncertainty as completion', () => { + vi.useFakeTimers(); + vi.setSystemTime(now); + const [value, setValue] = createSignal(completed); + const [running, setRunning] = createSignal(false); + render(() => ); + fireEvent.click(screen.getByRole('button', { name: /^Backup status:/ })); + const dialog = screen.getByRole('dialog'); + setRunning(true); + expect(dialog).toHaveTextContent('Backup running now'); + expect(dialog).toHaveTextContent('Last completed backup'); + setValue('not-a-date'); + expect(dialog).toHaveTextContent('invalid timestamp'); + expect(dialog).not.toHaveTextContent('Last completed backup'); + expect(dialog).not.toHaveTextContent('No backup has ever'); + setValue(now + 60000); + expect(dialog).toHaveTextContent('timestamp is in the future'); + expect(dialog).not.toHaveTextContent('Invalid Date'); + setValue(0); + expect(dialog).toHaveTextContent('No completed backup yet'); + setRunning(false); + expect(dialog).toHaveTextContent('No backup has ever been recorded'); + expect(dialog).not.toHaveTextContent('Backup running now'); + setValue(completed); + expect(dialog).toHaveTextContent('2 hours ago'); + expect(dialog).not.toHaveTextContent('unavailable'); + }); + + it('uses separate disclosures for adjacent guests and omits template indicators', () => { + vi.useFakeTimers(); + vi.setSystemTime(now); + render(() => ( + <> + + + + + )); + const triggers = screen.getAllByRole('button', { name: /^Backup status:/ }); + expect(triggers).toHaveLength(2); + fireEvent.click(triggers[0]); + fireEvent.click(screen.getByRole('button', { name: 'Close backup details' })); + fireEvent.click(triggers[1]); + expect(screen.getAllByRole('dialog')).toHaveLength(1); + expect(screen.getByRole('dialog')).toHaveTextContent('No backup has ever been recorded'); + expect(screen.getByRole('dialog')).not.toHaveTextContent('2 hours ago'); + expect(screen.queryByRole('button', { name: /^Last backup:/ })).toBeNull(); + }); +}); diff --git a/frontend-modern/src/components/Workloads/__tests__/BackupStatusCell.test.tsx b/frontend-modern/src/components/Workloads/__tests__/BackupStatusCell.test.tsx index 028de384f7..c4ad178d24 100644 --- a/frontend-modern/src/components/Workloads/__tests__/BackupStatusCell.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/BackupStatusCell.test.tsx @@ -9,7 +9,7 @@ const renderCell = (lastBackup: string | number | null, backupRunning?: boolean) const { container } = render(() => ( )); - const badge = container.querySelector('span[aria-label^="Backup status"]'); + const badge = container.querySelector('button[aria-label^="Backup status"]'); if (!badge) throw new Error('backup badge not rendered'); return badge as HTMLElement; }; diff --git a/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx b/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx index 657ddd04ad..079934ef2c 100644 --- a/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx @@ -2314,3 +2314,28 @@ describe('app-container update button visibility', () => { expect(screen.queryByTestId('update-button')).toBeNull(); }); }); + +describe('GuestRow backup evidence disclosure', () => { + it.each([{ columns: ['name', 'backup'] }, { columns: ['name'] }])( + 'opens read-only evidence without triggering the enclosing row for columns %j', + ({ columns }) => { + const onClick = vi.fn(); + renderGuestRow({ + guest: makeGuest({ lastBackup: Date.now() - 3600000 }), + visibleColumnIds: columns, + onClick, + }); + const trigger = screen.getByRole('button', { + name: columns.includes('backup') ? /^Backup status:/ : /^Last backup:/, + }); + fireEvent.click(trigger); + expect( + screen.getByRole('dialog', { name: 'Backup status details for test-vm' }), + ).toHaveTextContent('Last completed backup'); + expect(onClick).not.toHaveBeenCalled(); + fireEvent.click(screen.getByRole('button', { name: 'Close backup details' })); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(onClick).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/frontend-modern/src/utils/__tests__/docsLinks.test.ts b/frontend-modern/src/utils/__tests__/docsLinks.test.ts index c5d89ea99b..0ccae13f89 100644 --- a/frontend-modern/src/utils/__tests__/docsLinks.test.ts +++ b/frontend-modern/src/utils/__tests__/docsLinks.test.ts @@ -480,6 +480,40 @@ describe('docsLinks', () => { ); }); + it('explains container CPU scales without inventing a zero or forcing an alert', () => { + const docker = readFileSync(path.join(repoRoot, 'docs', 'DOCKER.md'), 'utf8'); + const cpu = docker + .split('## Container CPU readings\n')[1] + .split('\n---')[0] + .replace(/\s+/g, ' '); + expect(cpu).toContain("Docker/Podman host's total CPU capacity"); + expect(cpu).toContain('100% for one logical CPU'); + expect(cpu).toContain('CPU count reported for the runtime host'); + expect(cpu).toContain("not the physical Proxmox host's CPUs"); + expect(cpu).toContain("not utilisation of the container's own CPU quota or cpuset"); + expect(cpu).toContain('not an assumption about your host'); + expect(cpu).toContain('v6.5.0-rc.1'); + expect(cpu).toContain('not change stored readings or the values used by CPU alert evaluation'); + expect(cpu).toContain('unavailable data, not a measured zero'); + expect(cpu).toContain('Do not divide Pulse History again'); + expect(cpu).toContain('separate sampling intervals can legitimately differ'); + expect(cpu).toContain('Active does not prove fresh CPU samples'); + expect(cpu).toContain('not that an alert should fire or that its notification was delivered'); + expect(cpu).toContain('Do not restart workloads, create CPU load, lower alert thresholds'); + expect(cpu).toContain('no forced alert test is needed'); + + const troubleshooting = readFileSync(path.join(repoRoot, 'docs', 'TROUBLESHOOTING.md'), 'utf8'); + const entry = troubleshooting + .split('#### Container CPU differs from Docker or Podman stats\n')[1] + .split('\n#### ')[0] + .replace(/\s+/g, ' '); + expect(entry).toContain('DOCKER.md#container-cpu-readings'); + expect(entry).toContain('reported runtime host CPU count'); + expect(entry).toContain('same container and time interval'); + expect(entry).toContain('unavailable, not zero'); + expect(entry).toContain('Do not restart workloads, create CPU load or lower alert thresholds'); + }); + it('keeps clone identity recovery separate from destructive OS or credential resets', () => { const troubleshooting = readFileSync(path.join(repoRoot, 'docs', 'TROUBLESHOOTING.md'), 'utf8'); const agentGuide = readFileSync(path.join(repoRoot, 'docs', 'UNIFIED_AGENT.md'), 'utf8'); diff --git a/internal/api/org_handlers.go b/internal/api/org_handlers.go index 0704db88ef..fbae571366 100644 --- a/internal/api/org_handlers.go +++ b/internal/api/org_handlers.go @@ -387,7 +387,11 @@ func (h *OrgHandlers) HandleDeleteOrg(w http.ResponseWriter, r *http.Request) { } if h.mtMonitor != nil { - h.mtMonitor.BeginTenantDeletion(orgID) + if err := h.mtMonitor.BeginTenantDeletion(orgID); err != nil { + log.Error().Err(err).Str("org_id", orgID).Msg("Organization shutdown incomplete; retaining data") + writeErrorResponse(w, http.StatusServiceUnavailable, "tenant_shutdown_incomplete", "Organization monitoring has not shut down cleanly; its data has been retained", nil) + return + } defer h.mtMonitor.FinishTenantDeletion(orgID) } diff --git a/internal/api/org_shutdown_safety_test.go b/internal/api/org_shutdown_safety_test.go new file mode 100644 index 0000000000..90ee7f0855 --- /dev/null +++ b/internal/api/org_shutdown_safety_test.go @@ -0,0 +1,82 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" +) + +func TestOrgDeleteRetainsDataWhenGuestMetadataPersistenceFails(t *testing.T) { + t.Setenv("PULSE_DEV", "true") + previous := IsMultiTenantEnabled() + SetMultiTenantEnabled(true) + t.Cleanup(func() { SetMultiTenantEnabled(previous) }) + root := t.TempDir() + persistence := config.NewMultiTenantPersistence(root) + mtm := monitoring.NewMultiTenantMonitor(&config.Config{DataPath: root}, persistence, nil) + t.Cleanup(mtm.Stop) + h := NewOrgHandlers(persistence, mtm) + // Exercise the same session-owner setup as ordinary organisation creation. + create := withUser(httptest.NewRequest(http.MethodPost, "/api/orgs", strings.NewReader(`{"id":"retained","displayName":"Retained"}`)), "alice") + rec := httptest.NewRecorder() + h.HandleCreateOrg(rec, create) + if rec.Code != http.StatusCreated { + t.Fatalf("create: %d %s", rec.Code, rec.Body.String()) + } + org, err := persistence.LoadOrganization("retained") + if err != nil || org == nil { + t.Fatal(err) + } + monitor, err := mtm.GetMonitor("retained") + if err != nil { + t.Fatal(err) + } + orgPath := filepath.Join(root, "orgs", "retained") + // A real rename refusal, confined to this disposable test directory. + if err := os.Mkdir(filepath.Join(orgPath, "guest_metadata.json"), 0o700); err != nil { + t.Fatal(err) + } + if err := monitor.GuestMetadataStore().RememberIdentity("pve:node:100", "unsaved", "qemu"); err != nil { + t.Fatal(err) + } + if !monitor.GuestMetadataStore().WaitForPendingWrites(time.Second) { + t.Fatal("failed attempt did not quiesce") + } + called := false + h.SetOnDelete(func(context.Context, string) error { called = true; return nil }) + req := withUser(httptest.NewRequest(http.MethodDelete, "/api/orgs/retained", nil), "alice") + req.SetPathValue("id", "retained") + rec = httptest.NewRecorder() + h.HandleDeleteOrg(rec, req) + if rec.Code != http.StatusServiceUnavailable || !strings.Contains(rec.Body.String(), "tenant_shutdown_incomplete") { + t.Fatalf("unsafe deletion result: %d %s", rec.Code, rec.Body.String()) + } + if called { + t.Fatal("offboarding callback ran after incomplete shutdown") + } + if _, err := os.Stat(filepath.Join(orgPath, "org.json")); err != nil { + t.Fatalf("organisation data removed: %v", err) + } + if got, ok := mtm.PeekMonitor("retained"); !ok || got != monitor { + t.Fatal("failed writer owner lost") + } + if _, err := mtm.GetMonitor("retained"); err == nil { + t.Fatal("new monitor can write into retained shutdown path") + } + // Another tenant remains usable: the failed writer is not a global hold. + if err := persistence.SaveOrganization(&models.Organization{ID: "other"}); err != nil { + t.Fatal(err) + } + if _, err := persistence.GetPersistence("other"); err != nil { + t.Fatal(err) + } +} diff --git a/internal/api/rate_limit_config.go b/internal/api/rate_limit_config.go index 575364fc53..1ddbc3f55d 100644 --- a/internal/api/rate_limit_config.go +++ b/internal/api/rate_limit_config.go @@ -57,6 +57,21 @@ func newEndpointRateLimitConfig() *EndpointRateLimitConfig { } } +// Stop joins only the workers owned by this endpoint configuration. A Router +// must not stop the package-global configuration or another Router's budgets. +func (cfg *EndpointRateLimitConfig) Stop() { + if cfg == nil { + return + } + for _, limiter := range []*RateLimiter{ + cfg.AuthEndpoints, cfg.ConfigEndpoints, cfg.ExportEndpoints, + cfg.RecoveryEndpoints, cfg.UpdateEndpoints, cfg.WebSocketEndpoints, + cfg.GeneralAPI, cfg.PublicEndpoints, + } { + limiter.Stop() + } +} + func generalAPIRateLimit() int { if os.Getenv("PULSE_DEV") != "true" { return defaultGeneralAPIRateLimit diff --git a/internal/api/ratelimit.go b/internal/api/ratelimit.go index cb597e0bf4..9d207c4b3f 100644 --- a/internal/api/ratelimit.go +++ b/internal/api/ratelimit.go @@ -19,6 +19,8 @@ type RateLimiter struct { limit int window time.Duration stopCleanup chan struct{} + cleanupDone chan struct{} + stopOnce sync.Once } // NewRateLimiter creates a rate limiter that allows limit requests per window duration. @@ -44,10 +46,12 @@ func NewRateLimiter(limit int, window time.Duration) *RateLimiter { limit: limit, window: window, stopCleanup: make(chan struct{}), + cleanupDone: make(chan struct{}), } // Clean up old entries periodically go func() { + defer close(rl.cleanupDone) ticker := time.NewTicker(1 * time.Minute) defer ticker.Stop() @@ -64,13 +68,19 @@ func NewRateLimiter(limit int, window time.Duration) *RateLimiter { return rl } -// Stop stops the cleanup routine +// Stop stops and joins the cleanup routine. Concurrent owners may call it +// repeatedly without racing a channel close. Request budgets remain intact. func (rl *RateLimiter) Stop() { - select { - case <-rl.stopCleanup: + if rl == nil { return - default: - close(rl.stopCleanup) + } + rl.stopOnce.Do(func() { + if rl.stopCleanup != nil { + close(rl.stopCleanup) + } + }) + if rl.cleanupDone != nil { + <-rl.cleanupDone } } diff --git a/internal/api/router.go b/internal/api/router.go index 44e96339d8..082251c472 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -132,6 +132,7 @@ type Router struct { handoffExchangeRateLimiter *RateLimiter bootstrapTokenValidationLimiter *RateLimiter tenantRateLimiter *TenantRateLimiter + endpointRateLimitConfig *EndpointRateLimitConfig persistence *config.ConfigPersistence multiTenant *config.MultiTenantPersistence oidcMu sync.Mutex @@ -166,6 +167,9 @@ type Router struct { lifecycleCancel context.CancelFunc lifecycleWG sync.WaitGroup backgroundWorkersOnce sync.Once + backgroundShutdownOnce sync.Once + lifecycleMu sync.Mutex + lifecycleStopped bool hostedMode bool stripeWebhookHandlers *StripeWebhookHandlers patrolLifecycleMu sync.Mutex @@ -234,6 +238,7 @@ func NewRouter(cfg *config.Config, monitor *monitoring.Monitor, mtMonitor *monit wsHub: wsHub, reloadFunc: reloadFunc, updateManager: updateManager, + endpointRateLimitConfig: newEndpointRateLimitConfig(), updateHistory: updateHistory, exportLimiter: NewRateLimiter(5, 1*time.Minute), // 5 attempts per minute downloadLimiter: NewRateLimiter(60, 1*time.Minute), // downloads/installers per minute per IP @@ -350,7 +355,7 @@ func NewRouter(cfg *config.Config, monitor *monitoring.Monitor, mtMonitor *monit // Auth context middleware extracts user/token info BEFORE tenant middleware handler = AuthContextMiddleware(cfg, r.mtMonitor, handler) - handler = UniversalRateLimitMiddlewareWithConfig(newEndpointRateLimitConfig(), handler) + handler = UniversalRateLimitMiddlewareWithConfig(r.endpointRateLimitConfig, handler) r.wrapped = handler return r } @@ -1562,6 +1567,11 @@ func (r *Router) startLifecycleWorker(worker func()) { if r == nil || worker == nil { return } + r.lifecycleMu.Lock() + defer r.lifecycleMu.Unlock() + if r.lifecycleStopped { + return + } r.lifecycleWG.Add(1) go func() { defer r.lifecycleWG.Done() @@ -2978,40 +2988,64 @@ func (r *Router) ShutdownRBAC() { } } +// ShutdownBackgroundWorkers seals worker admission and joins the workers +// created by this Router. Call it after HTTP admission is stopped, before +// closing the monitor and removing its data directory. It does not close +// resource or RBAC stores; their existing owner shutdown methods still apply. +func (r *Router) ShutdownBackgroundWorkers() { + r.shutdownBackgroundWorkers() +} + func (r *Router) shutdownBackgroundWorkers() { - if r.lifecycleCancel != nil { - r.lifecycleCancel() + if r == nil { + return } - if r.updateManager != nil { - r.updateManager.Close() - } - if r.sessionStore != nil { - r.sessionStore.Shutdown() - } - if r.csrfStore != nil { - r.csrfStore.Shutdown() - } - if r.recoveryTokenStore != nil { - r.recoveryTokenStore.Shutdown() - } - if r.aiSettingsHandler != nil { - r.aiSettingsHandler.StopServices() - } - if r.aiHandler != nil { - r.aiHandler.clearApprovalStore() - } - if r.trueNASPoller != nil { - r.trueNASPoller.Stop() - } - if r.vmwarePoller != nil { - r.vmwarePoller.Stop() - } - if r.deployStore != nil { - if err := r.deployStore.Close(); err != nil { - log.Error().Err(err).Msg("Failed to close deploy store") + r.backgroundShutdownOnce.Do(func() { + r.lifecycleMu.Lock() + r.lifecycleStopped = true + if r.lifecycleCancel != nil { + r.lifecycleCancel() } - } - r.lifecycleWG.Wait() + r.lifecycleMu.Unlock() + for _, limiter := range []*RateLimiter{ + r.exportLimiter, r.downloadLimiter, r.signupRateLimiter, + r.handoffExchangeRateLimiter, r.bootstrapTokenValidationLimiter, + } { + limiter.Stop() + } + r.endpointRateLimitConfig.Stop() + r.tenantRateLimiter.Stop() + if r.updateManager != nil { + r.updateManager.Close() + } + if r.sessionStore != nil { + r.sessionStore.Shutdown() + } + if r.csrfStore != nil { + r.csrfStore.Shutdown() + } + if r.recoveryTokenStore != nil { + r.recoveryTokenStore.Shutdown() + } + if r.aiSettingsHandler != nil { + r.aiSettingsHandler.StopServices() + } + if r.aiHandler != nil { + r.aiHandler.clearApprovalStore() + } + if r.trueNASPoller != nil { + r.trueNASPoller.Stop() + } + if r.vmwarePoller != nil { + r.vmwarePoller.Stop() + } + if r.deployStore != nil { + if err := r.deployStore.Close(); err != nil { + log.Error().Err(err).Msg("Failed to close deploy store") + } + } + r.lifecycleWG.Wait() + }) } // StartAIChat starts the AI chat service diff --git a/internal/api/router_integration_test.go b/internal/api/router_integration_test.go index 8835f1febe..e5ac444e01 100644 --- a/internal/api/router_integration_test.go +++ b/internal/api/router_integration_test.go @@ -201,9 +201,17 @@ func newIntegrationServerWithRuntimeMode( return nil }, version) + // Register constructor cleanup before a listener or request assertion can + // fail. The fixture owns the same lifecycle used by the running server. + t.Cleanup(func() { + router.ShutdownBackgroundWorkers() + router.ShutdownResourceStores() + router.ShutdownRBAC() + }) srv := newIPv4HTTPServer(t, router.Handler()) t.Cleanup(func() { srv.Close() + router.ShutdownBackgroundWorkers() if monitor != nil { monitor.StopDiscoveryService() monitor.Stop() diff --git a/internal/api/router_limiter_lifecycle_test.go b/internal/api/router_limiter_lifecycle_test.go new file mode 100644 index 0000000000..b649e9ea39 --- /dev/null +++ b/internal/api/router_limiter_lifecycle_test.go @@ -0,0 +1,143 @@ +package api + +import ( + "net/http" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +func routerOwnedLimiters(r *Router) []*RateLimiter { + limiters := []*RateLimiter{r.exportLimiter, r.downloadLimiter, r.signupRateLimiter, + r.handoffExchangeRateLimiter, r.bootstrapTokenValidationLimiter} + limiters = append(limiters, rateLimitFixtureWorkers(r.endpointRateLimitConfig)...) + if r.tenantRateLimiter != nil { + limiters = append(limiters, r.tenantRateLimiter.limiter) + } + return limiters +} + +func requireLimiterJoined(t *testing.T, limiter *RateLimiter) { + t.Helper() + select { + case <-limiter.cleanupDone: + default: + t.Error("Stop returned with a cleanup worker still running") + } +} + +func TestRateLimiterConcurrentStopJoinsWithoutResettingBudget(t *testing.T) { + limiter := NewRateLimiter(1, time.Minute) + t.Cleanup(limiter.Stop) + if !limiter.Allow("192.0.2.1") || limiter.Allow("192.0.2.1") { + t.Fatal("fixture did not consume its budget") + } + var wg sync.WaitGroup + for i := 0; i < 32; i++ { + wg.Add(1) + go func() { + defer wg.Done() + limiter.Stop() + limiter.Allow("192.0.2.2") // In-flight requests retain normal accounting. + }() + } + wg.Wait() + requireLimiterJoined(t, limiter) + if limiter.Allow("192.0.2.1") { + t.Error("stopping cleanup reset an exhausted request budget") + } + (*RateLimiter)(nil).Stop() + (&RateLimiter{}).Stop() +} + +func TestRouterShutdownJoinsEveryOwnedLimiter(t *testing.T) { + for _, hosted := range []string{"false", "true"} { + t.Run(hosted, func(t *testing.T) { + t.Setenv("PULSE_HOSTED_MODE", hosted) + for i := 0; i < 3; i++ { + r := NewRouter(&config.Config{DataPath: t.TempDir()}, nil, nil, nil, nil, "test") + cleanupTestRouter(t, r) + owned := routerOwnedLimiters(r) + want := 13 + if hosted == "true" { + want++ + } + if len(owned) != want { + t.Fatalf("owned limiters = %d, want %d", len(owned), want) + } + r.ShutdownBackgroundWorkers() + r.ShutdownBackgroundWorkers() + for _, limiter := range owned { + requireLimiterJoined(t, limiter) + } + } + }) + } +} + +func TestRouterShutdownPreservesOtherRouterAndGlobalBudgets(t *testing.T) { + isolateRateLimitFixture(t) + InitializeRateLimiters() + global := globalRateLimitConfig + first := NewRouter(&config.Config{DataPath: t.TempDir()}, nil, nil, nil, nil, "test") + cleanupTestRouter(t, first) + second := NewRouter(&config.Config{DataPath: t.TempDir()}, nil, nil, nil, nil, "test") + cleanupTestRouter(t, second) + login := getRateLimiterForEndpoint(second.endpointRateLimitConfig, "/api/login", http.MethodPost) + for i := 0; i < login.limit; i++ { + if !login.Allow("192.0.2.1") { + t.Fatal("other router budget exhausted early") + } + } + first.ShutdownBackgroundWorkers() + if login.Allow("192.0.2.1") { + t.Error("another router's shutdown reset login enforcement") + } + for _, limiter := range append(routerOwnedLimiters(second), rateLimitFixtureWorkers(global)...) { + select { + case <-limiter.stopCleanup: + t.Error("shutdown stopped a limiter owned by another router/global configuration") + default: + } + } + if !second.exportLimiter.Allow("192.0.2.2") { + t.Error("other router no longer admits its independent export budget") + } +} + +func TestRouterShutdownSealsAndJoinsConcurrentWorkerAdmission(t *testing.T) { + r := NewRouter(&config.Config{DataPath: t.TempDir()}, nil, nil, nil, nil, "test") + cleanupTestRouter(t, r) + var started, exited atomic.Int64 + var callers sync.WaitGroup + for i := 0; i < 32; i++ { + callers.Add(2) + go func() { + defer callers.Done() + r.startLifecycleWorker(func() { + started.Add(1) + <-r.lifecycleCtx.Done() + exited.Add(1) + }) + }() + go func() { + defer callers.Done() + r.ShutdownBackgroundWorkers() + }() + } + callers.Wait() + if started.Load() != exited.Load() { + t.Fatalf("shutdown returned before joining admitted workers: started %d, exited %d", started.Load(), exited.Load()) + } + r.startLifecycleWorker(func() { t.Error("worker admitted after shutdown") }) + r.StartBackgroundWorkers() // Late application start cannot reopen admission. + r.ShutdownBackgroundWorkers() + for _, limiter := range routerOwnedLimiters(r) { + requireLimiterJoined(t, limiter) + } + (*Router)(nil).ShutdownBackgroundWorkers() + (&Router{}).ShutdownBackgroundWorkers() +} diff --git a/internal/config/guest_metadata.go b/internal/config/guest_metadata.go index 41e3bbae42..cba3f71834 100644 --- a/internal/config/guest_metadata.go +++ b/internal/config/guest_metadata.go @@ -2,11 +2,13 @@ package config import ( "encoding/json" + "errors" "fmt" "os" "path/filepath" "strings" "sync" + "sync/atomic" "time" "github.com/rs/zerolog/log" @@ -31,49 +33,202 @@ type GuestMetadataStore struct { dataPath string fs FileSystem - // inflight tracks background Set calls started by SetAsync so shutdown can - // wait for them. Callers used to spawn their own detached goroutines, which - // meant a write could land after the monitor stopped and after the data - // directory was being torn down, leaving a stray guest_metadata.json.tmp. - inflight sync.WaitGroup + // writeMu serializes disk snapshots with synchronous mutations. Background + // I/O never holds mu, so a burst can accumulate in one follow-up snapshot. + // Always acquire writeMu before mu when both locks are needed. + writeMu sync.Mutex + revision uint64 + attemptedRevision uint64 + lastWriteErr error + writerDone chan struct{} + closed atomic.Bool + closeMu sync.Mutex + closeAttempt *guestMetadataCloseAttempt } -// SetAsync persists metadata without blocking the caller, while keeping the -// write owned by the store so WaitForPendingWrites can drain it on shutdown. -func (s *GuestMetadataStore) SetAsync(guestID string, meta *GuestMetadata) { +type guestMetadataCloseAttempt struct { + done chan struct{} + err error +} + +var ErrGuestMetadataStoreClosed = errors.New("guest metadata store is closed") + +// SetAsync admits a detached replacement before returning. One store-owned +// writer persists the newest complete snapshot, coalescing changes that arrive +// during I/O. It does not start a goroutine or rewrite the file per guest. +func (s *GuestMetadataStore) SetAsync(guestID string, meta *GuestMetadata) error { if s == nil { + return nil + } + if meta == nil { + return fmt.Errorf("metadata cannot be nil") + } + clone := cloneGuestMetadata(meta) + clone.ID = guestID + s.mu.Lock() + defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } + s.metadata[guestID] = clone + s.queueSaveLocked() + return nil +} + +// RememberIdentity merges only monitor-owned identity fields at admission. +// A background name/type update must never restore an old URL, tag or note +// over a concurrent operator edit. OCI classification cannot be downgraded by +// a later poll lacking the classification evidence. +func (s *GuestMetadataStore) RememberIdentity(guestID, name, guestType string) error { + if s == nil { + return nil + } + guestType = strings.TrimSpace(guestType) + if guestType == "" { + return nil + } + s.mu.Lock() + defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } + meta := cloneGuestMetadata(s.metadata[guestID]) + if meta == nil { + meta = &GuestMetadata{ID: guestID, Tags: []string{}} + } + if meta.LastKnownType == "oci" { + guestType = "oci" + } + if meta.LastKnownName == name && meta.LastKnownType == guestType && s.lastWriteErr == nil { + return nil + } + meta.LastKnownName = name + meta.LastKnownType = guestType + s.metadata[guestID] = meta + s.queueSaveLocked() + return nil +} + +func (s *GuestMetadataStore) queueSaveLocked() { + s.revision++ + if s.writerDone != nil { return } - s.inflight.Add(1) - go func() { - defer s.inflight.Done() - if err := s.Set(guestID, meta); err != nil { - log.Error().Err(err).Str("guestID", guestID).Msg("failed to persist guest metadata") - } - }() + s.writerDone = make(chan struct{}) + go s.writePendingSnapshots() } -// WaitForPendingWrites blocks until background writes finish or the timeout -// elapses. It reports whether the store drained; a false result means a write -// may still be in flight and the data directory is not safe to remove. +func (s *GuestMetadataStore) writePendingSnapshots() { + for { + s.writeMu.Lock() + s.mu.Lock() + if s.attemptedRevision == s.revision { + close(s.writerDone) + s.writerDone = nil + s.mu.Unlock() + s.writeMu.Unlock() + return + } + revision := s.revision + data, err := json.Marshal(s.metadata) + s.mu.Unlock() + if err == nil { + err = persistMetadata(s.fs, s.dataPath, "guest_metadata.json", data) + } + s.mu.Lock() + s.attemptedRevision = revision + s.lastWriteErr = err + s.mu.Unlock() + s.writeMu.Unlock() + if err != nil { + // No unbounded automatic retry on a failing filesystem. A later + // admitted mutation (or ordinary identity poll) can try again. + log.Error().Err(err).Msg("failed to persist guest metadata snapshot") + } + } +} + +// WaitForPendingWrites reports quiescence, not persistence success. Producers +// must already be stopped if the caller plans to remove the data directory; +// Close seals admission as well. A timeout never makes teardown safe. func (s *GuestMetadataStore) WaitForPendingWrites(timeout time.Duration) bool { if s == nil { return true } - done := make(chan struct{}) - go func() { - s.inflight.Wait() - close(done) - }() + s.mu.RLock() + done := s.writerDone + s.mu.RUnlock() + if done == nil { + return true + } + timer := time.NewTimer(timeout) + defer timer.Stop() select { case <-done: return true - case <-time.After(timeout): + case <-timer.C: log.Warn().Dur("timeout", timeout).Msg("timed out waiting for guest metadata writes to drain") return false } } +// Close rejects further mutations, drains the owned writer within the supplied +// budget, and reports any final persistence error. Failure keeps the writer +// owned and must stop tenant-directory removal; it is not a successful stop. +func (s *GuestMetadataStore) Close(timeout time.Duration) error { + if s == nil { + return nil + } + // Seal before waiting for a synchronous mutation's I/O-held memory lock. + // One owned close observer, not one goroutine per waiter, covers both that + // mutation and the background writer within the caller's unchanged budget. + s.closed.Store(true) + s.closeMu.Lock() + attempt := s.closeAttempt + retry := false + if attempt != nil { + select { + case <-attempt.done: + // A later explicit shutdown/offboarding attempt may flush retained + // state after the filesystem is repaired. Never reopen admission, + // retry an in-flight write or spin-retry a failed close on its own. + if attempt.err != nil { + attempt = nil + retry = true + } + default: + } + } + if attempt == nil { + attempt = &guestMetadataCloseAttempt{done: make(chan struct{})} + s.closeAttempt = attempt + go func() { + s.mu.Lock() + if retry && s.lastWriteErr != nil && s.writerDone == nil { + s.queueSaveLocked() + } + done := s.writerDone + s.mu.Unlock() + if done != nil { + <-done + } + s.mu.RLock() + attempt.err = s.lastWriteErr + s.mu.RUnlock() + close(attempt.done) + }() + } + s.closeMu.Unlock() + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case <-attempt.done: + return attempt.err + case <-timer.C: + return fmt.Errorf("guest metadata writes did not drain within %s", timeout) + } +} + func cloneGuestMetadata(meta *GuestMetadata) *GuestMetadata { if meta == nil { return nil @@ -113,6 +268,16 @@ func NewGuestMetadataStore(dataPath string, fs FileSystem) *GuestMetadataStore { // Load reads metadata from disk func (s *GuestMetadataStore) Load() error { + s.writeMu.Lock() + defer s.writeMu.Unlock() + s.mu.Lock() + defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } + if s.revision != s.attemptedRevision || s.lastWriteErr != nil { + return fmt.Errorf("cannot reload guest metadata with unpersisted changes") + } filePath := filepath.Join(s.dataPath, "guest_metadata.json") log.Debug().Str("path", filePath).Msg("Loading guest metadata from disk") @@ -127,9 +292,6 @@ func (s *GuestMetadataStore) Load() error { return fmt.Errorf("failed to read metadata file: %w", err) } - s.mu.Lock() - defer s.mu.Unlock() - if err := json.Unmarshal(data, &s.metadata); err != nil { return fmt.Errorf("failed to unmarshal metadata: %w", err) } @@ -138,24 +300,16 @@ func (s *GuestMetadataStore) Load() error { return nil } -// save writes metadata to disk (must be called with lock held) +// save writes a synchronous mutation (writeMu and mu must both be held). func (s *GuestMetadataStore) save() error { - filePath := filepath.Join(s.dataPath, "guest_metadata.json") - - log.Debug().Str("path", filePath).Msg("Saving guest metadata to disk") - + s.revision++ data, err := json.Marshal(s.metadata) - if err != nil { - return fmt.Errorf("failed to marshal metadata: %w", err) + if err == nil { + err = persistMetadata(s.fs, s.dataPath, "guest_metadata.json", data) } - - if err := persistMetadata(s.fs, s.dataPath, "guest_metadata.json", data); err != nil { - return err - } - - log.Debug().Str("path", filePath).Int("entries", len(s.metadata)).Msg("Guest metadata saved successfully") - - return nil + s.attemptedRevision = s.revision + s.lastWriteErr = err + return err } // Get retrieves metadata for a guest @@ -188,8 +342,13 @@ func (s *GuestMetadataStore) GetWithLegacyMigration(guestID, instance, node stri // Helper to migrate a legacy ID to the new format migrate := func(legacyID string) *GuestMetadata { + s.writeMu.Lock() + defer s.writeMu.Unlock() s.mu.Lock() defer s.mu.Unlock() + if s.closed.Load() { + return nil + } if current := s.metadata[guestID]; current != nil { return cloneGuestMetadata(current) @@ -293,8 +452,13 @@ func (s *GuestMetadataStore) GetAll() map[string]*GuestMetadata { // Set updates or creates metadata for a guest func (s *GuestMetadataStore) Set(guestID string, meta *GuestMetadata) error { + s.writeMu.Lock() + defer s.writeMu.Unlock() s.mu.Lock() defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } if meta == nil { return fmt.Errorf("metadata cannot be nil") @@ -310,8 +474,13 @@ func (s *GuestMetadataStore) Set(guestID string, meta *GuestMetadata) error { // Delete removes metadata for a guest func (s *GuestMetadataStore) Delete(guestID string) error { + s.writeMu.Lock() + defer s.writeMu.Unlock() s.mu.Lock() defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } delete(s.metadata, guestID) @@ -321,8 +490,13 @@ func (s *GuestMetadataStore) Delete(guestID string) error { // ReplaceAll replaces all metadata entries and persists them to disk. func (s *GuestMetadataStore) ReplaceAll(metadata map[string]*GuestMetadata) error { + s.writeMu.Lock() + defer s.writeMu.Unlock() s.mu.Lock() defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } s.metadata = make(map[string]*GuestMetadata) @@ -354,8 +528,13 @@ func (s *GuestMetadataStore) UpdateAll( return nil } + s.writeMu.Lock() + defer s.writeMu.Unlock() s.mu.Lock() defer s.mu.Unlock() + if s.closed.Load() { + return ErrGuestMetadataStoreClosed + } working := make(map[string]*GuestMetadata, len(s.metadata)) for id, meta := range s.metadata { diff --git a/internal/config/guest_metadata_writer_test.go b/internal/config/guest_metadata_writer_test.go new file mode 100644 index 0000000000..f4c78ce497 --- /dev/null +++ b/internal/config/guest_metadata_writer_test.go @@ -0,0 +1,396 @@ +package config + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "sync" + "sync/atomic" + "testing" + "time" +) + +func guestWriterTestStore(t *testing.T) (*GuestMetadataStore, *controlledGuestWriterFS) { + t.Helper() + fs := &controlledGuestWriterFS{FileSystem: defaultFileSystem{}, entered: make(chan struct{}), release: make(chan struct{})} + store := NewGuestMetadataStore(t.TempDir(), fs) + var releaseOnce sync.Once + t.Cleanup(func() { + releaseOnce.Do(func() { + select { + case <-fs.release: + default: + close(fs.release) + } + }) + if !store.WaitForPendingWrites(time.Second) { + t.Error("writer did not exit during cleanup") + } + }) + if err := store.SetAsync("first", &GuestMetadata{LastKnownName: "first"}); err != nil { + t.Fatal(err) + } + select { + case <-fs.entered: + case <-time.After(time.Second): + t.Fatal("writer not started") + } + return store, fs +} +func capturedGuestSnapshot(t *testing.T, fs *controlledGuestWriterFS) map[string]*GuestMetadata { + t.Helper() + fs.mu.Lock() + data := append([]byte(nil), fs.data...) + fs.mu.Unlock() + var result map[string]*GuestMetadata + if err := json.Unmarshal(data, &result); err != nil { + t.Fatal(err) + } + return result +} +func TestGuestMetadataWriterOrderedReplacementsAndReadsDuringIO(t *testing.T) { + store, fs := guestWriterTestStore(t) + for i := 0; i < 100; i++ { + if err := store.SetAsync("same", &GuestMetadata{LastKnownName: fmt.Sprint(i)}); err != nil { + t.Fatal(err) + } + } + // This is an admission-state read, not a persistence-success assertion. + read := make(chan *GuestMetadata, 1) + go func() { read <- store.Get("same") }() + select { + case got := <-read: + if got.LastKnownName != "99" { + t.Fatalf("wrong admission order: %+v", got) + } + case <-time.After(time.Second): + t.Fatal("disk I/O blocked metadata reads") + } + close(fs.release) + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + if got := capturedGuestSnapshot(t, fs)["same"]; got.LastKnownName != "99" { + t.Fatalf("older update persisted last: %+v", got) + } + fs.mu.Lock() + defer fs.mu.Unlock() + if fs.writes != 2 { + t.Fatalf("100 replacements during one blocked write caused %d writes", fs.writes) + } +} +func TestGuestMetadataWriterSynchronousMutationsCannotBeOverwritten(t *testing.T) { + for _, operation := range []string{"set", "delete", "replace", "update", "migration"} { + t.Run(operation, func(t *testing.T) { + store, fs := guestWriterTestStore(t) + id := "pve:node:100" + if err := store.SetAsync(id, &GuestMetadata{LastKnownName: "queued"}); err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { + switch operation { + case "set": + done <- store.Set(id, &GuestMetadata{LastKnownName: "operator", Notes: []string{"new"}}) + case "delete": + done <- store.Delete(id) + case "replace": + done <- store.ReplaceAll(map[string]*GuestMetadata{"replacement": {LastKnownName: "replacement"}}) + case "update": + done <- store.UpdateAll(func(m map[string]*GuestMetadata) bool { m[id].Notes = []string{"edited"}; return true }) + case "migration": + if store.GetWithLegacyMigration("pve:other:100", "pve", "other", 100) == nil { + done <- errors.New("migration missing") + } else { + done <- nil + } + } + }() + close(fs.release) + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("synchronous mutation blocked") + } + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + saved := capturedGuestSnapshot(t, fs) + if !reflect.DeepEqual(saved, store.GetAll()) { + t.Fatalf("disk snapshot differs after %s: saved=%+v memory=%+v", operation, saved, store.GetAll()) + } + switch operation { + case "set": + if saved[id].LastKnownName != "operator" || saved[id].Notes[0] != "new" { + t.Fatal("operator replacement lost") + } + case "delete": + if saved[id] != nil { + t.Fatal("deleted entry resurrected") + } + case "replace": + if len(saved) != 1 || saved["replacement"] == nil { + t.Fatal("replace resurrected old entries") + } + case "update": + if saved[id].Notes[0] != "edited" { + t.Fatal("update lost") + } + case "migration": + if saved[id] != nil || saved["pve:other:100"] == nil { + t.Fatal("old node resurrected") + } + } + }) + } +} +func TestGuestMetadataWriterIdentityPreservesOperatorFieldsAndOCI(t *testing.T) { + store := NewGuestMetadataStore(t.TempDir(), nil) + original := &GuestMetadata{CustomURL: "https://operator.example", Description: "operator", Tags: []string{"tag"}, Notes: []string{"note"}, LastKnownName: "old", LastKnownType: "oci"} + if err := store.Set("pve:node:100", original); err != nil { + t.Fatal(err) + } + if err := store.RememberIdentity("pve:node:100", "new", " lxc "); err != nil { + t.Fatal(err) + } + got := store.Get("pve:node:100") + if got.LastKnownName != "new" || got.LastKnownType != "oci" { + t.Fatalf("identity/OCI lost: %+v", got) + } + original.ID = "pve:node:100" + original.LastKnownName = "new" + if !reflect.DeepEqual(got, original) { + t.Fatalf("identity changed operator data: %+v", got) + } + if !store.WaitForPendingWrites(time.Second) { + t.Fatal("identity not drained") + } + before := store.revision + if err := store.RememberIdentity("pve:node:100", "new", "oci"); err != nil { + t.Fatal(err) + } + if err := store.RememberIdentity("pve:node:100", "ignored", " "); err != nil { + t.Fatal(err) + } + if store.revision != before { + t.Fatal("unchanged/empty type triggered a write") + } + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + reloaded := NewGuestMetadataStore(store.dataPath, nil) + if !reflect.DeepEqual(reloaded.Get("pve:node:100"), got) { + t.Fatal("identity/operator fields not persisted") + } +} +func TestGuestMetadataWriterCloseBoundsBlockedAsyncAndSyncIO(t *testing.T) { + for _, synchronous := range []bool{false, true} { + t.Run(fmt.Sprint(synchronous), func(t *testing.T) { + fs := &controlledGuestWriterFS{FileSystem: defaultFileSystem{}, entered: make(chan struct{}), release: make(chan struct{})} + store := NewGuestMetadataStore(t.TempDir(), fs) + done := make(chan error, 1) + if synchronous { + go func() { done <- store.Set("first", &GuestMetadata{LastKnownName: "first"}) }() + } else { + done <- store.SetAsync("first", &GuestMetadata{LastKnownName: "first"}) + } + select { + case <-fs.entered: + case <-time.After(time.Second): + t.Fatal("write did not start") + } + t.Cleanup(func() { + select { + case <-fs.release: + default: + close(fs.release) + } + _ = store.Close(time.Second) + }) + start := time.Now() + if err := store.Close(10 * time.Millisecond); err == nil { + t.Fatal("blocked writer reported close success") + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Fatalf("close exceeded predeclared bounded budget: %s", elapsed) + } + close(fs.release) + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("write still running") + } + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + for _, mutation := range []func() error{ + func() error { return store.SetAsync("later", &GuestMetadata{}) }, + func() error { return store.RememberIdentity("later", "later", "qemu") }, + func() error { return store.Set("later", &GuestMetadata{}) }, + func() error { return store.Delete("first") }, + func() error { return store.ReplaceAll(nil) }, + func() error { return store.UpdateAll(func(map[string]*GuestMetadata) bool { return true }) }, + store.Load, + } { + if err := mutation(); !errors.Is(err, ErrGuestMetadataStoreClosed) { + t.Fatalf("closed admission accepted: %v", err) + } + } + if store.GetWithLegacyMigration("node:100", "pve", "node", 100) != nil { + t.Fatal("closed store migrated metadata") + } + if capturedGuestSnapshot(t, fs)["later"] != nil { + t.Fatal("post-close write") + } + }) + } +} + +type guestWriterFaultFS struct { + FileSystem + failWrite atomic.Bool + failRename atomic.Bool + writes atomic.Int32 +} + +func (f *guestWriterFaultFS) WriteFile(path string, data []byte, perm os.FileMode) error { + f.writes.Add(1) + if f.failWrite.Load() { + return errors.New("controlled write failure") + } + return f.FileSystem.WriteFile(path, data, perm) +} +func (f *guestWriterFaultFS) Rename(a, b string) error { + if f.failRename.Load() { + return errors.New("controlled rename failure") + } + return f.FileSystem.Rename(a, b) +} +func TestGuestMetadataWriterFailuresAreOwnedWithoutSpinRetry(t *testing.T) { + for _, phase := range []string{"write", "rename"} { + t.Run(phase, func(t *testing.T) { + fs := &guestWriterFaultFS{FileSystem: defaultFileSystem{}} + store := NewGuestMetadataStore(t.TempDir(), fs) + if err := store.Set("key", &GuestMetadata{LastKnownName: "original"}); err != nil { + t.Fatal(err) + } + fs.failWrite.Store(phase == "write") + fs.failRename.Store(phase == "rename") + if err := store.RememberIdentity("key", "changed", "qemu"); err != nil { + t.Fatal(err) + } + if !store.WaitForPendingWrites(time.Second) { + t.Fatal("failed attempt not quiescent") + } + if fs.writes.Load() != 2 { + t.Fatalf("failure retried automatically: %d", fs.writes.Load()) + } + if got := NewGuestMetadataStore(store.dataPath, nil).Get("key"); got.LastKnownName != "original" { + t.Fatal("failure replaced good disk snapshot") + } + fs.failWrite.Store(false) + fs.failRename.Store(false) + // A later ordinary poll retries the unpersisted identity, not a tight loop. + if err := store.RememberIdentity("key", "changed", "qemu"); err != nil { + t.Fatal(err) + } + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + if fs.writes.Load() != 3 { + t.Fatalf("explicit recovery writes=%d", fs.writes.Load()) + } + if got := NewGuestMetadataStore(store.dataPath, nil).Get("key"); got.LastKnownName != "changed" { + t.Fatal("recovery not persisted") + } + fs.failWrite.Store(true) + failed := NewGuestMetadataStore(t.TempDir(), fs) + if err := failed.SetAsync("lost", &GuestMetadata{}); err != nil { + t.Fatal(err) + } + if err := failed.Close(time.Second); err == nil { + t.Fatal("failed final write reported durable close") + } + }) + } +} +func TestGuestMetadataWriterRealFilesystemTenantIsolation(t *testing.T) { + root := t.TempDir() + a := NewGuestMetadataStore(filepath.Join(root, "orgs", "a"), nil) + b := NewGuestMetadataStore(filepath.Join(root, "orgs", "b"), nil) + for i := 0; i < 100; i++ { + id := fmt.Sprintf("pve:node:%d", i) + if err := a.RememberIdentity(id, "a", "qemu"); err != nil { + t.Fatal(err) + } + if err := b.RememberIdentity(id, "b", "lxc"); err != nil { + t.Fatal(err) + } + } + for _, store := range []*GuestMetadataStore{a, b} { + if err := store.Close(10 * time.Second); err != nil { + t.Fatal(err) + } + reloaded := NewGuestMetadataStore(store.dataPath, nil) + if !reflect.DeepEqual(reloaded.GetAll(), store.GetAll()) { + t.Fatal("real filesystem lost entries") + } + info, err := os.Stat(filepath.Join(store.dataPath, "guest_metadata.json")) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o600 { + t.Fatalf("unsafe file permissions: %v", info.Mode()) + } + if _, err := os.Stat(filepath.Join(store.dataPath, "guest_metadata.json.tmp")); !os.IsNotExist(err) { + t.Fatalf("temporary file survived close: %v", err) + } + } + if a.Get("pve:node:1").LastKnownName == b.Get("pve:node:1").LastKnownName { + t.Fatal("tenants share identity") + } +} + +// Retention after a failed close is not a permanent offboarding dead end. +// A later explicit close may flush retained state after the filesystem recovers, +// without accepting new mutations or overlapping the original writer. +func TestGuestMetadataWriterExplicitCloseRetryPersistsRetainedState(t *testing.T) { + fs := &guestWriterFaultFS{FileSystem: defaultFileSystem{}} + fs.failWrite.Store(true) + store := NewGuestMetadataStore(t.TempDir(), fs) + if err := store.SetAsync("retained", &GuestMetadata{LastKnownName: "retained"}); err != nil { + t.Fatal(err) + } + if err := store.Close(time.Second); err == nil { + t.Fatal("first failed close reported success") + } + if fs.writes.Load() != 1 { + t.Fatalf("failed close retried without an explicit later call: %d", fs.writes.Load()) + } + if err := store.SetAsync("late", &GuestMetadata{}); !errors.Is(err, ErrGuestMetadataStoreClosed) { + t.Fatalf("failed close reopened admission: %v", err) + } + fs.failWrite.Store(false) + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + if fs.writes.Load() != 2 { + t.Fatalf("explicit recovery or successful idempotent close wrote unexpected snapshots: %d", fs.writes.Load()) + } + reloaded := NewGuestMetadataStore(store.dataPath, nil) + if reloaded.Get("retained") == nil || reloaded.Get("late") != nil { + t.Fatal("explicit close recovery lost retained state or wrote a late admission") + } +} diff --git a/internal/config/guest_writer_control_test.go b/internal/config/guest_writer_control_test.go new file mode 100644 index 0000000000..a1c2e9e7ee --- /dev/null +++ b/internal/config/guest_writer_control_test.go @@ -0,0 +1,100 @@ +package config + +import ( + "encoding/json" + "fmt" + "os" + "sync" + "testing" + "time" +) + +type controlledGuestWriterFS struct { + FileSystem + mu sync.Mutex + writes int + data []byte + entered chan struct{} + release chan struct{} +} + +func (f *controlledGuestWriterFS) MkdirAll(string, os.FileMode) error { return nil } +func (f *controlledGuestWriterFS) Rename(string, string) error { return nil } +func (f *controlledGuestWriterFS) WriteFile(_ string, data []byte, _ os.FileMode) error { + f.mu.Lock() + f.writes++ + first := f.writes == 1 + f.mu.Unlock() + if first { + close(f.entered) + <-f.release + } + f.mu.Lock() + f.data = append([]byte(nil), data...) + f.mu.Unlock() + return nil +} +func TestGuestWriterControlledBurst(t *testing.T) { + for _, count := range []int{25, 100} { + t.Run(fmt.Sprint(count), func(t *testing.T) { + fs := &controlledGuestWriterFS{FileSystem: defaultFileSystem{}, entered: make(chan struct{}), release: make(chan struct{})} + store := NewGuestMetadataStore(t.TempDir(), fs) + store.SetAsync("first", &GuestMetadata{LastKnownName: "first"}) + select { + case <-fs.entered: + case <-time.After(time.Second): + t.Fatal("first write did not start") + } + for i := 0; i < count; i++ { + store.SetAsync(fmt.Sprint(i), &GuestMetadata{LastKnownName: fmt.Sprint(i)}) + } + close(fs.release) + if !store.WaitForPendingWrites(10 * time.Second) { + t.Fatal("drain failed") + } + fs.mu.Lock() + writes := fs.writes + data := append([]byte(nil), fs.data...) + fs.mu.Unlock() + t.Logf("admitted=%d complete-file-writes=%d", count+1, writes) + if writes > 2 { + t.Errorf("one blocked snapshot plus one complete follow-up needed, got %d", writes) + } + var saved map[string]*GuestMetadata + if err := json.Unmarshal(data, &saved); err != nil { + t.Fatal(err) + } + if len(saved) != count+1 { + t.Fatalf("lost guests: got %d want %d", len(saved), count+1) + } + for i := 0; i < count; i++ { + if saved[fmt.Sprint(i)].LastKnownName != fmt.Sprint(i) { + t.Errorf("wrong guest %d", i) + } + } + }) + } +} +func TestGuestWriterCopiesAtAdmission(t *testing.T) { + fs := &controlledGuestWriterFS{FileSystem: defaultFileSystem{}, entered: make(chan struct{}), release: make(chan struct{})} + store := NewGuestMetadataStore(t.TempDir(), fs) + store.SetAsync("first", &GuestMetadata{LastKnownName: "first"}) + select { + case <-fs.entered: + case <-time.After(time.Second): + t.Fatal("first write did not start") + } + meta := &GuestMetadata{LastKnownName: "original", Tags: []string{"original"}, Notes: []string{"original"}} + store.SetAsync("second", meta) + meta.LastKnownName = "mutated" + meta.Tags[0] = "mutated" + meta.Notes[0] = "mutated" + close(fs.release) + if !store.WaitForPendingWrites(10 * time.Second) { + t.Fatal("drain failed") + } + got := store.Get("second") + if got.LastKnownName != "original" || got.Tags[0] != "original" || got.Notes[0] != "original" { + t.Fatalf("caller mutation reached stored metadata: %+v", got) + } +} diff --git a/internal/monitoring/broadcast_metrics_snapshot_test.go b/internal/monitoring/broadcast_metrics_snapshot_test.go new file mode 100644 index 0000000000..02e18e99cc --- /dev/null +++ b/internal/monitoring/broadcast_metrics_snapshot_test.go @@ -0,0 +1,320 @@ +package monitoring + +import ( + "encoding/json" + "fmt" + "reflect" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/mock" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +func projectionFixtureGraph(t testing.TB) mock.FixtureGraph { + t.Helper() + previousEnabled, previousConfig := mock.IsMockEnabled(), mock.GetConfig() + t.Cleanup(func() { + mustSetMockEnabled(t, false) + mock.SetMockConfig(previousConfig) + mustSetMockEnabled(t, previousEnabled) + }) + mustSetMockEnabled(t, false) + cfg := mock.DefaultConfig + cfg.UpdateInterval = 5 * time.Minute + mock.SetMockConfig(cfg) + mustSetMockEnabled(t, true) + graph := mock.CurrentFixtureGraph() + mustSetMockEnabled(t, false) + return graph +} + +func projectionFixtureRegistry(graph mock.FixtureGraph) *unifiedresources.ResourceRegistry { + rr := unifiedresources.NewRegistry(nil) + rr.IngestSnapshot(graph.State) + for _, source := range []unifiedresources.DataSource{unifiedresources.SourceTrueNAS, unifiedresources.SourceVMware, unifiedresources.SourceAvailability} { + rr.IngestRecords(source, graph.SupplementalRecords(source)) + } + return rr +} + +// The original full projection, including target resolution after host +// coalescing. Compare all wire fields/catalogues, not selected metric values. +func legacyFrontendProjectionForTest(m *Monitor, snapshot models.StateSnapshot, resources []unifiedresources.Resource, resolver MetricsTargetResourceStore, now time.Time) models.StateFrontend { + rows := unifiedresources.CoalescePresentationHostResources(resources) + rows = m.applyPersistedMetadataToUnifiedResources(rows) + rows = unifiedresources.AttachResourceHealth(rows, resourceHealthAlerts(snapshot.ActiveAlerts), now) + projected, catalogs := convertResourcesForBroadcastReference(rows, resolver) + out := snapshot.ToFrontend() + out.Resources = projected + out.CapabilityCatalog = catalogs.capabilities + out.PolicyCatalog = catalogs.policies + out.AISafeSummaryCatalog = catalogs.aiSafeSummaries + out.ConnectedInfrastructure = buildConnectedInfrastructure(rows, snapshot) + return out +} + +func TestBroadcastMetricsSnapshotConnectedContent(t *testing.T) { + graph := projectionFixtureGraph(t) + rr := projectionFixtureRegistry(graph) + adapter := unifiedresources.NewMonitorAdapter(rr) + store := &broadcastProjectionCountingStore{MonitorAdapter: adapter} + m := &Monitor{resourceStore: store} + resources := adapter.GetAll() + if len(resources) < 1000 { + t.Fatalf("demo fixture too small: %d", len(resources)) + } + sources := map[unifiedresources.DataSource]bool{} + for _, r := range resources { + for _, source := range r.Sources { + sources[source] = true + } + } + for _, source := range []unifiedresources.DataSource{unifiedresources.SourceProxmox, unifiedresources.SourceAgent, unifiedresources.SourceDocker, unifiedresources.SourcePBS, unifiedresources.SourcePMG, unifiedresources.SourceK8s, unifiedresources.SourceTrueNAS, unifiedresources.SourceVMware, unifiedresources.SourceAvailability} { + if !sources[source] { + t.Fatalf("demo omits provider %s", source) + } + } + assertEqual := func(stage string) { + t.Helper() + evaluationTime := time.Now().UTC() + before, err := json.Marshal(adapter.GetAll()) + if err != nil { + t.Fatal(err) + } + want := legacyFrontendProjectionForTest(m, graph.State, adapter.GetAll(), adapter, evaluationTime) + // The parent's unified view fills a missing store watermark from the + // listed resources. Preserve that full-state header in the oracle too. + if freshness := latestUnifiedResourceLastSeen(adapter.GetAll()); !freshness.IsZero() { + want.LastUpdate = freshness.UnixMilli() + } + got := m.buildBroadcastFrontendStateFromSnapshotWithClock(graph.State, func() time.Time { return evaluationTime }) + wantJSON, err := json.Marshal(want) + if err != nil { + t.Fatal(err) + } + gotJSON, err := json.Marshal(got) + if err != nil { + t.Fatal(err) + } + if string(wantJSON) != string(gotJSON) { + first := 0 + for first < len(wantJSON) && first < len(gotJSON) && wantJSON[first] == gotJSON[first] { + first++ + } + start := max(0, first-80) + t.Fatalf("%s: complete frontend/catalogue/infrastructure JSON differs at byte %d: want %q, got %q (bytes %d/%d)", + stage, first, wantJSON[start:min(len(wantJSON), first+160)], gotJSON[start:min(len(gotJSON), first+160)], len(wantJSON), len(gotJSON)) + } + after, _ := json.Marshal(adapter.GetAll()) + if string(before) != string(after) { + t.Fatalf("%s: projection changed source data", stage) + } + t.Logf("%s: %d complete resources, %d frontend rows, %d wire bytes equal legacy projection", stage, len(adapter.GetAll()), len(got.Resources), len(gotJSON)) + } + assertEqual("cold-list-only") + rr.Workloads() + assertEqual("typed-views-clean") + // Same timestamps, new payload, policy and URL. No time-only invalidation. + resources = adapter.GetAll() + resources[0].Name = "changed-resource" + resources[0].Tags = []string{"customer-data"} + resources[0].CustomURL = "https://example.invalid/changed" + rr.IngestResources(resources) + assertEqual("dirty-same-time") +} + +func TestBroadcastMetricsSnapshotHealthClockBoundary(t *testing.T) { + m, adapter, id := newReadStateCloneTestMonitor(t, 1) + rows := adapter.GetAll() + original := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) + rows[0].LastSeen = original + rows[0].SourceStatus = map[unifiedresources.DataSource]unifiedresources.SourceStatus{ + unifiedresources.SourceProxmox: {Status: "stale", LastSeen: original}, + } + rr := unifiedresources.NewRegistry(nil) + rr.IngestResources(rows) + m.resourceStore = &broadcastProjectionCountingStore{MonitorAdapter: unifiedresources.NewMonitorAdapter(rr)} + for _, tc := range []struct { + age time.Duration + detail string + }{{59 * time.Second, "0m"}, {61 * time.Second, "1m"}} { + calls := 0 + got := m.buildBroadcastFrontendStateFromSnapshotWithClock(models.EmptyStateSnapshot(), func() time.Time { + calls++ + return original.Add(tc.age) + }) + if calls != 1 || len(got.Resources) != 1 || got.Resources[0].ID != id || got.Resources[0].LastSeen != original.UnixMilli() { + t.Fatal("health clock changed capture identity/original time or was not sampled once") + } + var health unifiedresources.ResourceHealth + if err := json.Unmarshal(got.Resources[0].Health, &health); err != nil { + t.Fatal(err) + } + if health.Verdict != unifiedresources.HealthStale || len(health.Reasons) != 1 || health.Reasons[0].Code != "telemetry_stale" || health.Reasons[0].Detail != tc.detail { + t.Fatalf("health age must advance across the minute boundary: %+v", health) + } + } +} + +func TestBroadcastMetricsSnapshotOwnsTargetsBeforeLiveReplacement(t *testing.T) { + m, adapter, _ := newReadStateCloneTestMonitor(t, 4) + view := m.currentUnifiedStateView() + if view.metricsTargets == nil { + t.Fatal("broadcast did not capture a bulk resolver") + } + old := view.resources[0] + want := adapter.MetricsTargetForResource(old.ID) + adapter.PopulateFromSnapshot(models.EmptyStateSnapshot()) + if got := view.metricsTargets.MetricsTargetForResource(old.ID); !reflect.DeepEqual(got, want) { + t.Fatal("captured resources were paired with a later generation's target") + } + if got := view.metricsTargets.MetricsTargetForResource("unknown"); got != nil { + t.Fatal("unknown target fell through to live data") + } + copyTarget := view.metricsTargets.MetricsTargetForResource(old.ID) + copyTarget.ResourceID = "caller-edit" + if got := view.metricsTargets.MetricsTargetForResource(old.ID); got.ResourceID == "caller-edit" { + t.Fatal("caller mutated the captured resolver") + } +} + +func TestBroadcastMetricsSnapshotContinuityAndLiveOverlays(t *testing.T) { + m, adapter, _ := newReadStateCloneTestMonitor(t, 2) + now := time.Now().UTC() + m.hostContinuityStore = config.NewHostContinuityStore(t.TempDir(), nil) + entry := config.HostContinuityEntry{HostID: "absent-host", Hostname: "absent.example", MachineID: "absent-machine", LastSeen: now.Add(-time.Hour)} + if err := m.hostContinuityStore.Upsert(entry); err != nil { + t.Fatal(err) + } + view := m.unifiedStateViewWithStandaloneHostContinuity(monitorUnifiedStateView{readState: adapter}) + if len(view.resources) != 3 || view.metricsTargets == nil { + t.Fatal("continuity projection omitted its resources or captured resolver") + } + var host unifiedresources.Resource + for _, r := range view.resources { + if r.Agent != nil && r.Agent.AgentID == entry.HostID { + host = r + } + if got, want := view.metricsTargets.MetricsTargetForResource(r.ID), broadcastMetricsTargetResolver(view.readState).MetricsTargetForResource(r.ID); !reflect.DeepEqual(got, want) { + t.Fatal("overlay target differs from overlay registry") + } + } + if host.ID == "" || host.Status == unifiedresources.StatusOnline || !host.LastSeen.Equal(entry.LastSeen) { + t.Fatal("continuity history was omitted or promoted to current") + } + m.hostMetadataStore = config.NewHostMetadataStore(t.TempDir(), nil) + for _, url := range []string{"https://example.invalid/operator", ""} { + if err := m.hostMetadataStore.Set(entry.HostID, &config.HostMetadata{CustomURL: url}); err != nil { + t.Fatal(err) + } + decorated := m.applyPersistedMetadataToUnifiedResources(view.resources) + for _, r := range decorated { + if r.ID == host.ID && r.CustomURL != url { + t.Fatal("operator URL change/clear was hidden by captured data") + } + } + } + for _, r := range view.resources { + if r.Type != unifiedresources.ResourceTypeVM { + continue + } + baseline := unifiedresources.EvaluateResourceHealth(r, nil, now) + critical := unifiedresources.EvaluateResourceHealth(r, []unifiedresources.ResourceHealthAlert{{ResourceID: r.ID, Level: "critical", Type: "cpu"}}, now) + cleared := unifiedresources.EvaluateResourceHealth(r, nil, now) + if baseline.Verdict != unifiedresources.HealthOK || critical.Verdict != unifiedresources.HealthCritical || !reflect.DeepEqual(cleared, baseline) { + t.Fatal("live alert/clear was hidden by captured data") + } + } + if len(adapter.GetAll()) != 2 { + t.Fatal("continuity overlay wrote back to the live registry") + } +} + +var completeProjectionSink models.StateFrontend +var completeProjectionJSONSink []byte + +// Exact parent broadcast shape (one coalesce, copy-to-attach, then conversion), +// not the older two-coalesce content oracle above. The unchanged conversion, +// metadata/health decoration and snapshot projection are part of both costs. +func parentFrontendProjectionForTest(m *Monitor, snapshot models.StateSnapshot, resources []unifiedresources.Resource, resolver MetricsTargetResourceStore, now time.Time) models.StateFrontend { + rows := unifiedresources.CoalescePresentationHostResources(resources) + healthAlerts := resourceHealthAlerts(snapshot.ActiveAlerts) + for i := range rows { + m.applyPersistedMetadataToUnifiedResource(&rows[i]) + health := unifiedresources.EvaluateResourceHealth(rows[i], healthAlerts, now) + rows[i].Health = &health + } + projected, catalogs := convertPresentationResourcesForBroadcast(attachBroadcastMetricsTargets(rows, resolver)) + out := snapshot.ToFrontend() + out.Resources = projected + out.CapabilityCatalog = catalogs.capabilities + out.PolicyCatalog = catalogs.policies + out.AISafeSummaryCatalog = catalogs.aiSafeSummaries + out.ConnectedInfrastructure = buildConnectedInfrastructure(rows, snapshot) + if freshness := latestUnifiedResourceLastSeen(resources); !freshness.IsZero() { + out.LastUpdate = freshness.UnixMilli() + } + return out +} + +func BenchmarkBroadcastMetricsSnapshot(b *testing.B) { + graph := projectionFixtureGraph(b) + for _, mode := range []string{"cold", "list-clean", "views-clean", "dirty"} { + for _, encode := range []bool{false, true} { + stage := "projection" + if encode { + stage = "json-frame" + } + for _, bulk := range []bool{false, true} { + path := "point-targets" + if bulk { + path = "bulk-targets" + } + b.Run(fmt.Sprintf("%s/%s/%s", mode, stage, path), func(b *testing.B) { + rr := projectionFixtureRegistry(graph) + adapter := unifiedresources.NewMonitorAdapter(rr) + store := &broadcastProjectionCountingStore{MonitorAdapter: adapter} + m := &Monitor{resourceStore: store} + if mode != "cold" { + rr.List() + } + if mode == "views-clean" { + rr.Workloads() + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if mode == "cold" || mode == "dirty" { + b.StopTimer() + if mode == "cold" { + rr = projectionFixtureRegistry(graph) + adapter = unifiedresources.NewMonitorAdapter(rr) + store.MonitorAdapter = adapter + } else { + changed := rr.List()[0] + changed.Tags = []string{fmt.Sprintf("edit-%d", i)} + rr.IngestResources([]unifiedresources.Resource{changed}) + } + b.StartTimer() + } + if bulk { + completeProjectionSink = m.buildBroadcastFrontendStateFromSnapshot(graph.State) + } else { + completeProjectionSink = parentFrontendProjectionForTest(m, graph.State, adapter.GetAll(), adapter, time.Now().UTC()) + } + if encode { + var err error + completeProjectionJSONSink, err = json.Marshal(completeProjectionSink) + if err != nil { + b.Fatal(err) + } + } + } + }) + } + } + } +} diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 165f94a540..f65c96057a 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -607,10 +607,11 @@ func TestBroadcastResourceProjectionCoalescesSplitHostIdentities(t *testing.T) { source := string(data) for _, snippet := range []string{ - "metricsTargetResolver := broadcastMetricsTargetResolver(unifiedView.readState)", + "metricsTargetResolver := unifiedView.metricsTargets", + "metricsTargetResolver = broadcastMetricsTargetResolver(unifiedView.readState)", "broadcastResources := unifiedresources.CoalescePresentationHostResources(unifiedView.resources)", - "broadcastFrontendResources, broadcastCatalogs := convertPresentationResourcesForBroadcast(", - "attachBroadcastMetricsTargets(broadcastResources, metricsTargetResolver)", + "broadcastFrontendResources, broadcastCatalogs := convertPresentationResourcesForBroadcast(broadcastResources)", + "attachBroadcastMetricsTargetsInPlace(broadcastResources, metricsTargetResolver)", "frontendState.CapabilityCatalog = broadcastCatalogs.capabilities", "frontendState.PolicyCatalog = broadcastCatalogs.policies", "frontendState.AISafeSummaryCatalog = broadcastCatalogs.aiSafeSummaries", @@ -2957,7 +2958,7 @@ func TestBroadcastProjectionMatchesPreviousPipeline(t *testing.T) { if !view.freshness.IsZero() { want.LastUpdate = view.freshness.UnixMilli() } - got := m.buildBroadcastFrontendStateFromSnapshot(snapshot) + got := m.buildBroadcastFrontendStateFromSnapshotWithClock(snapshot, func() time.Time { return now }) wantJSON, err := json.Marshal(want) if err != nil { t.Fatal(err) diff --git a/internal/monitoring/guest_metadata_shutdown_test.go b/internal/monitoring/guest_metadata_shutdown_test.go new file mode 100644 index 0000000000..8c9ad302ee --- /dev/null +++ b/internal/monitoring/guest_metadata_shutdown_test.go @@ -0,0 +1,125 @@ +package monitoring + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +type blockedGuestMetadataFS struct { + config.FileSystem + entered chan struct{} + release chan struct{} +} + +func (f *blockedGuestMetadataFS) ReadFile(p string) ([]byte, error) { return os.ReadFile(p) } +func (f *blockedGuestMetadataFS) Stat(p string) (os.FileInfo, error) { return os.Stat(p) } +func (f *blockedGuestMetadataFS) MkdirAll(p string, m os.FileMode) error { return os.MkdirAll(p, m) } +func (f *blockedGuestMetadataFS) WriteFile(p string, b []byte, m os.FileMode) error { + close(f.entered) + <-f.release + return os.WriteFile(p, b, m) +} +func (f *blockedGuestMetadataFS) Rename(a, b string) error { return os.Rename(a, b) } + +func TestTenantDeletionRetainsBlockedGuestWriterOwnership(t *testing.T) { + path := t.TempDir() + fs := &blockedGuestMetadataFS{entered: make(chan struct{}), release: make(chan struct{})} + store := config.NewGuestMetadataStore(path, fs) + monitor := &Monitor{guestMetadataStore: store} + mtm := NewMultiTenantMonitor(&config.Config{DataPath: path}, nil, nil) + mtm.monitors["blocked"] = monitor + t.Cleanup(func() { + select { + case <-fs.release: + default: + close(fs.release) + } + _ = store.Close(time.Second) + mtm.Stop() + }) + persistGuestIdentity(store, "pve:node:100", "retained", "qemu") + select { + case <-fs.entered: + case <-time.After(time.Second): + t.Fatal("write did not start") + } + if err := mtm.BeginTenantDeletion("blocked"); err == nil { + t.Fatal("deletion accepted incomplete drain") + } + if retained, ok := mtm.PeekMonitor("blocked"); !ok || retained != monitor { + t.Fatal("in-flight writer owner was discarded") + } + mtm.FinishTenantDeletion("blocked") + if _, err := mtm.GetMonitor("blocked"); err == nil { + t.Fatal("deferred cleanup released unsafe lifecycle guard") + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("data path not retained: %v", err) + } + close(fs.release) + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } + if err := mtm.BeginTenantDeletion("blocked"); err != nil { + t.Fatal(err) + } + if _, ok := mtm.PeekMonitor("blocked"); ok { + t.Fatal("completed shutdown not removed") + } + if err := store.RememberIdentity("late", "late", "qemu"); err == nil { + t.Fatal("stopped store accepted late identity") + } + if _, err := os.Stat(filepath.Join(path, "guest_metadata.json")); err != nil { + t.Fatal("accepted metadata not persisted") + } +} +func TestTenantDeletionRetainsUnfinishedMonitorLoop(t *testing.T) { + mtm := NewMultiTenantMonitor(&config.Config{}, nil, nil) + _, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + monitor := &Monitor{} + mtm.monitors["loop"] = monitor + mtm.tenantCancel["loop"] = cancel + mtm.tenantDone["loop"] = done + t.Cleanup(func() { + select { + case <-done: + default: + close(done) + } + mtm.Stop() + }) + if err := mtm.BeginTenantDeletion("loop"); err == nil { + t.Fatal("unfinished monitoring loop admitted directory deletion") + } + if retained, ok := mtm.PeekMonitor("loop"); !ok || retained != monitor { + t.Fatal("unfinished monitoring loop lost owner") + } + if _, err := mtm.GetMonitor("loop"); err == nil { + t.Fatal("unfinished monitor replaced") + } + close(done) + if err := mtm.BeginTenantDeletion("loop"); err != nil { + t.Fatal(err) + } +} +func TestPersistGuestIdentityEditsAreImmediateAndFieldScoped(t *testing.T) { + store := config.NewGuestMetadataStore(t.TempDir(), nil) + if err := store.Set("pve:node:100", &config.GuestMetadata{CustomURL: "https://operator.example", Tags: []string{"important"}, Notes: []string{"keep"}}); err != nil { + t.Fatal(err) + } + persistGuestIdentity(store, "pve:node:100", "old", "qemu") + persistGuestIdentity(store, "pve:node:100", "new", "qemu") + got := store.Get("pve:node:100") + if got.LastKnownName != "new" || got.CustomURL != "https://operator.example" || got.Tags[0] != "important" || got.Notes[0] != "keep" { + t.Fatalf("producer corrupted metadata: %+v", got) + } + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } +} diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 6e9091900e..c722fc70f0 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -4333,6 +4333,13 @@ func buildFrontendStateFromSnapshot(snapshot models.StateSnapshot) models.StateF } func (m *Monitor) buildBroadcastFrontendStateFromSnapshot(snapshot models.StateSnapshot) models.StateFrontend { + return m.buildBroadcastFrontendStateFromSnapshotWithClock(snapshot, time.Now) +} + +// The clock is sampled at the same post-read health-evaluation boundary as +// ordinary broadcasts. Tests can compare complete projections at one instant +// without mistaking a naturally advancing health-age label for lost content. +func (m *Monitor) buildBroadcastFrontendStateFromSnapshotWithClock(snapshot models.StateSnapshot, healthClock func() time.Time) models.StateFrontend { frontendState := buildFrontendStateFromSnapshot(snapshot) m.updateResourceStoreForRead(snapshot) if m != nil && m.alertManager != nil { @@ -4341,20 +4348,22 @@ func (m *Monitor) buildBroadcastFrontendStateFromSnapshot(snapshot models.StateS } } unifiedView := m.currentUnifiedStateView() - metricsTargetResolver := broadcastMetricsTargetResolver(unifiedView.readState) + metricsTargetResolver := unifiedView.metricsTargets + if metricsTargetResolver == nil { + metricsTargetResolver = broadcastMetricsTargetResolver(unifiedView.readState) + } broadcastResources := unifiedresources.CoalescePresentationHostResources(unifiedView.resources) // Coalescing owns the outer slice. Decorate that one projection in place, // not three full-resource copies; nested store data is still read-only. healthAlerts := resourceHealthAlerts(frontendState.ActiveAlerts) - now := time.Now().UTC() + now := healthClock().UTC() for i := range broadcastResources { m.applyPersistedMetadataToUnifiedResource(&broadcastResources[i]) health := unifiedresources.EvaluateResourceHealth(broadcastResources[i], healthAlerts, now) broadcastResources[i].Health = &health } - broadcastFrontendResources, broadcastCatalogs := convertPresentationResourcesForBroadcast( - attachBroadcastMetricsTargets(broadcastResources, metricsTargetResolver), - ) + attachBroadcastMetricsTargetsInPlace(broadcastResources, metricsTargetResolver) + broadcastFrontendResources, broadcastCatalogs := convertPresentationResourcesForBroadcast(broadcastResources) frontendState.Resources = broadcastFrontendResources frontendState.CapabilityCatalog = broadcastCatalogs.capabilities frontendState.PolicyCatalog = broadcastCatalogs.policies @@ -4871,23 +4880,50 @@ func (m *Monitor) RecentResourceChanges(resourceID string, since time.Time, limi } type monitorUnifiedStateView struct { - resources []unifiedresources.Resource - readState unifiedresources.ReadState - freshness time.Time + resources []unifiedresources.Resource + readState unifiedresources.ReadState + freshness time.Time + metricsTargets MetricsTargetResourceStore } type unifiedResourceReadStateLister interface { GetAll() []unifiedresources.Resource } +type unifiedResourceProjectionLister interface { + GetAllWithMetricsTargets() ([]unifiedresources.Resource, map[string]unifiedresources.MetricsTarget) +} + +// This resolver never falls through to the live store, including for an +// unknown ID. A missing target belongs to this captured generation too. +type projectionMetricsTargets map[string]unifiedresources.MetricsTarget + +func (targets projectionMetricsTargets) MetricsTargetForResource(id string) *unifiedresources.MetricsTarget { + target, ok := targets[unifiedresources.CanonicalResourceID(id)] + if !ok { + return nil + } + return &target +} + +func unifiedProjectionResources(lister unifiedResourceReadStateLister) ([]unifiedresources.Resource, MetricsTargetResourceStore) { + if projection, ok := lister.(unifiedResourceProjectionLister); ok { + resources, targets := projection.GetAllWithMetricsTargets() + return resources, projectionMetricsTargets(targets) + } + return lister.GetAll(), nil +} + func monitorUnifiedStateViewFromSnapshot(snapshot models.StateSnapshot) monitorUnifiedStateView { registry := unifiedresources.NewRegistry(nil) registry.IngestSnapshot(snapshot) adapter := unifiedresources.NewMonitorAdapter(registry) + resources, targets := unifiedProjectionResources(adapter) return monitorUnifiedStateView{ - resources: registry.List(), - readState: adapter, - freshness: snapshot.LastUpdate, + resources: resources, + readState: adapter, + freshness: snapshot.LastUpdate, + metricsTargets: targets, } } @@ -4895,10 +4931,12 @@ func monitorUnifiedStateViewFromResources(resources []unifiedresources.Resource, registry := unifiedresources.NewRegistry(nil) registry.IngestResources(resources) adapter := unifiedresources.NewMonitorAdapter(registry) + resources, targets := unifiedProjectionResources(adapter) return monitorUnifiedStateView{ - resources: registry.List(), - readState: adapter, - freshness: freshness, + resources: resources, + readState: adapter, + freshness: freshness, + metricsTargets: targets, } } @@ -4915,8 +4953,9 @@ func (m *Monitor) unifiedStateViewWithStandaloneHostContinuity(view monitorUnifi return view } - resources := lister.GetAll() + resources, targets := unifiedProjectionResources(lister) view.resources = resources + view.metricsTargets = targets if view.freshness.IsZero() { view.freshness = latestUnifiedResourceLastSeen(resources) } @@ -6479,17 +6518,24 @@ func attachBroadcastMetricsTargets( } out := make([]unifiedresources.Resource, len(resources)) - for i, resource := range resources { - if resource.MetricsTarget == nil { - resource.MetricsTarget = validBroadcastMetricsTarget( - resolver.MetricsTargetForResource(resource.ID), - ) - } - out[i] = resource - } + copy(out, resources) + attachBroadcastMetricsTargetsInPlace(out, resolver) return out } +// Only call with the owned outer slice of a coalesced broadcast projection. +// Nested resource data stays read-only, and the resolver returns owned targets. +func attachBroadcastMetricsTargetsInPlace(resources []unifiedresources.Resource, resolver MetricsTargetResourceStore) { + if resolver == nil { + return + } + for i := range resources { + if resources[i].MetricsTarget == nil { + resources[i].MetricsTarget = validBroadcastMetricsTarget(resolver.MetricsTargetForResource(resources[i].ID)) + } + } +} + func validBroadcastMetricsTarget( target *unifiedresources.MetricsTarget, ) *unifiedresources.MetricsTarget { @@ -7598,6 +7644,12 @@ func monitorLastSeenUnix(value time.Time) int64 { const guestMetadataDrainTimeout = 2 * time.Second func (m *Monitor) Stop() { + if err := m.stop(); err != nil { + log.Error().Err(err).Msg("monitor stopped with incomplete guest metadata persistence; data directory must be retained") + } +} + +func (m *Monitor) stop() error { log.Info().Msg("stopping monitor") // A trailing agent-report refresh must not run against stores closed below. @@ -7620,9 +7672,7 @@ func (m *Monitor) Stop() { // Drain background guest-metadata writes before the data directory can be // torn down. Without this a queued write lands after shutdown and leaves a // stray guest_metadata.json.tmp behind. - if m.guestMetadataStore != nil { - m.guestMetadataStore.WaitForPendingWrites(guestMetadataDrainTimeout) - } + drainErr := m.guestMetadataStore.Close(guestMetadataDrainTimeout) // Close persistent metrics store (flushes buffered data) if m.metricsStore != nil { @@ -7633,7 +7683,11 @@ func (m *Monitor) Stop() { } } + if drainErr != nil { + return drainErr + } log.Info().Msg("monitor stopped") + return nil } // recordAuthFailure records an authentication failure for a node diff --git a/internal/monitoring/monitor_backups.go b/internal/monitoring/monitor_backups.go index 1dc59f1184..d5475857eb 100644 --- a/internal/monitoring/monitor_backups.go +++ b/internal/monitoring/monitor_backups.go @@ -872,33 +872,8 @@ func enrichWithPersistedMetadata(metadataStore *config.GuestMetadataStore, byVMI // persistGuestIdentity updates the metadata store with the last-known name and type for a guest func persistGuestIdentity(metadataStore *config.GuestMetadataStore, guestKey, name, guestType string) { - existing := metadataStore.Get(guestKey) - if existing == nil { - existing = &config.GuestMetadata{ - ID: guestKey, - Tags: []string{}, - } - } - - guestType = strings.TrimSpace(guestType) - if guestType == "" { - return - } - - // Never "downgrade" OCI containers back to LXC. OCI classification can be transiently - // unavailable if Proxmox config reads fail due to permissions or transient API errors. - if existing.LastKnownType == "oci" && guestType != "oci" { - guestType = existing.LastKnownType - } - - // Only update if the name or type has changed - if existing.LastKnownName != name || existing.LastKnownType != guestType { - existing.LastKnownName = name - existing.LastKnownType = guestType - // Save without blocking the monitor. The store owns the goroutine so - // Monitor.Stop can drain it; a detached goroutine here could write - // after shutdown, into a data directory that was already being removed. - metadataStore.SetAsync(guestKey, existing) + if err := metadataStore.RememberIdentity(guestKey, name, guestType); err != nil { + log.Error().Err(err).Str("guestKey", guestKey).Msg("failed to admit guest identity update") } } diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index 12e934c142..b7d3708922 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -6718,6 +6718,10 @@ func (s *broadcastProjectionCountingStore) GetAll() []unifiedresources.Resource s.reads++ return s.MonitorAdapter.GetAll() } +func (s *broadcastProjectionCountingStore) GetAllWithMetricsTargets() ([]unifiedresources.Resource, map[string]unifiedresources.MetricsTarget) { + s.reads++ + return s.MonitorAdapter.GetAllWithMetricsTargets() +} func (*broadcastProjectionCountingStore) TryReplaceRegistryForRead(models.StateSnapshot, time.Duration, func() map[unifiedresources.DataSource][]unifiedresources.IngestRecord) bool { return false } @@ -6759,6 +6763,10 @@ func TestBroadcastProjectionListsRegistryOnceAndKeepsLiveChanges(t *testing.T) { if row == nil || row.CPU.Current != 77 || len(row.Tags) != 1 || row.Tags[0] != "new-tag" { t.Fatalf("mutable row was stale: %#v", row) } + var health unifiedresources.ResourceHealth + if err := json.Unmarshal(row.Health, &health); err != nil || health.Verdict != unifiedresources.HealthCritical { + t.Fatalf("live critical alert did not reach the freshly projected row: %s (%v)", row.Health, err) + } if first.Resources[0].CPU.Current != oldCPU { t.Fatal("later projection mutated an accepted baseline") } diff --git a/internal/monitoring/monitor_metadata_test.go b/internal/monitoring/monitor_metadata_test.go index 12f3d3e658..e63bd48fed 100644 --- a/internal/monitoring/monitor_metadata_test.go +++ b/internal/monitoring/monitor_metadata_test.go @@ -20,15 +20,6 @@ func TestPersistGuestIdentity_Concurrent(t *testing.T) { } defer os.RemoveAll(tmpDir) - // metadataFile := filepath.Join(tmpDir, "guest_metadata.json") // Actually NewGuestMetadataStore takes directory not file usually? No, it takes root dir? - // Let's verify standard usage. "store := NewGuestMetadataStore(tmpDir, nil)" - // Implementation: func NewGuestMetadataStore(dataPath string, fs FileSystem) - // Inside it does filepath.Join(dataPath, "guest_metadata.json") ? - // Let's re-read NewGuestMetadataStore in internal/config/guest_metadata.go via grep or similar if needed. - // But based on "config.NewGuestMetadataStore(metadataFile)" from my previous code failing, and grep showing "dataPath", it likely takes a Dir or File path. - // grep output: guestMetadataStore := NewGuestMetadataStore(dataPath, c.fs) - // Most likely directory. - store := config.NewGuestMetadataStore(tmpDir, nil) guestKey := "pve1:node1:100" @@ -36,8 +27,9 @@ func TestPersistGuestIdentity_Concurrent(t *testing.T) { // Test basic persistence persistGuestIdentity(store, guestKey, "VM 100", "qemu") - // Wait a bit since persistGuestIdentity is async - time.Sleep(50 * time.Millisecond) + if !store.WaitForPendingWrites(time.Second) { + t.Fatal("identity writer did not drain") + } meta := store.Get(guestKey) if meta == nil || meta.LastKnownName != "VM 100" || meta.LastKnownType != "qemu" { @@ -55,7 +47,9 @@ func TestPersistGuestIdentity_Concurrent(t *testing.T) { // Try to update to "lxc" persistGuestIdentity(store, guestKey, "VM 100", "lxc") - time.Sleep(50 * time.Millisecond) + if !store.WaitForPendingWrites(time.Second) { + t.Fatal("identity writer did not drain") + } meta = store.Get(guestKey) if meta.LastKnownType != "oci" { @@ -65,6 +59,9 @@ func TestPersistGuestIdentity_Concurrent(t *testing.T) { // Test persistence that shouldn't happen (no change) -> coverage of the if check // Should not trigger Set() persistGuestIdentity(store, guestKey, "VM 100", "oci") + if err := store.Close(time.Second); err != nil { + t.Fatal(err) + } } func TestEnrichWithPersistedMetadata_Detail(t *testing.T) { diff --git a/internal/monitoring/multi_tenant_monitor.go b/internal/monitoring/multi_tenant_monitor.go index 14a76d656b..248b842239 100644 --- a/internal/monitoring/multi_tenant_monitor.go +++ b/internal/monitoring/multi_tenant_monitor.go @@ -20,6 +20,7 @@ type MultiTenantMonitor struct { tenantCancel map[string]context.CancelFunc tenantDone map[string]chan struct{} tenantDeleting map[string]struct{} + tenantStopping map[string]struct{} persistence *config.MultiTenantPersistence baseConfig *config.Config wsHub *websocket.Hub @@ -37,6 +38,7 @@ func NewMultiTenantMonitor(baseCfg *config.Config, persistence *config.MultiTena tenantCancel: make(map[string]context.CancelFunc), tenantDone: make(map[string]chan struct{}), tenantDeleting: make(map[string]struct{}), + tenantStopping: make(map[string]struct{}), persistence: persistence, baseConfig: baseCfg, // Used as a template or for global settings wsHub: wsHub, @@ -294,47 +296,34 @@ func (mtm *MultiTenantMonitor) Stop() { log.Info().Msg("stopping MultiTenantMonitor and all tenant instances") mtm.mu.Lock() - mtm.globalCancel() - - monitors := make([]*Monitor, 0, len(mtm.monitors)) - cancels := make([]context.CancelFunc, 0, len(mtm.tenantCancel)) - doneSignals := make([]chan struct{}, 0, len(mtm.tenantDone)) - for orgID, monitor := range mtm.monitors { - if cancel := mtm.tenantCancel[orgID]; cancel != nil { - cancels = append(cancels, cancel) - } - if done := mtm.tenantDone[orgID]; done != nil { - doneSignals = append(doneSignals, done) - } - monitors = append(monitors, monitor) + if mtm.globalCancel != nil { + mtm.globalCancel() + } + orgIDs := make([]string, 0, len(mtm.monitors)) + for orgID := range mtm.monitors { + orgIDs = append(orgIDs, orgID) } - mtm.tenantCancel = make(map[string]context.CancelFunc) - mtm.tenantDone = make(map[string]chan struct{}) - mtm.monitors = make(map[string]*Monitor) mtm.mu.Unlock() - - for _, cancel := range cancels { - cancel() - } - for _, done := range doneSignals { - waitForTenantMonitorShutdown("", done) - } - for _, monitor := range monitors { - monitor.Stop() + for _, orgID := range orgIDs { + if err := mtm.removeTenant(orgID, false); err != nil { + log.Error().Err(err).Str("org_id", orgID).Msg("tenant shutdown incomplete; retaining runtime ownership and data") + } } } // RemoveTenant stops and removes a specific tenant's monitor. // Useful for offboarding or manual reloading. func (mtm *MultiTenantMonitor) RemoveTenant(orgID string) { - mtm.removeTenant(orgID, false) + if err := mtm.removeTenant(orgID, false); err != nil { + log.Error().Err(err).Str("org_id", orgID).Msg("tenant removal incomplete; retaining runtime ownership and data") + } } // BeginTenantDeletion prevents lazy monitor initialization while a tenant's // persistence is being removed. Call FinishTenantDeletion after the -// persistence operation completes, whether it succeeds or fails. -func (mtm *MultiTenantMonitor) BeginTenantDeletion(orgID string) { - mtm.removeTenant(orgID, true) +// persistence operation completes. An incomplete shutdown retains its guard. +func (mtm *MultiTenantMonitor) BeginTenantDeletion(orgID string) error { + return mtm.removeTenant(orgID, true) } // FinishTenantDeletion releases the temporary lifecycle guard installed by @@ -347,46 +336,70 @@ func (mtm *MultiTenantMonitor) FinishTenantDeletion(orgID string) { } mtm.mu.Lock() - delete(mtm.tenantDeleting, orgID) + // An incomplete removal still owns a runtime or writer. Do not release + // its guard merely because an offboarding caller ran deferred cleanup. + if _, retained := mtm.monitors[orgID]; !retained { + delete(mtm.tenantDeleting, orgID) + } mtm.mu.Unlock() } -func (mtm *MultiTenantMonitor) removeTenant(orgID string, deleting bool) { +func (mtm *MultiTenantMonitor) removeTenant(orgID string, deleting bool) error { orgID = strings.TrimSpace(orgID) if orgID == "" { - return + return nil } mtm.mu.Lock() - if deleting { - if mtm.tenantDeleting == nil { - mtm.tenantDeleting = make(map[string]struct{}) - } - mtm.tenantDeleting[orgID] = struct{}{} + if mtm.tenantDeleting == nil { + mtm.tenantDeleting = make(map[string]struct{}) } - monitor, exists := mtm.monitors[orgID] + if mtm.tenantStopping == nil { + mtm.tenantStopping = make(map[string]struct{}) + } + if _, stopping := mtm.tenantStopping[orgID]; stopping { + mtm.mu.Unlock() + return fmt.Errorf("organization %q shutdown is already in progress", orgID) + } + mtm.tenantDeleting[orgID] = struct{}{} + mtm.tenantStopping[orgID] = struct{}{} + monitor := mtm.monitors[orgID] cancel := mtm.tenantCancel[orgID] done := mtm.tenantDone[orgID] - delete(mtm.monitors, orgID) - delete(mtm.tenantCancel, orgID) - delete(mtm.tenantDone, orgID) mtm.mu.Unlock() - - if !exists { - return - } + defer func() { + mtm.mu.Lock() + delete(mtm.tenantStopping, orgID) + mtm.mu.Unlock() + }() log.Info().Str("org_id", orgID).Msg("stopping and removing tenant monitor") if cancel != nil { cancel() } - waitForTenantMonitorShutdown(orgID, done) - monitor.Stop() + if !waitForTenantMonitorShutdown(orgID, done) { + return fmt.Errorf("organization %q monitoring loop has not stopped", orgID) + } + if monitor != nil { + if err := monitor.stop(); err != nil { + return fmt.Errorf("organization %q guest metadata shutdown incomplete: %w", orgID, err) + } + } + + mtm.mu.Lock() + delete(mtm.monitors, orgID) + delete(mtm.tenantCancel, orgID) + delete(mtm.tenantDone, orgID) + if !deleting { + delete(mtm.tenantDeleting, orgID) + } + mtm.mu.Unlock() + return nil } -func waitForTenantMonitorShutdown(orgID string, done <-chan struct{}) { +func waitForTenantMonitorShutdown(orgID string, done <-chan struct{}) bool { if done == nil { - return + return true } timer := time.NewTimer(tenantMonitorShutdownTimeout) @@ -394,12 +407,14 @@ func waitForTenantMonitorShutdown(orgID string, done <-chan struct{}) { select { case <-done: + return true case <-timer.C: logger := log.Warn().Dur("timeout", tenantMonitorShutdownTimeout) if orgID != "" { logger = logger.Str("org_id", orgID) } logger.Msg("timed out waiting for tenant monitor loop to exit") + return false } } diff --git a/internal/unifiedresources/monitor_adapter.go b/internal/unifiedresources/monitor_adapter.go index 28eda9d110..8bac41ceaa 100644 --- a/internal/unifiedresources/monitor_adapter.go +++ b/internal/unifiedresources/monitor_adapter.go @@ -496,6 +496,18 @@ func (a *MonitorAdapter) GetAll() []Resource { return registry.List() } +// GetAllWithMetricsTargets materializes one complete broadcast read, binding +// its detached target map to the same registry as its detached resource list. +// The adapter may publish a replacement immediately after this capture; neither +// half of the returned read then consults that newer generation. +func (a *MonitorAdapter) GetAllWithMetricsTargets() ([]Resource, map[string]MetricsTarget) { + registry := a.currentRegistry() + if registry == nil { + return nil, nil + } + return registry.ListWithMetricsTargets() +} + func (a *MonitorAdapter) unifiedAIAdapter() *UnifiedAIAdapter { registry := a.currentRegistry() if registry == nil { diff --git a/internal/unifiedresources/monitor_adapter_read_state_test.go b/internal/unifiedresources/monitor_adapter_read_state_test.go index 5eb791f3cf..7eefcb5b99 100644 --- a/internal/unifiedresources/monitor_adapter_read_state_test.go +++ b/internal/unifiedresources/monitor_adapter_read_state_test.go @@ -971,3 +971,40 @@ func TestHostContinuityOverlayReusedWithinRegistryGeneration(t *testing.T) { t.Fatal("overlay was reused past its maximum age") } } + +func TestMonitorAdapterProjectionSnapshotKeepsReplacedGeneration(t *testing.T) { + var absent *MonitorAdapter + rows, targets := absent.GetAllWithMetricsTargets() + if rows != nil || targets != nil { + t.Fatal("nil adapter invented a projection") + } + adapter := NewMonitorAdapter(NewRegistry(nil)) + snapshot := readRefreshSnapshot("before") + adapter.PopulateFromSnapshot(snapshot) + first, firstTargets := adapter.GetAllWithMetricsTargets() + snapshot.VMs[0].Name = "after" + snapshot.VMs[0].ID = "new-node:201" + snapshot.VMs[0].VMID = 201 + // Observation time is deliberately unchanged. + adapter.PopulateFromSnapshot(snapshot) + second, secondTargets := adapter.GetAllWithMetricsTargets() + if len(first) != 1 || len(second) != 1 || first[0].Name != "before" || second[0].Name != "after" { + t.Fatal("replacement lost complete fresh resource content") + } + if firstTargets[first[0].ID].ResourceID == secondTargets[second[0].ID].ResourceID { + t.Fatal("replacement retained old history coordinates") + } + for _, r := range second { + if want := adapter.MetricsTargetForResource(r.ID); want == nil || *want != secondTargets[r.ID] { + t.Fatal("target differs from the generation's point resolver") + } + } + other := NewMonitorAdapter(NewRegistry(nil)) + snapshot.VMs[0].ID = "other-tenant:501" + snapshot.VMs[0].Name = "other-tenant" + other.PopulateFromSnapshot(snapshot) + otherRows, otherTargets := other.GetAllWithMetricsTargets() + if reflect.DeepEqual(otherRows, second) || reflect.DeepEqual(otherTargets, secondTargets) { + t.Fatal("separate adapters shared a tenant projection") + } +} diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 774f2d8930..34992538c1 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -1321,6 +1321,21 @@ func seededVMwareSourceID(resource *Resource) string { // the read lock and copy it without repeating identity/scope/policy derivation. // The typed views remain lazy, so a List-only consumer pays for no extra copy. func (rr *ResourceRegistry) List() []Resource { + resources, _ := rr.listMaterialized(false) + return resources +} + +// ListWithMetricsTargets captures detached resources and their history targets +// from the same registry generation. Resolve all source mappings in one pass, +// without forcing the unrelated typed views to be built. In particular a +// freshly ingested registry must not scan every mapping once per broadcast row. +// Targets are separate: host coalescing must select the presentation identity +// before a missing target is attached, just as with individual target lookups. +func (rr *ResourceRegistry) ListWithMetricsTargets() ([]Resource, map[string]MetricsTarget) { + return rr.listMaterialized(true) +} + +func (rr *ResourceRegistry) listMaterialized(withTargets bool) ([]Resource, map[string]MetricsTarget) { for { rr.mu.RLock() if rr.canonicalMetadataDirty { @@ -1332,13 +1347,27 @@ func (rr *ResourceRegistry) List() []Resource { // rather than copy a generation that is no longer materialized. continue } + var sourceTargets map[string][]SourceTarget + var targets map[string]MetricsTarget + if withTargets { + sourceTargets = rr.cachedSourceTargets + if rr.viewsDirty || sourceTargets == nil { + sourceTargets = rr.buildSourceTargetsIndexLocked() + } + targets = make(map[string]MetricsTarget, len(rr.resources)) + } out := make([]Resource, 0, len(rr.resources)) for _, r := range rr.resources { out = append(out, cloneMaterializedResource(r)) + if withTargets { + if target := rr.metricsTargetFromSourceTargets(r, sourceTargets[r.ID]); target != nil { + targets[r.ID] = *target + } + } } rr.mu.RUnlock() sortResourcesByName(out) - return out + return out, targets } } diff --git a/internal/unifiedresources/registry_projection_test.go b/internal/unifiedresources/registry_projection_test.go new file mode 100644 index 0000000000..ca9dda8383 --- /dev/null +++ b/internal/unifiedresources/registry_projection_test.go @@ -0,0 +1,99 @@ +package unifiedresources + +import ( + "fmt" + "sync" + "testing" +) + +// Exercise the capture while a writer changes the mapping and row together. +// Separate List/point reads can legitimately observe different generations; +// this paired bulk read cannot. The test never touches persistence or drivers. +func TestRegistryProjectionSnapshotConcurrentMappingEdits(t *testing.T) { + rr := NewRegistry(nil) + rr.IngestSnapshot(benchmarkVMState(1)) + id := rr.List()[0].ID + set := func(version int) { + rr.mu.Lock() + defer rr.mu.Unlock() + name := fmt.Sprintf("coordinate-%d", version) + rr.resources[id].Name = name + rr.bySource[SourceProxmox] = map[string]string{name: id} + rr.invalidateViewsLocked() + } + set(0) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 200; i++ { + set(i % 2) + } + }() + defer wg.Wait() + for i := 0; i < 200; i++ { + rows, targets := rr.ListWithMetricsTargets() + if len(rows) != 1 || rows[0].Name != targets[id].ResourceID { + t.Fatalf("mixed row/target generation: row=%q target=%q", rows[0].Name, targets[id].ResourceID) + } + } +} + +var projectionResourceSink []Resource +var projectionTargetSink map[string]MetricsTarget + +// Include the entire owned List and every history coordinate. These are not +// point-lookup timings, nor a replacement for the old clone-allocation envelope. +func BenchmarkRegistryProjectionSnapshot(b *testing.B) { + for _, count := range []int{0, 1, 1000} { + for _, mode := range []string{"cold", "list-clean", "views-clean", "dirty"} { + for _, bulk := range []bool{false, true} { + name := "point-targets" + if bulk { + name = "bulk-targets" + } + b.Run(fmt.Sprintf("%d/%s/%s", count, mode, name), func(b *testing.B) { + state := benchmarkVMState(count) + rr := NewRegistry(nil) + rr.IngestSnapshot(state) + if mode != "cold" { + rr.List() + } + if mode == "views-clean" { + rr.VMs() + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if mode == "cold" { + b.StopTimer() + rr = NewRegistry(nil) + rr.IngestSnapshot(state) + b.StartTimer() + } + if mode == "dirty" { + rr.mu.Lock() + for _, r := range rr.resources { + r.Tags = []string{fmt.Sprintf("version-%d", i)} + break + } + rr.invalidateViewsLocked() + rr.mu.Unlock() + } + if bulk { + projectionResourceSink, projectionTargetSink = rr.ListWithMetricsTargets() + } else { + projectionResourceSink = rr.List() + projectionTargetSink = make(map[string]MetricsTarget, len(projectionResourceSink)) + for _, row := range projectionResourceSink { + if target := rr.MetricsTarget(row.ID); target != nil { + projectionTargetSink[row.ID] = *target + } + } + } + } + }) + } + } + } +} diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index e7490bef07..8eacc8f37b 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -6968,3 +6968,68 @@ func TestEvaluateResourceHealthKeepsStaleTelemetryBehindAWinningAlert(t *testing t.Fatalf("expected only the alert reason for a fresh source, got %+v", health.Reasons) } } + +// A List-only broadcast must capture history coordinates without constructing +// every typed view, and without trusting freshness timestamps as mutation keys. +func TestRegistryProjectionSnapshotDetachesAndTracksMappings(t *testing.T) { + rr := NewRegistry(nil) + rr.IngestSnapshot(benchmarkVMState(3)) + resources, targets := rr.ListWithMetricsTargets() + if !reflect.DeepEqual(resources, rr.List()) { + t.Fatal("bulk read changed resource content") + } + if !rr.viewsDirty || rr.cachedVMs != nil { + t.Fatal("projection forced unrelated typed views to materialize") + } + if len(targets) != len(resources) { + t.Fatalf("targets=%d resources=%d", len(targets), len(resources)) + } + for _, r := range resources { + if want := rr.MetricsTarget(r.ID); want == nil || *want != targets[r.ID] { + t.Fatalf("target for %s differs from point resolver", r.ID) + } + } + id := resources[0].ID + observedAt := resources[0].LastSeen + rr.mu.Lock() + for key, mapped := range rr.bySource[SourceProxmox] { + if mapped == id { + delete(rr.bySource[SourceProxmox], key) + } + } + rr.bySource[SourceProxmox]["new-history-coordinate"] = id + rr.resources[id].Tags = []string{"customer-data"} + rr.invalidateViewsLocked() + rr.mu.Unlock() + changed, updatedTargets := rr.ListWithMetricsTargets() + if updatedTargets[id].ResourceID != "new-history-coordinate" { + t.Fatal("same-time mapping change left an old target") + } + if targets[id].ResourceID == "new-history-coordinate" { + t.Fatal("later mapping edit mutated a prior target snapshot") + } + for _, r := range changed { + if r.ID == id { + if !r.LastSeen.Equal(observedAt) || r.Policy.Sensitivity != ResourceSensitivityRestricted { + t.Fatal("same-time metadata edit was stale") + } + r.Canonical.Aliases[0] = "caller-change" + r.Tags[0] = "caller-change" + r.Policy.Routing.Redact = append(r.Policy.Routing.Redact, ResourceRedactionPath) + } + } + again, againTargets := rr.ListWithMetricsTargets() + if !reflect.DeepEqual(againTargets, updatedTargets) { + t.Fatal("detached map mutation escaped") + } + for _, r := range again { + if r.ID == id && (r.Tags[0] != "customer-data" || r.Canonical.Aliases[0] == "caller-change") { + t.Fatal("caller changed live registry") + } + } + empty := NewRegistry(nil) + rows, emptyTargets := empty.ListWithMetricsTargets() + if len(rows) != 0 || len(emptyTargets) != 0 { + t.Fatal("empty projection invented resources or targets") + } +} diff --git a/scripts/installtests/container_cpu_guidance_test.go b/scripts/installtests/container_cpu_guidance_test.go new file mode 100644 index 0000000000..9cbbe8aa93 --- /dev/null +++ b/scripts/installtests/container_cpu_guidance_test.go @@ -0,0 +1,52 @@ +package installtests + +import ( + "math" + "os" + "regexp" + "strconv" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) + +// Check the guide's actual worked examples against the normaliser used by +// container ingestion, History and alert evaluation, not a second formula. +func TestContainerCPUGuidanceExamplesMatchNormalisation(t *testing.T) { + guide, err := os.ReadFile(repoFile("docs", "DOCKER.md")) + if err != nil { + t.Fatal(err) + } + rows := regexp.MustCompile(`(?m)^\| (\d+) \| ([\d.]+)% \| (?:About )?([\d.]+)% \|$`).FindAllStringSubmatch(string(guide), -1) + if len(rows) != 3 { + t.Fatalf("expected three worked host-capacity CPU examples, got %d", len(rows)) + } + for _, row := range rows { + cpus, err := strconv.Atoi(row[1]) + if err != nil { + t.Fatal(err) + } + raw, err := strconv.ParseFloat(row[2], 64) + if err != nil { + t.Fatal(err) + } + printed, err := strconv.ParseFloat(row[3], 64) + if err != nil { + t.Fatal(err) + } + actual := models.NormalizeDockerContainerCPUCapacityPercent(raw, cpus) + if math.Abs(actual-printed) > 0.005 { + t.Errorf("%s disagrees with the production normaliser: %g", row[0], actual) + } + } + for _, name := range []string{"DOCKER.md", "TROUBLESHOOTING.md"} { + doc, err := os.ReadFile(repoFile("docs", name)) + if err != nil { + t.Fatal(err) + } + shipped, err := os.ReadFile(repoFile("frontend-modern", "public", "docs", name)) + if err != nil || string(doc) != string(shipped) { + t.Fatalf("%s shipped help does not match the checked canonical guide", name) + } + } +} diff --git a/scripts/installtests/native_guidance_tools_test.go b/scripts/installtests/native_guidance_tools_test.go new file mode 100644 index 0000000000..c136e70d7f --- /dev/null +++ b/scripts/installtests/native_guidance_tools_test.go @@ -0,0 +1,164 @@ +//go:build !windows + +package installtests + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "gopkg.in/yaml.v3" +) + +type nativeGuidanceStep struct { + Name, If, Shell, Run string +} + +func nativeGuidanceToolSteps(content []byte) (nativeGuidanceStep, nativeGuidanceStep, error) { + var workflow struct { + Jobs map[string]struct{ Steps []nativeGuidanceStep } + } + if err := yaml.Unmarshal(content, &workflow); err != nil { + return nativeGuidanceStep{}, nativeGuidanceStep{}, err + } + steps := workflow.Jobs["native-agent"].Steps + bootstrapIndex, verifyIndex, installerIndex := -1, -1, -1 + for i, step := range steps { + switch step.Name { + case "Supply GNU timeout for macOS guidance checks": + bootstrapIndex = i + case "Verify native Unix timeout prerequisite": + verifyIndex = i + case "Test native Unix installer contracts": + installerIndex = i + } + } + if bootstrapIndex < 0 || verifyIndex <= bootstrapIndex || installerIndex <= verifyIndex { + return nativeGuidanceStep{}, nativeGuidanceStep{}, fmt.Errorf("native guidance needs GNU tool supply and verification before installer tests") + } + bootstrap, verify := steps[bootstrapIndex], steps[verifyIndex] + if bootstrap.If != "runner.os == 'macOS'" || verify.If != "matrix.unix" || steps[installerIndex].If != "matrix.unix" || bootstrap.Shell != "bash" || verify.Shell != "bash" { + return bootstrap, verify, fmt.Errorf("GNU tool preparation must cover macOS, verification and installer tests must cover every native Unix job") + } + return bootstrap, verify, nil +} + +func TestNativeGuidanceToolsSupplyAndVerifyRealGNUTimeout(t *testing.T) { + content, err := os.ReadFile(repoFile(".github", "workflows", "unified-agent-native.yml")) + if err != nil { + t.Fatal(err) + } + bootstrap, verify, err := nativeGuidanceToolSteps(content) + if err != nil { + t.Fatal(err) + } + realTimeout, err := exec.LookPath("timeout") + if err != nil { + t.Fatal("native Unix verification requires the real GNU timeout prerequisite") + } + for _, scenario := range []string{"real GNU tool", "install failure", "missing tool", "non-GNU tool"} { + t.Run(scenario, func(t *testing.T) { + root := t.TempDir() + bin, prefix := filepath.Join(root, "bin"), filepath.Join(root, "coreutils with spaces") + coreutilsBin, runnerTemp := filepath.Join(prefix, "bin"), filepath.Join(root, "runner temp") + for _, dir := range []string{bin, coreutilsBin, runnerTemp} { + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + } + // Only Homebrew's installer is simulated. The positive timeout is + // the real executable; the copied TERM/KILL recipes remain unchanged. + const brew = `#!/bin/sh +printf '%s\n' "$*" >> "$BREW_CALLS" +case "$*" in + 'install coreutils') exit "${BREW_EXIT:-0}" ;; + '--prefix coreutils') printf '%s\n' "$COREUTILS_PREFIX" ;; + *) exit 64 ;; +esac +` + if err := os.WriteFile(filepath.Join(bin, "brew"), []byte(brew), 0700); err != nil { + t.Fatal(err) + } + tool := filepath.Join(coreutilsBin, "gtimeout") + switch scenario { + case "real GNU tool", "install failure": + if err := os.Symlink(realTimeout, tool); err != nil { + t.Fatal(err) + } + case "non-GNU tool": + if err := os.WriteFile(tool, []byte("#!/bin/sh\nprintf 'BSD timeout\\n'\n"), 0700); err != nil { + t.Fatal(err) + } + } + pathFile, callsFile := filepath.Join(root, "github-path"), filepath.Join(root, "brew-calls") + installExit := "0" + if scenario == "install failure" { + installExit = "23" + } + env := append(os.Environ(), "PATH="+bin+":"+os.Getenv("PATH"), "GITHUB_PATH="+pathFile, + "COREUTILS_PREFIX="+prefix, "BREW_CALLS="+callsFile, "BREW_EXIT="+installExit, "RUNNER_TEMP="+runnerTemp) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "bash", "-c", bootstrap.Run) + cmd.Env = env + output, runErr := cmd.CombinedOutput() + if ctx.Err() != nil { + t.Fatalf("prerequisite observation did not finish: %v", ctx.Err()) + } + if scenario != "real GNU tool" { + wantExit := 1 + if scenario == "install failure" { + wantExit = 23 + } + exit, ok := runErr.(*exec.ExitError) + if !ok || exit.ExitCode() != wantExit { + t.Fatalf("prerequisite exit %v, want %d: %s", runErr, wantExit, output) + } + if _, err := os.Stat(pathFile); !os.IsNotExist(err) { + t.Fatal("failed prerequisite exported an executable search path") + } + return + } + if runErr != nil { + t.Fatalf("GNU prerequisite failed: %v: %s", runErr, output) + } + path, err := os.ReadFile(pathFile) + if err != nil { + t.Fatal(err) + } + timeoutBin := strings.TrimSuffix(string(path), "\n") + if filepath.Dir(timeoutBin) != runnerTemp || !strings.HasPrefix(filepath.Base(timeoutBin), "pulse-guidance-tools.") { + t.Fatal("bootstrap did not export its newly owned timeout directory") + } + entries, err := os.ReadDir(timeoutBin) + if err != nil || len(entries) != 1 || entries[0].Name() != "timeout" { + t.Fatal("bootstrap must not replace native macOS utilities other than timeout") + } + if target, err := os.Readlink(filepath.Join(timeoutBin, "timeout")); err != nil || target != tool { + t.Fatal("timeout must resolve to the exact installed GNU executable") + } + calls, err := os.ReadFile(callsFile) + if err != nil || string(calls) != "install coreutils\n--prefix coreutils\n" { + t.Fatal("bootstrap changed its fixed Homebrew dependency") + } + cmd = exec.CommandContext(ctx, "bash", "-c", verify.Run) + cmd.Env = append(env, "PATH="+timeoutBin+":"+os.Getenv("PATH")) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("next workflow step cannot use supplied GNU timeout: %v: %s", err, output) + } + cmd = exec.CommandContext(ctx, "bash", "-c", verify.Run) + cmd.Env = append(env, "PATH="+bin+":"+os.Getenv("PATH")) + if err := os.WriteFile(filepath.Join(bin, "timeout"), []byte("#!/bin/sh\nprintf 'BSD timeout\\n'\n"), 0700); err != nil { + t.Fatal(err) + } + if output, err := cmd.CombinedOutput(); err == nil { + t.Fatalf("Unix prerequisite probe accepted a non-GNU timeout: %s", output) + } + }) + } +}