From 982101743e92acffa4f6bc1b42285aa7b020342e Mon Sep 17 00:00:00 2001 From: Santhi Prakash <38608178+santhiprakash@users.noreply.github.com> Date: Wed, 16 Sep 2026 22:58:47 +0530 Subject: [PATCH] fix(zfs): skip zpool list when /dev/zfs unavailable in Linux (#2325) Co-authored-by: henrygd --- agent/storage_pool.go | 2 +- agent/storage_pool_test.go | 15 ++++++++ agent/zfs/zfs.go | 6 ++++ agent/zfs/zfs_linux.go | 18 +++++++++- agent/zfs/zfs_linux_test.go | 63 ++++++++++++++++++++++++++++++++++ agent/zfs/zfs_nonlinux.go | 9 +++++ agent/zfs/zfs_nonlinux_test.go | 33 ++++++++++++++++++ 7 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 agent/zfs/zfs_nonlinux.go create mode 100644 agent/zfs/zfs_nonlinux_test.go diff --git a/agent/storage_pool.go b/agent/storage_pool.go index ab23dda3..6adcf4e6 100644 --- a/agent/storage_pool.go +++ b/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), } diff --git a/agent/storage_pool_test.go b/agent/storage_pool_test.go index 9f87fed6..d9d7bf0e 100644 --- a/agent/storage_pool_test.go +++ b/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 diff --git a/agent/zfs/zfs.go b/agent/zfs/zfs.go index 55c9f706..0cd224d6 100644 --- a/agent/zfs/zfs.go +++ b/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) diff --git a/agent/zfs/zfs_linux.go b/agent/zfs/zfs_linux.go index 6be9b90e..b5a25779 100644 --- a/agent/zfs/zfs_linux.go +++ b/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. diff --git a/agent/zfs/zfs_linux_test.go b/agent/zfs/zfs_linux_test.go index 8c8e8623..7ecb59a0 100644 --- a/agent/zfs/zfs_linux_test.go +++ b/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) +} diff --git a/agent/zfs/zfs_nonlinux.go b/agent/zfs/zfs_nonlinux.go new file mode 100644 index 00000000..3ac5fc54 --- /dev/null +++ b/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 +} diff --git a/agent/zfs/zfs_nonlinux_test.go b/agent/zfs/zfs_nonlinux_test.go new file mode 100644 index 00000000..b7e71c42 --- /dev/null +++ b/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) +}