containerImageRef/containerImageSource: don't buffer uncompressed layers

Instead of extracting layer content to a temporary file when we're
committing to transports that aren't containers-storage, record the ID
of a layer and the uncompressed size it has recorded for its contents.

When later asked for a blob, if we cached a layer ID and size, generate
the layer diff on the fly, otherwise check for a file named after the
digest of the requested blob in our cache directory location (usually
used for new layers that we're adding) and the supplemental location
(which can be supplied by a caller).

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
This commit is contained in:
Nalin Dahyabhai 2021-05-25 16:34:36 -04:00
parent df14b1c770
commit ff1f9a3ee0
3 changed files with 60 additions and 48 deletions

View File

@ -291,21 +291,8 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
}
logrus.Debugf("committing image with reference %q is allowed by policy", transports.ImageName(dest))
// Check if the base image is already in the destination and it's some kind of local
// storage. If so, we can skip recompressing any layers that come from the base image.
exportBaseLayers := true
if transport, destIsStorage := dest.Transport().(is.StoreTransport); destIsStorage && options.OciEncryptConfig != nil {
return imgID, nil, "", errors.New("unable to use local storage with image encryption")
} else if destIsStorage && b.FromImageID != "" {
if baseref, err := transport.ParseReference(b.FromImageID); baseref != nil && err == nil {
if img, err := transport.GetImage(baseref); img != nil && err == nil {
logrus.Debugf("base image %q is already present in local storage, no need to copy its layers", b.FromImageID)
exportBaseLayers = false
}
}
}
// Build an image reference from which we can copy the finished image.
src, err := b.makeImageRef(options, exportBaseLayers)
src, err := b.makeImageRef(options)
if err != nil {
return imgID, nil, "", errors.Wrapf(err, "error computing layer digests and building metadata for container %q", b.ContainerID)
}

View File

