From 09aaffe010899d96570576f138c4cca59d0c971e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 24 Sep 2024 21:10:50 +0200 Subject: [PATCH] Add oci/layout.PutBlobFromLocalFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Try to reflink the file and restort to copying it in case of failure. Also add an Options struct to be future proof. Signed-off-by: Miloslav Trmač Signed-off-by: Valentin Rothberg --- go.mod | 2 +- internal/reflink/reflink_linux.go | 22 ++++++ internal/reflink/reflink_unsupported.go | 15 ++++ oci/layout/fixtures/files/a.txt | 1 + oci/layout/fixtures/files/b.txt | 1 + oci/layout/oci_dest.go | 93 ++++++++++++++++++++++--- oci/layout/oci_dest_test.go | 38 ++++++++++ 7 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 internal/reflink/reflink_linux.go create mode 100644 internal/reflink/reflink_unsupported.go create mode 100644 oci/layout/fixtures/files/a.txt create mode 100644 oci/layout/fixtures/files/b.txt diff --git a/go.mod b/go.mod index 65c21cc14e..66e0bdef7e 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,7 @@ require ( golang.org/x/exp v0.0.0-20241108190413-2d47ceb2692f golang.org/x/oauth2 v0.24.0 golang.org/x/sync v0.10.0 + golang.org/x/sys v0.28.0 golang.org/x/term v0.27.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -140,7 +141,6 @@ require ( go.opentelemetry.io/otel/trace v1.28.0 // indirect golang.org/x/mod v0.22.0 // indirect golang.org/x/net v0.31.0 // indirect - golang.org/x/sys v0.28.0 // indirect golang.org/x/text v0.21.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect google.golang.org/grpc v1.68.1 // indirect diff --git a/internal/reflink/reflink_linux.go b/internal/reflink/reflink_linux.go new file mode 100644 index 0000000000..2cfd97bda1 --- /dev/null +++ b/internal/reflink/reflink_linux.go @@ -0,0 +1,22 @@ +//go:build linux + +package reflink + +import ( + "io" + "os" + + "golang.org/x/sys/unix" +) + +// LinkOrCopy attempts to reflink the source to the destination fd. +// If reflinking fails or is unsupported, it falls back to io.Copy(). +func LinkOrCopy(src, dst *os.File) error { + _, _, errno := unix.Syscall(unix.SYS_IOCTL, dst.Fd(), unix.FICLONE, src.Fd()) + if errno == 0 { + return nil + } + + _, err := io.Copy(dst, src) + return err +} diff --git a/internal/reflink/reflink_unsupported.go b/internal/reflink/reflink_unsupported.go new file mode 100644 index 0000000000..8ba11db84f --- /dev/null +++ b/internal/reflink/reflink_unsupported.go @@ -0,0 +1,15 @@ +//go:build !linux + +package reflink + +import ( + "io" + "os" +) + +// LinkOrCopy attempts to reflink the source to the destination fd. +// If reflinking fails or is unsupported, it falls back to io.Copy(). +func LinkOrCopy(src, dst *os.File) error { + _, err := io.Copy(dst, src) + return err +} diff --git a/oci/layout/fixtures/files/a.txt b/oci/layout/fixtures/files/a.txt new file mode 100644 index 0000000000..5582bcf401 --- /dev/null +++ b/oci/layout/fixtures/files/a.txt @@ -0,0 +1 @@ +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa diff --git a/oci/layout/fixtures/files/b.txt b/oci/layout/fixtures/files/b.txt new file mode 100644 index 0000000000..59f630b80c --- /dev/null +++ b/oci/layout/fixtures/files/b.txt @@ -0,0 +1 @@ +bbbbbbbbbbbbbbbbbbbbbbbb diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index 19c0cff570..b87cef4f7d 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -17,6 +17,7 @@ import ( "github.com/containers/image/v5/internal/manifest" "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/internal/putblobdigest" + "github.com/containers/image/v5/internal/reflink" "github.com/containers/image/v5/types" "github.com/containers/storage/pkg/fileutils" digest "github.com/opencontainers/go-digest" @@ -141,9 +142,21 @@ func (d *ociImageDestination) PutBlobWithOptions(ctx context.Context, stream io. if inputInfo.Size != -1 && size != inputInfo.Size { return private.UploadedBlob{}, fmt.Errorf("Size mismatch when copying %s, expected %d, got %d", blobDigest, inputInfo.Size, size) } - if err := blobFile.Sync(); err != nil { + + if err := d.blobFileSyncAndRename(blobFile, blobDigest, &explicitClosed); err != nil { return private.UploadedBlob{}, err } + succeeded = true + return private.UploadedBlob{Digest: blobDigest, Size: size}, nil +} + +// blobFileSyncAndRename syncs the specified blobFile on the filesystem and renames it to the +// specific blob path determined by the blobDigest. The closed pointer indicates to the caller +// whether blobFile has been closed or not. +func (d *ociImageDestination) blobFileSyncAndRename(blobFile *os.File, blobDigest digest.Digest, closed *bool) error { + if err := blobFile.Sync(); err != nil { + return err + } // On POSIX systems, blobFile was created with mode 0600, so we need to make it readable. // On Windows, the “permissions of newly created files” argument to syscall.Open is @@ -151,26 +164,27 @@ func (d *ociImageDestination) PutBlobWithOptions(ctx context.Context, stream io. // always fails on Windows. if runtime.GOOS != "windows" { if err := blobFile.Chmod(0644); err != nil { - return private.UploadedBlob{}, err + return err } } blobPath, err := d.ref.blobPath(blobDigest, d.sharedBlobDir) if err != nil { - return private.UploadedBlob{}, err + return err } if err := ensureParentDirectoryExists(blobPath); err != nil { - return private.UploadedBlob{}, err + return err } - // need to explicitly close the file, since a rename won't otherwise not work on Windows + // need to explicitly close the file, since a rename won't otherwise work on Windows blobFile.Close() - explicitClosed = true + *closed = true + if err := os.Rename(blobFile.Name(), blobPath); err != nil { - return private.UploadedBlob{}, err + return err } - succeeded = true - return private.UploadedBlob{Digest: blobDigest, Size: size}, nil + + return nil } // TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination @@ -303,6 +317,67 @@ func (d *ociImageDestination) CommitWithOptions(ctx context.Context, options pri return os.WriteFile(d.ref.indexPath(), indexJSON, 0644) } +// PutBlobFromLocalFileOption is unused but may receive functionality in the future. +type PutBlobFromLocalFileOption struct{} + +// PutBlobFromLocalFile arranges the data from path to be used as blob with digest. +// It computes, and returns, the digest and size of the used file. +// +// This function can be used instead of dest.PutBlob() where the ImageDestination requires PutBlob() to be called. +func PutBlobFromLocalFile(ctx context.Context, dest types.ImageDestination, file string, options ...PutBlobFromLocalFileOption) (digest.Digest, int64, error) { + d, ok := dest.(*ociImageDestination) + if !ok { + return "", -1, errors.New("internal error: PutBlobFromLocalFile called with a non-oci: destination") + } + + succeeded := false + blobFileClosed := false + blobFile, err := os.CreateTemp(d.ref.dir, "oci-put-blob") + if err != nil { + return "", -1, err + } + defer func() { + if !blobFileClosed { + blobFile.Close() + } + if !succeeded { + os.Remove(blobFile.Name()) + } + }() + + srcFile, err := os.Open(file) + if err != nil { + return "", -1, err + } + defer srcFile.Close() + + err = reflink.LinkOrCopy(srcFile, blobFile) + if err != nil { + return "", -1, err + } + + _, err = blobFile.Seek(0, io.SeekStart) + if err != nil { + return "", -1, err + } + blobDigest, err := digest.FromReader(blobFile) + if err != nil { + return "", -1, err + } + + fileInfo, err := blobFile.Stat() + if err != nil { + return "", -1, err + } + + if err := d.blobFileSyncAndRename(blobFile, blobDigest, &blobFileClosed); err != nil { + return "", -1, err + } + + succeeded = true + return blobDigest, fileInfo.Size(), nil +} + func ensureDirectoryExists(path string) error { if err := fileutils.Exists(path); err != nil && errors.Is(err, fs.ErrNotExist) { if err := os.MkdirAll(path, 0755); err != nil { diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go index 57981c37a3..9a8bd03ce4 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -178,3 +178,41 @@ func putTestManifest(t *testing.T, ociRef ociReference, tmpDir string) { digest := digest.FromBytes(data).Encoded() assert.Contains(t, paths, filepath.Join(tmpDir, "blobs", "sha256", digest), "The OCI directory does not contain the new manifest data") } + +func TestPutblobFromLocalFile(t *testing.T) { + ref, _ := refToTempOCI(t, false) + dest, err := ref.NewImageDestination(context.Background(), nil) + require.NoError(t, err) + defer dest.Close() + ociDest, ok := dest.(*ociImageDestination) + require.True(t, ok) + + for _, test := range []struct { + path string + size int64 + digest string + }{ + {path: "fixtures/files/a.txt", size: 31, digest: "sha256:c8a3f498ce6aaa13c803fa3a6a0d5fd6b5d75be5781f98f56c0f960efcc53174"}, + {path: "fixtures/files/b.txt", size: 25, digest: "sha256:8c1e9b03116b95e6dfac68c588190d56bfc82b9cc550ada726e882e138a3b93b"}, + {path: "fixtures/files/b.txt", size: 25, digest: "sha256:8c1e9b03116b95e6dfac68c588190d56bfc82b9cc550ada726e882e138a3b93b"}, // Must not fail + } { + digest, size, err := PutBlobFromLocalFile(context.Background(), dest, test.path) + require.NoError(t, err) + require.Equal(t, test.size, size) + require.Equal(t, test.digest, digest.String()) + + blobPath, err := ociDest.ref.blobPath(digest, ociDest.sharedBlobDir) + require.NoError(t, err) + require.FileExists(t, blobPath) + + expectedContent, err := os.ReadFile(test.path) + require.NoError(t, err) + require.NotEmpty(t, expectedContent) + blobContent, err := os.ReadFile(blobPath) + require.NoError(t, err) + require.Equal(t, expectedContent, blobContent) + } + + err = ociDest.CommitWithOptions(context.Background(), private.CommitOptions{}) + require.NoError(t, err) +}