Skip to content

Commit 5601568

Browse files
sean-sean-
committed
pkg/storage/disk: add unit tests for parsing various outputs
Epic: none Release note: None
1 parent de45035 commit 5601568

File tree

3 files changed

+281
-18
lines changed

3 files changed

+281
-18
lines changed

‎pkg/storage/disk/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ go_test(
4949
"linux_parse_test.go",
5050
"monitor_test.go",
5151
"monitor_tracer_test.go",
52+
"platform_linux_test.go",
5253
],
5354
data = glob(["testdata/**"]),
5455
embed = [":disk"],

‎pkg/storage/disk/platform_linux.go‎

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,16 @@ func deviceIDFromFileInfo(finfo fs.FileInfo, path string) DeviceID {
134134
type _ZPoolName string
135135

136136
func deviceIDForZFS(path string) (uint32, uint32, error) {
137-
zpoolName, err := getZFSPoolName(path)
137+
zpoolName, err := zfsGetPoolName(path)
138138
if err != nil {
139139
return 0, 0, errors.Newf("unable to find the zpool for %q: %v", path, err) // nolint:errwrap
140140
}
141141

142-
devName, err := getZPoolDevice(zpoolName)
143-
if err != nil {
142+
// If there are multiple devices for a zpool, an error is returned along with
143+
// a device name. Continue resolving the device's major:minor numbers,
144+
// despite the multiple drives.
145+
devName, err := zpoolGetDevice(zpoolName)
146+
if err != nil && devName == "" {
144147
return 0, 0, errors.Newf("unable to find the device for pool %q: %v", zpoolName, err) // nolint:errwrap
145148
}
146149

@@ -152,15 +155,19 @@ func deviceIDForZFS(path string) (uint32, uint32, error) {
152155
return major, minor, nil
153156
}
154157

155-
func getZFSPoolName(path string) (_ZPoolName, error) {
158+
func zfsGetPoolName(path string) (_ZPoolName, error) {
156159
out, err := exec.Command("df", "--no-sync", "--output=source,fstype", path).Output()
157160
if err != nil {
158161
return "", errors.Newf("unable to exec df(1): %v", err) // nolint:errwrap
159162
}
160163

161-
lines := strings.Split(strings.TrimSpace(string(out)), "\n")
164+
return zfsParseDF(out)
165+
}
166+
167+
func zfsParseDF(df []byte) (_ZPoolName, error) {
168+
lines := strings.Split(strings.TrimSpace(string(df)), "\n")
162169
if len(lines) < 2 {
163-
return "", fmt.Errorf("unexpected df(1) output: %q", out)
170+
return "", fmt.Errorf("unexpected df(1) output: %q", df)
164171
}
165172

166173
fields := strings.Fields(lines[1])
@@ -178,32 +185,42 @@ func getZFSPoolName(path string) (_ZPoolName, error) {
178185
return _ZPoolName(poolName), nil
179186
}
180187

181-
func getZPoolDevice(poolName _ZPoolName) (string, error) {
188+
func zpoolGetDevice(poolName _ZPoolName) (string, error) {
182189
ctx := context.TODO()
183190

184191
out, err := exec.Command("zpool", "status", "-pPL", string(poolName)).Output()
185192
if err != nil {
186193
return "", errors.Newf("unable to find the devices attached to pool %q: %v", poolName, err) // nolint:errwrap
187194
}
188195

189-
scanner := bufio.NewScanner(bytes.NewReader(out))
190-
var devPart string
196+
return zpoolParseStatus(ctx, poolName, out)
197+
}
198+
199+
func zpoolParseStatus(ctx context.Context, poolName _ZPoolName, output []byte) (string, error) {
200+
scanner := bufio.NewScanner(bytes.NewReader(output))
201+
var devName string
202+
var devCount int
191203
for scanner.Scan() {
192204
line := strings.TrimSpace(scanner.Text())
193205
fields := strings.Fields(line)
194206
if len(fields) >= 2 && fields[1] == "ONLINE" && strings.HasPrefix(fields[0], "/dev/") {
195-
if devPart == "" {
196-
devPart = stripDevicePartition(fields[0])
207+
devCount++
208+
if devName == "" {
209+
devName = stripDevicePartition(fields[0])
197210
} else {
198-
maybeWarnf(ctx, "unsupported configuration: multiple devices (i.e. %q, %q) detected for zpool %q", devPart, fields[0], string(poolName))
211+
maybeWarnf(ctx, "unsupported configuration: multiple devices (i.e. %q, %q) detected for zpool %q", devName, fields[0], string(poolName))
199212
}
200213
}
201214
}
202-
if devPart != "" {
203-
return devPart, nil
204-
}
205215

206-
return "", fmt.Errorf("no device found for zpool %q", poolName)
216+
switch {
217+
case devCount == 1:
218+
return devName, nil
219+
case devCount > 1:
220+
return devName, errors.Newf("unsupported configuration: %d devices detected for zpool %q", devCount, string(poolName))
221+
default:
222+
return "", fmt.Errorf("no device found for zpool %q", poolName)
223+
}
207224
}
208225

209226
var (
@@ -221,7 +238,7 @@ func stripDevicePartition(devicePath string) string {
221238
}
222239

223240
scsiMatches := scsiPartitionRegex.FindStringSubmatch(base)
224-
if len(scsiMatches) == 3 {
241+
if len(scsiMatches) >= 3 {
225242
return scsiMatches[1]
226243
}
227244

@@ -238,14 +255,18 @@ func getDeviceID(devPath string) (uint32, uint32, error) {
238255
return 0, 0, errors.Newf("unable to read %q: %v", devFilePath, err) // nolint:errwrap
239256
}
240257

258+
return parseDeviceID(devFilePath, data)
259+
}
260+
261+
func parseDeviceID(devFilePath string, data []byte) (uint32, uint32, error) {
241262
devStr := strings.TrimSpace(string(data))
242263
parts := strings.Split(devStr, ":")
243264
if len(parts) != 2 {
244265
return 0, 0, fmt.Errorf("unexpected device string format in %q: %s", devFilePath, devStr)
245266
}
246267

247268
var maj, min uint32
248-
_, err = fmt.Sscanf(devStr, "%d:%d", &maj, &min)
269+
_, err := fmt.Sscanf(devStr, "%d:%d", &maj, &min)
249270
if err != nil {
250271
return 0, 0, errors.Newf("failed parsing device numbers: %v", err) // nolint:errwrap
251272
}

‎pkg/storage/disk/platform_linux_test.go‎

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
//go:build linux
7+
8+
package disk
9+
10+
import (
11+
"context"
12+
"testing"
13+
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestLinux_zfsParseDF(t *testing.T) {
18+
testCases := []struct {
19+
name string
20+
path string
21+
mockOutput string
22+
mockError error
23+
expectName _ZPoolName
24+
expectError bool
25+
}{
26+
{
27+
name: "valid ZFS pool with nested dataset",
28+
path: "/mnt/data1/",
29+
mockOutput: "Filesystem Type\ndata1/crdb1 zfs\n",
30+
expectName: "data1",
31+
},
32+
{
33+
name: "valid ZFS pool without nested dataset",
34+
path: "/mnt/data2/",
35+
mockOutput: "Filesystem Type\ndata2 zfs\n",
36+
expectName: "data2",
37+
},
38+
{
39+
name: "unexpected filesystem type",
40+
path: "/mnt/other/",
41+
mockOutput: "Filesystem Type\n/dev/sda1 ext4\n",
42+
expectError: true,
43+
},
44+
{
45+
name: "unexpected output format",
46+
path: "/mnt/bad/",
47+
mockOutput: "Filesystem\n",
48+
expectError: true,
49+
},
50+
}
51+
52+
for _, tc := range testCases {
53+
t.Run(tc.name, func(t *testing.T) {
54+
poolName, err := zfsParseDF([]byte(tc.mockOutput))
55+
if tc.expectError {
56+
require.Error(t, err)
57+
} else {
58+
require.NoError(t, err)
59+
require.Equal(t, tc.expectName, poolName)
60+
}
61+
})
62+
}
63+
}
64+
65+
func TestLinux_zpoolParseStatus(t *testing.T) {
66+
ctx := context.Background()
67+
68+
testCases := []struct {
69+
name string
70+
poolName _ZPoolName
71+
output string
72+
expectDev string
73+
expectError bool
74+
}{
75+
{
76+
name: "Valid single device",
77+
poolName: "data1",
78+
output: `
79+
pool: data1
80+
state: ONLINE
81+
scan: resilvered 72.9G in 00:18:00 with 0 errors on Mon May 19 19:18:10 2025
82+
config:
83+
84+
NAME STATE READ WRITE CKSUM
85+
data1 ONLINE 0 0 0
86+
/dev/nvme1n1p1 ONLINE 0 0 0
87+
88+
errors: No known data errors
89+
`,
90+
expectDev: "nvme1n1",
91+
},
92+
{
93+
name: "Invalid multiple devices",
94+
poolName: "data1",
95+
output: `
96+
pool: data1
97+
state: ONLINE
98+
status: One or more devices is currently being resilvered.
99+
continue to function, possibly in a degraded state.
100+
action: Wait for the resilver to complete.
101+
scan: resilver in progress since Tue May 20 22:22:02 2025
102+
18.3G / 18.4G scanned, 2.10G / 17.9G issued at 79.8M/s
103+
2.12G resilvered, 11.75% done, 00:03:22 to go
104+
config:
105+
106+
NAME STATE READ WRITE CKSUM
107+
data1 ONLINE 0 0 0
108+
mirror-0 ONLINE 0 0 0
109+
/dev/nvme1n1p1 ONLINE 0 0 0
110+
/dev/nvme5n1p1 ONLINE 0 0 0 (resilvering)
111+
112+
errors: No known data errors
113+
`,
114+
expectDev: "nvme1n1",
115+
expectError: true,
116+
},
117+
{
118+
name: "Invalid output",
119+
poolName: "data1",
120+
output: `
121+
pool: data1
122+
state: ONLINE
123+
status: One or more devices is currently being resilvered.
124+
continue to function, possibly in a degraded state.
125+
action: Wait for the resilver to complete.
126+
scan: resilver in progress since Tue May 20 22:22:02 2025
127+
18.3G / 18.4G scanned, 2.10G / 17.9G issued at 79.8M/s
128+
2.12G resilvered, 11.75% done, 00:03:22 to go
129+
config:
130+
131+
NAME STATE READ WRITE CKSUM
132+
data1 ONLINE 0 0 0
133+
mirror-0 ONLINE 0 0 0
134+
135+
errors: No known data errors
136+
`,
137+
expectDev: "nvme1n1",
138+
expectError: true,
139+
},
140+
}
141+
142+
for _, tc := range testCases {
143+
t.Run(tc.name, func(t *testing.T) {
144+
device, err := zpoolParseStatus(ctx, tc.poolName, []byte(tc.output))
145+
if tc.expectError && device != "" {
146+
require.Error(t, err)
147+
require.Equal(t, tc.expectDev, device)
148+
} else if tc.expectError && device == "" {
149+
require.Error(t, err)
150+
} else {
151+
require.NoError(t, err)
152+
require.Equal(t, tc.expectDev, device)
153+
}
154+
})
155+
}
156+
}
157+
158+
func TestLinux_stripDevicePartition(t *testing.T) {
159+
tests := []struct {
160+
name string
161+
input string
162+
expected string
163+
}{
164+
{"NVME with partition", "/dev/nvme0n1p1", "nvme0n1"},
165+
{"NVME without partition", "/dev/nvme0n1", "nvme0n1"},
166+
{"SCSI with partition", "/dev/sda1", "sda"},
167+
{"SCSI without partition", "/dev/sda", "sda"},
168+
{"RAM device with partition", "/dev/ram0", "ram"},
169+
{"Loop device", "/dev/loop0", "loop"},
170+
{"Invalid device", "/dev/randomdevice", "/dev/randomdevice"},
171+
{"Empty string", "", ""},
172+
{"Device path without prefix", "nvme0n1p3", "nvme0n1"},
173+
{"Complex invalid input", "/dev/nvme0n1p1x", "/dev/nvme0n1p1x"},
174+
}
175+
176+
for _, test := range tests {
177+
t.Run(test.name, func(t *testing.T) {
178+
got := stripDevicePartition(test.input)
179+
require.Equal(t, test.expected, got)
180+
})
181+
}
182+
}
183+
184+
func TestLinux_parseDeviceID(t *testing.T) {
185+
testCases := []struct {
186+
name string
187+
devFilePath string
188+
data []byte
189+
wantMaj uint32
190+
wantMin uint32
191+
wantErr bool
192+
}{
193+
{
194+
name: "valid device numbers",
195+
devFilePath: "/sys/block/sda/dev",
196+
data: []byte("8:0\n"),
197+
wantMaj: 8,
198+
wantMin: 0,
199+
wantErr: false,
200+
},
201+
{
202+
name: "valid device numbers with whitespace",
203+
devFilePath: "/sys/block/nvme1n1/dev",
204+
data: []byte(" 259:5\n"),
205+
wantMaj: 259,
206+
wantMin: 5,
207+
wantErr: false,
208+
},
209+
{
210+
name: "invalid format missing colon",
211+
devFilePath: "/sys/block/sdc/dev",
212+
data: []byte("2593\n"),
213+
wantErr: true,
214+
},
215+
{
216+
name: "non-numeric values",
217+
devFilePath: "/sys/block/sdd/dev",
218+
data: []byte("a:b\n"),
219+
wantErr: true,
220+
},
221+
{
222+
name: "empty data",
223+
devFilePath: "/sys/block/sde/dev",
224+
data: []byte("\n"),
225+
wantErr: true,
226+
},
227+
}
228+
229+
for _, tc := range testCases {
230+
t.Run(tc.name, func(t *testing.T) {
231+
maj, min, err := parseDeviceID(tc.devFilePath, tc.data)
232+
if tc.wantErr {
233+
require.Error(t, err)
234+
} else {
235+
require.NoError(t, err)
236+
require.Equal(t, tc.wantMaj, maj)
237+
require.Equal(t, tc.wantMin, min)
238+
}
239+
})
240+
}
241+
}

0 commit comments

Comments
 (0)