diff --git a/docs/snapshotters/erofs.md b/docs/snapshotters/erofs.md index 358b72a93f..33e0f22d73 100644 --- a/docs/snapshotters/erofs.md +++ b/docs/snapshotters/erofs.md @@ -141,6 +141,13 @@ forget to restart containerd after changing the configuration. # Enable fsverity support for EROFS layers, default is false enable_fsverity = true + # fsync the snapshot directory, everything in it and its entry in + # snapshots/ before a snapshot is committed, so a power loss cannot + # leave a committed snapshot without its blob or directory. Blobs served + # from layer_content_caches are not synced. Costs a few fsyncs per layer, + # default is false + fsync_on_commit = true + # Optional: Additional mount options for overlayfs ovl_mount_options = [] diff --git a/plugins/snapshots/erofs/erofs.go b/plugins/snapshots/erofs/erofs.go index c9fcc84ef2..3d9d926db5 100644 --- a/plugins/snapshots/erofs/erofs.go +++ b/plugins/snapshots/erofs/erofs.go @@ -51,6 +51,9 @@ type SnapshotterConfig struct { enableFsverity bool // setImmutable enables IMMUTABLE_FL file attribute for EROFS layers setImmutable bool + // fsyncOnCommit makes Commit sync the layer blob and the snapshot + // directories before the snapshot is recorded as committed + fsyncOnCommit bool // defaultSize creates a default size writable layer for active snapshots defaultSize int64 remapIDs bool @@ -88,6 +91,14 @@ func WithImmutable() Opt { } } +// WithFsyncOnCommit makes Commit sync the layer blob and the snapshot +// directories to disk before the snapshot is recorded as committed +func WithFsyncOnCommit() Opt { + return func(config *SnapshotterConfig) { + config.fsyncOnCommit = true + } +} + // WithDmverityMode sets the dm-verity mode: "auto" (default), "on" (required), or "off" (disabled) func WithDmverityMode(mode string) Opt { return func(config *SnapshotterConfig) { @@ -131,6 +142,7 @@ type snapshotter struct { ovlOptions []string enableFsverity bool setImmutable bool + fsyncOnCommit bool defaultWritable int64 blockMode bool remapIDs bool @@ -234,6 +246,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, remapIDs: config.remapIDs, @@ -283,6 +296,33 @@ func (s *snapshotter) layerBlobPath(id string) string { return filepath.Join(s.root, "snapshots", id, "layer.erofs") } +// syncDir fsyncs the immediate entries of dir, then dir itself and its parent, +// so that the files in dir, its entry list and its own entry in the parent are +// on disk before the caller records them elsewhere. It does not recurse. +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) fsMetaPath(id string) string { return filepath.Join(s.root, "snapshots", id, "fsmeta.erofs") } @@ -963,6 +1003,22 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap // Note: dm-verity formatting is handled by the EROFS differ, not here + // The metadata store syncs its own file when the snapshot is committed, so + // with fsync_on_commit enabled the layer is made durable first; otherwise + // a power loss shortly after a successful pull can leave a committed + // snapshot whose blob is truncated or whose directory does not exist, and + // nothing re-pulls a layer whose chain ID already exists. Everything in + // the snapshot directory is synced rather than a list of known files, then + // the directory and its entry in snapshots/, which was created by a rename + // in Prepare and may still be unwritten. A blob served from the layer + // content cache lives elsewhere and is not synced; whoever populates the + // cache must make it durable. + 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) diff --git a/plugins/snapshots/erofs/erofs_linux_test.go b/plugins/snapshots/erofs/erofs_linux_test.go index 751f76ab55..08acc89df7 100644 --- a/plugins/snapshots/erofs/erofs_linux_test.go +++ b/plugins/snapshots/erofs/erofs_linux_test.go @@ -143,6 +143,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) +} + // TestWritableSize exercises the LabelSnapshotMaxSize override that the // block-mode mkfs path passes to X-containerd.mkfs.size. Covers the // happy path (label overrides default), fallback cases (missing, empty, diff --git a/plugins/snapshots/erofs/plugin/plugin.go b/plugins/snapshots/erofs/plugin/plugin.go index 960f0c3bfe..f77ed3273d 100644 --- a/plugins/snapshots/erofs/plugin/plugin.go +++ b/plugins/snapshots/erofs/plugin/plugin.go @@ -51,6 +51,13 @@ type Config struct { // If `SetImmutable` is enabled, IMMUTABLE_FL will be set on layer blobs. SetImmutable bool `toml:"set_immutable"` + // FsyncOnCommit makes Commit fsync the snapshot directory, everything in + // it and its entry in snapshots/ before the snapshot is recorded, so that + // a power loss cannot leave a committed snapshot without its blob or + // directory. Blobs served from a layer content cache are not synced. Off + // by default, like the diff service's sync_fs. + FsyncOnCommit bool `toml:"fsync_on_commit"` + // DefaultSize is the default size of a writable layer in string DefaultSize string `toml:"default_size"` @@ -114,6 +121,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 {