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 4 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
  •  
  •  
  •  
18 changes: 9 additions & 9 deletions builder/builder-next/adapters/containerimage/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
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 Down Expand Up @@ -63,7 +63,7 @@ type SourceOpt struct {
// Source is the source implementation for accessing container images
type Source struct {
SourceOpt
g flightcontrol.Group
g flightcontrol.Group[interface{}]
neersighted marked this conversation as resolved.
Show resolved Hide resolved
}

// NewSource creates a new image source
Expand Down Expand Up @@ -187,7 +187,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 +258,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 +287,16 @@ 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)
if err != nil {
return nil, err
return struct{}{}, err
}

p.config = dt
}
return nil, nil
return struct{}{}, nil
})
return err
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
//go:build !windows

package buildkit
thaJeztah marked this conversation as resolved.
Show resolved Hide resolved

import (
Expand All @@ -16,6 +14,7 @@ import (
"github.com/docker/docker/pkg/stringid"
"github.com/moby/buildkit/executor"
"github.com/moby/buildkit/executor/oci"
"github.com/moby/buildkit/executor/resources"
"github.com/moby/buildkit/executor/runcexecutor"
"github.com/moby/buildkit/identity"
"github.com/moby/buildkit/solver/pb"
Expand Down Expand Up @@ -51,6 +50,11 @@ func newExecutor(root, cgroupParent string, net *libnetwork.Controller, dnsConfi
pidmap = nil
}

rm, err := resources.NewMonitor()
if err != nil {
return nil, err
}

return runcexecutor.New(runcexecutor.Opt{
Root: filepath.Join(root, "executor"),
CommandCandidates: []string{"runc"},
Expand All @@ -60,6 +64,7 @@ func newExecutor(root, cgroupParent string, net *libnetwork.Controller, dnsConfi
IdentityMapping: pidmap,
DNS: dnsConfig,
ApparmorProfile: apparmorProfile,
ResourceMonitor: rm,
}, networkProviders)
}

Expand Down Expand Up @@ -121,6 +126,11 @@ func (iface *lnInterface) init(c *libnetwork.Controller, n *libnetwork.Network)
iface.ep = ep
}

// TODO(neersighted): Unstub Sample(), and collect data from the libnetwork Endpoint.
func (iface *lnInterface) Sample() (*network.Sample, error) {
neersighted marked this conversation as resolved.
Show resolved Hide resolved
return &network.Sample{}, nil
}

func (iface *lnInterface) Set(s *specs.Spec) error {
<-iface.ready
if iface.err != nil {
Expand Down
34 changes: 34 additions & 0 deletions builder/builder-next/executor_nolinux.go
Copy link
Contributor

Choose a reason for hiding this comment

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

If we get a builder-next executor for Windows, will this file need to be renamed again? What about executor_notimplemented.go, since we're not relying on the GOOS filename tag anymore, and then any future executors only need to update the build-tag to exclude their newly-implemented GOOS from the not-implemented list.

Or am I misunderstanding, and this can never be implemented on non-Linux, e.g., even if we have BuildKit working for Windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

This could be implemented for Windows, based on @gabriel-samfira and your work, as I understand it. We would need to rename the file, but I like it like this for now as _nolinux is a BuildKit-ism that we are symmetric with (when you look at the built-in executor implementations).

When BuildKit changes, I expect we'll rename this/change the build tags to once again be symmetric.

Copy link
Member

Choose a reason for hiding this comment

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

The only consideration could be to name this _unsupported.go (done that in some other cases, to be slightly more generic, and sometimes preventing some renames).

Not a blocker for me though

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//go:build !linux

package buildkit

import (
"context"
"errors"
"runtime"

"github.com/docker/docker/daemon/config"
"github.com/docker/docker/libnetwork"
"github.com/docker/docker/pkg/idtools"
"github.com/moby/buildkit/executor"
"github.com/moby/buildkit/executor/oci"
resourcetypes "github.com/moby/buildkit/executor/resources/types"
)

func newExecutor(_, _ string, _ *libnetwork.Controller, _ *oci.DNSConfig, _ bool, _ idtools.IdentityMapping, _ string) (executor.Executor, error) {
return &stubExecutor{}, nil
}

type stubExecutor struct{}

func (w *stubExecutor) Run(ctx context.Context, id string, root executor.Mount, mounts []executor.Mount, process executor.ProcessInfo, started chan<- struct{}) (resourcetypes.Recorder, error) {
return nil, errors.New("buildkit executor not implemented for "+runtime.GOOS)
}

func (w *stubExecutor) Exec(ctx context.Context, id string, process executor.ProcessInfo) error {
return errors.New("buildkit executor not implemented for "+runtime.GOOS)
}

func getDNSConfig(config.DNSConfig) *oci.DNSConfig {
return nil
}
30 changes: 0 additions & 30 deletions builder/builder-next/executor_windows.go

This file was deleted.

2 changes: 1 addition & 1 deletion daemon/containerd/image_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (i *ImageService) PullImage(ctx context.Context, ref reference.Named, platf

// Allow pulling application/vnd.docker.distribution.manifest.v1+prettyjws images
// by converting them to OCI manifests.
opts = append(opts, containerd.WithSchema1Conversion)
opts = append(opts, containerd.WithSchema1Conversion) //nolint:staticcheck // Ignore SA1019: containerd.WithSchema1Conversion is deprecated: use Schema 2 or OCI images.

img, err := i.client.Pull(ctx, ref.String(), opts...)
if err != nil {
Expand Down
11 changes: 4 additions & 7 deletions daemon/containerd/image_push.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
cerrdefs "github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/images"
containerdimages "github.com/containerd/containerd/images"
containerdlabels "github.com/containerd/containerd/labels"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/platforms"
"github.com/containerd/containerd/remotes"
Expand Down Expand Up @@ -246,23 +247,19 @@ func getDigestSources(ctx context.Context, store content.Manager, digest digest.

sources := extractDistributionSources(info.Labels)
if sources == nil {
return nil, errdefs.NotFound(fmt.Errorf("label %q is not attached to %s", labelDistributionSource, digest.String()))
return nil, errdefs.NotFound(fmt.Errorf("label %q is not attached to %s", containerdlabels.LabelDistributionSource, digest.String()))
}

return sources, nil
}

// TODO(vvoland): Remove and use containerd const in containerd 1.7+
// https://github.com/containerd/containerd/pull/8224
const labelDistributionSource = "containerd.io/distribution.source."

func extractDistributionSources(labels map[string]string) []distributionSource {
var sources []distributionSource

// Check if this blob has a distributionSource label
// if yes, read it as source
for k, v := range labels {
if reg := strings.TrimPrefix(k, labelDistributionSource); reg != k {
if reg := strings.TrimPrefix(k, containerdlabels.LabelDistributionSource); reg != k {
for _, repo := range strings.Split(v, ",") {
ref, err := reference.ParseNamed(reg + "/" + repo)
if err != nil {
Expand All @@ -287,7 +284,7 @@ type distributionSource struct {
func (source distributionSource) ToAnnotation() (string, string) {
domain := reference.Domain(source.registryRef)
v := reference.Path(source.registryRef)
return labelDistributionSource + domain, v
return containerdlabels.LabelDistributionSource + domain, v
}

func (source distributionSource) GetReference(dgst digest.Digest) (reference.Named, error) {
Expand Down
3 changes: 2 additions & 1 deletion daemon/containerd/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/containerd/containerd/content"
cerrdefs "github.com/containerd/containerd/errdefs"
containerdlabels "github.com/containerd/containerd/labels"
"github.com/distribution/reference"
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
Expand Down Expand Up @@ -42,7 +43,7 @@ func (p fakeStoreWithSources) Info(ctx context.Context, dgst digest.Digest) (con
return info, err
}

key := labelDistributionSource + reference.Domain(source.registryRef)
key := containerdlabels.LabelDistributionSource + reference.Domain(source.registryRef)
value := reference.Path(source.registryRef)
return content.Info{
Digest: dgst,
Expand Down
11 changes: 8 additions & 3 deletions daemon/runtime_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (
"github.com/docker/docker/api/types/system"
"github.com/docker/docker/daemon/config"
"github.com/docker/docker/errdefs"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/imdario/mergo"
"google.golang.org/protobuf/proto"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
Expand Down Expand Up @@ -242,11 +244,11 @@ func TestGetRuntime(t *testing.T) {
assert.Assert(t, ok, "stock runtime could not be found (test needs to be updated)")
stockRuntime.Features = nil

configdOpts := *stockRuntime.Opts.(*v2runcoptions.Options)
configdOpts := proto.Clone(stockRuntime.Opts.(*v2runcoptions.Options)).(*v2runcoptions.Options)
neersighted marked this conversation as resolved.
Show resolved Hide resolved
configdOpts.BinaryName = configuredRuntime.Path
wantConfigdRuntime := &shimConfig{
Shim: stockRuntime.Shim,
Opts: &configdOpts,
Opts: configdOpts,
}

for _, tt := range []struct {
Expand Down Expand Up @@ -334,7 +336,10 @@ func TestGetRuntime(t *testing.T) {
if tt.want != nil {
assert.Check(t, err)
got := &shimConfig{Shim: shim, Opts: opts}
assert.Check(t, is.DeepEqual(got, tt.want))
assert.Check(t, is.DeepEqual(got, tt.want,
cmpopts.IgnoreUnexported(runtimeoptions_v1.Options{}),
cmpopts.IgnoreUnexported(v2runcoptions.Options{}),
))
} else {
assert.Check(t, is.Equal(shim, ""))
assert.Check(t, is.Nil(opts))
Expand Down
2 changes: 1 addition & 1 deletion integration/build/build_traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestBuildkitHistoryTracePropagation(t *testing.T) {
}()

eg.Go(func() error {
_, err := progressui.DisplaySolveStatus(ctxGo, "test", nil, &testWriter{t}, ch)
_, err := progressui.DisplaySolveStatus(ctxGo, nil, &testWriter{t}, ch, progressui.WithPhase("test"))
neersighted marked this conversation as resolved.
Show resolved Hide resolved
return err
})

Expand Down
17 changes: 10 additions & 7 deletions libcontainerd/remote/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,21 @@ import (
cerrdefs "github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/images"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/protobuf"
v2runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
"github.com/containerd/typeurl/v2"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/libcontainerd/queue"
libcontainerdtypes "github.com/docker/docker/libcontainerd/types"
"github.com/docker/docker/pkg/ioutils"
"github.com/hashicorp/go-multierror"
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
)

// DockerContainerBundlePath is the label key pointing to the container's bundle path
Expand Down Expand Up @@ -163,7 +166,7 @@ func (c *container) Start(ctx context.Context, checkpointDir string, withStdin b
// remove the checkpoint when we're done
defer func() {
if checkpoint != nil {
err := c.client.client.ContentStore().Delete(ctx, checkpoint.Digest)
err := c.client.client.ContentStore().Delete(ctx, digest.Digest(checkpoint.Digest))
if err != nil {
c.client.logger.WithError(err).WithFields(log.Fields{
"ref": checkpointDir,
Expand Down Expand Up @@ -205,10 +208,10 @@ func (c *container) Start(ctx context.Context, checkpointDir string, withStdin b
if runtime.GOOS != "windows" {
taskOpts = append(taskOpts, func(_ context.Context, _ *containerd.Client, info *containerd.TaskInfo) error {
if c.v2runcoptions != nil {
opts := *c.v2runcoptions
opts := proto.Clone(c.v2runcoptions).(*v2runcoptions.Options)
opts.IoUid = uint32(uid)
opts.IoGid = uint32(gid)
info.Options = &opts
info.Options = opts
}
return nil
})
Expand Down Expand Up @@ -342,7 +345,7 @@ func (t *task) Stats(ctx context.Context) (*libcontainerdtypes.Stats, error) {
if err != nil {
return nil, err
}
return libcontainerdtypes.InterfaceToStats(m.Timestamp, v), nil
return libcontainerdtypes.InterfaceToStats(protobuf.FromTimestamp(m.Timestamp), v), nil
}

func (t *task) Summary(ctx context.Context) ([]libcontainerdtypes.Summary, error) {
Expand Down Expand Up @@ -675,7 +678,7 @@ func (c *client) processEventStream(ctx context.Context, ns string) {
ProcessID: t.ID,
Pid: t.Pid,
ExitCode: t.ExitStatus,
ExitedAt: t.ExitedAt,
ExitedAt: protobuf.FromTimestamp(t.ExitedAt),
})
case *apievents.TaskOOM:
c.processEvent(ctx, libcontainerdtypes.EventOOM, libcontainerdtypes.EventInfo{
Expand Down Expand Up @@ -734,8 +737,8 @@ func (c *client) writeContent(ctx context.Context, mediaType, ref string, r io.R
}
return &types.Descriptor{
MediaType: mediaType,
Digest: writer.Digest(),
Size_: size,
Digest: writer.Digest().Encoded(),
Size: size,
}, nil
}

Expand Down