Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short-term kludges for recent AdditionalLayerStore changes #2428

Merged
merged 3 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions storage/storage_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,17 @@ func (s *storageImageDestination) tryReusingBlobAsPending(blobDigest digest.Dige
if err != nil && !errors.Is(err, storage.ErrLayerUnknown) {
return false, private.ReusedBlob{}, fmt.Errorf(`looking for compressed layers with digest %q and labels: %w`, blobDigest, err)
} else if err == nil {
d := aLayer.TOCDigest()
if d == "" {
return false, private.ReusedBlob{}, fmt.Errorf(`failed to get TOCDigest of %q: %w`, blobDigest, err)
alsTOCDigest := aLayer.TOCDigest()
if alsTOCDigest != options.TOCDigest {
// FIXME: If alsTOCDigest is "", the Additional Layer Store FUSE server is probably just too old, and we could
// probably go on reading the layer from other sources.
//
// Currently it should not be possible for alsTOCDigest to be set and not the expected value, but there’s
// not that much benefit to checking for equality — we trust the FUSE server to validate the digest either way.
return false, private.ReusedBlob{}, fmt.Errorf("additional layer for TOCDigest %q reports unexpected TOCDigest %q",
options.TOCDigest, alsTOCDigest)
}
s.lockProtected.indexToTOCDigest[*options.LayerIndex] = d
s.lockProtected.indexToTOCDigest[*options.LayerIndex] = options.TOCDigest
s.lockProtected.indexToAdditionalLayer[*options.LayerIndex] = aLayer
return true, private.ReusedBlob{
Digest: blobDigest,
Expand Down
18 changes: 11 additions & 7 deletions storage/storage_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,6 @@ func (s *storageImageSource) LayerInfosForCopy(ctx context.Context, instanceDige
if err != nil {
return nil, fmt.Errorf("reading layer %q in image %q: %w", layerID, s.image.ID, err)
}
if layer.UncompressedSize < 0 {
layer.UncompressedSize = -1
}

blobDigest := layer.UncompressedDigest
if blobDigest == "" {
Expand All @@ -331,12 +328,16 @@ func (s *storageImageSource) LayerInfosForCopy(ctx context.Context, instanceDige
return nil, fmt.Errorf("parsing expected diffID %q for layer %q: %w", expectedDigest, layerID, err)
}
}
size := layer.UncompressedSize
if size < 0 {
size = -1
}
s.getBlobMutex.Lock()
s.getBlobMutexProtected.digestToLayerID[blobDigest] = layer.ID
s.getBlobMutex.Unlock()
blobInfo := types.BlobInfo{
Digest: blobDigest,
Size: layer.UncompressedSize,
Size: size,
MediaType: uncompressedLayerType,
}
physicalBlobInfos = append([]types.BlobInfo{blobInfo}, physicalBlobInfos...)
Expand Down Expand Up @@ -455,10 +456,13 @@ func (s *storageImageSource) getSize() (int64, error) {
if (layer.TOCDigest == "" && layer.UncompressedDigest == "") || (layer.TOCDigest == "" && layer.UncompressedSize < 0) {
return -1, fmt.Errorf("size for layer %q is unknown, failing getSize()", layerID)
}
if layer.UncompressedSize < 0 {
sum = 0
// FIXME: We allow layer.UncompressedSize < 0 above, because currently images in an Additional Layer Store don’t provide that value.
// Right now, various callers in Podman (and, also, newImage in this package) don’t expect the size computation to fail.
// Should we update the callers, or do we need to continue returning inaccurate information here? Or should we pay the cost
// to compute the size from the diff?
if layer.UncompressedSize >= 0 {
sum += layer.UncompressedSize
}
sum += layer.UncompressedSize
if layer.Parent == "" {
break
}
Expand Down