diff --git a/agent/disk.go b/agent/disk.go index 8574beff0..ec7006f99 100644 --- a/agent/disk.go +++ b/agent/disk.go @@ -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 -// key. The key selection itself lives in buildFsStatRegistration so that logic -// can stay directly unit-tested. -func (d *diskDiscovery) addFsStat(device, mountpoint string, root bool, customName string) { +// key and reports whether it was added. The key selection itself lives in +// buildFsStatRegistration so that logic can stay directly unit-tested. +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) if !ok { - return + return false } d.agent.fsStats[key] = fsStats name := key @@ -167,6 +167,7 @@ func (d *diskDiscovery) addFsStat(device, mountpoint string, root bool, customNa name = customName } slog.Info("Detected disk", "name", name, "device", device, "mount", mountpoint, "io", key, "root", root) + return true } // addConfiguredRootFs resolves FILESYSTEM against partitions first, then falls @@ -212,9 +213,10 @@ func (d *diskDiscovery) addPartitionRootFs(device, mountpoint string) bool { return false } // The resolved I/O device is already known here, so use it directly to avoid - // a second fallback search inside buildFsStatRegistration. - d.addFsStat(fs, mountpoint, true, "") - return true + // a second fallback search inside buildFsStatRegistration. Report failure if + // the key was already taken (e.g. root drive listed in EXTRA_FILESYSTEMS) so + // the caller can still fall back to addLastResortRootFs. + return d.addFsStat(fs, mountpoint, true, "") } // 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 -// 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 // 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') { return "", false } - if len(value) == 2 { - return value, true - } // Only separators may follow the specifier. "C:data" is a drive-relative // path, not a volume. for i := 2; i < len(value); i++ { @@ -566,7 +566,7 @@ func windowsVolumeName(value string) (string, bool) { return "", false } } - return value[:2], true + return strings.ToUpper(value[:2]), true } // Sets start values for disk I/O stats. diff --git a/agent/disk_test.go b/agent/disk_test.go index 59f423bfd..7d5a3e4d7 100644 --- a/agent/disk_test.go +++ b/agent/disk_test.go @@ -1066,9 +1066,10 @@ func TestNormalizeDeviceName(t *testing.T) { for _, spelling := range []string{"C:", `C:\`, "C:/", `C:\\`} { assert.Equal(t, "C:", normalizeDeviceName(spelling), "spelling %q", spelling) } - // Case is left to the caller, as it already is for Linux device names. - assert.Equal(t, "d:", normalizeDeviceName("d:")) - assert.Equal(t, "c:", normalizeDeviceName(" c: ")) + // Drive letters are case-insensitive, so the letter is uppercased. + assert.Equal(t, "D:", normalizeDeviceName("d:")) + assert.Equal(t, "C:", normalizeDeviceName(" c: ")) + assert.Equal(t, "C:", normalizeDeviceName(`c:\`)) // Non-volume inputs keep using filepath.Base. assert.Equal(t, "sda1", normalizeDeviceName("/dev/sda1")) @@ -1124,3 +1125,32 @@ func TestAddPartitionRootFsWindowsDrive(t *testing.T) { assert.True(t, exists) 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) +}