mirror of
https://github.com/henrygd/beszel.git
synced 2026-09-26 11:27:47 +02:00
fix(agent): fall back to last-resort root when root key is taken
addPartitionRootFs reported success even when addFsStat skipped the registration because the I/O key was already in use, e.g. when the root drive is also listed in EXTRA_FILESYSTEMS. hasRoot was then set without any root filesystem, and the last-resort fallback never ran. addFsStat now reports whether it registered the filesystem and addPartitionRootFs propagates that. Also uppercase the drive letter in windowsVolumeName so "c:" and "C:" normalize to the same key.
This commit is contained in:
@@ -154,12 +154,12 @@ func registerFilesystemStats(existing map[string]*system.FsStats, device, mountp
|
|||||||
}
|
}
|
||||||
|
|
||||||
// addFsStat inserts a discovered filesystem if it resolves to a new tracking
|
// addFsStat inserts a discovered filesystem if it resolves to a new tracking
|
||||||
// key. The key selection itself lives in buildFsStatRegistration so that logic
|
// key and reports whether it was added. The key selection itself lives in
|
||||||
// can stay directly unit-tested.
|
// buildFsStatRegistration so that logic can stay directly unit-tested.
|
||||||
func (d *diskDiscovery) addFsStat(device, mountpoint string, root bool, customName string) {
|
func (d *diskDiscovery) addFsStat(device, mountpoint string, root bool, customName string) bool {
|
||||||
key, fsStats, ok := registerFilesystemStats(d.agent.fsStats, device, mountpoint, root, customName, d.ctx)
|
key, fsStats, ok := registerFilesystemStats(d.agent.fsStats, device, mountpoint, root, customName, d.ctx)
|
||||||
if !ok {
|
if !ok {
|
||||||
return
|
return false
|
||||||
}
|
}
|
||||||
d.agent.fsStats[key] = fsStats
|
d.agent.fsStats[key] = fsStats
|
||||||
name := key
|
name := key
|
||||||
@@ -167,6 +167,7 @@ func (d *diskDiscovery) addFsStat(device, mountpoint string, root bool, customNa
|
|||||||
name = customName
|
name = customName
|
||||||
}
|
}
|
||||||
slog.Info("Detected disk", "name", name, "device", device, "mount", mountpoint, "io", key, "root", root)
|
slog.Info("Detected disk", "name", name, "device", device, "mount", mountpoint, "io", key, "root", root)
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
// addConfiguredRootFs resolves FILESYSTEM against partitions first, then falls
|
// addConfiguredRootFs resolves FILESYSTEM against partitions first, then falls
|
||||||
@@ -212,9 +213,10 @@ func (d *diskDiscovery) addPartitionRootFs(device, mountpoint string) bool {
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
// The resolved I/O device is already known here, so use it directly to avoid
|
// The resolved I/O device is already known here, so use it directly to avoid
|
||||||
// a second fallback search inside buildFsStatRegistration.
|
// a second fallback search inside buildFsStatRegistration. Report failure if
|
||||||
d.addFsStat(fs, mountpoint, true, "")
|
// the key was already taken (e.g. root drive listed in EXTRA_FILESYSTEMS) so
|
||||||
return true
|
// the caller can still fall back to addLastResortRootFs.
|
||||||
|
return d.addFsStat(fs, mountpoint, true, "")
|
||||||
}
|
}
|
||||||
|
|
||||||
// addLastResortRootFs is only used when neither FILESYSTEM nor partition-based
|
// addLastResortRootFs is only used when neither FILESYSTEM nor partition-based
|
||||||
@@ -542,7 +544,8 @@ func normalizeDeviceName(value string) string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// windowsVolumeName returns the canonical form of a bare Windows volume
|
// windowsVolumeName returns the canonical form of a bare Windows volume
|
||||||
// specifier, so that "C:", `C:\` and "C:/" all name the same drive.
|
// specifier, so that "C:", "c:", `C:\` and "C:/" all name the same drive.
|
||||||
|
// Drive letters are case-insensitive on Windows, so the letter is uppercased.
|
||||||
//
|
//
|
||||||
// filepath.Base cannot do this. On Windows it treats "C:" as a volume name
|
// filepath.Base cannot do this. On Windows it treats "C:" as a volume name
|
||||||
// with no path element to take the base of and returns "\", so every drive
|
// with no path element to take the base of and returns "\", so every drive
|
||||||
@@ -556,9 +559,6 @@ func windowsVolumeName(value string) (string, bool) {
|
|||||||
if c := value[0]; !('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z') {
|
if c := value[0]; !('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z') {
|
||||||
return "", false
|
return "", false
|
||||||
}
|
}
|
||||||
if len(value) == 2 {
|
|
||||||
return value, true
|
|
||||||
}
|
|
||||||
// Only separators may follow the specifier. "C:data" is a drive-relative
|
// Only separators may follow the specifier. "C:data" is a drive-relative
|
||||||
// path, not a volume.
|
// path, not a volume.
|
||||||
for i := 2; i < len(value); i++ {
|
for i := 2; i < len(value); i++ {
|
||||||
@@ -566,7 +566,7 @@ func windowsVolumeName(value string) (string, bool) {
|
|||||||
return "", false
|
return "", false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return value[:2], true
|
return strings.ToUpper(value[:2]), true
|
||||||
}
|
}
|
||||||
|
|
||||||
// Sets start values for disk I/O stats.
|
// Sets start values for disk I/O stats.
|
||||||
|
|||||||
@@ -1066,9 +1066,10 @@ func TestNormalizeDeviceName(t *testing.T) {
|
|||||||
for _, spelling := range []string{"C:", `C:\`, "C:/", `C:\\`} {
|
for _, spelling := range []string{"C:", `C:\`, "C:/", `C:\\`} {
|
||||||
assert.Equal(t, "C:", normalizeDeviceName(spelling), "spelling %q", spelling)
|
assert.Equal(t, "C:", normalizeDeviceName(spelling), "spelling %q", spelling)
|
||||||
}
|
}
|
||||||
// Case is left to the caller, as it already is for Linux device names.
|
// Drive letters are case-insensitive, so the letter is uppercased.
|
||||||
assert.Equal(t, "d:", normalizeDeviceName("d:"))
|
assert.Equal(t, "D:", normalizeDeviceName("d:"))
|
||||||
assert.Equal(t, "c:", normalizeDeviceName(" c: "))
|
assert.Equal(t, "C:", normalizeDeviceName(" c: "))
|
||||||
|
assert.Equal(t, "C:", normalizeDeviceName(`c:\`))
|
||||||
|
|
||||||
// Non-volume inputs keep using filepath.Base.
|
// Non-volume inputs keep using filepath.Base.
|
||||||
assert.Equal(t, "sda1", normalizeDeviceName("/dev/sda1"))
|
assert.Equal(t, "sda1", normalizeDeviceName("/dev/sda1"))
|
||||||
@@ -1124,3 +1125,32 @@ func TestAddPartitionRootFsWindowsDrive(t *testing.T) {
|
|||||||
assert.True(t, exists)
|
assert.True(t, exists)
|
||||||
assert.True(t, stats.Root)
|
assert.True(t, stats.Root)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestAddPartitionRootFsKeyAlreadyRegistered(t *testing.T) {
|
||||||
|
// The root drive is also listed in EXTRA_FILESYSTEMS, so its key is taken
|
||||||
|
// before the root fallback runs. addPartitionRootFs must report failure so
|
||||||
|
// the caller still falls back to addLastResortRootFs instead of ending up
|
||||||
|
// with no root filesystem.
|
||||||
|
agent := &Agent{fsStats: map[string]*system.FsStats{
|
||||||
|
"C:": {Mountpoint: `C:\`},
|
||||||
|
}}
|
||||||
|
discovery := diskDiscovery{
|
||||||
|
agent: agent,
|
||||||
|
rootMountPoint: `C:\`,
|
||||||
|
ctx: fsRegistrationContext{
|
||||||
|
isWindows: true,
|
||||||
|
diskIoCounters: map[string]disk.IOCountersStat{
|
||||||
|
"C:": {Name: "C:", ReadBytes: 10},
|
||||||
|
"D:": {Name: "D:"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
ok := discovery.addPartitionRootFs("C:", `C:\`)
|
||||||
|
assert.False(t, ok)
|
||||||
|
assert.False(t, agent.fsStats["C:"].Root)
|
||||||
|
|
||||||
|
discovery.addLastResortRootFs()
|
||||||
|
assert.Len(t, agent.fsStats, 1)
|
||||||
|
assert.True(t, agent.fsStats["C:"].Root)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user