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

Update to BuildKit 0.12 #45966

Merged
merged 11 commits into from
Sep 22, 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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
9 changes: 7 additions & 2 deletions .github/workflows/buildkit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ jobs:
disabledFeatures="${disabledFeatures},merge_diff"
fi
echo "BUILDKIT_TEST_DISABLE_FEATURES=${disabledFeatures}" >> $GITHUB_ENV
# Expose `ACTIONS_RUNTIME_TOKEN` and `ACTIONS_CACHE_URL`, which is used
# in BuildKit's test suite to skip/unskip cache exporters:
# https://github.com/moby/buildkit/blob/567a99433ca23402d5e9b9f9124005d2e59b8861/client/client_test.go#L5407-L5411
-
name: Expose GitHub Runtime
uses: crazy-max/ghaction-github-runtime@v2
neersighted marked this conversation as resolved.
Show resolved Hide resolved
-
name: Checkout
uses: actions/checkout@v3
Expand Down Expand Up @@ -118,6 +124,5 @@ jobs:
TEST_DOCKERD: "1"
TEST_DOCKERD_BINARY: "./build/moby/dockerd"
TESTPKGS: "./${{ matrix.pkg }}"
# Skip buildkit tests checking the digest (see https://github.com/moby/buildkit/pull/3736)
TESTFLAGS: "-v --parallel=1 --timeout=30m --run=/^Test([^R]|.[^e]|..[^p]|...[^r]|....[^o]|.....[^S])/worker=${{ matrix.worker }}$"
TESTFLAGS: "-v --parallel=1 --timeout=30m --run=//worker=${{ matrix.worker }}$"
working-directory: buildkit
119 changes: 77 additions & 42 deletions builder/builder-next/adapters/containerimage/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"path"
"strings"
"sync"
"time"

