Skip to content

Commit fabf650

Browse files
committed
fix(applycheck): exclude kernel-virtual disks from selector candidates and hints
Real Talos hosts running cozystack accumulate dozens of virtual block devices (dm-* from LVM, drbd* from DRBD-replicated PVs, loop* from container image layers). Talos's block.DiskSpec marks all of these with bus_path="/virtual"; the install-disk picker never lands on them, and a pre-apply selector hitting one would be a silent misdirect. Surface that exclusion in both validation paths: - matchSelector now skips bus_path=/virtual disks the same way it skips Readonly and CDROM, so a 'type: ssd' selector on a host with hundreds of loop/dm/drbd devices doesn't surface a spurious 'multiple matches' warning. - diskPathList and summarizeDisks omit the virtual class from operator- facing hints, replacing a 19-entry scroll of dm/drbd/loop noise on a cozystack host with the actionable 'available disks: /dev/sda, /dev/sdb' two-name list verified on a live OCI v1.12.6 cluster. Refs: #172 Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent 13e3a8a commit fabf650

2 files changed

Lines changed: 107 additions & 16 deletions

File tree

pkg/applycheck/selector.go

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,16 @@ const (
3535
transportNVMe = "nvme"
3636
)
3737

38+
// virtualBusPath is the BusPath value Talos assigns to kernel-virtual
39+
// block devices (loop, dm, drbd, ram, ...). Real disks live under a
40+
// PCI / virtio / etc. bus path; matchSelector excludes virtual devices
41+
// from candidacy so a selector never silently lands on /dev/loop0.
42+
const virtualBusPath = "/virtual"
43+
3844
// matchSelector returns every disk in candidates that satisfies every
39-
// non-empty field of sel, excluding read-only and CD-ROM devices the
40-
// way Talos's install-disk resolution does. An empty selector matches
41-
// every non-excluded candidate.
45+
// non-empty field of sel, excluding read-only, CD-ROM, and kernel-virtual
46+
// devices the way Talos's install-disk resolution does. An empty
47+
// selector matches every non-excluded candidate.
4248
//
4349
// Match semantics mirror Talos's InstallDiskSelector at the level the
4450
// pre-apply gate cares about — model and busPath are shell-globs (Talos
@@ -52,7 +58,7 @@ func matchSelector(sel *DiskSelector, candidates []DiskInfo) []DiskInfo {
5258
for i := range candidates {
5359
disk := &candidates[i]
5460

55-
if disk.Readonly || disk.CDROM {
61+
if isExcludedDisk(disk) {
5662
continue
5763
}
5864

@@ -64,6 +70,14 @@ func matchSelector(sel *DiskSelector, candidates []DiskInfo) []DiskInfo {
6470
return matches
6571
}
6672

73+
// isExcludedDisk reports whether a disk should never be a selector
74+
// candidate. Readonly and CDROM are explicit Talos exclusions; virtual
75+
// devices (dm-*, drbd*, loop*) carry bus_path "/virtual" and are
76+
// inappropriate install / volume targets.
77+
func isExcludedDisk(disk *DiskInfo) bool {
78+
return disk.Readonly || disk.CDROM || disk.BusPath == virtualBusPath
79+
}
80+
6781
// diskPredicate is one rule the candidate disk has to satisfy. matchAll
6882
// short-circuits on the first false.
6983
type diskPredicate func(sel *DiskSelector, disk *DiskInfo) bool
@@ -232,12 +246,19 @@ func parseSize(raw string) (uint64, bool) {
232246
return value, true
233247
}
234248

235-
// diskPathList returns the sorted set of DiskInfo.DevPath values so error
236-
// messages list them in a stable, scannable order.
249+
// diskPathList returns the sorted set of DiskInfo.DevPath values for
250+
// non-excluded candidates so error messages list them in a stable,
251+
// scannable order without burying real candidates under noise from
252+
// dm/drbd/loop entries.
237253
func diskPathList(disks []DiskInfo) []string {
238-
paths := make([]string, len(disks))
254+
var paths []string
255+
239256
for i := range disks {
240-
paths[i] = disks[i].DevPath
257+
if isExcludedDisk(&disks[i]) {
258+
continue
259+
}
260+
261+
paths = append(paths, disks[i].DevPath)
241262
}
242263

243264
sort.Strings(paths)
@@ -266,17 +287,24 @@ func joinAvailable(items []string) string {
266287
return strings.Join(sorted, ", ")
267288
}
268289

269-
// summarizeDisks returns a compact human-readable list of disks, one per
270-
// `path model serial size` triple. Used in selector-mismatch findings so
271-
// the operator can pick the right disk without running another command.
290+
// summarizeDisks returns a compact human-readable list of non-excluded
291+
// disks, one per `path model serial size` triple. Used in selector-
292+
// mismatch findings so the operator can pick the right disk without
293+
// running another command. Excluded devices (virtual, readonly, CD)
294+
// are omitted from the summary to keep the hint actionable.
272295
func summarizeDisks(disks []DiskInfo) string {
273-
if len(disks) == 0 {
274-
return noneText
275-
}
296+
var parts []string
276297

277-
parts := make([]string, len(disks))
278298
for i := range disks {
279-
parts[i] = formatDisk(&disks[i])
299+
if isExcludedDisk(&disks[i]) {
300+
continue
301+
}
302+
303+
parts = append(parts, formatDisk(&disks[i]))
304+
}
305+
306+
if len(parts) == 0 {
307+
return noneText
280308
}
281309

282310
sort.Strings(parts)

pkg/applycheck/validate_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,69 @@ func TestValidateRefs_ReadonlyAndCDROMExcluded(t *testing.T) {
238238
}
239239
}
240240

241+
// TestValidateRefs_VirtualDisksExcluded pins the exclusion of kernel-
242+
// virtual block devices (loop, dm, drbd, ram). Talos block.DiskSpec
243+
// reports BusPath="/virtual" for those; the gate must skip them so a
244+
// selector like `type: ssd` on a cozystack host (which hosts many
245+
// loop/dm/drbd devices for DRBD-replicated PVs) doesn't surface a
246+
// spurious "multiple matches" warning across every install. Verified
247+
// on a live OCI cluster: real disks (sda, sdb) have PCI bus paths,
248+
// virtual disks (dm-*, drbd*, loop*) carry "/virtual".
249+
func TestValidateRefs_VirtualDisksExcluded(t *testing.T) {
250+
t.Parallel()
251+
252+
refs := []applycheck.Ref{{
253+
Kind: applycheck.RefKindDiskSelector,
254+
Selector: applycheck.DiskSelector{Type: "ssd"},
255+
}}
256+
snapshot := applycheck.HostSnapshot{
257+
Disks: []applycheck.DiskInfo{
258+
{DevPath: "/dev/sda", BusPath: "/pci0000:00/0000:00:04.0", Transport: "virtio", Rotational: false},
259+
{DevPath: "/dev/dm-0", BusPath: "/virtual", Rotational: false},
260+
{DevPath: "/dev/drbd1000", BusPath: "/virtual", Rotational: false},
261+
{DevPath: "/dev/loop0", BusPath: "/virtual", Rotational: false},
262+
},
263+
}
264+
265+
findings := applycheck.ValidateRefs(refs, snapshot)
266+
if len(findings) != 0 {
267+
t.Errorf("expected one clean match (sda); virtual disks must be excluded, got findings=%+v", findings)
268+
}
269+
}
270+
271+
// TestValidateRefs_DiskLiteralHint_OmitsVirtual pins the hint output
272+
// scope: when a literal disk path doesn't resolve, the operator-facing
273+
// hint must list real block devices only, not virtual noise.
274+
func TestValidateRefs_DiskLiteralHint_OmitsVirtual(t *testing.T) {
275+
t.Parallel()
276+
277+
refs := []applycheck.Ref{{
278+
Kind: applycheck.RefKindDiskLiteral,
279+
Name: "/dev/sdz",
280+
}}
281+
snapshot := applycheck.HostSnapshot{
282+
Disks: []applycheck.DiskInfo{
283+
{DevPath: "/dev/sda", BusPath: "/pci0000:00/0000:00:04.0"},
284+
{DevPath: "/dev/dm-0", BusPath: "/virtual"},
285+
{DevPath: "/dev/loop0", BusPath: "/virtual"},
286+
},
287+
}
288+
289+
findings := applycheck.ValidateRefs(refs, snapshot)
290+
if len(findings) != 1 {
291+
t.Fatalf("expected one blocker, got %+v", findings)
292+
}
293+
294+
hint := findings[0].Hint
295+
if strings.Contains(hint, "/dev/dm-0") || strings.Contains(hint, "/dev/loop0") {
296+
t.Errorf("hint should omit virtual devices, got %q", hint)
297+
}
298+
299+
if !strings.Contains(hint, "/dev/sda") {
300+
t.Errorf("hint should list real disks, got %q", hint)
301+
}
302+
}
303+
241304
func TestValidateRefs_SelectorBySize_GreaterThanOrEqual(t *testing.T) {
242305
t.Parallel()
243306

0 commit comments

Comments
 (0)