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

Import/export support for OCI compatible image manifest version of cache manifest (opt-in on export, inferred on import) #3724

Merged
merged 2 commits into from May 13, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Expand Up @@ -388,6 +388,7 @@ buildctl build ... \
* `min`: only export layers for the resulting image
* `max`: export all the layers of all intermediate steps
* `ref=<ref>`: specify repository reference to store cache, e.g. `docker.io/user/image:tag`
* `image-manifest=<true|false>`: whether to export cache manifest as an OCI-compatible image manifest rather than a manifest list/index (default: `false`, must be used with `oci-mediatypes=true`)
* `oci-mediatypes=<true|false>`: whether to use OCI mediatypes in exported manifests (default: `true`, since BuildKit `v0.8`)
* `compression=<uncompressed|gzip|estargz|zstd>`: choose compression type for layers newly created and cached, gzip is default value. estargz and zstd should be used with `oci-mediatypes=true`
* `compression-level=<value>`: choose compression level for gzip, estargz (0-9) and zstd (0-22)
Expand All @@ -414,6 +415,7 @@ The directory layout conforms to OCI Image Spec v1.0.
* `max`: export all the layers of all intermediate steps
* `dest=<path>`: destination directory for cache exporter
* `tag=<tag>`: specify custom tag of image to write to local index (default: `latest`)
* `image-manifest=<true|false>`: whether to export cache manifest as an OCI-compatible image manifest rather than a manifest list/index (default: `false`, must be used with `oci-mediatypes=true`)
* `oci-mediatypes=<true|false>`: whether to use OCI mediatypes in exported manifests (default `true`, since BuildKit `v0.8`)
* `compression=<uncompressed|gzip|estargz|zstd>`: choose compression type for layers newly created and cached, gzip is default value. estargz and zstd should be used with `oci-mediatypes=true`.
* `compression-level=<value>`: compression level for gzip, estargz (0-9) and zstd (0-22)
Expand Down
167 changes: 136 additions & 31 deletions cache/remotecache/export.go
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/moby/buildkit/util/progress"
"github.com/moby/buildkit/util/progress/logs"
digest "github.com/opencontainers/go-digest"
specs "github.com/opencontainers/image-spec/specs-go"
"github.com/opencontainers/image-spec/specs-go"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
)
Expand All @@ -37,24 +37,135 @@ type Config struct {
Compression compression.Config
}

type CacheType int

const (
// ExportResponseManifestDesc is a key for the map returned from Exporter.Finalize.
// The map value is a JSON string of an OCI desciptor of a manifest.
ExporterResponseManifestDesc = "cache.manifest"
)

