diff --git a/snapshots/blockfile/blockfile.go b/snapshots/blockfile/blockfile.go index dd6257936..16666f199 100644 --- a/snapshots/blockfile/blockfile.go +++ b/snapshots/blockfile/blockfile.go @@ -19,6 +19,7 @@ package blockfile import ( "context" "fmt" + "io" "os" "path/filepath" @@ -27,10 +28,11 @@ import ( "github.com/containerd/containerd/plugin" "github.com/containerd/containerd/snapshots" "github.com/containerd/containerd/snapshots/storage" - - "github.com/containerd/continuity/fs" ) +// viewHookHelper is only used in test for recover the filesystem. +type viewHookHelper func(backingFile string, fsType string, defaultOpts []string) error + // SnapshotterConfig holds the configurable properties for the blockfile snapshotter type SnapshotterConfig struct { // recreateScratch is whether scratch should be recreated even @@ -44,6 +46,17 @@ type SnapshotterConfig struct { // mountOptions are the base options added to the mount (defaults to ["loop"]) mountOptions []string + + // testViewHookHelper is used to fsck or mount with rw to handle + // the recovery. If we mount ro for view snapshot, we might hit + // the issue like + // + // (ext4) INFO: recovery required on readonly filesystem + // (ext4) write access unavailable, cannot proceed (try mounting with noload) + // + // FIXME(fuweid): I don't hit the readonly issue in ssd storage. But it's + // easy to reproduce it in slow-storage. + testViewHookHelper viewHookHelper } // Opt is an option to configure the overlay snapshotter @@ -55,7 +68,7 @@ func WithScratchFile(src string) Opt { return func(root string, config *SnapshotterConfig) { config.scratchGenerator = func(dst string) error { // Copy src to dst - if err := fs.CopyFile(dst, src); err != nil { + if err := copyFileWithSync(dst, src); err != nil { return fmt.Errorf("failed to copy scratch: %w", err) } return nil @@ -78,12 +91,30 @@ func WithMountOptions(options []string) Opt { } +// WithRecreateScratch is used to determine that scratch should be recreated +// even if already exists. +func WithRecreateScratch(recreate bool) Opt { + return func(root string, config *SnapshotterConfig) { + config.recreateScratch = recreate + } +} + +// withViewHookHelper introduces hook for preparing snapshot for View. It +// should be used in test only. +func withViewHookHelper(fn viewHookHelper) Opt { + return func(_ string, config *SnapshotterConfig) { + config.testViewHookHelper = fn + } +} + type snapshotter struct { root string scratch string fsType string options []string ms *storage.MetaStore + + testViewHookHelper viewHookHelper } // NewSnapshotter returns a Snapshotter which copies layers on the underlying @@ -140,6 +171,8 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { fsType: config.fsType, options: config.mountOptions, ms: ms, + + testViewHookHelper: config.testViewHookHelper, }, nil } @@ -343,18 +376,27 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return fmt.Errorf("failed to create snapshot: %w", err) } + var path string if len(s.ParentIDs) == 0 || s.Kind == snapshots.KindActive { - path := o.getBlockFile(s.ID) + path = o.getBlockFile(s.ID) if len(s.ParentIDs) > 0 { - if err = fs.CopyFile(path, o.getBlockFile(s.ParentIDs[0])); err != nil { + if err = copyFileWithSync(path, o.getBlockFile(s.ParentIDs[0])); err != nil { return fmt.Errorf("copying of parent failed: %w", err) } } else { - if err = fs.CopyFile(path, o.scratch); err != nil { + if err = copyFileWithSync(path, o.scratch); err != nil { return fmt.Errorf("copying of scratch failed: %w", err) } } + } else { + path = o.getBlockFile(s.ParentIDs[0]) + } + + if o.testViewHookHelper != nil { + if err := o.testViewHookHelper(path, o.fsType, o.options); err != nil { + return fmt.Errorf("failed to handle the viewHookHelper: %w", err) + } } return nil @@ -401,3 +443,20 @@ func (o *snapshotter) mounts(s storage.Snapshot) []mount.Mount { func (o *snapshotter) Close() error { return o.ms.Close() } + +func copyFileWithSync(target, source string) error { + src, err := os.Open(source) + if err != nil { + return fmt.Errorf("failed to open source %s: %w", source, err) + } + defer src.Close() + tgt, err := os.Create(target) + if err != nil { + return fmt.Errorf("failed to open target %s: %w", target, err) + } + defer tgt.Close() + defer tgt.Sync() + + _, err = io.Copy(tgt, src) + return err +} diff --git a/snapshots/blockfile/blockfile_loopsetup_test.go b/snapshots/blockfile/blockfile_loopsetup_test.go index 06b4dd306..bda55f58b 100644 --- a/snapshots/blockfile/blockfile_loopsetup_test.go +++ b/snapshots/blockfile/blockfile_loopsetup_test.go @@ -26,9 +26,6 @@ import ( "testing" "github.com/containerd/containerd/mount" - "github.com/containerd/continuity/fs" - "github.com/containerd/continuity/testutil/loopback" - "golang.org/x/sys/unix" ) func setupSnapshotter(t *testing.T) ([]Opt, error) { @@ -37,52 +34,94 @@ func setupSnapshotter(t *testing.T) ([]Opt, error) { t.Skipf("Could not find mkfs.ext4: %v", err) } - loopbackSize := int64(128 << 20) // 128 MB + loopbackSize := int64(8 << 20) // 8 MB if os.Getpagesize() > 4096 { loopbackSize = int64(650 << 20) // 650 MB } - loop, err := loopback.New(loopbackSize) + scratch := filepath.Join(t.TempDir(), "scratch") + scratchDevFile, err := os.OpenFile(scratch, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to create %s: %w", scratch, err) } - defer loop.Close() - if out, err := exec.Command(mkfs, loop.Device).CombinedOutput(); err != nil { + if err := scratchDevFile.Truncate(loopbackSize); err != nil { + scratchDevFile.Close() + return nil, fmt.Errorf("failed to resize %s file: %w", scratch, err) + } + + if err := scratchDevFile.Sync(); err != nil { + scratchDevFile.Close() + return nil, fmt.Errorf("failed to sync %s file: %w", scratch, err) + } + scratchDevFile.Close() + + if out, err := exec.Command(mkfs, scratch).CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to make ext4 filesystem (out: %q): %w", out, err) } - // sync after a mkfs on the loopback before trying to mount the device - unix.Sync() - if err := testMount(t, loop.Device); err != nil { - return nil, err - } + defaultOpts := []string{"loop", "direct-io", "sync"} - scratch := filepath.Join(t.TempDir(), "scratch") - err = fs.CopyFile(scratch, loop.File) - if err != nil { + if err := testMount(t, scratch, defaultOpts); err != nil { return nil, err } return []Opt{ WithScratchFile(scratch), WithFSType("ext4"), - WithMountOptions([]string{"loop", "sync"}), + WithMountOptions(defaultOpts), + WithRecreateScratch(false), // reduce IO presure in CI + withViewHookHelper(testViewHook), }, nil } -func testMount(t *testing.T, device string) error { +func testMount(t *testing.T, scratchFile string, opts []string) error { root, err := os.MkdirTemp(t.TempDir(), "") if err != nil { return err } defer os.RemoveAll(root) - if out, err := exec.Command("mount", device, root).CombinedOutput(); err != nil { - return fmt.Errorf("failed to mount device %s (out: %q): %w", device, out, err) + m := []mount.Mount{ + { + Type: "ext4", + Source: scratchFile, + Options: opts, + }, } + + if err := mount.All(m, root); err != nil { + return fmt.Errorf("failed to mount device %s: %w", scratchFile, err) + } + if err := os.Remove(filepath.Join(root, "lost+found")); err != nil { return err } - return mount.UnmountAll(root, unix.MNT_DETACH) + return mount.UnmountAll(root, 0) +} + +func testViewHook(backingFile string, fsType string, defaultOpts []string) error { + root, err := os.MkdirTemp("", "blockfile-testViewHook-XXXX") + if err != nil { + return err + } + defer os.RemoveAll(root) + + // FIXME(fuweid): Mount with rw to force fs to handle recover + mountOpts := []mount.Mount{ + { + Type: fsType, + Source: backingFile, + Options: defaultOpts, + }, + } + + if err := mount.All(mountOpts, root); err != nil { + return fmt.Errorf("failed to mount device %s: %w", backingFile, err) + } + + if err := mount.UnmountAll(root, 0); err != nil { + return fmt.Errorf("failed to unmount device %s: %w", backingFile, err) + } + return nil }