fix(agent): keep every EXTRA_FILESYSTEMS mount when device keys collide#1932
Open
BootstrapperSBL wants to merge 1 commit intohenrygd:mainfrom
Open
fix(agent): keep every EXTRA_FILESYSTEMS mount when device keys collide#1932BootstrapperSBL wants to merge 1 commit intohenrygd:mainfrom
BootstrapperSBL wants to merge 1 commit intohenrygd:mainfrom
Conversation
systemd-automount (x-systemd.automount in fstab) reports the same device name — "systemd-1" — for every automount target. `registerFilesystemStats` dedupes by key, so only the first mount survived and every additional mount in `EXTRA_FILESYSTEMS` was silently dropped with no log line. Before, the agent log showed only Video for: EXTRA_FILESYSTEMS=/media/video__Video,/media/backup__Backup because both were registered under key "systemd-1" and the second call hit the "already exists" short-circuit. The dedupe check now distinguishes two cases: * same mountpoint under an existing key → still rejected (genuine double-registration) * different mountpoint under an existing key → fall back to a unique key derived from the mountpoint basename (with a -N suffix if that collides too) The original "skips existing key" test is updated to describe the real case (same mount re-registered) and two new tests cover the pseudo-device collision and the suffix-disambiguation paths. Fixes henrygd#1931
4 tasks
Contributor
Author
|
Hey, just checking if you've had a chance to look at this — happy to adjust anything if needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
EXTRA_FILESYSTEMSnow correctly registers every listed mount when multiple entries sit behind systemd-automount (or any other pseudo-device that reuses a single device identifier across mounts).Fixes #1931.
Why
agent/disk.go'sregisterFilesystemStatsdedupes by device-derived key. For most real devices that's correct —/dev/sda1can only be mounted in one place at a time. Butx-systemd.automountreports every mount with the literal devicesystemd-1, so the fsStats key collides on the first entry and every subsequent mount is silently dropped (no log line, no error).Reporter's config, which should register two disks, only surfaces one:
Only
Videoever makes it intofsStats;Backupis silently discarded becausefsStats["systemd-1"]was already set.How
registerFilesystemStatsnow distinguishes two "existing key" cases:ok=false(the old behaviour)./media/backup→backup), with a-Nsuffix if that basename also collides.A small
uniqueFsStatsKeyhelper encapsulates the basename/suffix logic.Before / After
For
EXTRA_FILESYSTEMS=/media/video__Video,/media/backup__Backup:{"systemd-1": Video}{"systemd-1": Video, "backup": Backup}No change for any setup where the device key uniquely identifies the mount (i.e. every real block device). The new path only triggers on key collisions, so existing user-visible names stay stable.
Test plan
go build ./agent/...gofmt -l agent/disk.go agent/disk_test.go— cleango vet ./agent/...go test -race ./agent/...— all pass, including three tests covering the new paths: