reset Docker client connections after repeated old-engine list failures (#1728)

- 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

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

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")

agent/docker.go

@@ -32,6 +32,10 @@ var ansiEscapePattern = regexp.MustCompile(`\x1b\[[0-9;]*[a-zA-Z]|\x1b\][^\x07]*
 const (
 	// Docker API timeout in milliseconds
 	dockerTimeoutMs = 2100
+	// Number of consecutive /containers/json failures before forcing a client reset on old Docker versions
+	dockerClientResetFailureThreshold = 3
+	// Minimum time between Docker client resets to avoid reset flapping
+	dockerClientResetCooldown = 30 * time.Second
 	// Maximum realistic network speed (5 GB/s) to detect bad deltas
 	maxNetworkSpeedBps uint64 = 5e9
 	// Maximum conceivable memory usage of a container (100TB) to detect bad memory stats
@@ -55,12 +59,16 @@ type dockerManager struct {
 	containerStatsMap   map[string]*container.Stats // Keeps track of container stats
 	validIds            map[string]struct{}         // Map of valid container ids, used to prune invalid containers from containerStatsMap
 	goodDockerVersion   bool                        // Whether docker version is at least 25.0.0 (one-shot works correctly)
+	versionChecked      bool                        // Whether docker version detection completed successfully
 	isWindows           bool                        // Whether the Docker Engine API is running on Windows
 	buf                 *bytes.Buffer               // Buffer to store and read response bodies
 	decoder             *json.Decoder               // Reusable JSON decoder that reads from buf
 	apiStats            *container.ApiStats         // Reusable API stats object
 	excludeContainers   []string                    // Patterns to exclude containers by name
 	usingPodman         bool                        // Whether the Docker Engine API is running on Podman
+	transport           *http.Transport             // Base transport used by client for connection resets
+	consecutiveListFailures int                     // Number of consecutive /containers/json request failures
+	lastClientReset     time.Time                   // Last time the Docker client connections were reset
 
 	// Cache-time-aware tracking for CPU stats (similar to cpu.go)
 	// Maps cache time intervals to container-specific CPU usage tracking
@@ -119,8 +127,10 @@ func (dm *dockerManager) shouldExcludeContainer(name string) bool {
 func (dm *dockerManager) getDockerStats(cacheTimeMs uint16) ([]*container.Stats, error) {
 	resp, err := dm.client.Get("http://localhost/containers/json")
 	if err != nil {
+		dm.handleContainerListError(err)
 		return nil, err
 	}
+	dm.consecutiveListFailures = 0
 
 	dm.apiContainerList = dm.apiContainerList[:0]
 	if err := dm.decode(resp, &dm.apiContainerList); err != nil {
@@ -204,6 +214,50 @@ func (dm *dockerManager) getDockerStats(cacheTimeMs uint16) ([]*container.Stats,
 	return stats, nil
 }
 
+func (dm *dockerManager) handleContainerListError(err error) {
+	dm.consecutiveListFailures++
+	if !dm.shouldResetDockerClient(err) {
+		return
+	}
+	dm.resetDockerClientConnections()
+}
+
+func (dm *dockerManager) shouldResetDockerClient(err error) bool {
+	if !dm.versionChecked || dm.goodDockerVersion {
+		return false
+	}
+	if dm.consecutiveListFailures < dockerClientResetFailureThreshold {
+		return false
+	}
+	if !dm.lastClientReset.IsZero() && time.Since(dm.lastClientReset) < dockerClientResetCooldown {
+		return false
+	}
+	return isDockerApiOverloadError(err)
+}
+
+func isDockerApiOverloadError(err error) bool {
+	if err == nil {
+		return false
+	}
+	if errors.Is(err, context.DeadlineExceeded) {
+		return true
+	}
+	msg := err.Error()
+	return strings.Contains(msg, "Client.Timeout exceeded") ||
+		strings.Contains(msg, "request canceled") ||
+		strings.Contains(msg, "context deadline exceeded") ||
+		strings.Contains(msg, "EOF")
+}
+
+func (dm *dockerManager) resetDockerClientConnections() {
+	if dm.transport == nil {
+		return
+	}
+	dm.transport.CloseIdleConnections()
+	dm.lastClientReset = time.Now()
+	slog.Warn("Reset Docker client connections after repeated /containers/json failures", "failures", dm.consecutiveListFailures)
+}
+
 // initializeCpuTracking initializes CPU tracking maps for a specific cache time interval
 func (dm *dockerManager) initializeCpuTracking(cacheTimeMs uint16) {
 	// Initialize cache time maps if they don't exist
@@ -553,6 +607,7 @@ func newDockerManager() *dockerManager {
 			Timeout:   timeout,
 			Transport: userAgentTransport,
 		},
+		transport:         transport,
 		containerStatsMap: make(map[string]*container.Stats),
 		sem:               make(chan struct{}, 5),
 		apiContainerList:  []*container.ApiInfo{},
@@ -611,6 +666,7 @@ func (dm *dockerManager) checkDockerVersion() {
 	if err := dm.decode(resp, &versionInfo); err != nil {
 		return
 	}
+	dm.versionChecked = true
 	// if version > 24, one-shot works correctly and we can limit concurrent operations
 	if dockerVersion, err := semver.Parse(versionInfo.Version); err == nil && dockerVersion.Major > 24 {
 		dm.goodDockerVersion = true

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)