@ -60,7 +60,6 @@ type containerImageRef struct {
historyComment string
annotations map[string]string
preferredManifestType string
exporting bool
squash bool
emptyLayer bool
idMappingOptions *define.IDMappingOptions
@ -70,6 +69,11 @@ type containerImageRef struct {
postEmptyLayers []v1.History
}
type blobLayerInfo struct {
ID string
Size int64
}
type containerImageSource struct {
path string
ref *containerImageRef
@ -83,8 +87,8 @@ type containerImageSource struct {
configDigest digest.Digest
manifest []byte
manifestType string
exporting bool
blobDirectory string
blobLayers map[digest.Digest]blobLayerInfo
}
func (i *containerImageRef) NewImage(ctx context.Context, sc *types.SystemContext) (types.ImageCloser, error) {
@ -310,6 +314,7 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, sc *types.System
}
// Extract each layer and compute its digests, both compressed (if requested) and uncompressed.
blobLayers := make(map[digest.Digest]blobLayerInfo)
for _, layerID := range layers {
what := fmt.Sprintf("layer %q", layerID)
if i.squash {
@ -328,9 +333,9 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, sc *types.System
if i.emptyLayer && layerID == i.layerID {
continue
}
// If we're not re-exporting the data, and we're reusing layers individually, reuse
// the blobsum and diff IDs.
if !i.exporting && !i.squash && layerID != i.layerID && layer.UncompressedDigest != "" {
// If we already know the digest of the contents of parent
// layers, reuse their blobsums, diff IDs, and sizes.
if !i.squash && layerID != i.layerID && layer.UncompressedDigest != "" {
layerBlobSum := layer.UncompressedDigest
layerBlobSize := layer.UncompressedSize
diffID := layer.UncompressedDigest
@ -350,6 +355,10 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, sc *types.System
// Note this layer in the list of diffIDs, again using the uncompressed digest.
oimage.RootFS.DiffIDs = append(oimage.RootFS.DiffIDs, diffID)
dimage.RootFS.DiffIDs = append(dimage.RootFS.DiffIDs, diffID)
blobLayers[diffID] = blobLayerInfo{
ID: layer.ID,
Size: layerBlobSize,
}
continue
}
// Figure out if we need to change the media type, in case we've changed the compression.
@ -599,8 +608,8 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, sc *types.System
configDigest: digest.Canonical.FromBytes(config),
manifest: imageManifest,
manifestType: manifestType,
exporting: i.exporting,
blobDirectory: i.blobDirectory,
blobLayers: blobLayers,
}
return src, nil
}
@ -675,39 +684,51 @@ func (i *containerImageSource) GetBlob(ctx context.Context, blob types.BlobInfo,
}
return ioutils.NewReadCloserWrapper(reader, closer), reader.Size(), nil
}
var layerFile *os.File
for _, path := range []string{i.blobDirectory, i.path} {
layerFile, err = os.OpenFile(filepath.Join(path, blob.Digest.String()), os.O_RDONLY, 0600)
if err == nil {
break
}
if !os.IsNotExist(err) {
logrus.Debugf("error checking for layer %q in %q: %v", blob.Digest.String(), path, err)
}
}
if err != nil || layerFile == nil {
logrus.Debugf("error reading layer %q: %v", blob.Digest.String(), err)
return nil, -1, errors.Wrapf(err, "error opening file %q to buffer layer blob", filepath.Join(i.path, blob.Digest.String()))
}
var layerReadCloser io.ReadCloser
size = -1
st, err := layerFile.Stat()
if err != nil {
logrus.Warnf("error reading size of layer %q: %v", blob.Digest.String(), err)
if blobLayerInfo, ok := i.blobLayers[blob.Digest]; ok {
noCompression := archive.Uncompressed
diffOptions := &storage.DiffOptions{
Compression: &noCompression,
}
layerReadCloser, err = i.store.Diff("", blobLayerInfo.ID, diffOptions)
size = blobLayerInfo.Size
} else {
size = st.Size()
for _, blobDir := range []string{i.blobDirectory, i.path} {
var layerFile *os.File
layerFile, err = os.OpenFile(filepath.Join(blobDir, blob.Digest.String()), os.O_RDONLY, 0600)
if err == nil {
st, err := layerFile.Stat()
if err != nil {
logrus.Warnf("error reading size of layer file %q: %v", blob.Digest.String(), err)
} else {
size = st.Size()
layerReadCloser = layerFile
break
}
layerFile.Close()
}
if !os.IsNotExist(err) {
logrus.Debugf("error checking for layer %q in %q: %v", blob.Digest.String(), blobDir, err)
}
}
}
if err != nil || layerReadCloser == nil || size == -1 {
logrus.Debugf("error reading layer %q: %v", blob.Digest.String(), err)
return nil, -1, errors.Wrap(err, "error opening layer blob")
}
logrus.Debugf("reading layer %q", blob.Digest.String())
closer := func() error {
logrus.Debugf("finished reading layer %q", blob.Digest.String())
if err := layerFile.Close(); err != nil {
if err := layerReadCloser.Close(); err != nil {
return errors.Wrapf(err, "error closing layer %q after reading", blob.Digest.String())
}
return nil
}
return ioutils.NewReadCloserWrapper(layerFile, closer), size, nil
return ioutils.NewReadCloserWrapper(layerReadCloser, closer), size, nil
}
func (b *Builder) makeImageRef(options CommitOptions, exporting bool) (types.ImageReference, error) {
func (b *Builder) makeImageRef(options CommitOptions) (types.ImageReference, error) {
var name reference.Named
container, err := b.store.Container(b.ContainerID)
if err != nil {
@ -768,7 +789,6 @@ func (b *Builder) makeImageRef(options CommitOptions, exporting bool) (types.Ima
historyComment: b.HistoryComment(),
annotations: b.Annotations(),
preferredManifestType: manifestType,
exporting: exporting,
squash: options.Squash,
emptyLayer: options.EmptyLayer && !options.Squash,
idMappingOptions: &b.IDMappingOptions,

View File

@ -46,19 +46,24 @@ load helpers
run_buildah from --quiet --pull-always --signature-policy ${TESTSDIR}/policy.json dir:${elsewhere}
expect_output "$(basename ${elsewhere})-working-container"
run_buildah from --pull --signature-policy ${TESTSDIR}/policy.json scratch
cid=$output
run_buildah commit --signature-policy ${TESTSDIR}/policy.json $cid oci-archive:${elsewhere}.oci
run_buildah rm $cid
run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json oci-archive:${elsewhere}.oci
expect_output "oci-archive-working-container"
run_buildah rm $output
run_buildah from --pull --signature-policy ${TESTSDIR}/policy.json scratch
cid=$output
run_buildah commit --signature-policy ${TESTSDIR}/policy.json $cid dir:${elsewhere}
run_buildah commit --signature-policy ${TESTSDIR}/policy.json $cid docker-archive:${elsewhere}.docker
run_buildah rm $cid
run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json dir:${elsewhere}
expect_output "elsewhere-img-working-container"
run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json docker-archive:${elsewhere}.docker
expect_output "docker-archive-working-container"
run_buildah rm $output
run_buildah from --quiet --pull-always --signature-policy ${TESTSDIR}/policy.json dir:${elsewhere}
expect_output "$(basename ${elsewhere})-working-container"
}
@test "from-tagged-image" {