From b024e301464a8363b08bd0d977e8daf21032f5c6 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 13 Sep 2023 15:22:50 +0200 Subject: [PATCH 1/5] overlay: do not append empty option if d.options.mountOptions is the empty string, we should not append an empty option. Signed-off-by: Giuseppe Scrivano --- drivers/overlay/overlay.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 0f6d74021f..91ca273de0 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -1447,7 +1447,9 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO needsIDMapping := !disableShifting && len(options.UidMaps) > 0 && len(options.GidMaps) > 0 && d.options.mountProgram == "" if len(optsList) == 0 { - optsList = strings.Split(d.options.mountOptions, ",") + if d.options.mountOptions != "" { + optsList = strings.Split(d.options.mountOptions, ",") + } } else { // If metacopy=on is present in d.options.mountOptions it must be present in the mount // options otherwise the kernel refuses to follow the metacopy xattr. From 6dadae0f0df6f81535525fcb648fed00f6d8ef60 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 13 Sep 2023 15:36:27 +0200 Subject: [PATCH 2/5] overlay, composefs: use data-only lower layers use the new overlay data-only feature to mount the composefs data directory so there is no need for upper layers to create whiteouts to hide payload files. The feature was added to Linux 6.5. Signed-off-by: Giuseppe Scrivano --- drivers/overlay/check.go | 33 +++++++++++++++++++++++ drivers/overlay/mount.go | 21 ++++++++++++--- drivers/overlay/overlay.go | 54 +++++++++++++++++++++++++++++++------- 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/drivers/overlay/check.go b/drivers/overlay/check.go index 60980994b2..d8139f6566 100644 --- a/drivers/overlay/check.go +++ b/drivers/overlay/check.go @@ -275,3 +275,36 @@ func supportsIdmappedLowerLayers(home string) (bool, error) { }() return true, nil } + +// supportsDataOnlyLayers checks if the kernel supports mounting a overlay file system +// that uses data-only layers. +func supportsDataOnlyLayers(home string) (bool, error) { + layerDir, err := os.MkdirTemp(home, "compat") + if err != nil { + return false, err + } + defer func() { + _ = os.RemoveAll(layerDir) + }() + + mergedDir := filepath.Join(layerDir, "merged") + lowerDir := filepath.Join(layerDir, "lower") + lowerDirDataOnly := filepath.Join(layerDir, "lower-data") + upperDir := filepath.Join(layerDir, "upper") + workDir := filepath.Join(layerDir, "work") + + _ = idtools.MkdirAs(mergedDir, 0o700, 0, 0) + _ = idtools.MkdirAs(lowerDir, 0o700, 0, 0) + _ = idtools.MkdirAs(lowerDirDataOnly, 0o700, 0, 0) + _ = idtools.MkdirAs(upperDir, 0o700, 0, 0) + _ = idtools.MkdirAs(workDir, 0o700, 0, 0) + + opts := fmt.Sprintf("lowerdir=%s::%s,upperdir=%s,workdir=%s,metacopy=on", lowerDir, lowerDirDataOnly, upperDir, workDir) + flags := uintptr(0) + if err := unix.Mount("overlay", mergedDir, "overlay", flags, opts); err != nil { + return false, err + } + _ = unix.Unmount(mergedDir, unix.MNT_DETACH) + + return true, nil +} diff --git a/drivers/overlay/mount.go b/drivers/overlay/mount.go index 33e60b1189..8829e55e98 100644 --- a/drivers/overlay/mount.go +++ b/drivers/overlay/mount.go @@ -141,14 +141,27 @@ func mountOverlayFromMain() { // the new value for the list of lowers, because it's shorter. if lowerv != "" { lowers := strings.Split(lowerv, ":") - for i := range lowers { - lowerFd, err := unix.Open(lowers[i], unix.O_RDONLY, 0) + var newLowers []string + dataOnly := false + for _, lowerPath := range lowers { + if lowerPath == "" { + dataOnly = true + continue + } + lowerFd, err := unix.Open(lowerPath, unix.O_RDONLY, 0) if err != nil { fatal(err) } - lowers[i] = fmt.Sprintf("%d", lowerFd) + var lower string + if dataOnly { + lower = fmt.Sprintf(":%d", lowerFd) + dataOnly = false + } else { + lower = fmt.Sprintf("%d", lowerFd) + } + newLowers = append(newLowers, lower) } - lowerv = strings.Join(lowers, ":") + lowerv = strings.Join(newLowers, ":") } // Reconstruct the Label field. diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 91ca273de0..525655331d 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -1542,7 +1542,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO } }() - maybeAddComposefsMount := func(lowerID string, i int) (string, error) { + maybeAddComposefsMount := func(lowerID string, i int, readWrite bool) (string, error) { composefsBlob := d.getComposefsData(lowerID) _, err = os.Stat(composefsBlob) if err != nil { @@ -1553,6 +1553,10 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO } logrus.Debugf("overlay: using composefs blob %s for lower %s", composefsBlob, lowerID) + if readWrite && i == 0 { + return "", fmt.Errorf("cannot mount a composefs layer as writeable") + } + dest := filepath.Join(composefsLayers, fmt.Sprintf("%d", i)) if err := os.MkdirAll(dest, 0o700); err != nil { return "", err @@ -1573,7 +1577,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO diffDir := path.Join(workDirBase, "diff") - if dest, err := maybeAddComposefsMount(id, 0); err != nil { + if dest, err := maybeAddComposefsMount(id, 0, readWrite); err != nil { return "", err } else if dest != "" { diffDir = dest @@ -1625,7 +1629,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO return "", err } lowerID := filepath.Base(filepath.Dir(linkContent)) - composefsMount, err := maybeAddComposefsMount(lowerID, i+1) + composefsMount, err := maybeAddComposefsMount(lowerID, i+1, readWrite) if err != nil { return "", err } @@ -1657,8 +1661,6 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO optsList = append(optsList, "metacopy=on", "redirect_dir=on") } - absLowers = append(absLowers, composeFsLayers...) - if len(absLowers) == 0 { absLowers = append(absLowers, path.Join(dir, "empty")) } @@ -1752,11 +1754,20 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO absLowers = newAbsDir } + lowerDirs := strings.Join(absLowers, ":") + if len(composeFsLayers) > 0 { + composeFsLayersLowerDirs := strings.Join(composeFsLayers, "::") + lowerDirs = lowerDirs + "::" + composeFsLayersLowerDirs + } + // absLowers is not valid anymore now as we have added composeFsLayers to it, so prevent + // its usage. + absLowers = nil //nolint:ineffassign + var opts string if readWrite { - opts = fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", strings.Join(absLowers, ":"), diffDir, workdir) + opts = fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", lowerDirs, diffDir, workdir) } else { - opts = fmt.Sprintf("lowerdir=%s:%s", diffDir, strings.Join(absLowers, ":")) + opts = fmt.Sprintf("lowerdir=%s:%s", diffDir, lowerDirs) } if len(optsList) > 0 { opts = fmt.Sprintf("%s,%s", opts, strings.Join(optsList, ",")) @@ -1800,9 +1811,9 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO if readWrite { diffDir := path.Join(id, "diff") workDir := path.Join(id, "work") - opts = fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", strings.Join(absLowers, ":"), diffDir, workDir) + opts = fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", lowerDirs, diffDir, workDir) } else { - opts = fmt.Sprintf("lowerdir=%s:%s", diffDir, strings.Join(absLowers, ":")) + opts = fmt.Sprintf("lowerdir=%s:%s", diffDir, lowerDirs) } if len(optsList) > 0 { opts = strings.Join(append([]string{opts}, optsList...), ",") @@ -2009,11 +2020,34 @@ func (d *Driver) CleanupStagingDirectory(stagingDirectory string) error { return os.RemoveAll(stagingDirectory) } +func (d *Driver) supportsDataOnlyLayers() (bool, error) { + feature := "dataonly-layers" + overlayCacheResult, overlayCacheText, err := cachedFeatureCheck(d.runhome, feature) + if err == nil { + if overlayCacheResult { + logrus.Debugf("Cached value indicated that data-only layers for overlay are supported") + return true, nil + } + logrus.Debugf("Cached value indicated that data-only layers for overlay are not supported") + return false, errors.New(overlayCacheText) + } + supportsDataOnly, err := supportsDataOnlyLayers(d.home) + if err2 := cachedFeatureRecord(d.runhome, feature, supportsDataOnly, ""); err2 != nil { + return false, fmt.Errorf("recording overlay data-only layers support status: %w", err2) + } + return supportsDataOnly, err +} + func (d *Driver) useComposeFs() bool { if !composeFsSupported() || unshare.IsRootless() { return false } - return true + supportsDataOnlyLayers, err := d.supportsDataOnlyLayers() + if err != nil { + logrus.Debugf("Check for data-only layers failed with: %v", err) + return false + } + return supportsDataOnlyLayers } // ApplyDiff applies the changes in the new layer using the specified function From 1dd7a5516d3c37f00f453d68e29816e78a830cb9 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 19 Sep 2023 13:42:56 +0200 Subject: [PATCH 3/5] overlay, composefs: include file path in the error Signed-off-by: Giuseppe Scrivano --- drivers/overlay/composefs_supported.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/overlay/composefs_supported.go b/drivers/overlay/composefs_supported.go index 58c15eac46..2f45c0f685 100644 --- a/drivers/overlay/composefs_supported.go +++ b/drivers/overlay/composefs_supported.go @@ -93,7 +93,7 @@ func generateComposeFsBlob(toc []byte, composefsDir string) error { fd, err := unix.Openat(unix.AT_FDCWD, destFile, unix.O_WRONLY|unix.O_CREAT|unix.O_TRUNC|unix.O_EXCL|unix.O_CLOEXEC, 0o644) if err != nil { - return fmt.Errorf("failed to open output file: %w", err) + return fmt.Errorf("failed to open output file %q: %w", destFile, err) } outFd := os.NewFile(uintptr(fd), "outFd") From c40dde3f9ec08a6b222189990cb4566c1d91e7fc Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 19 Sep 2023 13:43:19 +0200 Subject: [PATCH 4/5] cmd: add applydiff-using-staging-dir add a new command to exercise the ApplyDiff from a staging directory. Signed-off-by: Giuseppe Scrivano --- cmd/containers-storage/diff.go | 100 ++++++++++++++++++ ...ers-storage-applydiff-using-staging-dir.md | 27 +++++ docs/containers-storage.md | 76 ++++++------- 3 files changed, 166 insertions(+), 37 deletions(-) create mode 100644 docs/containers-storage-applydiff-using-staging-dir.md diff --git a/cmd/containers-storage/diff.go b/cmd/containers-storage/diff.go index f1d8f97fa4..2ae30f5d4c 100644 --- a/cmd/containers-storage/diff.go +++ b/cmd/containers-storage/diff.go @@ -1,12 +1,17 @@ package main import ( + "context" "fmt" "io" "os" "github.com/containers/storage" + graphdriver "github.com/containers/storage/drivers" "github.com/containers/storage/pkg/archive" + "github.com/containers/storage/pkg/chunked" + "github.com/containers/storage/pkg/chunked/compressor" + "github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/mflag" ) @@ -96,6 +101,93 @@ func diff(flags *mflag.FlagSet, action string, m storage.Store, args []string) ( return 0, nil } +type fileFetcher struct { + file *os.File +} + +func sendFileParts(f *fileFetcher, chunks []chunked.ImageSourceChunk, streams chan io.ReadCloser, errors chan error) { + defer close(streams) + defer close(errors) + + for _, chunk := range chunks { + l := io.NewSectionReader(f.file, int64(chunk.Offset), int64(chunk.Length)) + streams <- ioutils.NewReadCloserWrapper(l, func() error { + return nil + }) + } +} + +func (f fileFetcher) GetBlobAt(chunks []chunked.ImageSourceChunk) (chan io.ReadCloser, chan error, error) { + streams := make(chan io.ReadCloser) + errs := make(chan error) + go sendFileParts(&f, chunks, streams, errs) + return streams, errs, nil +} + +func applyDiffUsingStagingDirectory(flags *mflag.FlagSet, action string, m storage.Store, args []string) (int, error) { + if len(args) < 2 { + return 2, nil + } + + layer := args[0] + sourceDirectory := args[1] + + tOptions := archive.TarOptions{} + tr, err := archive.TarWithOptions(sourceDirectory, &tOptions) + if err != nil { + return 1, err + } + defer tr.Close() + + tar, err := os.CreateTemp("", "layer-diff-tar-") + if err != nil { + return 1, err + } + defer os.Remove(tar.Name()) + defer tar.Close() + + // we go through the zstd:chunked compressor first so that it generates the metadata required to mount + // a composefs image. + + metadata := make(map[string]string) + compressor, err := compressor.ZstdCompressor(tar, metadata, nil) + if err != nil { + return 1, err + } + + if _, err := io.Copy(compressor, tr); err != nil { + return 1, err + } + if err := compressor.Close(); err != nil { + return 1, err + } + + size, err := tar.Seek(0, io.SeekCurrent) + if err != nil { + return 1, err + } + + fetcher := fileFetcher{ + file: tar, + } + + differ, err := chunked.GetDiffer(context.Background(), m, size, metadata, &fetcher) + if err != nil { + return 1, err + } + + var options graphdriver.ApplyDiffOpts + out, err := m.ApplyDiffWithDiffer("", &options, differ) + if err != nil { + return 1, err + } + if err := m.ApplyDiffFromStagingDirectory(layer, out.Target, out, &options); err != nil { + m.CleanupStagingDirectory(out.Target) + return 1, err + } + return 0, nil +} + func applyDiff(flags *mflag.FlagSet, action string, m storage.Store, args []string) (int, error) { if len(args) < 1 { return 1, nil @@ -179,4 +271,12 @@ func init() { flags.StringVar(&applyDiffFile, []string{"-file", "f"}, "", "Read from file instead of stdin") }, }) + commands = append(commands, command{ + names: []string{"applydiff-using-staging-dir"}, + optionsHelp: "layerNameOrID directory", + usage: "Apply a diff to a layer using a staging directory", + minArgs: 2, + maxArgs: 2, + action: applyDiffUsingStagingDirectory, + }) } diff --git a/docs/containers-storage-applydiff-using-staging-dir.md b/docs/containers-storage-applydiff-using-staging-dir.md new file mode 100644 index 0000000000..c7fcc4c1f0 --- /dev/null +++ b/docs/containers-storage-applydiff-using-staging-dir.md @@ -0,0 +1,27 @@ +## containers-storage-applydiff-using-staging-dir 1 "September 2023" + +## NAME +containers-storage applydiff-using-staging-dir - Apply a layer diff to a layer using a staging directory + +## SYNOPSIS +**containers-storage** **applydiff-using-staging-dir** *layerNameOrID* *source* + +## DESCRIPTION +When a layer is first created, it contains no changes relative to its parent +layer. The layer can either be mounted read-write and its contents modified +directly, or contents can be added (or removed) by applying a layer diff. A +layer diff takes the form of a (possibly compressed) tar archive with +additional information present in its headers, and can be produced by running +*containers-storage diff* or an equivalent. + +Differently than **apply-diff**, the command **applydiff-using-staging-dir** +first creates a staging directory and then moves the final result to the destination. + +## EXAMPLE +**containers-storage applydiff-using-staging-dir 5891b5b522 /path/to/diff** + +## SEE ALSO +containers-storage-apply-diff(1) +containers-storage-changes(1) +containers-storage-diff(1) +containers-storage-diffsize(1) diff --git a/docs/containers-storage.md b/docs/containers-storage.md index c851befa92..1e95fc4944 100644 --- a/docs/containers-storage.md +++ b/docs/containers-storage.md @@ -52,79 +52,81 @@ configuration will be stored as data items. ## SUB-COMMANDS The *containers-storage* command's features are broken down into several subcommands: - **containers-storage add-names(1)** Add layer, image, or container name or names + **containers-storage add-names(1)** Add layer, image, or container name or names - **containers-storage applydiff(1)** Apply a diff to a layer + **containers-storage applydiff(1)** Apply a diff to a layer - **containers-storage changes(1)** Compare two layers + **containers-storage applydiff-using-staging-dir(1)** Apply a diff to a layer staging the new content first. - **containers-storage check(1)** Check for and possibly remove damaged layers/images/containers + **containers-storage changes(1)** Compare two layers - **containers-storage container(1)** Examine a container + **containers-storage check(1)** Check for and possibly remove damaged layers/images/containers - **containers-storage containers(1)** List containers + **containers-storage container(1)** Examine a container - **containers-storage create-container(1)** Create a new container from an image + **containers-storage containers(1)** List containers - **containers-storage create-image(1)** Create a new image using layers + **containers-storage create-container(1)** Create a new container from an image - **containers-storage create-layer(1)** Create a new layer + **containers-storage create-image(1)** Create a new image using layers - **containers-storage create-storage-layer(1)** Create a new layer in the lower-level storage driver + **containers-storage create-layer(1)** Create a new layer - **containers-storage delete(1)** Delete a layer or image or container, with no safety checks + **containers-storage create-storage-layer(1)** Create a new layer in the lower-level storage driver - **containers-storage delete-container(1)** Delete a container, with safety checks + **containers-storage delete(1)** Delete a layer or image or container, with no safety checks - **containers-storage delete-image(1)** Delete an image, with safety checks + **containers-storage delete-container(1)** Delete a container, with safety checks - **containers-storage delete-layer(1)** Delete a layer, with safety checks + **containers-storage delete-image(1)** Delete an image, with safety checks - **containers-storage diff(1)** Compare two layers + **containers-storage delete-layer(1)** Delete a layer, with safety checks - **containers-storage diffsize(1)** Compare two layers + **containers-storage diff(1)** Compare two layers - **containers-storage exists(1)** Check if a layer or image or container exists + **containers-storage diffsize(1)** Compare two layers - **containers-storage get-container-data(1)** Get data that is attached to a container + **containers-storage exists(1)** Check if a layer or image or container exists - **containers-storage get-image-data(1)** Get data that is attached to an image + **containers-storage get-container-data(1)** Get data that is attached to a container - **containers-storage image(1)** Examine an image + **containers-storage get-image-data(1)** Get data that is attached to an image - **containers-storage images(1)** List images + **containers-storage image(1)** Examine an image - **containers-storage layers(1)** List layers + **containers-storage images(1)** List images - **containers-storage list-container-data(1)** List data items that are attached to a container + **containers-storage layers(1)** List layers - **containers-storage list-image-data(1)** List data items that are attached to an image + **containers-storage list-container-data(1)** List data items that are attached to a container - **containers-storage metadata(1)** Retrieve layer, image, or container metadata + **containers-storage list-image-data(1)** List data items that are attached to an image - **containers-storage mount(1)** Mount a layer or container + **containers-storage metadata(1)** Retrieve layer, image, or container metadata - **containers-storage mounted(1)** Check if a file system is mounted + **containers-storage mount(1)** Mount a layer or container - **containers-storage set-container-data(1)** Set data that is attached to a container + **containers-storage mounted(1)** Check if a file system is mounted - **containers-storage set-image-data(1)** Set data that is attached to an image + **containers-storage set-container-data(1)** Set data that is attached to a container - **containers-storage set-metadata(1)** Set layer, image, or container metadata + **containers-storage set-image-data(1)** Set data that is attached to an image - **containers-storage set-names(1)** Set layer, image, or container name or names + **containers-storage set-metadata(1)** Set layer, image, or container metadata - **containers-storage shutdown(1)** Shut down graph driver + **containers-storage set-names(1)** Set layer, image, or container name or names - **containers-storage status(1)** Check on graph driver status + **containers-storage shutdown(1)** Shut down graph driver - **containers-storage unmount(1)** Unmount a layer or container + **containers-storage status(1)** Check on graph driver status - **containers-storage unshare(1)** Run a command in a user namespace + **containers-storage unmount(1)** Unmount a layer or container - **containers-storage version(1)** Return containers-storage version information + **containers-storage unshare(1)** Run a command in a user namespace - **containers-storage wipe(1)** Wipe all layers, images, and containers + **containers-storage version(1)** Return containers-storage version information + + **containers-storage wipe(1)** Wipe all layers, images, and containers ## OPTIONS **--help** From 42fa4a987d8a0b8df1766cf37bdb36d460add167 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 19 Sep 2023 13:44:12 +0200 Subject: [PATCH 5/5] test: test create layer from staging directory Signed-off-by: Giuseppe Scrivano --- tests/apply-diff.bats | 57 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/apply-diff.bats b/tests/apply-diff.bats index 0f4351dd1d..83807c845e 100644 --- a/tests/apply-diff.bats +++ b/tests/apply-diff.bats @@ -44,3 +44,60 @@ load helpers checkchanges checkdiffs } + +@test "apply-diff-from-staging-directory" { + case "$STORAGE_DRIVER" in + overlay*) + ;; + *) + skip "driver $STORAGE_DRIVER does not support diff-from-staging-directory" + ;; + esac + + SRC=$TESTDIR/source + mkdir -p $SRC + createrandom $SRC/file1 + createrandom $SRC/file2 + createrandom $SRC/file3 + + local sconf=$TESTDIR/storage.conf + + local root=`storage status 2>&1 | awk '/^Root:/{print $2}'` + local runroot=`storage status 2>&1 | awk '/^Run Root:/{print $3}'` + + cat >$sconf <