type contentCacheExporter struct {
solver.CacheExporterTarget
chains *v1.CacheChains
ingester content.Ingester
oci bool
ref string
comp compression.Config
const (
NotSet CacheType = iota
ManifestList
ImageManifest
)

func (data CacheType) String() string {
switch data {
case ManifestList:
return "Manifest List"
case ImageManifest:
return "Image Manifest"
default:
return "Not Set"
}
}

func NewExporter(ingester content.Ingester, ref string, oci bool, compressionConfig compression.Config) Exporter {
func NewExporter(ingester content.Ingester, ref string, oci bool, imageManifest bool, compressionConfig compression.Config) Exporter {
cc := v1.NewCacheChains()
return &contentCacheExporter{CacheExporterTarget: cc, chains: cc, ingester: ingester, oci: oci, ref: ref, comp: compressionConfig}
return &contentCacheExporter{CacheExporterTarget: cc, chains: cc, ingester: ingester, oci: oci, imageManifest: imageManifest, ref: ref, comp: compressionConfig}
}

type ExportableCache struct {
// This cache describes two distinct styles of exportable cache, one is an Index (or Manifest List) of blobs,
// or as an artifact using the OCI image manifest format.
ExportedManifest ocispecs.Manifest
ExportedIndex ocispecs.Index
CacheType CacheType
OCI bool
}

func NewExportableCache(oci bool, imageManifest bool) (*ExportableCache, error) {
var mediaType string

if imageManifest {
mediaType = ocispecs.MediaTypeImageManifest
if !oci {
return nil, errors.Errorf("invalid configuration for remote cache")
}
} else {
if oci {
mediaType = ocispecs.MediaTypeImageIndex
} else {
mediaType = images.MediaTypeDockerSchema2ManifestList
}
}

cacheType := ManifestList
if imageManifest {
cacheType = ImageManifest
}

schemaVersion := specs.Versioned{SchemaVersion: 2}
switch cacheType {
case ManifestList:
return &ExportableCache{ExportedIndex: ocispecs.Index{
MediaType: mediaType,
Versioned: schemaVersion,
},
CacheType: cacheType,
OCI: oci,
}, nil
case ImageManifest:
return &ExportableCache{ExportedManifest: ocispecs.Manifest{
MediaType: mediaType,
Versioned: schemaVersion,
},
CacheType: cacheType,
OCI: oci,
}, nil
default:
return nil, errors.Errorf("exportable cache type not set")
}
}

func (ec *ExportableCache) MediaType() string {
if ec.CacheType == ManifestList {
return ec.ExportedIndex.MediaType
}
return ec.ExportedManifest.MediaType
}

func (ec *ExportableCache) AddCacheBlob(blob ocispecs.Descriptor) {
if ec.CacheType == ManifestList {
ec.ExportedIndex.Manifests = append(ec.ExportedIndex.Manifests, blob)
} else {
ec.ExportedManifest.Layers = append(ec.ExportedManifest.Layers, blob)
}
}

func (ec *ExportableCache) FinalizeCache(ctx context.Context) {
if ec.CacheType == ManifestList {
ec.ExportedIndex.Manifests = compression.ConvertAllLayerMediaTypes(ctx, ec.OCI, ec.ExportedIndex.Manifests...)
} else {
ec.ExportedManifest.Layers = compression.ConvertAllLayerMediaTypes(ctx, ec.OCI, ec.ExportedManifest.Layers...)
}
}

func (ec *ExportableCache) SetConfig(config ocispecs.Descriptor) {
if ec.CacheType == ManifestList {
ec.ExportedIndex.Manifests = append(ec.ExportedIndex.Manifests, config)
} else {
ec.ExportedManifest.Config = config
}
}

func (ec *ExportableCache) MarshalJSON() ([]byte, error) {
if ec.CacheType == ManifestList {
return json.Marshal(ec.ExportedIndex)
}
return json.Marshal(ec.ExportedManifest)
}

type contentCacheExporter struct {
solver.CacheExporterTarget
chains *v1.CacheChains
ingester content.Ingester
oci bool
imageManifest bool
ref string
comp compression.Config
}

func (ce *contentCacheExporter) Name() string {
Expand All @@ -74,21 +185,9 @@ func (ce *contentCacheExporter) Finalize(ctx context.Context) (map[string]string
return nil, err
}

// own type because oci type can't be pushed and docker type doesn't have annotations
type manifestList struct {
specs.Versioned

MediaType string `json:"mediaType,omitempty"`

// Manifests references platform specific manifests.
Manifests []ocispecs.Descriptor `json:"manifests"`
}

var mfst manifestList
mfst.SchemaVersion = 2
mfst.MediaType = images.MediaTypeDockerSchema2ManifestList
if ce.oci {
mfst.MediaType = ocispecs.MediaTypeImageIndex
cache, err := NewExportableCache(ce.oci, ce.imageManifest)
if err != nil {
return nil, err
}

for _, l := range config.Layers {
Expand All @@ -101,10 +200,10 @@ func (ce *contentCacheExporter) Finalize(ctx context.Context) (map[string]string
return nil, layerDone(errors.Wrap(err, "error writing layer blob"))
}
layerDone(nil)
mfst.Manifests = append(mfst.Manifests, dgstPair.Descriptor)
cache.AddCacheBlob(dgstPair.Descriptor)
}

mfst.Manifests = compression.ConvertAllLayerMediaTypes(ctx, ce.oci, mfst.Manifests...)
cache.FinalizeCache(ctx)

dt, err := json.Marshal(config)
if err != nil {
Expand All @@ -122,9 +221,9 @@ func (ce *contentCacheExporter) Finalize(ctx context.Context) (map[string]string
}
configDone(nil)

mfst.Manifests = append(mfst.Manifests, desc)
cache.SetConfig(desc)

dt, err = json.Marshal(mfst)
dt, err = cache.MarshalJSON()
if err != nil {
return nil, errors.Wrap(err, "failed to marshal manifest")
}
Expand All @@ -133,9 +232,14 @@ func (ce *contentCacheExporter) Finalize(ctx context.Context) (map[string]string
desc = ocispecs.Descriptor{
Digest: dgst,
Size: int64(len(dt)),
MediaType: mfst.MediaType,
MediaType: cache.MediaType(),
}
mfstDone := progress.OneOff(ctx, fmt.Sprintf("writing manifest %s", dgst))

mfstLog := fmt.Sprintf("writing cache manifest %s", dgst)
if ce.imageManifest {
mfstLog = fmt.Sprintf("writing cache image manifest %s", dgst)
}
mfstDone := progress.OneOff(ctx, mfstLog)
if err := content.WriteBlob(ctx, ce.ingester, dgst.String(), bytes.NewReader(dt), desc); err != nil {
return nil, mfstDone(errors.Wrap(err, "error writing manifest blob"))
}
Expand All @@ -145,5 +249,6 @@ func (ce *contentCacheExporter) Finalize(ctx context.Context) (map[string]string
}
res[ExporterResponseManifestDesc] = string(descJSON)
mfstDone(nil)

return res, nil
}
50 changes: 40 additions & 10 deletions cache/remotecache/import.go
Expand Up @@ -3,6 +3,7 @@ package remotecache
import (
"context"
"encoding/json"
"fmt"
"io"
"sync"
"time"
Expand All @@ -14,6 +15,7 @@ import (
"github.com/moby/buildkit/solver"
"github.com/moby/buildkit/util/bklog"
"github.com/moby/buildkit/util/imageutil"
"github.com/moby/buildkit/util/progress"
"github.com/moby/buildkit/worker"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
Expand Down Expand Up @@ -47,24 +49,52 @@ func (ci *contentCacheImporter) Resolve(ctx context.Context, desc ocispecs.Descr
return nil, err
}

var mfst ocispecs.Index
if err := json.Unmarshal(dt, &mfst); err != nil {
manifestType, err := imageutil.DetectManifestBlobMediaType(dt)
if err != nil {
return nil, err
}

allLayers := v1.DescriptorProvider{}
layerDone := progress.OneOff(ctx, fmt.Sprintf("inferred cache manifest type: %s", manifestType))
layerDone(nil)

allLayers := v1.DescriptorProvider{}
var configDesc ocispecs.Descriptor

for _, m := range mfst.Manifests {
if m.MediaType == v1.CacheConfigMediaTypeV0 {
configDesc = m
continue
switch manifestType {
case images.MediaTypeDockerSchema2ManifestList, ocispecs.MediaTypeImageIndex:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be follow-up, but I guess manifest list of "cache image manifests" should also be a valid configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not sure I should tackle this because I don't know how such a cache would/could be exported.

var mfst ocispecs.Index
if err := json.Unmarshal(dt, &mfst); err != nil {
return nil, err
}
allLayers[m.Digest] = v1.DescriptorProviderPair{
Descriptor: m,
Provider: ci.provider,

for _, m := range mfst.Manifests {
if m.MediaType == v1.CacheConfigMediaTypeV0 {
configDesc = m
continue
}
allLayers[m.Digest] = v1.DescriptorProviderPair{
Descriptor: m,
Provider: ci.provider,
}
}
case images.MediaTypeDockerSchema2Manifest, ocispecs.MediaTypeImageManifest:
var mfst ocispecs.Manifest
if err := json.Unmarshal(dt, &mfst); err != nil {
return nil, err
}

if mfst.Config.MediaType == v1.CacheConfigMediaTypeV0 {
configDesc = mfst.Config
}
for _, m := range mfst.Layers {
allLayers[m.Digest] = v1.DescriptorProviderPair{
Descriptor: m,
Provider: ci.provider,
}
}
default:
err = errors.Wrapf(err, "unsupported or uninferrable manifest type")
return nil, err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is the inline case still hit in line 112?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would work fine if the ref'd manifest happens to be a Manifest List/Index and had an inline cache in its config (can that even happen?), but it would fail to hit the inline case if the ref is an Image Manifest because of line 88. I propose just ripping out lines 88 through 91 and letting any malform failures happen in importInlineCache.

Copy link
Contributor Author

@kattmang kattmang May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, my apologies for not seeing the integ test failure indicating this issue: I did see some integ tests fail without any changes from my part so I thought that was attributable to other changes in master. Very good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested in my local, ripping out 88-91 does indeed fix the integ tests failing below.

}

if dsls, ok := ci.provider.(DistributionSourceLabelSetter); ok {
Expand Down
11 changes: 10 additions & 1 deletion cache/remotecache/local/local.go
Expand Up @@ -19,6 +19,7 @@ const (
attrDigest = "digest"
attrSrc = "src"
attrDest = "dest"
attrImageManifest = "image-manifest"
attrOCIMediatypes = "oci-mediatypes"
contentStoreIDPrefix = "local:"
)
Expand Down Expand Up @@ -50,12 +51,20 @@ func ResolveCacheExporterFunc(sm *session.Manager) remotecache.ResolveCacheExpor
}
ociMediatypes = b
}
imageManifest := false
if v, ok := attrs[attrImageManifest]; ok {
b, err := strconv.ParseBool(v)
if err != nil {
return nil, errors.Wrapf(err, "failed to parse %s", attrImageManifest)
}
imageManifest = b
}
csID := contentStoreIDPrefix + store
cs, err := getContentStore(ctx, sm, g, csID)
if err != nil {
return nil, err
}
return &exporter{remotecache.NewExporter(cs, "", ociMediatypes, compressionConfig)}, nil
return &exporter{remotecache.NewExporter(cs, "", ociMediatypes, imageManifest, compressionConfig)}, nil
}
}

Expand Down
11 changes: 10 additions & 1 deletion cache/remotecache/registry/registry.go
Expand Up @@ -36,6 +36,7 @@ func canonicalizeRef(rawRef string) (reference.Named, error) {

const (
attrRef = "ref"
attrImageManifest = "image-manifest"
attrOCIMediatypes = "oci-mediatypes"
attrInsecure = "registry.insecure"
)
Expand Down Expand Up @@ -67,6 +68,14 @@ func ResolveCacheExporterFunc(sm *session.Manager, hosts docker.RegistryHosts) r
}
ociMediatypes = b
}
imageManifest := false
if v, ok := attrs[attrImageManifest]; ok {
b, err := strconv.ParseBool(v)
if err != nil {
return nil, errors.Wrapf(err, "failed to parse %s", attrImageManifest)
}
imageManifest = b
}
insecure := false
if v, ok := attrs[attrInsecure]; ok {
b, err := strconv.ParseBool(v)
Expand All @@ -82,7 +91,7 @@ func ResolveCacheExporterFunc(sm *session.Manager, hosts docker.RegistryHosts) r
if err != nil {
return nil, err
}
return &exporter{remotecache.NewExporter(contentutil.FromPusher(pusher), refString, ociMediatypes, compressionConfig)}, nil
return &exporter{remotecache.NewExporter(contentutil.FromPusher(pusher), refString, ociMediatypes, imageManifest, compressionConfig)}, nil
}
}

Expand Down