From 26dd527fabc1bd9a314a7caa657ce3f6b3556089 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Mon, 11 Jun 2018 18:48:42 +0000 Subject: [PATCH 1/2] builder: return image ID in API when using buildkit Signed-off-by: Tibor Vass (cherry picked from commit ca8022ec63a9d0e2f9660e2a3455d821abf8f517) Signed-off-by: Tibor Vass --- api/server/backend/build/backend.go | 2 +- api/server/router/build/build_routes.go | 8 +++---- builder/builder-next/builder.go | 25 +++++---------------- builder/dockerfile/builder.go | 2 +- pkg/streamformatter/streamformatter.go | 4 ++-- pkg/streamformatter/streamformatter_test.go | 2 +- 6 files changed, 14 insertions(+), 29 deletions(-) diff --git a/api/server/backend/build/backend.go b/api/server/backend/build/backend.go index 546ad5f86d81d..5e04e837a1bde 100644 --- a/api/server/backend/build/backend.go +++ b/api/server/backend/build/backend.go @@ -73,7 +73,7 @@ func (b *Backend) Build(ctx context.Context, config backend.BuildConfig) (string return "", err } if config.ProgressWriter.AuxFormatter != nil { - if err = config.ProgressWriter.AuxFormatter.Emit(types.BuildResult{ID: imageID}); err != nil { + if err = config.ProgressWriter.AuxFormatter.Emit("moby.image.id", types.BuildResult{ID: imageID}); err != nil { return "", err } } diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index 071402ae70a9b..827ba3a713c79 100644 --- a/api/server/router/build/build_routes.go +++ b/api/server/router/build/build_routes.go @@ -243,6 +243,10 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * return errdefs.InvalidParameter(errors.New("squash is only supported with experimental mode")) } + if buildOptions.Version == types.BuilderBuildKit && !br.daemon.HasExperimental() { + return errdefs.InvalidParameter(errors.New("buildkit is only supported with experimental mode")) + } + out := io.Writer(output) if buildOptions.SuppressOutput { out = notVerboseBuffer @@ -255,10 +259,6 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * return progress.NewProgressReader(in, progressOutput, r.ContentLength, "Downloading context", buildOptions.RemoteContext) } - if buildOptions.Version == types.BuilderBuildKit && !br.daemon.HasExperimental() { - return errdefs.InvalidParameter(errors.New("buildkit is only supported with experimental mode")) - } - wantAux := versions.GreaterThanOrEqualTo(version, "1.30") imgID, err := br.backend.Build(ctx, backend.BuildConfig{ diff --git a/builder/builder-next/builder.go b/builder/builder-next/builder.go index 5a82cddf444ea..73520db9fd0ec 100644 --- a/builder/builder-next/builder.go +++ b/builder/builder-next/builder.go @@ -2,7 +2,6 @@ package buildkit import ( "context" - "encoding/json" "io" "strings" "sync" @@ -14,7 +13,7 @@ import ( "github.com/docker/docker/api/types/backend" "github.com/docker/docker/builder" "github.com/docker/docker/daemon/images" - "github.com/docker/docker/pkg/jsonmessage" + "github.com/docker/docker/pkg/streamformatter" controlapi "github.com/moby/buildkit/api/services/control" "github.com/moby/buildkit/control" "github.com/moby/buildkit/identity" @@ -228,6 +227,8 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder. Session: opt.Options.SessionID, } + aux := streamformatter.AuxFormatter{opt.ProgressWriter.Output} + eg, ctx := errgroup.WithContext(ctx) eg.Go(func() error { @@ -240,7 +241,7 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder. return errors.Errorf("missing image id") } out.ImageID = id - return nil + return aux.Emit("moby.image.id", types.BuildResult{ID: id}) }) ch := make(chan *controlapi.StatusResponse) @@ -258,25 +259,9 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder. if err != nil { return err } - - auxJSONBytes, err := json.Marshal(dt) - if err != nil { + if err := aux.Emit("moby.buildkit.trace", dt); err != nil { return err } - auxJSON := new(json.RawMessage) - *auxJSON = auxJSONBytes - msgJSON, err := json.Marshal(&jsonmessage.JSONMessage{ID: "moby.buildkit.trace", Aux: auxJSON}) - if err != nil { - return err - } - msgJSON = append(msgJSON, []byte("\r\n")...) - n, err := opt.ProgressWriter.Output.Write(msgJSON) - if err != nil { - return err - } - if n != len(msgJSON) { - return io.ErrShortWrite - } } return nil }) diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index b58534707916d..1a0b680c37e73 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -257,7 +257,7 @@ func emitImageID(aux *streamformatter.AuxFormatter, state *dispatchState) error if aux == nil || state.imageID == "" { return nil } - return aux.Emit(types.BuildResult{ID: state.imageID}) + return aux.Emit("", types.BuildResult{ID: state.imageID}) } func processMetaArg(meta instructions.ArgCommand, shlex *shell.Lex, args *BuildArgs) error { diff --git a/pkg/streamformatter/streamformatter.go b/pkg/streamformatter/streamformatter.go index 2b5e713040eb4..04917d49ab29d 100644 --- a/pkg/streamformatter/streamformatter.go +++ b/pkg/streamformatter/streamformatter.go @@ -139,14 +139,14 @@ type AuxFormatter struct { } // Emit emits the given interface as an aux progress message -func (sf *AuxFormatter) Emit(aux interface{}) error { +func (sf *AuxFormatter) Emit(id string, aux interface{}) error { auxJSONBytes, err := json.Marshal(aux) if err != nil { return err } auxJSON := new(json.RawMessage) *auxJSON = auxJSONBytes - msgJSON, err := json.Marshal(&jsonmessage.JSONMessage{Aux: auxJSON}) + msgJSON, err := json.Marshal(&jsonmessage.JSONMessage{ID: id, Aux: auxJSON}) if err != nil { return err } diff --git a/pkg/streamformatter/streamformatter_test.go b/pkg/streamformatter/streamformatter_test.go index 4399a6509b444..f630699d73e11 100644 --- a/pkg/streamformatter/streamformatter_test.go +++ b/pkg/streamformatter/streamformatter_test.go @@ -106,7 +106,7 @@ func TestAuxFormatterEmit(t *testing.T) { sampleAux := &struct { Data string }{"Additional data"} - err := aux.Emit(sampleAux) + err := aux.Emit("", sampleAux) assert.NilError(t, err) assert.Check(t, is.Equal(`{"aux":{"Data":"Additional data"}}`+streamNewline, b.String())) } From f030a49747a88b523aa2ac1ab416e1ea1f86da2f Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 3 Jul 2018 02:31:05 +0000 Subject: [PATCH 2/2] api: Change Platform field back to string (temporary workaround) This partially reverts https://github.com/moby/moby/pull/37350 Although specs.Platform is desirable in the API, there is more work to be done on helper functions, namely containerd's platforms.Parse that assumes the default platform of the Go runtime. That prevents a client to use the recommended Parse function to retrieve a specs.Platform object. With this change, no parsing is expected from the client. Signed-off-by: Tibor Vass (cherry picked from commit facad557440a0c955beb615495b8d0175f25e4e3) Signed-off-by: Tibor Vass --- api/server/router/build/build_routes.go | 16 ++----------- api/types/client.go | 5 ++-- builder/builder-next/builder.go | 14 +++++++++-- builder/dockerfile/builder.go | 31 +++++++++++++++++++++---- builder/dockerfile/copy.go | 2 +- builder/dockerfile/dispatchers.go | 4 ++-- builder/dockerfile/dispatchers_test.go | 6 ++--- builder/dockerfile/internals.go | 10 ++++---- client/image_build.go | 15 +++++------- 9 files changed, 60 insertions(+), 43 deletions(-) diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index 827ba3a713c79..c4699f3d865aa 100644 --- a/api/server/router/build/build_routes.go +++ b/api/server/router/build/build_routes.go @@ -14,7 +14,6 @@ import ( "strings" "sync" - "github.com/containerd/containerd/platforms" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" @@ -24,8 +23,7 @@ import ( "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" - "github.com/docker/docker/pkg/system" - "github.com/docker/go-units" + units "github.com/docker/go-units" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -72,17 +70,7 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui options.Target = r.FormValue("target") options.RemoteContext = r.FormValue("remote") if versions.GreaterThanOrEqualTo(version, "1.32") { - apiPlatform := r.FormValue("platform") - if apiPlatform != "" { - sp, err := platforms.Parse(apiPlatform) - if err != nil { - return nil, err - } - if err := system.ValidatePlatform(sp); err != nil { - return nil, err - } - options.Platform = &sp - } + options.Platform = r.FormValue("platform") } if r.Form.Get("shmsize") != "" { diff --git a/api/types/client.go b/api/types/client.go index 33bc98e0bb401..3b698c2c240d0 100644 --- a/api/types/client.go +++ b/api/types/client.go @@ -7,8 +7,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" - "github.com/docker/go-units" - specs "github.com/opencontainers/image-spec/specs-go/v1" + units "github.com/docker/go-units" ) // CheckpointCreateOptions holds parameters to create a checkpoint from a container @@ -181,7 +180,7 @@ type ImageBuildOptions struct { ExtraHosts []string // List of extra hosts Target string SessionID string - Platform *specs.Platform + Platform string // Version specifies the version of the unerlying builder to use Version BuilderVersion // BuildID is an optional identifier that can be passed together with the diff --git a/builder/builder-next/builder.go b/builder/builder-next/builder.go index 73520db9fd0ec..b1d31a5225635 100644 --- a/builder/builder-next/builder.go +++ b/builder/builder-next/builder.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/daemon/images" "github.com/docker/docker/pkg/streamformatter" + "github.com/docker/docker/pkg/system" controlapi "github.com/moby/buildkit/api/services/control" "github.com/moby/buildkit/control" "github.com/moby/buildkit/identity" @@ -208,8 +209,17 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder. frontendAttrs["no-cache"] = "" } - if opt.Options.Platform != nil { - frontendAttrs["platform"] = platforms.Format(*opt.Options.Platform) + if opt.Options.Platform != "" { + // same as in newBuilder in builder/dockerfile.builder.go + // TODO: remove once opt.Options.Platform is of type specs.Platform + sp, err := platforms.Parse(opt.Options.Platform) + if err != nil { + return nil, err + } + if err := system.ValidatePlatform(sp); err != nil { + return nil, err + } + frontendAttrs["platform"] = opt.Options.Platform } exporterAttrs := map[string]string{} diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 1a0b680c37e73..ee95f236892fe 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/containerd/containerd/platforms" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" @@ -25,6 +26,7 @@ import ( "github.com/moby/buildkit/frontend/dockerfile/parser" "github.com/moby/buildkit/frontend/dockerfile/shell" "github.com/moby/buildkit/session" + specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sync/syncmap" @@ -111,7 +113,11 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) ( PathCache: bm.pathCache, IDMappings: bm.idMappings, } - return newBuilder(ctx, builderOptions).build(source, dockerfile) + b, err := newBuilder(ctx, builderOptions) + if err != nil { + return nil, err + } + return b.build(source, dockerfile) } func (bm *BuildManager) initializeClientSession(ctx context.Context, cancel func(), options *types.ImageBuildOptions) (builder.Source, error) { @@ -175,10 +181,11 @@ type Builder struct { pathCache pathCache containerManager *containerManager imageProber ImageProber + platform *specs.Platform } // newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options. -func newBuilder(clientCtx context.Context, options builderOptions) *Builder { +func newBuilder(clientCtx context.Context, options builderOptions) (*Builder, error) { config := options.Options if config == nil { config = new(types.ImageBuildOptions) @@ -199,7 +206,20 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder { containerManager: newContainerManager(options.Backend), } - return b + // same as in Builder.Build in builder/builder-next/builder.go + // TODO: remove once config.Platform is of type specs.Platform + if config.Platform != "" { + sp, err := platforms.Parse(config.Platform) + if err != nil { + return nil, err + } + if err := system.ValidatePlatform(sp); err != nil { + return nil, err + } + b.platform = &sp + } + + return b, nil } // Build 'LABEL' command(s) from '--label' options and add to the last stage @@ -365,9 +385,12 @@ func BuildFromConfig(config *container.Config, changes []string, os string) (*co return nil, errdefs.InvalidParameter(err) } - b := newBuilder(context.Background(), builderOptions{ + b, err := newBuilder(context.Background(), builderOptions{ Options: &types.ImageBuildOptions{NoCache: true}, }) + if err != nil { + return nil, err + } // ensure that the commands are valid for _, n := range dockerfile.AST.Children { diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index 7e9dc6036a460..74e245bdc40b9 100644 --- a/builder/dockerfile/copy.go +++ b/builder/dockerfile/copy.go @@ -87,7 +87,7 @@ func copierFromDispatchRequest(req dispatchRequest, download sourceDownloader, i pathCache: req.builder.pathCache, download: download, imageSource: imageSource, - platform: req.builder.options.Platform, + platform: req.builder.platform, } } diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 1032c6cdada89..5d72237cdb429 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -146,7 +146,7 @@ func (d *dispatchRequest) getImageMount(imageRefOrID string) (*imageMount, error imageRefOrID = stage.Image localOnly = true } - return d.builder.imageSources.Get(imageRefOrID, localOnly, d.builder.options.Platform) + return d.builder.imageSources.Get(imageRefOrID, localOnly, d.builder.platform) } // FROM [--platform=platform] imagename[:tag | @digest] [AS build-stage-name] @@ -238,7 +238,7 @@ func (d *dispatchRequest) getImageOrStage(name string, platform *specs.Platform) } if platform == nil { - platform = d.builder.options.Platform + platform = d.builder.platform } // Windows cannot support a container with no base image unless it is LCOW. diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 047a8742e8300..c61a45b03a1d6 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -6,7 +6,6 @@ import ( "runtime" "testing" - "github.com/containerd/containerd/platforms" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" @@ -23,8 +22,7 @@ import ( func newBuilderWithMockBackend() *Builder { mockBackend := &MockBackend{} - defaultPlatform := platforms.DefaultSpec() - opts := &types.ImageBuildOptions{Platform: &defaultPlatform} + opts := &types.ImageBuildOptions{} ctx := context.Background() b := &Builder{ options: opts, @@ -116,7 +114,7 @@ func TestFromScratch(t *testing.T) { err := initializeStage(sb, cmd) if runtime.GOOS == "windows" && !system.LCOWSupported() { - assert.Check(t, is.Error(err, "Windows does not support FROM scratch")) + assert.Check(t, is.Error(err, "Linux containers are not supported on this system")) return } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 5e2c286d75453..1b3a5b0f03ac2 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -169,7 +169,7 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error { return err } - imageMount, err := b.imageSources.Get(state.imageID, true, req.builder.options.Platform) + imageMount, err := b.imageSources.Get(state.imageID, true, req.builder.platform) if err != nil { return errors.Wrapf(err, "failed to get destination image %q", state.imageID) } @@ -416,7 +416,9 @@ func (b *Builder) probeAndCreate(dispatchState *dispatchState, runConfig *contai func (b *Builder) create(runConfig *container.Config) (string, error) { logrus.Debugf("[BUILDER] Command to be executed: %v", runConfig.Cmd) - hostConfig := hostConfigFromOptions(b.options) + + isWCOW := runtime.GOOS == "windows" && b.platform != nil && b.platform.OS == "windows" + hostConfig := hostConfigFromOptions(b.options, isWCOW) container, err := b.containerManager.Create(runConfig, hostConfig) if err != nil { return "", err @@ -429,7 +431,7 @@ func (b *Builder) create(runConfig *container.Config) (string, error) { return container.ID, nil } -func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConfig { +func hostConfigFromOptions(options *types.ImageBuildOptions, isWCOW bool) *container.HostConfig { resources := container.Resources{ CgroupParent: options.CgroupParent, CPUShares: options.CPUShares, @@ -457,7 +459,7 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf // is too small for builder scenarios where many users are // using RUN statements to install large amounts of data. // Use 127GB as that's the default size of a VHD in Hyper-V. - if runtime.GOOS == "windows" && options.Platform != nil && options.Platform.OS == "windows" { + if isWCOW { hc.StorageOpt = make(map[string]string) hc.StorageOpt["size"] = "127GB" } diff --git a/client/image_build.go b/client/image_build.go index e5013176a2f96..9add3c10b348b 100644 --- a/client/image_build.go +++ b/client/image_build.go @@ -8,8 +8,8 @@ import ( "net/http" "net/url" "strconv" + "strings" - "github.com/containerd/containerd/platforms" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" ) @@ -30,12 +30,6 @@ func (cli *Client) ImageBuild(ctx context.Context, buildContext io.Reader, optio } headers.Add("X-Registry-Config", base64.URLEncoding.EncodeToString(buf)) - if options.Platform != nil { - if err := cli.NewVersionError("1.32", "platform"); err != nil { - return types.ImageBuildResponse{}, err - } - query.Set("platform", platforms.Format(*options.Platform)) - } headers.Set("Content-Type", "application/x-tar") serverResp, err := cli.postRaw(ctx, "/build", query, buildContext, headers) @@ -130,8 +124,11 @@ func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (ur if options.SessionID != "" { query.Set("session", options.SessionID) } - if options.Platform != nil { - query.Set("platform", platforms.Format(*options.Platform)) + if options.Platform != "" { + if err := cli.NewVersionError("1.32", "platform"); err != nil { + return query, err + } + query.Set("platform", strings.ToLower(options.Platform)) } if options.BuildID != "" { query.Set("buildid", options.BuildID)