Expand All @@ -16,10 +17,11 @@ import (
"github.com/containerd/containerd/leases"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/platforms"
cdreference "github.com/containerd/containerd/reference"
ctdreference "github.com/containerd/containerd/reference"
"github.com/containerd/containerd/remotes"
"github.com/containerd/containerd/remotes/docker"
"github.com/containerd/containerd/remotes/docker/schema1"
"github.com/containerd/containerd/remotes/docker/schema1" //nolint:staticcheck // Ignore SA1019: "github.com/containerd/containerd/remotes/docker/schema1" is deprecated: use images formatted in Docker Image Manifest v2, Schema 2, or OCI Image Spec v1.
neersighted marked this conversation as resolved.
Show resolved Hide resolved
distreference "github.com/distribution/reference"
dimages "github.com/docker/docker/daemon/images"
"github.com/docker/docker/distribution/metadata"
Expand All @@ -32,8 +34,12 @@ import (
"github.com/moby/buildkit/client/llb"
"github.com/moby/buildkit/session"
"github.com/moby/buildkit/solver"
"github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/source"
srctypes "github.com/moby/buildkit/source/types"
"github.com/moby/buildkit/sourcepolicy"
policy "github.com/moby/buildkit/sourcepolicy/pb"
spb "github.com/moby/buildkit/sourcepolicy/pb"
"github.com/moby/buildkit/util/flightcontrol"
"github.com/moby/buildkit/util/imageutil"
"github.com/moby/buildkit/util/leaseutil"
Expand Down Expand Up @@ -63,7 +69,7 @@ type SourceOpt struct {
// Source is the source implementation for accessing container images
type Source struct {
SourceOpt
g flightcontrol.Group
g flightcontrol.Group[*resolveRemoteResult]
}

// NewSource creates a new image source
Expand Down Expand Up @@ -92,45 +98,49 @@ func (is *Source) resolveLocal(refStr string) (*image.Image, error) {
return img, nil
}

func (is *Source) resolveRemote(ctx context.Context, ref string, platform *ocispec.Platform, sm *session.Manager, g session.Group) (digest.Digest, []byte, error) {
type t struct {
dgst digest.Digest
dt []byte
}
type resolveRemoteResult struct {
ref string
dgst digest.Digest
dt []byte
}

func (is *Source) resolveRemote(ctx context.Context, ref string, platform *ocispec.Platform, sm *session.Manager, g session.Group) (string, digest.Digest, []byte, error) {
p := platforms.DefaultSpec()
if platform != nil {
p = *platform
}
// key is used to synchronize resolutions that can happen in parallel when doing multi-stage.
key := "getconfig::" + ref + "::" + platforms.Format(p)
res, err := is.g.Do(ctx, key, func(ctx context.Context) (interface{}, error) {
res, err := is.g.Do(ctx, key, func(ctx context.Context) (*resolveRemoteResult, error) {
res := resolver.DefaultPool.GetResolver(is.RegistryHosts, ref, "pull", sm, g)
dgst, dt, err := imageutil.Config(ctx, ref, res, is.ContentStore, is.LeaseManager, platform)
ref, dgst, dt, err := imageutil.Config(ctx, ref, res, is.ContentStore, is.LeaseManager, platform, []*policy.Policy{})
if err != nil {
return nil, err
}
return &t{dgst: dgst, dt: dt}, nil
return &resolveRemoteResult{ref: ref, dgst: dgst, dt: dt}, nil
})
var typed *t
if err != nil {
return "", nil, err
return ref, "", nil, err
}
typed = res.(*t)
return typed.dgst, typed.dt, nil
return res.ref, res.dgst, res.dt, nil
}

// ResolveImageConfig returns image config for an image
func (is *Source) ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt, sm *session.Manager, g session.Group) (digest.Digest, []byte, error) {
func (is *Source) ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt, sm *session.Manager, g session.Group) (string, digest.Digest, []byte, error) {
ref, err := applySourcePolicies(ctx, ref, opt.SourcePolicies)
if err != nil {
return "", "", nil, err
}
resolveMode, err := source.ParseImageResolveMode(opt.ResolveMode)
if err != nil {
return "", nil, err
return ref, "", nil, err
}
switch resolveMode {
case source.ResolveModeForcePull:
dgst, dt, err := is.resolveRemote(ctx, ref, opt.Platform, sm, g)
ref, dgst, dt, err := is.resolveRemote(ctx, ref, opt.Platform, sm, g)
// TODO: pull should fallback to local in case of failure to allow offline behavior
// the fallback doesn't work currently
return dgst, dt, err
return ref, dgst, dt, err
/*
if err == nil {
return dgst, dt, err
Expand All @@ -152,14 +162,14 @@ func (is *Source) ResolveImageConfig(ctx context.Context, ref string, opt llb.Re
path.Join(img.OS, img.Architecture, img.Variant),
)
} else {
return "", img.RawJSON(), err
return ref, "", img.RawJSON(), err
}
}
// fallback to remote
return is.resolveRemote(ctx, ref, opt.Platform, sm, g)
}
// should never happen
return "", nil, fmt.Errorf("builder cannot resolve image %s: invalid mode %q", ref, opt.ResolveMode)
return ref, "", nil, fmt.Errorf("builder cannot resolve image %s: invalid mode %q", ref, opt.ResolveMode)
}

// Resolve returns access to pulling for an identifier
Expand Down Expand Up @@ -187,7 +197,7 @@ func (is *Source) Resolve(ctx context.Context, id source.Identifier, sm *session
type puller struct {
is *Source
resolveLocalOnce sync.Once
g flightcontrol.Group
g flightcontrol.Group[struct{}]
src *source.ImageIdentifier
desc ocispec.Descriptor
ref string
Expand Down Expand Up @@ -258,21 +268,21 @@ func (p *puller) resolveLocal() {
}

func (p *puller) resolve(ctx context.Context, g session.Group) error {
_, err := p.g.Do(ctx, "", func(ctx context.Context) (_ interface{}, err error) {
_, err := p.g.Do(ctx, "", func(ctx context.Context) (_ struct{}, err error) {
resolveProgressDone := oneOffProgress(ctx, "resolve "+p.src.Reference.String())
defer func() {
resolveProgressDone(err)
}()

ref, err := distreference.ParseNormalizedNamed(p.src.Reference.String())
if err != nil {
return nil, err
return struct{}{}, err
}

if p.desc.Digest == "" && p.config == nil {
origRef, desc, err := p.resolver(g).Resolve(ctx, ref.String())
if err != nil {
return nil, err
return struct{}{}, err
}

p.desc = desc
Expand All @@ -287,16 +297,17 @@ func (p *puller) resolve(ctx context.Context, g session.Group) error {
if p.config == nil && p.desc.MediaType != images.MediaTypeDockerSchema1Manifest {
ref, err := distreference.WithDigest(ref, p.desc.Digest)
if err != nil {
return nil, err
return struct{}{}, err
}
_, dt, err := p.is.ResolveImageConfig(ctx, ref.String(), llb.ResolveImageConfigOpt{Platform: &p.platform, ResolveMode: resolveModeToString(p.src.ResolveMode)}, p.sm, g)
newRef, _, dt, err := p.is.ResolveImageConfig(ctx, ref.String(), llb.ResolveImageConfigOpt{Platform: &p.platform, ResolveMode: p.src.ResolveMode.String()}, p.sm, g)
if err != nil {
return nil, err
return struct{}{}, err
}

p.ref = newRef
p.config = dt
}
return nil, nil
return struct{}{}, nil
})
return err
}
Expand Down Expand Up @@ -837,20 +848,6 @@ func cacheKeyFromConfig(dt []byte) digest.Digest {
return identity.ChainID(img.RootFS.DiffIDs)
}

// resolveModeToString is the equivalent of github.com/moby/buildkit/solver/llb.ResolveMode.String()
// FIXME: add String method on source.ResolveMode
func resolveModeToString(rm source.ResolveMode) string {
switch rm {
case source.ResolveModeDefault:
return "default"
case source.ResolveModeForcePull:
return "pull"
case source.ResolveModePreferLocal:
return "local"
}
return ""
}

func platformMatches(img *image.Image, p *ocispec.Platform) bool {
return dimages.OnlyPlatformWithFallback(*p).Match(ocispec.Platform{
Architecture: img.Architecture,
Expand All @@ -860,3 +857,41 @@ func platformMatches(img *image.Image, p *ocispec.Platform) bool {
Variant: img.Variant,
})
}

func applySourcePolicies(ctx context.Context, str string, spls []*spb.Policy) (string, error) {
ref, err := cdreference.Parse(str)
if err != nil {
return "", errors.WithStack(err)
}
op := &pb.Op{
Op: &pb.Op_Source{
Source: &pb.SourceOp{
Identifier: srctypes.DockerImageScheme + "://" + ref.String(),
},
},
}

mut, err := sourcepolicy.NewEngine(spls).Evaluate(ctx, op)
if err != nil {
return "", errors.Wrap(err, "could not resolve image due to policy")
}

if mut {
var (
t string
ok bool
)
t, newRef, ok := strings.Cut(op.GetSource().GetIdentifier(), "://")
if !ok {
return "", errors.Errorf("could not parse ref: %s", op.GetSource().GetIdentifier())
}
if ok && t != srctypes.DockerImageScheme {
return "", &imageutil.ResolveToNonImageError{Ref: str, Updated: newRef}
Copy link
Member

Choose a reason for hiding this comment

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

This is probably fine as is since it is what is still in buildkit, but this was meant to be the full identifier.

ref: moby/buildkit#4209

Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally we wouldn't be reinventing this since it's sufficiently abstracted from the snapshotters/graphdrivers, but the implementation in BuildKit isn't really shaped right for us to re-use. I've asked @tonistiigi and he's interested in cleaning this up so we can share it; but assuming that doesn't happen immediately, would you be willing to port your fix once it's accepted in BuildKit?

}
ref, err = cdreference.Parse(newRef)
if err != nil {
return "", errors.WithStack(err)
}
}
return ref.String(), nil
}
8 changes: 5 additions & 3 deletions builder/builder-next/adapters/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/docker/docker/pkg/idtools"
"github.com/moby/buildkit/identity"
"github.com/moby/buildkit/snapshot"
"github.com/moby/buildkit/util/leaseutil"
"github.com/opencontainers/go-digest"
"github.com/pkg/errors"
bolt "go.etcd.io/bbolt"
Expand Down Expand Up @@ -57,7 +58,7 @@ type snapshotter struct {
}

// NewSnapshotter creates a new snapshotter
func NewSnapshotter(opt Opt, prevLM leases.Manager) (snapshot.Snapshotter, leases.Manager, error) {
func NewSnapshotter(opt Opt, prevLM leases.Manager, ns string) (snapshot.Snapshotter, *leaseutil.Manager, error) {
dbPath := filepath.Join(opt.Root, "snapshots.db")
db, err := bolt.Open(dbPath, 0o600, nil)
if err != nil {
Expand All @@ -76,7 +77,8 @@ func NewSnapshotter(opt Opt, prevLM leases.Manager) (snapshot.Snapshotter, lease
reg: reg,
}

lm := newLeaseManager(s, prevLM)
slm := newLeaseManager(s, prevLM)
lm := leaseutil.WithNamespace(slm, ns)

ll, err := lm.List(context.TODO())
if err != nil {
Expand All @@ -89,7 +91,7 @@ func NewSnapshotter(opt Opt, prevLM leases.Manager) (snapshot.Snapshotter, lease
}
for _, r := range rr {
if r.Type == "snapshots/default" {
lm.addRef(l.ID, r.ID)
slm.addRef(l.ID, r.ID)
}
}
}
Expand Down
16 changes: 7 additions & 9 deletions builder/builder-next/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ import (
"github.com/moby/buildkit/frontend/gateway"
"github.com/moby/buildkit/frontend/gateway/forwarder"
containerdsnapshot "github.com/moby/buildkit/snapshot/containerd"
"github.com/moby/buildkit/solver"
"github.com/moby/buildkit/solver/bboltcachestorage"
"github.com/moby/buildkit/util/archutil"
"github.com/moby/buildkit/util/entitlements"
"github.com/moby/buildkit/util/leaseutil"
"github.com/moby/buildkit/util/network/netproviders"
"github.com/moby/buildkit/util/tracing/detect"
"github.com/moby/buildkit/worker"
Expand Down Expand Up @@ -135,7 +135,7 @@ func newSnapshotterController(ctx context.Context, rt http.RoundTripper, opt Opt
SessionManager: opt.SessionManager,
WorkerController: wc,
Frontends: frontends,
CacheKeyStorage: cacheStorage,
CacheManager: solver.NewCacheManager(ctx, "local", cacheStorage, worker.NewCacheResultStorage(wc)),
ResolveCacheImporterFuncs: map[string]remotecache.ResolveCacheImporterFunc{
"gha": gha.ResolveCacheImporterFunc(),
"local": localremotecache.ResolveCacheImporterFunc(opt.SessionManager),
Expand Down Expand Up @@ -202,7 +202,7 @@ func newGraphDriverController(ctx context.Context, rt http.RoundTripper, opt Opt
return nil, errors.Errorf("could not access graphdriver")
}

store, err := local.NewStore(filepath.Join(root, "content"))
innerStore, err := local.NewStore(filepath.Join(root, "content"))
if err != nil {
return nil, err
}
Expand All @@ -212,18 +212,16 @@ func newGraphDriverController(ctx context.Context, rt http.RoundTripper, opt Opt
return nil, errors.WithStack(err)
}

mdb := ctdmetadata.NewDB(db, store, map[string]snapshots.Snapshotter{})
mdb := ctdmetadata.NewDB(db, innerStore, map[string]snapshots.Snapshotter{})

store = containerdsnapshot.NewContentStore(mdb.ContentStore(), "buildkit")

lm := leaseutil.WithNamespace(ctdmetadata.NewLeaseManager(mdb), "buildkit")
store := containerdsnapshot.NewContentStore(mdb.ContentStore(), "buildkit")

snapshotter, lm, err := snapshot.NewSnapshotter(snapshot.Opt{
GraphDriver: driver,
LayerStore: dist.LayerStore,
Root: root,
IdentityMapping: opt.IdentityMapping,
}, lm)
}, ctdmetadata.NewLeaseManager(mdb), "buildkit")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -358,7 +356,7 @@ func newGraphDriverController(ctx context.Context, rt http.RoundTripper, opt Opt
SessionManager: opt.SessionManager,
WorkerController: wc,
Frontends: frontends,
CacheKeyStorage: cacheStorage,
CacheManager: solver.NewCacheManager(ctx, "local", cacheStorage, worker.NewCacheResultStorage(wc)),
ResolveCacheImporterFuncs: map[string]remotecache.ResolveCacheImporterFunc{
"registry": localinlinecache.ResolveCacheImporterFunc(opt.SessionManager, opt.RegistryHosts, store, dist.ReferenceStore, dist.ImageStore),
"local": localremotecache.ResolveCacheImporterFunc(opt.SessionManager),
Expand Down