Skip to content

Commit

Permalink
Fix: archive/tar: write too long error when using Docker Desktop co…
Browse files Browse the repository at this point in the history
…ntainerd backend (#275)

Fixes buildpacks/pack#2174

This is somewhat of a band-aid solution and degrades performance
because we have to read the uncompressed layer bits to get the uncompressed layer size
for layers that we `docker save` out of the daemon (re-used layers and all the base layers).

The better longer-term fix will be to send OCI-layout formatted tars
when we have all the layers available (when using containerd as the backing store, this will always be true).

Signed-off-by: Natalie Arellano <narellano@vmware.com>
  • Loading branch information
natalieparellano authored Jun 3, 2024
1 parent 4af8786 commit 47db70c
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 78 deletions.
52 changes: 5 additions & 47 deletions local/local.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package local

import (
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"

v1 "github.com/google/go-containerregistry/pkg/v1"
Expand Down Expand Up @@ -110,67 +106,29 @@ func (i *Image) SetOS(osVal string) error {
var emptyHistory = v1.History{Created: v1.Time{Time: imgutil.NormalizedDateTime}}

func (i *Image) AddLayer(path string) error {
diffID, err := calculateChecksum(path)
if err != nil {
return err
}
layer, err := i.addLayerToStore(path, diffID)
layer, err := i.store.AddLayer(path)
if err != nil {
return err
}
return i.AddLayerWithHistory(layer, emptyHistory)
}

func calculateChecksum(path string) (string, error) {
f, err := os.Open(filepath.Clean(path))
if err != nil {
return "", fmt.Errorf("failed to open layer at path %s: %w", path, err)
}
defer f.Close()
hasher := sha256.New()
if _, err := io.Copy(hasher, f); err != nil {
return "", fmt.Errorf("failed to calculate checksum for layer at path %s: %w", path, err)
}
return "sha256:" + hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), nil
}

func (i *Image) AddLayerWithDiffID(path, diffID string) error {
layer, err := i.addLayerToStore(path, diffID)
func (i *Image) AddLayerWithDiffID(path, _ string) error {
layer, err := i.store.AddLayer(path)
if err != nil {
return err
}
return i.AddLayerWithHistory(layer, emptyHistory)
}

func (i *Image) AddLayerWithDiffIDAndHistory(path, diffID string, history v1.History) error {
layer, err := i.addLayerToStore(path, diffID)
func (i *Image) AddLayerWithDiffIDAndHistory(path, _ string, history v1.History) error {
layer, err := i.store.AddLayer(path)
if err != nil {
return err
}
return i.AddLayerWithHistory(layer, history)
}

func (i *Image) addLayerToStore(fromPath, withDiffID string) (v1.Layer, error) {
var (
layer v1.Layer
err error
)
diffID, err := v1.NewHash(withDiffID)
if err != nil {
return nil, err
}
layer = newPopulatedLayer(diffID, fromPath, 1)
if err != nil {
return nil, err
}
fi, err := os.Stat(fromPath)
if err != nil {
return nil, err
}
i.store.AddLayer(layer, diffID, fi.Size())
return layer, nil
}

func (i *Image) AddOrReuseLayerWithHistory(path string, diffID string, history v1.History) error {
prevLayerExists, err := i.PreviousImageHasLayer(diffID)
if err != nil {
Expand Down
54 changes: 40 additions & 14 deletions local/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/docker/docker/pkg/jsonmessage"
registryName "github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/tarball"
"golang.org/x/sync/errgroup"

"github.com/buildpacks/imgutil"
Expand Down Expand Up @@ -288,11 +289,13 @@ func (s *Store) getLayerSize(layer v1.Layer) (int64, error) {
return 0, err
}
knownLayer, layerFound := s.onDiskLayersByDiffID[diffID]
if layerFound {
if layerFound && knownLayer.uncompressedSize != -1 {
return knownLayer.uncompressedSize, nil
}
// FIXME: this is a time sink and should be avoided if the daemon accepts OCI layout-formatted tars
// If layer was not seen previously, we need to read it to get the uncompressed size
// In practice, we should not get here
// In practice, we should only get here if layers saved from the daemon via `docker save`
// are output compressed.
layerReader, err := layer.Uncompressed()
if err != nil {
return 0, err
Expand Down Expand Up @@ -450,18 +453,11 @@ func (s *Store) doDownloadLayersFor(identifier string) error {
return err
}

for idx, diffID := range configFile.RootFS.DiffIDs {
for idx := range configFile.RootFS.DiffIDs {
layerPath := filepath.Join(tmpDir, manifest[0].Layers[idx])
hash, err := v1.NewHash(diffID)
if err != nil {
if _, err := s.AddLayer(layerPath); err != nil {
return err
}
layer := newPopulatedLayer(hash, layerPath, 1)
fi, err := os.Stat(layerPath)
if err != nil {
return err
}
s.AddLayer(layer, hash, fi.Size())
}
return nil
}
Expand Down Expand Up @@ -546,9 +542,39 @@ func (s *Store) findLayer(withHash v1.Hash) v1.Layer {
return aLayer.layer
}

func (s *Store) AddLayer(layer v1.Layer, withDiffID v1.Hash, withSize int64) {
s.onDiskLayersByDiffID[withDiffID] = annotatedLayer{
func (s *Store) AddLayer(fromPath string) (v1.Layer, error) {
layer, err := tarball.LayerFromFile(fromPath)
if err != nil {
return nil, err
}
diffID, err := layer.DiffID()
if err != nil {
return nil, err
}
var uncompressedSize int64
fileSize, err := func() (int64, error) {
fi, err := os.Stat(fromPath)
if err != nil {
return -1, err
}
return fi.Size(), nil
}()
if err != nil {
return nil, err
}
compressedSize, err := layer.Size()
if err != nil {
return nil, err
}
if fileSize == compressedSize {
// the layer is compressed, we don't know the uncompressed size
uncompressedSize = -1
} else {
uncompressedSize = fileSize
}
s.onDiskLayersByDiffID[diffID] = annotatedLayer{
layer: layer,
uncompressedSize: withSize,
uncompressedSize: uncompressedSize,
}
return layer, nil
}
17 changes: 0 additions & 17 deletions local/v1_facade.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"fmt"
"io"
"os"
"time"

"github.com/docker/docker/api/types"
Expand Down Expand Up @@ -234,22 +233,6 @@ func newDownloadableEmptyLayer(diffID v1.Hash, store *Store, imageID string) *v1
}
}

func newPopulatedLayer(diffID v1.Hash, fromPath string, uncompressedSize int64) *v1LayerFacade {
return &v1LayerFacade{
diffID: diffID,
uncompressed: func() (io.ReadCloser, error) {
f, err := os.Open(fromPath)
if err != nil {
return nil, err
}
return f, nil
},
uncompressedSize: func() (int64, error) {
return uncompressedSize, nil
},
}
}

func newEmptyLayerListFrom(configFile *v1.ConfigFile, downloadOnAccess bool, withStore *Store, withImageIdentifier string) []v1.Layer {
layers := make([]v1.Layer, len(configFile.RootFS.DiffIDs))
for idx, diffID := range configFile.RootFS.DiffIDs {
Expand Down

0 comments on commit 47db70c

Please sign in to comment.