From fdb2a07f5613ff14a39e10626e4ee3ef2ecd56cd Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Thu, 18 Dec 2025 16:23:56 +0000
Subject: [PATCH] fix(agent): find zpool binary on TrueNAS SCALE (#718)

Enhanced zpool binary lookup to try common paths when exec.LookPath fails.
This fixes issue #718 where TrueNAS SCALE reports inflated storage because
the agent runs with a restricted PATH that doesn't include /usr/sbin.

Changes:
- Added findZpool() helper that tries common paths like /usr/sbin/zpool,
  /sbin/zpool, /usr/local/sbin/zpool for TrueNAS/FreeBSD/Linux systems
- Added commonZpoolPaths variable listing typical zpool locations
- Added tests for the new findZpool function

This ensures zpool list is used for accurate pool-level capacity instead
of falling back to dataset-level summation.
---
 internal/hostmetrics/zfs.go      | 36 +++++++++++++++++++++-
 internal/hostmetrics/zfs_test.go | 52 ++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/internal/hostmetrics/zfs.go b/internal/hostmetrics/zfs.go
index 1e66fc2bb..501d604a4 100644
--- a/internal/hostmetrics/zfs.go
+++ b/internal/hostmetrics/zfs.go
@@ -5,6 +5,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"os"
 	"os/exec"
 	"sort"
 	"strconv"
@@ -139,12 +140,45 @@ func fallbackZFSDisks(bestDatasets map[string]zfsDatasetUsage, mountpoints map[s
 	return disks
 }
 
+// commonZpoolPaths lists common locations for the zpool binary.
+// TrueNAS SCALE, FreeBSD, and various Linux distributions may install
+// zpool in different locations that might not be in the agent's PATH.
+// This helps fix issue #718 where TrueNAS reports inflated storage.
+var commonZpoolPaths = []string{
+	"/usr/sbin/zpool",         // TrueNAS SCALE, Debian, Ubuntu
+	"/sbin/zpool",             // FreeBSD, older Linux
+	"/usr/local/sbin/zpool",   // FreeBSD ports, custom builds
+	"/usr/local/bin/zpool",    // Custom installations
+	"/opt/zfs/bin/zpool",      // Some enterprise Linux
+	"/usr/bin/zpool",          // Some distributions
+}
+
+// findZpool returns the path to the zpool binary by first trying exec.LookPath,
+// then falling back to common hardcoded paths for TrueNAS/FreeBSD/Linux systems.
+func findZpool() (string, error) {
+	// First, try the standard PATH lookup
+	if path, err := exec.LookPath("zpool"); err == nil {
+		return path, nil
+	}
+
+	// If that fails, try common absolute paths
+	// This is especially important for TrueNAS SCALE where the agent
+	// might run with a restricted PATH that doesn't include /usr/sbin
+	for _, path := range commonZpoolPaths {
+		if _, err := os.Stat(path); err == nil {
+			return path, nil
+		}
+	}
+
+	return "", fmt.Errorf("zpool binary not found in PATH or common locations")
+}
+
 func fetchZpoolStats(ctx context.Context, pools []string) (map[string]zpoolStats, error) {
 	if len(pools) == 0 {
 		return nil, nil
 	}
 
-	path, err := exec.LookPath("zpool")
+	path, err := findZpool()
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/hostmetrics/zfs_test.go b/internal/hostmetrics/zfs_test.go
index 1685a2fa7..740d6f9f8 100644
--- a/internal/hostmetrics/zfs_test.go
+++ b/internal/hostmetrics/zfs_test.go
@@ -2,6 +2,7 @@ package hostmetrics
 
 import (
 	"context"
+	"os"
 	"testing"
 )
 
@@ -71,6 +72,57 @@ func TestSummarizeZFSPoolsFallback(t *testing.T) {
 	}
 }
 
+func TestFindZpool(t *testing.T) {
+	// This test verifies that findZpool correctly:
+	// 1. Uses exec.LookPath first (if zpool is in PATH)
+	// 2. Falls back to common absolute paths
+
+	// We can't easily mock os.Stat, so we just verify the function
+	// returns a path (if zpool is installed) or an error (if not)
+	path, err := findZpool()
+
+	// On a system without zpool, we expect an error
+	if err != nil {
+		if path != "" {
+			t.Errorf("findZpool() returned path %q with error: %v", path, err)
+		}
+		// Expected behavior on systems without zpool
+		return
+	}
+
+	// On a system with zpool, verify the path looks valid
+	if path == "" {
+		t.Error("findZpool() returned empty path without error")
+	}
+
+	// Verify the returned path exists
+	if _, statErr := os.Stat(path); statErr != nil {
+		t.Errorf("findZpool() returned path %q but os.Stat failed: %v", path, statErr)
+	}
+}
+
+func TestCommonZpoolPaths(t *testing.T) {
+	// Verify that commonZpoolPaths contains expected paths for TrueNAS
+	expectedPaths := []string{
+		"/usr/sbin/zpool",       // TrueNAS SCALE, Debian, Ubuntu
+		"/sbin/zpool",           // FreeBSD, older Linux
+		"/usr/local/sbin/zpool", // FreeBSD ports
+	}
+
+	for _, expected := range expectedPaths {
+		found := false
+		for _, path := range commonZpoolPaths {
+			if path == expected {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("commonZpoolPaths missing expected path: %s", expected)
+		}
+	}
+}
+
 func TestZFSPoolFromDevice(t *testing.T) {
 	tests := []struct {
 		device string