Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 2 additions & 14 deletions api/server/router/build/build_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we keep the validation/error-handling as it was before we changed to this? https://github.com/moby/moby/pull/37350/files#diff-f0ccc740f5a5ed38075be6855298809cL77

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, I see, you moved the validation to the build/next-builder.

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") != "" {
Expand Down
5 changes: 2 additions & 3 deletions api/types/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions builder/builder-next/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}
Expand Down
31 changes: 27 additions & 4 deletions builder/dockerfile/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion builder/dockerfile/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions builder/dockerfile/dispatchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions builder/dockerfile/dispatchers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -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"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As follow-up we should add a test that uses platform=goos and checks that it returns the original windows specific message.

return
}

Expand Down
10 changes: 6 additions & 4 deletions builder/dockerfile/internals.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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"
}
Expand Down
15 changes: 6 additions & 9 deletions client/image_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down