Merge pull request #14396 from mike-sul/backport/release-2.2-fsync-on-commit

[release/2.2] erofs: add fsync_on_commit to sync a layer before its commit

docs/snapshotters/erofs.md

@@ -98,6 +98,10 @@ forget to restart containerd after changing the configuration.
       # Enable fsverity support for EROFS layers, default is false
       enable_fsverity = true
 
+      # Enable fsyncing a snapshot's directory before the snapshot is committed,
+      # default is false
+      fsync_on_commit = true
+
       # Optional: Additional mount options for overlayfs
       ovl_mount_options = []
 

plugins/snapshots/erofs/erofs.go

@@ -44,6 +44,8 @@ type SnapshotterConfig struct {
 	enableFsverity bool
 	// setImmutable enables IMMUTABLE_FL file attribute for EROFS layers
 	setImmutable bool
+	// fsyncOnCommit enables fsyncing a snapshot's directory before the snapshot is committed
+	fsyncOnCommit bool
 	// defaultSize creates a default size writable layer for active snapshots
 	defaultSize int64
 }
@@ -72,6 +74,13 @@ func WithImmutable() Opt {
 	}
 }
 
+// WithFsyncOnCommit enables fsyncing a snapshot's directory before the snapshot is committed
+func WithFsyncOnCommit() Opt {
+	return func(config *SnapshotterConfig) {
+		config.fsyncOnCommit = true
+	}
+}
+
 // WithDefaultSize creates a default size writable layer for active snapshots
 func WithDefaultSize(size int64) Opt {
 	return func(config *SnapshotterConfig) {
@@ -91,6 +100,7 @@ type snapshotter struct {
 	ovlOptions      []string
 	enableFsverity  bool
 	setImmutable    bool
+	fsyncOnCommit   bool
 	defaultWritable int64
 	blockMode       bool
 }
@@ -147,6 +157,7 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) {
 		ovlOptions:      config.ovlOptions,
 		enableFsverity:  config.enableFsverity,
 		setImmutable:    config.setImmutable,
+		fsyncOnCommit:   config.fsyncOnCommit,
 		defaultWritable: config.defaultSize,
 		blockMode:       config.defaultSize > 0,
 	}, nil
@@ -174,6 +185,31 @@ func (s *snapshotter) layerBlobPath(id string) string {
 	return filepath.Join(s.root, "snapshots", id, "layer.erofs")
 }
 
+// syncDir fsyncs the entries of dir, then dir and its parent.
+func syncDir(dir string) error {
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return fmt.Errorf("failed to read %s for sync: %w", dir, err)
+	}
+	paths := make([]string, 0, len(entries)+2)
+	for _, e := range entries {
+		paths = append(paths, filepath.Join(dir, e.Name()))
+	}
+	paths = append(paths, dir, filepath.Dir(dir))
+	for _, p := range paths {
+		f, err := os.Open(p)
+		if err != nil {
+			return fmt.Errorf("failed to open %s for sync: %w", p, err)
+		}
+		err = f.Sync()
+		f.Close()
+		if err != nil {
+			return fmt.Errorf("failed to sync %s: %w", p, err)
+		}
+	}
+	return nil
+}
+
 func (s *snapshotter) lowerPath(id string) (string, error) {
 	layerBlob := s.layerBlobPath(id)
 	if _, err := os.Stat(layerBlob); err != nil {
@@ -492,6 +528,12 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap
 		}
 	}
 
+	if s.fsyncOnCommit {
+		if err := syncDir(filepath.Join(s.root, "snapshots", id)); err != nil {
+			return err
+		}
+	}
+
 	return s.ms.WithTransaction(ctx, true, func(ctx context.Context) error {
 		if _, err := os.Stat(layerBlob); err != nil {
 			return fmt.Errorf("failed to get the converted erofs blob: %w", err)

plugins/snapshots/erofs/erofs_linux_test.go

@@ -137,6 +137,21 @@ func TestGetCleanupDirectoriesSkipsSnapshotTempDirs(t *testing.T) {
 	assert.Equal(t, []string{orphanDir}, cleanup)
 }
 
+// TestSyncDir checks that a snapshot directory holding files and
+// subdirectories can be synced, and that a directory that cannot be read
+// returns an error instead of being skipped.
+func TestSyncDir(t *testing.T) {
+	root := t.TempDir()
+	snapshotDir := filepath.Join(root, "snapshots", "1")
+	require.NoError(t, os.MkdirAll(filepath.Join(snapshotDir, "fs"), 0700))
+	require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, "layer.erofs"), []byte("erofs"), 0644))
+
+	require.NoError(t, syncDir(snapshotDir))
+
+	err := syncDir(filepath.Join(root, "snapshots", "missing"))
+	require.ErrorIs(t, err, os.ErrNotExist)
+}
+
 func TestErofsFsverity(t *testing.T) {
 	testutil.RequiresRoot(t)
 	ctx := context.Background()

plugins/snapshots/erofs/plugin/plugin.go

@@ -44,6 +44,9 @@ type Config struct {
 	// If `SetImmutable` is enabled, IMMUTABLE_FL will be set on layer blobs.
 	SetImmutable bool `toml:"set_immutable"`
 
+	// FsyncOnCommit enables fsyncing a snapshot's directory before the snapshot is committed
+	FsyncOnCommit bool `toml:"fsync_on_commit"`
+
 	// DefaultSize is the default size of a writable layer in string
 	DefaultSize string `toml:"default_size"`
 }
@@ -79,6 +82,10 @@ func init() {
 				opts = append(opts, erofs.WithImmutable())
 			}
 
+			if config.FsyncOnCommit {
+				opts = append(opts, erofs.WithFsyncOnCommit())
+			}
+
 			if config.DefaultSize != "" {
 				size, err := units.RAMInBytes(config.DefaultSize)
 				if err != nil {