Skip to content

Commit b823b4f

Browse files
fuweiddcantah
fuweid
authored and
dcantah
committed
snapshots/blockfile: deflaky the testsuite
* Use direct-io mode to reduce IO. * Add testViewHook helper to recovery the backing file since the ext4 might need writable permission to handle recovery. If the backing file needs recovery and it's for View snapshot, the readonly mount will cause error. * Use 8 MiB as capacity to reduce the IO. Signed-off-by: Wei Fu <[email protected]> (cherry picked from commit 7de95cb) Signed-off-by: Danny Canter <[email protected]>
1 parent 6daeceb commit b823b4f

2 files changed

Lines changed: 125 additions & 27 deletions

File tree

‎snapshots/blockfile/blockfile.go‎

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package blockfile
1919
import (
2020
"context"
2121
"fmt"
22+
"io"
2223
"os"
2324
"path/filepath"
2425

@@ -27,10 +28,11 @@ import (
2728
"github.com/containerd/containerd/plugin"
2829
"github.com/containerd/containerd/snapshots"
2930
"github.com/containerd/containerd/snapshots/storage"
30-
31-
"github.com/containerd/continuity/fs"
3231
)
3332

33+
// viewHookHelper is only used in test for recover the filesystem.
34+
type viewHookHelper func(backingFile string, fsType string, defaultOpts []string) error
35+
3436
// SnapshotterConfig holds the configurable properties for the blockfile snapshotter
3537
type SnapshotterConfig struct {
3638
// recreateScratch is whether scratch should be recreated even
@@ -44,6 +46,17 @@ type SnapshotterConfig struct {
4446

4547
// mountOptions are the base options added to the mount (defaults to ["loop"])
4648
mountOptions []string
49+
50+
// testViewHookHelper is used to fsck or mount with rw to handle
51+
// the recovery. If we mount ro for view snapshot, we might hit
52+
// the issue like
53+
//
54+
// (ext4) INFO: recovery required on readonly filesystem
55+
// (ext4) write access unavailable, cannot proceed (try mounting with noload)
56+
//
57+
// FIXME(fuweid): I don't hit the readonly issue in ssd storage. But it's
58+
// easy to reproduce it in slow-storage.
59+
testViewHookHelper viewHookHelper
4760
}
4861

4962
// Opt is an option to configure the overlay snapshotter
@@ -55,7 +68,7 @@ func WithScratchFile(src string) Opt {
5568
return func(root string, config *SnapshotterConfig) {
5669
config.scratchGenerator = func(dst string) error {
5770
// Copy src to dst
58-
if err := fs.CopyFile(dst, src); err != nil {
71+
if err := copyFileWithSync(dst, src); err != nil {
5972
return fmt.Errorf("failed to copy scratch: %w", err)
6073
}
6174
return nil
@@ -78,12 +91,30 @@ func WithMountOptions(options []string) Opt {
7891

7992
}
8093

94+
// WithRecreateScratch is used to determine that scratch should be recreated
95+
// even if already exists.
96+
func WithRecreateScratch(recreate bool) Opt {
97+
return func(root string, config *SnapshotterConfig) {
98+
config.recreateScratch = recreate
99+
}
100+
}
101+
102+
// withViewHookHelper introduces hook for preparing snapshot for View. It
103+
// should be used in test only.
104+
func withViewHookHelper(fn viewHookHelper) Opt {
105+
return func(_ string, config *SnapshotterConfig) {
106+
config.testViewHookHelper = fn
107+
}
108+
}
109+
81110
type snapshotter struct {
82111
root string
83112
scratch string
84113
fsType string
85114
options []string
86115
ms *storage.MetaStore
116+
117+
testViewHookHelper viewHookHelper
87118
}
88119

89120
// NewSnapshotter returns a Snapshotter which copies layers on the underlying
@@ -140,6 +171,8 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) {
140171
fsType: config.fsType,
141172
options: config.mountOptions,
142173
ms: ms,
174+
175+
testViewHookHelper: config.testViewHookHelper,
143176
}, nil
144177
}
145178

@@ -343,18 +376,27 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
343376
return fmt.Errorf("failed to create snapshot: %w", err)
344377
}
345378

379+
var path string
346380
if len(s.ParentIDs) == 0 || s.Kind == snapshots.KindActive {
347-
path := o.getBlockFile(s.ID)
381+
path = o.getBlockFile(s.ID)
348382

349383
if len(s.ParentIDs) > 0 {
350-
if err = fs.CopyFile(path, o.getBlockFile(s.ParentIDs[0])); err != nil {
384+
if err = copyFileWithSync(path, o.getBlockFile(s.ParentIDs[0])); err != nil {
351385
return fmt.Errorf("copying of parent failed: %w", err)
352386
}
353387
} else {
354-
if err = fs.CopyFile(path, o.scratch); err != nil {
388+
if err = copyFileWithSync(path, o.scratch); err != nil {
355389
return fmt.Errorf("copying of scratch failed: %w", err)
356390
}
357391
}
392+
} else {
393+
path = o.getBlockFile(s.ParentIDs[0])
394+
}
395+
396+
if o.testViewHookHelper != nil {
397+
if err := o.testViewHookHelper(path, o.fsType, o.options); err != nil {
398+
return fmt.Errorf("failed to handle the viewHookHelper: %w", err)
399+
}
358400
}
359401

360402
return nil
@@ -401,3 +443,20 @@ func (o *snapshotter) mounts(s storage.Snapshot) []mount.Mount {
401443
func (o *snapshotter) Close() error {
402444
return o.ms.Close()
403445
}
446+
447+
func copyFileWithSync(target, source string) error {
448+
src, err := os.Open(source)
449+
if err != nil {
450+
return fmt.Errorf("failed to open source %s: %w", source, err)
451+
}
452+
defer src.Close()
453+
tgt, err := os.Create(target)
454+
if err != nil {
455+
return fmt.Errorf("failed to open target %s: %w", target, err)
456+
}
457+
defer tgt.Close()
458+
defer tgt.Sync()
459+
460+
_, err = io.Copy(tgt, src)
461+
return err
462+
}

‎snapshots/blockfile/blockfile_loopsetup_test.go‎

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ import (
2626
"testing"
2727

2828
"github.com/containerd/containerd/mount"
29-
"github.com/containerd/continuity/fs"
30-
"github.com/containerd/continuity/testutil/loopback"
31-
"golang.org/x/sys/unix"
3229
)
3330

3431
func setupSnapshotter(t *testing.T) ([]Opt, error) {
@@ -37,52 +34,94 @@ func setupSnapshotter(t *testing.T) ([]Opt, error) {
3734
t.Skipf("Could not find mkfs.ext4: %v", err)
3835
}
3936

40-
loopbackSize := int64(128 << 20) // 128 MB
37+
loopbackSize := int64(8 << 20) // 8 MB
4138
if os.Getpagesize() > 4096 {
4239
loopbackSize = int64(650 << 20) // 650 MB
4340
}
4441

45-
loop, err := loopback.New(loopbackSize)
42+
scratch := filepath.Join(t.TempDir(), "scratch")
43+
scratchDevFile, err := os.OpenFile(scratch, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600)
4644
if err != nil {
47-
return nil, err
45+
return nil, fmt.Errorf("failed to create %s: %w", scratch, err)
4846
}
49-
defer loop.Close()
5047

51-
if out, err := exec.Command(mkfs, loop.Device).CombinedOutput(); err != nil {
52-
return nil, fmt.Errorf("failed to make ext4 filesystem (out: %q): %w", out, err)
48+
if err := scratchDevFile.Truncate(loopbackSize); err != nil {
49+
scratchDevFile.Close()
50+
return nil, fmt.Errorf("failed to resize %s file: %w", scratch, err)
5351
}
54-
// sync after a mkfs on the loopback before trying to mount the device
55-
unix.Sync()
5652

57-
if err := testMount(t, loop.Device); err != nil {
58-
return nil, err
53+
if err := scratchDevFile.Sync(); err != nil {
54+
scratchDevFile.Close()
55+
return nil, fmt.Errorf("failed to sync %s file: %w", scratch, err)
5956
}
57+
scratchDevFile.Close()
6058

61-
scratch := filepath.Join(t.TempDir(), "scratch")
62-
err = fs.CopyFile(scratch, loop.File)
63-
if err != nil {
59+
if out, err := exec.Command(mkfs, scratch).CombinedOutput(); err != nil {
60+
return nil, fmt.Errorf("failed to make ext4 filesystem (out: %q): %w", out, err)
61+
}
62+
63+
defaultOpts := []string{"loop", "direct-io", "sync"}
64+
65+
if err := testMount(t, scratch, defaultOpts); err != nil {
6466
return nil, err
6567
}
6668

6769
return []Opt{
6870
WithScratchFile(scratch),
6971
WithFSType("ext4"),
70-
WithMountOptions([]string{"loop", "sync"}),
72+
WithMountOptions(defaultOpts),
73+
WithRecreateScratch(false), // reduce IO presure in CI
74+
withViewHookHelper(testViewHook),
7175
}, nil
7276
}
7377

74-
func testMount(t *testing.T, device string) error {
78+
func testMount(t *testing.T, scratchFile string, opts []string) error {
7579
root, err := os.MkdirTemp(t.TempDir(), "")
7680
if err != nil {
7781
return err
7882
}
7983
defer os.RemoveAll(root)
8084

81-
if out, err := exec.Command("mount", device, root).CombinedOutput(); err != nil {
82-
return fmt.Errorf("failed to mount device %s (out: %q): %w", device, out, err)
85+
m := []mount.Mount{
86+
{
87+
Type: "ext4",
88+
Source: scratchFile,
89+
Options: opts,
90+
},
8391
}
92+
93+
if err := mount.All(m, root); err != nil {
94+
return fmt.Errorf("failed to mount device %s: %w", scratchFile, err)
95+
}
96+
8497
if err := os.Remove(filepath.Join(root, "lost+found")); err != nil {
8598
return err
8699
}
87-
return mount.UnmountAll(root, unix.MNT_DETACH)
100+
return mount.UnmountAll(root, 0)
101+
}
102+
103+
func testViewHook(backingFile string, fsType string, defaultOpts []string) error {
104+
root, err := os.MkdirTemp("", "blockfile-testViewHook-XXXX")
105+
if err != nil {
106+
return err
107+
}
108+
defer os.RemoveAll(root)
109+
110+
// FIXME(fuweid): Mount with rw to force fs to handle recover
111+
mountOpts := []mount.Mount{
112+
{
113+
Type: fsType,
114+
Source: backingFile,
115+
Options: defaultOpts,
116+
},
117+
}
118+
119+
if err := mount.All(mountOpts, root); err != nil {
120+
return fmt.Errorf("failed to mount device %s: %w", backingFile, err)
121+
}
122+
123+
if err := mount.UnmountAll(root, 0); err != nil {
124+
return fmt.Errorf("failed to unmount device %s: %w", backingFile, err)
125+
}
126+
return nil
88127
}

0 commit comments

Comments
 (0)