From 993a267e0c4d71ccaf9db153e7d0ae4ae478ffb1 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 20 Aug 2026 15:37:10 +0200 Subject: [PATCH] fix(truenas): keep reporting pools without usable dataset space (#6635) --- apps/docs/docs/integrations/truenas/index.mdx | 2 +- .../truenas/test/truenas-integration.spec.ts | 121 ++++++++++++++++- .../src/truenas/truenas-integration.ts | 122 +++++++++++++----- 3 files changed, 209 insertions(+), 36 deletions(-) diff --git a/apps/docs/docs/integrations/truenas/index.mdx b/apps/docs/docs/integrations/truenas/index.mdx index 5e57f3a89..00f8f0093 100644 --- a/apps/docs/docs/integrations/truenas/index.mdx +++ b/apps/docs/docs/integrations/truenas/index.mdx @@ -27,7 +27,7 @@ import Alert from '@theme/Admonition'; ]} /> -The storage values shown by these widgets use the root ZFS dataset's usable `used` and `available` space, including RAIDZ parity overhead and dataset limits. +The storage values shown by these widgets come from each pool's root ZFS dataset, so they already account for RAIDZ parity overhead and dataset limits and match what `zfs list` and the TrueNAS web interface report. When a pool cannot report its dataset space, Homarr falls back to the pool's physical values, which read higher on RAIDZ. ### Adding the integration diff --git a/packages/integrations/src/truenas/test/truenas-integration.spec.ts b/packages/integrations/src/truenas/test/truenas-integration.spec.ts index d0856630e..237b41e3f 100644 --- a/packages/integrations/src/truenas/test/truenas-integration.spec.ts +++ b/packages/integrations/src/truenas/test/truenas-integration.spec.ts @@ -59,10 +59,12 @@ const ws = vi.hoisted(() => { } const result = responder(payload.method as string, (payload.params as unknown[] | undefined) ?? []); + // A responder returning an Error mimics a method the server rejects, e.g. an unsupported filter. + const body = result instanceof Error ? { error: { message: result.message } } : { result }; const envelope = payload.jsonrpc === "2.0" - ? { jsonrpc: "2.0", id: payload.id, result } - : { msg: "result", id: payload.id, result }; + ? { jsonrpc: "2.0", id: payload.id, ...body } + : { msg: "result", id: payload.id, ...body }; this.respond(JSON.stringify(envelope)); } @@ -91,7 +93,10 @@ const ws = vi.hoisted(() => { }; }); +const logs = vi.hoisted(() => ({ debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() })); + vi.mock("ws", () => ({ WebSocket: ws.FakeWebSocket })); +vi.mock("@homarr/core/infrastructure/logs", () => ({ createLogger: () => logs })); vi.mock("@homarr/core/infrastructure/certificates", () => ({ getAllTrustedCertificatesAsync: vi.fn().mockResolvedValue([]), getTrustedCertificateHostnamesAsync: vi.fn().mockResolvedValue([]), @@ -167,6 +172,11 @@ const happyResponder = (method: string) => { const emptyAggregations = () => ({ min: {}, mean: {}, max: {} }); +// `pool.query` reports 500 allocated of a 1000 byte pool, leaving 500 free. +const physicalFallbackFileSystem = [{ deviceName: "tank", available: "500", used: "500", percentage: 50 }]; + +const ERROR_MESSAGE_LIMIT = 200; + let integrationCounter = 0; const createIntegration = (decryptedSecrets: IntegrationSecret[]) => new TrueNasIntegration({ @@ -188,6 +198,7 @@ describe("TrueNasIntegration", () => { ws.constructedUrls.length = 0; ws.failures.length = 0; ws.setResponder(happyResponder); + logs.warn.mockClear(); }); test("fetches and maps system info over the JSON-RPC API", async () => { @@ -230,6 +241,112 @@ describe("TrueNasIntegration", () => { }); }); + test("falls back to physical pool space when the root dataset is not returned", async () => { + ws.setResponder((method) => (method === "pool.dataset.query" ? [] : happyResponder(method))); + const integration = createIntegration(credentials); + + const result = await integration.getSystemInfoAsync(); + + expect(result.fileSystem).toMatchObject(physicalFallbackFileSystem); + // Dropping the pool would also hide its health status. + expect(result.smart).toMatchObject([{ deviceName: "tank", healthy: true, overallStatus: "ONLINE" }]); + }); + + test("falls back to physical pool space when the root dataset reports no usable values", async () => { + ws.setResponder((method) => + method === "pool.dataset.query" + ? [{ id: "tank", used: { parsed: null }, available: null }] + : happyResponder(method), + ); + const integration = createIntegration(credentials); + + const result = await integration.getSystemInfoAsync(); + + expect(result.fileSystem).toMatchObject(physicalFallbackFileSystem); + }); + + test("falls back to physical pool space when the root dataset omits the requested properties", async () => { + // Datasets that cannot report space, e.g. while locked, come back without `used` and `available`. + ws.setResponder((method) => + method === "pool.dataset.query" ? [{ id: "tank", locked: true }] : happyResponder(method), + ); + const integration = createIntegration(credentials); + + const result = await integration.getSystemInfoAsync(); + + expect(result.fileSystem).toMatchObject(physicalFallbackFileSystem); + }); + + test("falls back to physical pool space when the dataset response fails schema validation", async () => { + ws.setResponder((method) => + method === "pool.dataset.query" ? { unexpected: "not an array" } : happyResponder(method), + ); + const integration = createIntegration(credentials); + + const result = await integration.getSystemInfoAsync(); + + expect(result.fileSystem).toMatchObject(physicalFallbackFileSystem); + // A malformed payload must not take the rest of the system info down with it. + expect(result).toMatchObject({ cpuTemp: 45, memAvailableInBytes: 6_000_000_000, uptime: 3600 }); + }); + + test("falls back to physical pool space when the dataset query is rejected", async () => { + ws.setResponder((method) => + method === "pool.dataset.query" + ? new Error("[EINVAL] filters: Value error, Invalid operation: in") + : happyResponder(method), + ); + const integration = createIntegration(credentials); + + const result = await integration.getSystemInfoAsync(); + + expect(result.fileSystem).toMatchObject(physicalFallbackFileSystem); + // A rejected dataset query must not take the rest of the system info down with it. + expect(result).toMatchObject({ cpuTemp: 45, memAvailableInBytes: 6_000_000_000, uptime: 3600 }); + }); + + test("logs the truncated rejection reason rather than the generic wrapper message", async () => { + // TrueNAS answers a rejected method with its Python traceback, which runs to several kilobytes. + ws.setResponder((method) => + method === "pool.dataset.query" + ? new Error(`[EINVAL] query-filters ${"x".repeat(5000)}`) + : happyResponder(method), + ); + const integration = createIntegration(credentials); + + await integration.getSystemInfoAsync(); + + const logged = logs.warn.mock.calls.find(([message]) => String(message).includes("Could not retrieve")); + const reason = String((logged?.[1] as { error?: string } | undefined)?.error); + // The decorator would otherwise surface only "An unknown error occured while executing Integration method". + expect(reason).toContain("[EINVAL] query-filters"); + expect(reason).toHaveLength(ERROR_MESSAGE_LIMIT + 1); // truncated, plus the ellipsis + }); + + test("keeps reporting a pool that is unhealthy but has no usable dataset space", async () => { + ws.setResponder((method) => { + if (method === "pool.query") + return [ + { name: "tank", status: "ONLINE", healthy: true, free: 500, size: 1000, allocated: 500 }, + { name: "backup", status: "DEGRADED", healthy: false, free: 10, size: 100, allocated: 90 }, + ]; + if (method === "pool.dataset.query") return [{ id: "tank", used: { parsed: 200 }, available: { parsed: 300 } }]; + return happyResponder(method); + }); + const integration = createIntegration(credentials); + + const result = await integration.getSystemInfoAsync(); + + expect(result.smart).toMatchObject([ + { deviceName: "tank", healthy: true, overallStatus: "ONLINE" }, + { deviceName: "backup", healthy: false, overallStatus: "DEGRADED" }, + ]); + expect(result.fileSystem).toMatchObject([ + { deviceName: "tank", available: "300", used: "200", percentage: 40 }, + { deviceName: "backup", available: "10", used: "90", percentage: 90 }, + ]); + }); + test("reports zero usage for an empty root dataset", async () => { ws.setResponder((method) => method === "pool.dataset.query" diff --git a/packages/integrations/src/truenas/truenas-integration.ts b/packages/integrations/src/truenas/truenas-integration.ts index c5ba02c76..ffb2225d9 100644 --- a/packages/integrations/src/truenas/truenas-integration.ts +++ b/packages/integrations/src/truenas/truenas-integration.ts @@ -15,6 +15,20 @@ const logger = createLogger({ module: "trueNasIntegration" }); const NETWORK_MULTIPLIER = 100; +const ERROR_MESSAGE_LIMIT = 200; + +/** + * The error decorator replaces a rejected request with a generic message and moves the actionable + * reason to `cause`. TrueNAS puts its middleware traceback in there, several kilobytes of it, so the + * reason is truncated before it reaches a log line that repeats on every poll. + */ +const describeRequestError = (error: unknown) => { + const cause = error instanceof Error ? error.cause : undefined; + const reason = cause instanceof Error ? cause.message : error instanceof Error ? error.message : String(error); + + return reason.length > ERROR_MESSAGE_LIMIT ? `${reason.slice(0, ERROR_MESSAGE_LIMIT)}…` : reason; +}; + export class TrueNasIntegration extends Integration implements ISystemHealthMonitoringIntegration { private client?: TrueNasClient; @@ -108,9 +122,10 @@ export class TrueNasIntegration extends Integration implements ISystemHealthMoni .map((pool) => { if (pool.allocated !== null && pool.size !== null && pool.free !== null) { return { - ...pool, + name: pool.name, + status: pool.status, + healthy: pool.healthy, allocated: pool.allocated, - size: pool.size, free: pool.free, }; } @@ -119,39 +134,75 @@ export class TrueNasIntegration extends Integration implements ISystemHealthMoni }) .filter((pool) => pool !== null); - if (activePools.length === 0) return []; - - const datasets = await this.requestAsync("pool.dataset.query", [ - [["id", "in", activePools.map((pool) => pool.name)]], - { - extra: { - properties: ["used", "available"], - }, - }, - ]); - const parsedDatasets = await poolDatasetSchema.parseAsync(datasets); - - const poolsWithUsableSpace = activePools - .map((pool) => { - const dataset = parsedDatasets.find((candidate) => candidate.id === pool.name); - if (!dataset || dataset.used.parsed === null || dataset.available.parsed === null) return null; - - return { - name: pool.name, - status: pool.status, - healthy: pool.healthy, - used: dataset.used.parsed, - available: dataset.available.parsed, - }; - }) - .filter((pool) => pool !== null); - logger.debug("Retrieved pools", { url: this.integration.url, totalCount: result.length, - activeCount: poolsWithUsableSpace.length, + activeCount: activePools.length, }); - return poolsWithUsableSpace; + + if (activePools.length === 0) return []; + + const usableSpaceByPool = await this.getUsableSpaceByPoolAsync(activePools.map((pool) => pool.name)); + + return activePools.map((pool) => { + const usableSpace = usableSpaceByPool.get(pool.name); + if (!usableSpace) { + logger.warn("Falling back to physical pool space, the root dataset reported no usable space", { + url: this.integration.url, + pool: pool.name, + }); + } + + return { + name: pool.name, + status: pool.status, + healthy: pool.healthy, + used: usableSpace?.used ?? pool.allocated, + available: usableSpace?.available ?? pool.free, + }; + }); + } + + /** + * Returns the usable space of each pool's root ZFS dataset, keyed by pool name. Unlike the physical + * `pool.query` values, these account for RAIDZ parity overhead and dataset limits, matching `zfs list`. + * See https://github.com/homarr-labs/homarr/issues/6566 + * + * A pool is left out when its usable space is unavailable, so the caller keeps reporting the pool with + * its physical values instead of dropping it, which would also hide its health status: + * - request rejected → method or `extra.properties` unsupported, or the API key lacks the dataset role + * - response rejected → the payload does not match the expected shape + * - dataset missing → the pool's root dataset was not returned by the query + * - property absent → the dataset does not report `used` / `available`, e.g. while locked + */ + private async getUsableSpaceByPoolAsync(poolNames: string[]) { + const usableSpaceByPool = new Map(); + + try { + const datasets = await this.requestAsync("pool.dataset.query", [ + [["id", "in", poolNames]], + { + extra: { + properties: ["used", "available"], + }, + }, + ]); + + for (const dataset of await poolDatasetSchema.parseAsync(datasets)) { + const used = dataset.used?.parsed; + const available = dataset.available?.parsed; + if (typeof used !== "number" || typeof available !== "number") continue; + + usableSpaceByPool.set(dataset.id, { used, available }); + } + } catch (error) { + logger.warn("Could not retrieve root dataset space, continuing with physical pool space", { + url: this.integration.url, + error: describeRequestError(error), + }); + } + + return usableSpaceByPool; } /** @@ -292,11 +343,16 @@ const poolSchema = z.array( }), ); +// Properties are absent or unparsed for datasets that do not report them, e.g. while locked, +// so they are optional here and the pool falls back to its physical values instead of failing +// the whole request, which would also drop cpu, memory and network data +const poolDatasetPropertySchema = z.object({ parsed: z.number().min(0).nullish() }).nullish(); + const poolDatasetSchema = z.array( z.object({ id: z.string(), - used: z.object({ parsed: z.number().min(0).nullable() }), - available: z.object({ parsed: z.number().min(0).nullable() }), + used: poolDatasetPropertySchema, + available: poolDatasetPropertySchema, }), );