Skip to content

Commit 3cb378d

Browse files
authored
nydusify commit: fix mount option parsing in manager.go (#19)
The Inspect() function parsed snapshot mount options by hardcoded array index, assuming Options[0]=workdir, Options[1]=upperdir, Options[2]=lowerdir. The nydus snapshotter can return a "volatile" flag at index 0, shifting all other options by one position. This produced malformed overlay mount data ("lowerdir=upperdir=/path:...") causing "no such file or directory" errors. Replace hardcoded indices with prefix-based iteration, matching the pattern already used in overlay_linux.go:GetOverlayLayers. Extract parseMountOptions() helper with unit tests covering volatile prefix, different orderings, and error cases.
1 parent 3d01b05 commit 3cb378d

2 files changed

Lines changed: 123 additions & 5 deletions

File tree

contrib/nydusify/pkg/committer/manager.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,14 @@ func (m *Manager) Inspect(ctx context.Context, containerID string) (*InspectResu
109109
}
110110

111111
snapshot := client.SnapshotService("nydus")
112-
lowerDirs := ""
113-
upperDir := ""
114112
mount, err := snapshot.Mounts(ctx, containerInfo.SnapshotKey)
115113
if err != nil {
116114
return nil, errors.Wrapf(err, "get snapshot mount")
117115
}
118-
// snapshot Mount Options[0] "workdir=$workdir", Options[1] "upperdir=$upperdir", Options[2] "lowerdir=$lowerdir".
119-
lowerDirs = strings.TrimPrefix(mount[0].Options[2], "lowerdir=")
120-
upperDir = strings.TrimPrefix(mount[0].Options[1], "upperdir=")
116+
lowerDirs, upperDir, err := parseMountOptions(mount[0].Options)
117+
if err != nil {
118+
return nil, errors.Wrapf(err, "parse snapshot mount options")
119+
}
121120

122121
return &InspectResult{
123122
LowerDirs: lowerDirs,
@@ -127,3 +126,17 @@ func (m *Manager) Inspect(ctx context.Context, containerID string) (*InspectResu
127126
Pid: pid,
128127
}, nil
129128
}
129+
130+
func parseMountOptions(options []string) (lowerDirs, upperDir string, err error) {
131+
for _, opt := range options {
132+
if strings.HasPrefix(opt, "lowerdir=") {
133+
lowerDirs = strings.TrimPrefix(opt, "lowerdir=")
134+
} else if strings.HasPrefix(opt, "upperdir=") {
135+
upperDir = strings.TrimPrefix(opt, "upperdir=")
136+
}
137+
}
138+
if lowerDirs == "" || upperDir == "" {
139+
return "", "", errors.Errorf("snapshot mount missing lowerdir or upperdir in options: %v", options)
140+
}
141+
return lowerDirs, upperDir, nil
142+
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package committer
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestParseMountOptions(t *testing.T) {
11+
tests := []struct {
12+
Name string
13+
Options []string
14+
WantLower string
15+
WantUpper string
16+
WantErr bool
17+
ErrSubstring string
18+
}{
19+
{
20+
Name: "StandardOrder",
21+
Options: []string{
22+
"workdir=/var/lib/containerd/snapshots/123/work",
23+
"upperdir=/var/lib/containerd/snapshots/123/fs",
24+
"lowerdir=/var/lib/containerd/snapshots/100/mnt",
25+
},
26+
WantLower: "/var/lib/containerd/snapshots/100/mnt",
27+
WantUpper: "/var/lib/containerd/snapshots/123/fs",
28+
},
29+
{
30+
Name: "WithVolatilePrefix",
31+
Options: []string{
32+
"volatile",
33+
"workdir=/var/lib/containerd/io.containerd.snapshotter.v1.nydus/snapshots/1443/work",
34+
"upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.nydus/snapshots/1443/fs",
35+
"lowerdir=/var/lib/containerd/io.containerd.snapshotter.v1.nydus/snapshots/282/mnt",
36+
},
37+
WantLower: "/var/lib/containerd/io.containerd.snapshotter.v1.nydus/snapshots/282/mnt",
38+
WantUpper: "/var/lib/containerd/io.containerd.snapshotter.v1.nydus/snapshots/1443/fs",
39+
},
40+
{
41+
Name: "ReversedOrder",
42+
Options: []string{
43+
"lowerdir=/lower",
44+
"upperdir=/upper",
45+
"workdir=/work",
46+
},
47+
WantLower: "/lower",
48+
WantUpper: "/upper",
49+
},
50+
{
51+
Name: "MultipleLowerDirs",
52+
Options: []string{
53+
"workdir=/work",
54+
"upperdir=/upper",
55+
"lowerdir=/lower1:/lower2:/lower3",
56+
},
57+
WantLower: "/lower1:/lower2:/lower3",
58+
WantUpper: "/upper",
59+
},
60+
{
61+
Name: "WithIndexOff",
62+
Options: []string{
63+
"volatile",
64+
"index=off",
65+
"workdir=/work",
66+
"upperdir=/upper",
67+
"lowerdir=/lower",
68+
},
69+
WantLower: "/lower",
70+
WantUpper: "/upper",
71+
},
72+
{
73+
Name: "MissingLowerDir",
74+
Options: []string{"workdir=/work", "upperdir=/upper"},
75+
WantErr: true,
76+
ErrSubstring: "missing lowerdir or upperdir",
77+
},
78+
{
79+
Name: "MissingUpperDir",
80+
Options: []string{"workdir=/work", "lowerdir=/lower"},
81+
WantErr: true,
82+
ErrSubstring: "missing lowerdir or upperdir",
83+
},
84+
{
85+
Name: "EmptyOptions",
86+
Options: []string{},
87+
WantErr: true,
88+
ErrSubstring: "missing lowerdir or upperdir",
89+
},
90+
}
91+
92+
for _, tt := range tests {
93+
t.Run(tt.Name, func(t *testing.T) {
94+
lowerDirs, upperDir, err := parseMountOptions(tt.Options)
95+
if tt.WantErr {
96+
require.Error(t, err)
97+
assert.Contains(t, err.Error(), tt.ErrSubstring)
98+
return
99+
}
100+
require.NoError(t, err)
101+
assert.Equal(t, tt.WantLower, lowerDirs)
102+
assert.Equal(t, tt.WantUpper, upperDir)
103+
})
104+
}
105+
}

0 commit comments

Comments
 (0)