Skip to content

Commit 77a96f8

Browse files
committed
fix(applycheck): accept stable disk symlinks for RefKindDiskLiteral
machine.install.disk is most safely written as a /dev/disk/by-id/ or /dev/disk/by-path/ form — the kernel-assigned /dev/sdX names reorder across reboots, but the udev-managed symlinks pin to the underlying device. Talos's block.DiskSpec exposes those alternate forms in spec.symlinks; the gate must accept any of them as equivalent to spec.dev_path. Without this fix, the recommended-for-production literal 'disk: /dev/disk/by-id/wwn-0x...' surfaced a Phase 1 blocker with a hint listing only /dev/sda — pushing operators toward the unstable kernel name. cosiLinksDisksReader now carries Symlinks into the snapshot; ValidateRefs builds the path set from DevPath + Symlinks. Verified on the dev17 OCI cluster: a by-id literal that previously blocked now passes Phase 1. Refs: #172 Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent bafa4a8 commit 77a96f8

3 files changed

Lines changed: 54 additions & 2 deletions

File tree

pkg/applycheck/validate.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ type HostSnapshot struct {
2929
// predicate over Transport + Rotational, mirrored in matchSelector.
3030
// CDROM and Readonly devices are excluded by Talos's install-disk
3131
// resolution; matchSelector mirrors that exclusion.
32+
//
33+
// Symlinks captures the alternate path forms Talos exposes for the
34+
// same device (/dev/disk/by-id/wwn-…, /dev/disk/by-path/pci-…,
35+
// /dev/disk/by-diskseq/…); RefKindDiskLiteral validation accepts any
36+
// of these as equivalent to DevPath, because by-id paths are the
37+
// recommended stable form for `machine.install.disk` and an operator
38+
// using them is doing the right thing.
3239
type DiskInfo struct {
3340
DevPath string // /dev/sda
3441
Model string
@@ -42,6 +49,7 @@ type DiskInfo struct {
4249
Rotational bool
4350
Readonly bool
4451
CDROM bool
52+
Symlinks []string
4553
}
4654

4755
// Severity classifies a finding's blocker status.
@@ -90,9 +98,18 @@ func ValidateRefs(refs []Ref, snapshot HostSnapshot) []Finding {
9098
linkSet[name] = struct{}{}
9199
}
92100

93-
diskPaths := make(map[string]struct{}, len(snapshot.Disks))
101+
// Disk-literal validation accepts DevPath (`/dev/sda`) and every
102+
// stable Symlink alternative (/dev/disk/by-id/wwn-…, by-path/…,
103+
// by-diskseq/…). The recommended Talos pattern is by-id, so the
104+
// gate must not reject by-id literals.
105+
diskPaths := make(map[string]struct{}, len(snapshot.Disks)*4) //nolint:mnd // upper-bound estimate for sym-count.
94106
for i := range snapshot.Disks {
95-
diskPaths[snapshot.Disks[i].DevPath] = struct{}{}
107+
d := &snapshot.Disks[i]
108+
diskPaths[d.DevPath] = struct{}{}
109+
110+
for _, sym := range d.Symlinks {
111+
diskPaths[sym] = struct{}{}
112+
}
96113
}
97114

98115
var findings []Finding

pkg/applycheck/validate_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,40 @@ func TestValidateRefs_DiskLiteralMissing_EmitsBlocker(t *testing.T) {
8585
}
8686
}
8787

88+
// TestValidateRefs_DiskLiteralByID_AcceptedViaSymlink pins the
89+
// contract: machine.install.disk accepts the by-id /
90+
// by-path / by-diskseq stable forms Talos exposes via Disk.Symlinks.
91+
// These are the recommended forms for stable boot ordering, so the
92+
// gate must not block them — verified on the dev17 OCI cluster where
93+
// /dev/sda surfaces /dev/disk/by-id/wwn-0x602742… as a symlink.
94+
func TestValidateRefs_DiskLiteralByID_AcceptedViaSymlink(t *testing.T) {
95+
t.Parallel()
96+
97+
refs := []applycheck.Ref{{
98+
Kind: applycheck.RefKindDiskLiteral,
99+
Name: "/dev/disk/by-id/wwn-0x602742ce4e9046729ae81c05166f4d8e",
100+
}}
101+
snapshot := applycheck.HostSnapshot{
102+
Disks: []applycheck.DiskInfo{
103+
{
104+
DevPath: "/dev/sda",
105+
WWID: "naa.602742ce4e9046729ae81c05166f4d8e",
106+
Symlinks: []string{
107+
"/dev/disk/by-diskseq/23",
108+
"/dev/disk/by-id/scsi-3602742ce4e9046729ae81c05166f4d8e",
109+
"/dev/disk/by-id/wwn-0x602742ce4e9046729ae81c05166f4d8e",
110+
"/dev/disk/by-path/pci-0000:00:04.0-scsi-0:0:0:1",
111+
},
112+
},
113+
},
114+
}
115+
116+
findings := applycheck.ValidateRefs(refs, snapshot)
117+
if len(findings) != 0 {
118+
t.Errorf("by-id symlink should resolve to /dev/sda; gate must not block, got findings=%+v", findings)
119+
}
120+
}
121+
88122
func TestValidateRefs_SelectorZeroMatches_Blocker(t *testing.T) {
89123
t.Parallel()
90124

pkg/commands/preflight_apply_safety.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ func cosiLinksDisksReader(c *client.Client) linksDisksReader {
301301
Rotational: spec.Rotational,
302302
Readonly: spec.Readonly,
303303
CDROM: spec.CDROM,
304+
Symlinks: append([]string(nil), spec.Symlinks...),
304305
})
305306
}
306307

0 commit comments

Comments
 (0)