fix(zfs): skip zpool list when /dev/zfs unavailable in Linux (#2325)

Co-authored-by: henrygd <hank@henrygd.me>

agent/storage_pool.go

@@ -80,7 +80,7 @@ func newZfsBackend() *poolBackend {
 	return &poolBackend{
 		name:           "zfs",
 		poolStatsFn:    optionalPoolSource(zfs.PoolStats),
-		datasetsFn:     zfs.Datasets,
+		datasetsFn:     optionalPoolSource(zfs.Datasets),
 		kernelStatsFn:  optionalPoolSource(zfs.PoolKernelStats),
 		poolStatusesFn: optionalPoolSource(zfs.PoolStatuses),
 	}

agent/storage_pool_test.go

@@ -323,6 +323,21 @@ func TestDatasetUsageRefreshOnErrorKeepsPrevious(t *testing.T) {
 	assert.Len(t, usage, 1, "previous usage should be retained on error")
 }
 
+func TestDatasetUsageClearsAbsentBackend(t *testing.T) {
+	b := newZfsBackend()
+	b.datasetUsage = map[string]zfsDatasetUsage{"/tank": {used: 1, avail: 1}}
+	b.datasetsFn = optionalPoolSource(func() ([]zfs.Dataset, error) {
+		return nil, zfs.ErrNoZfs
+	})
+
+	datasets, err := b.datasets()
+	require.NoError(t, err, "an absent backend must not produce an error to log")
+	assert.Empty(t, datasets)
+	b.refreshDatasetUsage()
+	assert.Empty(t, b.datasetUsage)
+	assert.False(t, b.lastUsageRefresh.IsZero())
+}
+
 func TestGetDetailForceRefresh(t *testing.T) {
 	zm := &StoragePoolManager{detailInterval: time.Hour, backends: []*poolBackend{{name: "zfs"}}}
 	poolCalls := 0

agent/zfs/zfs.go

@@ -70,6 +70,9 @@ type Dataset struct {
 // PoolStats returns capacity and health for all pools on the system using
 // `zpool list`. Frequent health and I/O sampling uses PoolKernelStats instead.
 func PoolStats() ([]PoolStat, error) {
+	if err := checkZfsDevice(); err != nil {
+		return nil, err
+	}
 	out, err := commandOutput("zpool", "list", "-Hp", "-o", "name,size,alloc,free,health")
 	if err != nil {
 		var exitErr *exec.ExitError
@@ -84,6 +87,9 @@ func PoolStats() ([]PoolStat, error) {
 // Datasets returns all datasets on the system with usage and mountpoint
 // information using `zfs list` (recursive by default).
 func Datasets() ([]Dataset, error) {
+	if err := checkZfsDevice(); err != nil {
+		return nil, err
+	}
 	out, err := commandOutput("zfs", "list", "-Hp", "-o", "name,used,avail,mountpoint")
 	if err != nil {
 		return nil, fmt.Errorf("zfs list: %w", err)

agent/zfs/zfs_linux.go

@@ -13,7 +13,10 @@ import (
 	"strings"
 )
 
-var procZfsPath = "/proc/spl/kstat/zfs"
+var (
+	procZfsPath = "/proc/spl/kstat/zfs"
+	devZfsPath  = "/dev/zfs"
+)
 
 func ARCSize() (uint64, error) {
 	file, err := os.Open(filepath.Join(procZfsPath, "arcstats"))
@@ -40,6 +43,19 @@ func ARCSize() (uint64, error) {
 	return 0, fmt.Errorf("size field not found in arcstats")
 }
 
+// checkZfsDevice lets containers without /dev/zfs fail fast instead of
+// waiting for ZFS utility commands to time out.
+func checkZfsDevice() error {
+	_, err := os.Stat(devZfsPath)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return ErrNoZfs
+		}
+		return err
+	}
+	return nil
+}
+
 // PoolKernelStats reads pool state and cumulative I/O counters directly from
 // procfs. These kstats are the same interfaces used by node_exporter's Linux
 // ZFS collector and avoid keeping a `zpool iostat` subprocess alive.

agent/zfs/zfs_linux_test.go

@@ -88,3 +88,66 @@ func TestReadObjsetIORequiresAllCounters(t *testing.T) {
 	_, _, err := readObjsetIO(path)
 	require.Error(t, err)
 }
+
+func TestCollectorsSkipCommandsWhenDevZfsMissing(t *testing.T) {
+	root := t.TempDir()
+	oldDevZfsPath := devZfsPath
+	devZfsPath = filepath.Join(root, "missing")
+	t.Cleanup(func() { devZfsPath = oldDevZfsPath })
+
+	oldCommandOutput := commandOutput
+	commandOutput = func(name string, args ...string) ([]byte, error) {
+		t.Fatalf("unexpected %s call with %v", name, args)
+		return nil, nil
+	}
+	t.Cleanup(func() { commandOutput = oldCommandOutput })
+
+	_, err := PoolStats()
+	assert.ErrorIs(t, err, ErrNoZfs)
+	_, err = Datasets()
+	assert.ErrorIs(t, err, ErrNoZfs)
+}
+
+func TestDatasetsDelegatesWhenDevZfsPresent(t *testing.T) {
+	oldDevZfsPath := devZfsPath
+	devZfsPath = filepath.Join(t.TempDir(), "zfs")
+	require.NoError(t, os.WriteFile(devZfsPath, nil, 0o644))
+	t.Cleanup(func() { devZfsPath = oldDevZfsPath })
+
+	oldCommandOutput := commandOutput
+	commandOutput = func(name string, args ...string) ([]byte, error) {
+		assert.Equal(t, "zfs", name)
+		assert.Equal(t, []string{"list", "-Hp", "-o", "name,used,avail,mountpoint"}, args)
+		return []byte("tank\t50\t50\t/tank\n"), nil
+	}
+	t.Cleanup(func() { commandOutput = oldCommandOutput })
+
+	datasets, err := Datasets()
+	require.NoError(t, err)
+	assert.Equal(t, []Dataset{{Name: "tank", Used: 50, Avail: 50, Mountpoint: "/tank"}}, datasets)
+}
+
+func TestPoolStatsDelegatesToZpoolWhenDevZfsPresent(t *testing.T) {
+	root := t.TempDir()
+	devFile := filepath.Join(root, "zfs")
+	require.NoError(t, os.WriteFile(devFile, []byte(""), 0o644))
+
+	oldDevZfsPath := devZfsPath
+	devZfsPath = devFile
+	t.Cleanup(func() { devZfsPath = oldDevZfsPath })
+
+	oldCommandOutput := commandOutput
+	called := false
+	commandOutput = func(name string, args ...string) ([]byte, error) {
+		called = true
+		assert.Equal(t, "zpool", name)
+		assert.Equal(t, []string{"list", "-Hp", "-o", "name,size,alloc,free,health"}, args)
+		return []byte("tank\t100\t50\t50\tONLINE\n"), nil
+	}
+	t.Cleanup(func() { commandOutput = oldCommandOutput })
+
+	pools, err := PoolStats()
+	require.NoError(t, err)
+	assert.True(t, called)
+	assert.Equal(t, []PoolStat{{Name: "tank", Size: 100, Alloc: 50, Free: 50, Health: "ONLINE"}}, pools)
+}

agent/zfs/zfs_nonlinux.go

@@ -0,0 +1,9 @@
+//go:build !linux
+
+package zfs
+
+// The /dev/zfs probe is Linux-specific. Other platforms detect availability
+// through the ZFS utilities themselves.
+func checkZfsDevice() error {
+	return nil
+}

agent/zfs/zfs_nonlinux_test.go

@@ -0,0 +1,33 @@
+//go:build testing && !linux
+
+package zfs
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCollectorsUseUtilitiesOnNonLinux(t *testing.T) {
+	oldCommandOutput := commandOutput
+	commandOutput = func(name string, args ...string) ([]byte, error) {
+		switch name {
+		case "zpool":
+			return []byte("tank\t100\t50\t50\tONLINE\n"), nil
+		case "zfs":
+			return []byte("tank\t50\t50\t/tank\n"), nil
+		default:
+			t.Fatalf("unexpected command %s", name)
+			return nil, nil
+		}
+	}
+	t.Cleanup(func() { commandOutput = oldCommandOutput })
+
+	pools, err := PoolStats()
+	require.NoError(t, err)
+	assert.Equal(t, []PoolStat{{Name: "tank", Size: 100, Alloc: 50, Free: 50, Health: "ONLINE"}}, pools)
+	datasets, err := Datasets()
+	require.NoError(t, err)
+	assert.Equal(t, []Dataset{{Name: "tank", Used: 50, Avail: 50, Mountpoint: "/tank"}}, datasets)
+}