Skip to content

Commit

Permalink
Don't set UncompressedSize on chunked pull
Browse files Browse the repository at this point in the history
The current value obtained by summing the sizes of regular file contents
does not match the size of the uncompressed layer tarball.

We don't have a convenient source to compute the correct size
for estargz without pulling the full layer and defeating the point.

For recent zstd:chunked images, we have the full tar-split,
so we can compute the correct size; for now, this doesn't do that.
That might slow down image size computation.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
mtrmac committed Oct 10, 2024
1 parent 250a170 commit 96b0ad1
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 23 deletions.
2 changes: 1 addition & 1 deletion drivers/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ type Driver interface {
type DriverWithDifferOutput struct {
Differ Differ
Target string
Size int64
Size int64 // Size of the uncompressed layer, -1 if unknown. Must be known if UncompressedDigest is set.
UIDs []uint32
GIDs []uint32
UncompressedDigest digest.Digest
Expand Down
13 changes: 8 additions & 5 deletions layers.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,12 @@ type Layer struct {
TOCDigest digest.Digest `json:"toc-digest,omitempty"`

// UncompressedSize is the length of the blob that was last passed to
// ApplyDiff() or create(), after we decompressed it. If
// UncompressedDigest is not set, this should be treated as if it were
// an uninitialized value.
// ApplyDiff() or create(), after we decompressed it.
//
// - If UncompressedDigest is set, this must be set to a valid value.
// - Otherwise, if TOCDigest is set, this is either valid or -1.
// - If neither of this digests is set, this should be treated as if it were
// an uninitialized value.
UncompressedSize int64 `json:"diff-size,omitempty"`

// CompressionType is the type of compression which we detected on the blob
Expand Down Expand Up @@ -1214,8 +1217,8 @@ func (r *layerStore) Size(name string) (int64, error) {
// We use the presence of a non-empty digest as an indicator that the size value was intentionally set, and that
// a zero value is not just present because it was never set to anything else (which can happen if the layer was
// created by a version of this library that didn't keep track of digest and size information).
if layer.TOCDigest != "" || layer.UncompressedDigest != "" {
return layer.UncompressedSize, nil
if layer.UncompressedDigest != "" || layer.TOCDigest != "" {
return layer.UncompressedSize, nil // This may return -1 if only TOCDigest is set
}
return -1, nil
}
Expand Down
22 changes: 6 additions & 16 deletions pkg/chunked/storage_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ func (c *chunkedDiffer) ApplyDiff(dest string, options *archive.TarOptions, diff

var compressedDigest digest.Digest
var uncompressedDigest digest.Digest
var convertedBlobSize int64
var uncompressedSize int64 = -1

if c.convertToZstdChunked {
fd, err := unix.Open(dest, unix.O_TMPFILE|unix.O_RDWR|unix.O_CLOEXEC, 0o600)
Expand Down Expand Up @@ -1185,7 +1185,7 @@ func (c *chunkedDiffer) ApplyDiff(dest string, options *archive.TarOptions, diff
if err != nil {
return graphdriver.DriverWithDifferOutput{}, err
}
convertedBlobSize = tarSize
uncompressedSize = tarSize
// fileSource is a O_TMPFILE file descriptor, so we
// need to keep it open until the entire file is processed.
defer fileSource.Close()
Expand Down Expand Up @@ -1255,6 +1255,7 @@ func (c *chunkedDiffer) ApplyDiff(dest string, options *archive.TarOptions, diff
TOCDigest: c.tocDigest,
UncompressedDigest: uncompressedDigest,
CompressedDigest: compressedDigest,
Size: uncompressedSize,
}

// When the hard links deduplication is used, file attributes are ignored because setting them
Expand All @@ -1274,13 +1275,6 @@ func (c *chunkedDiffer) ApplyDiff(dest string, options *archive.TarOptions, diff
}

output.UIDs, output.GIDs = collectIDs(mergedEntries)
if convertedBlobSize > 0 {
// if the image was converted, store the original tar size, so that
// it can be recreated correctly.
output.Size = convertedBlobSize
} else {
output.Size = totalSizeFromTOC
}

if err := maybeDoIDRemap(mergedEntries, options); err != nil {
return output, err
Expand Down Expand Up @@ -1597,9 +1591,7 @@ func mustSkipFile(fileType compressedFileType, e internal.FileMetadata) bool {
return false
}

func (c *chunkedDiffer) mergeTocEntries(fileType compressedFileType, entries []internal.FileMetadata) ([]fileMetadata, int64, error) {
var totalFilesSize int64

func (c *chunkedDiffer) mergeTocEntries(fileType compressedFileType, entries []internal.FileMetadata) ([]fileMetadata, error) {
countNextChunks := func(start int) int {
count := 0
for _, e := range entries[start:] {
Expand Down Expand Up @@ -1629,10 +1621,8 @@ func (c *chunkedDiffer) mergeTocEntries(fileType compressedFileType, entries []i
continue
}

totalFilesSize += e.Size

if e.Type == TypeChunk {
return nil, -1, fmt.Errorf("chunk type without a regular file")
return nil, fmt.Errorf("chunk type without a regular file")
}

if e.Type == TypeReg {
Expand Down Expand Up @@ -1668,7 +1658,7 @@ func (c *chunkedDiffer) mergeTocEntries(fileType compressedFileType, entries []i
lastChunkOffset = mergedEntries[i].chunks[j].Offset
}
}
return mergedEntries, totalFilesSize, nil
return mergedEntries, nil
}

// validateChunkChecksum checks if the file at $root/$path[offset:chunk.ChunkSize] has the
Expand Down
2 changes: 1 addition & 1 deletion store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2201,7 +2201,7 @@ func (s *store) ImageSize(id string) (int64, error) {
}
// The UncompressedSize is only valid if there's a digest to go with it.
n := layer.UncompressedSize
if layer.UncompressedDigest == "" {
if layer.UncompressedDigest == "" || n == -1 {
// Compute the size.
n, err = layerStore.DiffSize("", layer.ID)
if err != nil {
Expand Down

0 comments on commit 96b0ad1

Please sign in to comment.