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.
This commit is contained in:
rcourtman 2025-12-18 16:23:56 +00:00
parent 0182cc8310
commit fdb2a07f56
2 changed files with 87 additions and 1 deletions

View file

@ -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
}

View file

@ -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