containers/image
1
0
Fork 0
mirror of https://github.com/containers/image.git synced 2025-04-18 19:44:05 +03:00
Code

Fix excessive memory and disk usage when sharing layers

Browse Source 
createNewLayer can add new entries to s.filenames for
uncompressed versions of layers; these entries don't match
manifest.LayerInfos, causing the existing logic to assume
they are a config and to store them as ImageBigData.

That's both a potentially unlimited disk space waste (recording
uncompressed versions of layers unnecessarily), and a memory
pressure concern (ImageBigData is created by reading the whole
blob into memory).

Instead, precisely track the digest of the config, and never
include more than one file in ImageBigData.

Blobs added via PutBlob/TryReusingBlob but not referenced from
the manifest are now ignored instead of being recorded inside
the image.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač 2024-11-18 23:13:17 +01:00 committed by Krzysztof Wilczyński
parent f8d6234cf1
commit 64440ba01d
No known key found for this signature in database
GPG Key ID: 7C64768D3DE334E7
2 changed files with 25 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
storage/storage_dest.go
View File

@ -21,7 +21,6 @@ import (
"github.com/containers/image/v5/internal/imagedestination/stubs"
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/internal/putblobdigest"
"github.com/containers/image/v5/internal/set"
"github.com/containers/image/v5/internal/signature"
"github.com/containers/image/v5/internal/tmpdir"
"github.com/containers/image/v5/manifest"
@ -84,6 +83,9 @@ type storageImageDestination struct {
indexToAddedLayerInfo map[int]addedLayerInfo // Mapping from layer (by index) to blob to add to the image
blobAdditionalLayer map[digest.Digest]storage.AdditionalLayer // Mapping from layer blobsums to their corresponding additional layer
diffOutputs map[digest.Digest]*graphdriver.DriverWithDifferOutput // Mapping from digest to differ output
// Config
configDigest digest.Digest // "" if N/A or not known yet.
}
// addedLayerInfo records data about a layer to use in this image.
@ -170,7 +172,17 @@ func (s *storageImageDestination) PutBlobWithOptions(ctx context.Context, stream
return info, err
}
if options.IsConfig || options.LayerIndex == nil {
if options.IsConfig {
s.lock.Lock()
defer s.lock.Unlock()
if s.configDigest != "" {
return private.UploadedBlob{}, fmt.Errorf("after config %q, refusing to record another config %q",
s.configDigest.String(), info.Digest.String())
}
s.configDigest = info.Digest
return info, nil
}
if options.LayerIndex == nil {
return info, nil
}
@ -776,22 +788,14 @@ func (s *storageImageDestination) Commit(ctx context.Context, unparsedToplevel t
options.CreationDate = *inspect.Created
}
// Set up to save the non-layer blobs as data items. Since we only share layers, they should all be in files, so
// we just need to screen out the ones that are actually layers to get the list of non-layers.
dataBlobs := set.New[digest.Digest]()
for blob := range s.filenames {
dataBlobs.Add(blob)
}
for _, layerBlob := range layerBlobs {
dataBlobs.Delete(layerBlob.Digest)
}
for _, blob := range dataBlobs.Values() {
v, err := os.ReadFile(s.filenames[blob])
// Set up to save the config as a data item. Since we only share layers, the config should be in a file.
if s.configDigest != "" {
v, err := os.ReadFile(s.filenames[s.configDigest])
if err != nil {
return fmt.Errorf("copying non-layer blob %q to image: %w", blob, err)
return fmt.Errorf("copying config blob %q to image: %w", s.configDigest, err)
}
options.BigData = append(options.BigData, storage.ImageBigDataOption{
Key: blob.String(),
Key: s.configDigest.String(),
Data: v,
Digest: digest.Canonical.FromBytes(v),
})

12
storage/storage_test.go
View File

@ -277,11 +277,11 @@ func makeLayer(t *testing.T, compression archive.Compression) testBlob {
}
}
func (l testBlob) storeBlob(t *testing.T, dest types.ImageDestination, cache types.BlobInfoCache, mimeType string) manifest.Schema2Descriptor {
func (l testBlob) storeBlob(t *testing.T, dest types.ImageDestination, cache types.BlobInfoCache, mimeType string, isConfig bool) manifest.Schema2Descriptor {
_, err := dest.PutBlob(context.Background(), bytes.NewReader(l.data), types.BlobInfo{
Size: l.compressedSize,
Digest: l.compressedDigest,
}, cache, false)
}, cache, isConfig)
require.NoError(t, err)
return manifest.Schema2Descriptor{
MediaType: mimeType,
@ -312,12 +312,12 @@ func createUncommittedImageDest(t *testing.T, ref types.ImageReference, cache ty
layerDescriptors := []manifest.Schema2Descriptor{}
for _, layer := range layers {
desc := layer.storeBlob(t, dest, cache, manifest.DockerV2Schema2LayerMediaType)
desc := layer.storeBlob(t, dest, cache, manifest.DockerV2Schema2LayerMediaType, false)
layerDescriptors = append(layerDescriptors, desc)
}
configDescriptor := manifest.Schema2Descriptor{} // might be good enough
if config != nil {
configDescriptor = config.storeBlob(t, dest, cache, manifest.DockerV2Schema2ConfigMediaType)
configDescriptor = config.storeBlob(t, dest, cache, manifest.DockerV2Schema2ConfigMediaType, true)
}
manifest := manifest.Schema2FromComponents(configDescriptor, layerDescriptors)
@ -429,9 +429,9 @@ func TestWriteRead(t *testing.T) {
compression = archive.Gzip
}
layer := makeLayer(t, compression)
_ = layer.storeBlob(t, dest, cache, manifest.DockerV2Schema2LayerMediaType)
_ = layer.storeBlob(t, dest, cache, manifest.DockerV2Schema2LayerMediaType, false)
t.Logf("Wrote randomly-generated layer %q (%d/%d bytes) to destination", layer.compressedDigest, layer.compressedSize, layer.uncompressedSize)
_ = config.storeBlob(t, dest, cache, manifest.DockerV2Schema2ConfigMediaType)
_ = config.storeBlob(t, dest, cache, manifest.DockerV2Schema2ConfigMediaType, true)
manifest := strings.ReplaceAll(manifestFmt, "%lh", layer.compressedDigest.String())
manifest = strings.ReplaceAll(manifest, "%ch", config.compressedDigest.String())