From dba3519b2c0b0e190a732fa302d117684d207f74 Mon Sep 17 00:00:00 2001 From: henrygd Date: Tue, 10 Feb 2026 18:12:04 -0500 Subject: [PATCH] fix(agent): avoid mismatched root disk I/O mapping in docker (#1737) - Stop using max-read fallback when mapping root filesystem to diskstats. - Keep root usage reporting even when root I/O cannot be mapped. - Expand docker fallback mount detection to include /etc/resolv.conf and /etc/hostname (in addition to /etc/hosts). - Add clearer warnings when root block device detection is uncertain. --- agent/disk.go | 50 +++++++++++++++++++++++------------------ agent/disk_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++ agent/smart.go | 4 +--- 3 files changed, 85 insertions(+), 25 deletions(-) diff --git a/agent/disk.go b/agent/disk.go index 4cf33cee..8e36fad7 100644 --- a/agent/disk.go +++ b/agent/disk.go @@ -26,6 +26,15 @@ func parseFilesystemEntry(entry string) (device, customName string) { return device, customName } +func isDockerSpecialMountpoint(mountpoint string) bool { + switch mountpoint { + case "/etc/hosts", "/etc/resolv.conf", "/etc/hostname": + return true + default: + return false + } +} + // Sets up the filesystems to monitor for disk usage and I/O. func (a *Agent) initializeDiskInfo() { filesystem, _ := GetEnv("FILESYSTEM") @@ -69,11 +78,15 @@ func (a *Agent) initializeDiskInfo() { if _, exists := a.fsStats[key]; !exists { if root { slog.Info("Detected root device", "name", key) - // Check if root device is in /proc/diskstats, use fallback if not + // Check if root device is in /proc/diskstats. Do not guess a + // fallback device for root: that can misattribute root I/O to a + // different disk while usage remains tied to root mountpoint. if _, ioMatch = diskIoCounters[key]; !ioMatch { - key, ioMatch = findIoDevice(filesystem, diskIoCounters, a.fsStats) - if !ioMatch { - slog.Info("Using I/O fallback", "device", device, "mountpoint", mountpoint, "fallback", key) + if matchedKey, match := findIoDevice(filesystem, diskIoCounters); match { + key = matchedKey + ioMatch = true + } else { + slog.Warn("Root I/O unmapped; set FILESYSTEM", "device", device, "mountpoint", mountpoint) } } } else { @@ -141,8 +154,8 @@ func (a *Agent) initializeDiskInfo() { for _, p := range partitions { // fmt.Println(p.Device, p.Mountpoint) // Binary root fallback or docker root fallback - if !hasRoot && (p.Mountpoint == rootMountPoint || (p.Mountpoint == "/etc/hosts" && strings.HasPrefix(p.Device, "/dev"))) { - fs, match := findIoDevice(filepath.Base(p.Device), diskIoCounters, a.fsStats) + if !hasRoot && (p.Mountpoint == rootMountPoint || (isDockerSpecialMountpoint(p.Mountpoint) && strings.HasPrefix(p.Device, "/dev"))) { + fs, match := findIoDevice(filepath.Base(p.Device), diskIoCounters) if match { addFsStat(fs, p.Mountpoint, true) hasRoot = true @@ -176,33 +189,26 @@ func (a *Agent) initializeDiskInfo() { // If no root filesystem set, use fallback if !hasRoot { - rootDevice, _ := findIoDevice(filepath.Base(filesystem), diskIoCounters, a.fsStats) - slog.Info("Root disk", "mountpoint", rootMountPoint, "io", rootDevice) - a.fsStats[rootDevice] = &system.FsStats{Root: true, Mountpoint: rootMountPoint} + rootKey := filepath.Base(rootMountPoint) + if _, exists := a.fsStats[rootKey]; exists { + rootKey = "root" + } + slog.Warn("Root device not detected; root I/O disabled", "mountpoint", rootMountPoint) + a.fsStats[rootKey] = &system.FsStats{Root: true, Mountpoint: rootMountPoint} } a.initializeDiskIoStats(diskIoCounters) } -// Returns matching device from /proc/diskstats, -// or the device with the most reads if no match is found. +// Returns matching device from /proc/diskstats. // bool is true if a match was found. -func findIoDevice(filesystem string, diskIoCounters map[string]disk.IOCountersStat, fsStats map[string]*system.FsStats) (string, bool) { - var maxReadBytes uint64 - maxReadDevice := "/" +func findIoDevice(filesystem string, diskIoCounters map[string]disk.IOCountersStat) (string, bool) { for _, d := range diskIoCounters { if d.Name == filesystem || (d.Label != "" && d.Label == filesystem) { return d.Name, true } - if d.ReadBytes > maxReadBytes { - // don't use if device already exists in fsStats - if _, exists := fsStats[d.Name]; !exists { - maxReadBytes = d.ReadBytes - maxReadDevice = d.Name - } - } } - return maxReadDevice, false + return "", false } // Sets start values for disk I/O stats. diff --git a/agent/disk_test.go b/agent/disk_test.go index 4bfff867..f180c9e8 100644 --- a/agent/disk_test.go +++ b/agent/disk_test.go @@ -94,6 +94,62 @@ func TestParseFilesystemEntry(t *testing.T) { } } +func TestFindIoDevice(t *testing.T) { + t.Run("matches by device name", func(t *testing.T) { + ioCounters := map[string]disk.IOCountersStat{ + "sda": {Name: "sda"}, + "sdb": {Name: "sdb"}, + } + + device, ok := findIoDevice("sdb", ioCounters) + assert.True(t, ok) + assert.Equal(t, "sdb", device) + }) + + t.Run("matches by device label", func(t *testing.T) { + ioCounters := map[string]disk.IOCountersStat{ + "sda": {Name: "sda", Label: "rootfs"}, + "sdb": {Name: "sdb"}, + } + + device, ok := findIoDevice("rootfs", ioCounters) + assert.True(t, ok) + assert.Equal(t, "sda", device) + }) + + t.Run("returns no fallback when not found", func(t *testing.T) { + ioCounters := map[string]disk.IOCountersStat{ + "sda": {Name: "sda"}, + "sdb": {Name: "sdb"}, + } + + device, ok := findIoDevice("nvme0n1p1", ioCounters) + assert.False(t, ok) + assert.Equal(t, "", device) + }) +} + +func TestIsDockerSpecialMountpoint(t *testing.T) { + testCases := []struct { + name string + mountpoint string + expected bool + }{ + {name: "hosts", mountpoint: "/etc/hosts", expected: true}, + {name: "resolv", mountpoint: "/etc/resolv.conf", expected: true}, + {name: "hostname", mountpoint: "/etc/hostname", expected: true}, + {name: "root", mountpoint: "/", expected: false}, + {name: "passwd", mountpoint: "/etc/passwd", expected: false}, + {name: "extra-filesystem", mountpoint: "/extra-filesystems/sda1", expected: false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, isDockerSpecialMountpoint(tc.mountpoint)) + }) + } +} + func TestInitializeDiskInfoWithCustomNames(t *testing.T) { // Set up environment variables oldEnv := os.Getenv("EXTRA_FILESYSTEMS") diff --git a/agent/smart.go b/agent/smart.go index 6b38d438..51b1004c 100644 --- a/agent/smart.go +++ b/agent/smart.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "os" "os/exec" "path/filepath" @@ -18,8 +19,6 @@ import ( "time" "github.com/henrygd/beszel/internal/entities/smart" - - "log/slog" ) // SmartManager manages data collection for SMART devices @@ -1125,7 +1124,6 @@ func NewSmartManager() (*SmartManager, error) { sm.refreshExcludedDevices() path, err := sm.detectSmartctl() if err != nil { - slog.Debug(err.Error()) return nil, err } slog.Debug("smartctl", "path", path)