Skip to content

Commit facad55

Browse files
author
Tibor Vass
committed
api: Change Platform field back to string (temporary workaround)
This partially reverts moby#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 <[email protected]>
1 parent 1da7d2e commit facad55

9 files changed

Lines changed: 60 additions & 43 deletions

File tree

‎api/server/router/build/build_routes.go‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"strings"
1515
"sync"
1616

17-
"github.com/containerd/containerd/platforms"
1817
"github.com/docker/docker/api/server/httputils"
1918
"github.com/docker/docker/api/types"
2019
"github.com/docker/docker/api/types/backend"
@@ -24,8 +23,7 @@ import (
2423
"github.com/docker/docker/pkg/ioutils"
2524
"github.com/docker/docker/pkg/progress"
2625
"github.com/docker/docker/pkg/streamformatter"
27-
"github.com/docker/docker/pkg/system"
28-
"github.com/docker/go-units"
26+
units "github.com/docker/go-units"
2927
"github.com/pkg/errors"
3028
"github.com/sirupsen/logrus"
3129
)
@@ -72,17 +70,7 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui
7270
options.Target = r.FormValue("target")
7371
options.RemoteContext = r.FormValue("remote")
7472
if versions.GreaterThanOrEqualTo(version, "1.32") {
75-
apiPlatform := r.FormValue("platform")
76-
if apiPlatform != "" {
77-
sp, err := platforms.Parse(apiPlatform)
78-
if err != nil {
79-
return nil, err
80-
}
81-
if err := system.ValidatePlatform(sp); err != nil {
82-
return nil, err
83-
}
84-
options.Platform = &sp
85-
}
73+
options.Platform = r.FormValue("platform")
8674
}
8775

8876
if r.Form.Get("shmsize") != "" {

‎api/types/client.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import (
77

88
"github.com/docker/docker/api/types/container"
99
"github.com/docker/docker/api/types/filters"
10-
"github.com/docker/go-units"
11-
specs "github.com/opencontainers/image-spec/specs-go/v1"
10+
units "github.com/docker/go-units"
1211
)
1312

1413
// CheckpointCreateOptions holds parameters to create a checkpoint from a container
@@ -181,7 +180,7 @@ type ImageBuildOptions struct {
181180
ExtraHosts []string // List of extra hosts
182181
Target string
183182
SessionID string
184-
Platform *specs.Platform
183+
Platform string
185184
// Version specifies the version of the unerlying builder to use
186185
Version BuilderVersion
187186
// BuildID is an optional identifier that can be passed together with the

‎builder/builder-next/builder.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/docker/docker/builder"
1515
"github.com/docker/docker/daemon/images"
1616
"github.com/docker/docker/pkg/streamformatter"
17+
"github.com/docker/docker/pkg/system"
1718
controlapi "github.com/moby/buildkit/api/services/control"
1819
"github.com/moby/buildkit/control"
1920
"github.com/moby/buildkit/identity"
@@ -208,8 +209,17 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder.
208209
frontendAttrs["no-cache"] = ""
209210
}
210211

211-
if opt.Options.Platform != nil {
212-
frontendAttrs["platform"] = platforms.Format(*opt.Options.Platform)
212+
if opt.Options.Platform != "" {
213+
// same as in newBuilder in builder/dockerfile.builder.go
214+
// TODO: remove once opt.Options.Platform is of type specs.Platform
215+
sp, err := platforms.Parse(opt.Options.Platform)
216+
if err != nil {
217+
return nil, err
218+
}
219+
if err := system.ValidatePlatform(sp); err != nil {
220+
return nil, err
221+
}
222+
frontendAttrs["platform"] = opt.Options.Platform
213223
}
214224

215225
exporterAttrs := map[string]string{}

‎builder/dockerfile/builder.go‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111
"time"
1212

13+
"github.com/containerd/containerd/platforms"
1314
"github.com/docker/docker/api/types"
1415
"github.com/docker/docker/api/types/backend"
1516
"github.com/docker/docker/api/types/container"
@@ -25,6 +26,7 @@ import (
2526
"github.com/moby/buildkit/frontend/dockerfile/parser"
2627
"github.com/moby/buildkit/frontend/dockerfile/shell"
2728
"github.com/moby/buildkit/session"
29+
specs "github.com/opencontainers/image-spec/specs-go/v1"
2830
"github.com/pkg/errors"
2931
"github.com/sirupsen/logrus"
3032
"golang.org/x/sync/syncmap"
@@ -111,7 +113,11 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) (
111113
PathCache: bm.pathCache,
112114
IDMappings: bm.idMappings,
113115
}
114-
return newBuilder(ctx, builderOptions).build(source, dockerfile)
116+
b, err := newBuilder(ctx, builderOptions)
117+
if err != nil {
118+
return nil, err
119+
}
120+
return b.build(source, dockerfile)
115121
}
116122

117123
func (bm *BuildManager) initializeClientSession(ctx context.Context, cancel func(), options *types.ImageBuildOptions) (builder.Source, error) {
@@ -175,10 +181,11 @@ type Builder struct {
175181
pathCache pathCache
176182
containerManager *containerManager
177183
imageProber ImageProber
184+
platform *specs.Platform
178185
}
179186

180187
// newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options.
181-
func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
188+
func newBuilder(clientCtx context.Context, options builderOptions) (*Builder, error) {
182189
config := options.Options
183190
if config == nil {
184191
config = new(types.ImageBuildOptions)
@@ -199,7 +206,20 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
199206
containerManager: newContainerManager(options.Backend),
200207
}
201208

202-
return b
209+
// same as in Builder.Build in builder/builder-next/builder.go
210+
// TODO: remove once config.Platform is of type specs.Platform
211+
if config.Platform != "" {
212+
sp, err := platforms.Parse(config.Platform)
213+
if err != nil {
214+
return nil, err
215+
}
216+
if err := system.ValidatePlatform(sp); err != nil {
217+
return nil, err
218+
}
219+
b.platform = &sp
220+
}
221+
222+
return b, nil
203223
}
204224

205225
// 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
365385
return nil, errdefs.InvalidParameter(err)
366386
}
367387

368-
b := newBuilder(context.Background(), builderOptions{
388+
b, err := newBuilder(context.Background(), builderOptions{
369389
Options: &types.ImageBuildOptions{NoCache: true},
370390
})
391+
if err != nil {
392+
return nil, err
393+
}
371394

372395
// ensure that the commands are valid
373396
for _, n := range dockerfile.AST.Children {

‎builder/dockerfile/copy.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func copierFromDispatchRequest(req dispatchRequest, download sourceDownloader, i
8787
pathCache: req.builder.pathCache,
8888
download: download,
8989
imageSource: imageSource,
90-
platform: req.builder.options.Platform,
90+
platform: req.builder.platform,
9191
}
9292
}
9393

‎builder/dockerfile/dispatchers.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func (d *dispatchRequest) getImageMount(imageRefOrID string) (*imageMount, error
146146
imageRefOrID = stage.Image
147147
localOnly = true
148148
}
149-
return d.builder.imageSources.Get(imageRefOrID, localOnly, d.builder.options.Platform)
149+
return d.builder.imageSources.Get(imageRefOrID, localOnly, d.builder.platform)
150150
}
151151

152152
// FROM [--platform=platform] imagename[:tag | @digest] [AS build-stage-name]
@@ -238,7 +238,7 @@ func (d *dispatchRequest) getImageOrStage(name string, platform *specs.Platform)
238238
}
239239

240240
if platform == nil {
241-
platform = d.builder.options.Platform
241+
platform = d.builder.platform
242242
}
243243

244244
// Windows cannot support a container with no base image unless it is LCOW.

‎builder/dockerfile/dispatchers_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"runtime"
77
"testing"
88

9-
"github.com/containerd/containerd/platforms"
109
"github.com/docker/docker/api/types"
1110
"github.com/docker/docker/api/types/backend"
1211
"github.com/docker/docker/api/types/container"
@@ -23,8 +22,7 @@ import (
2322

2423
func newBuilderWithMockBackend() *Builder {
2524
mockBackend := &MockBackend{}
26-
defaultPlatform := platforms.DefaultSpec()
27-
opts := &types.ImageBuildOptions{Platform: &defaultPlatform}
25+
opts := &types.ImageBuildOptions{}
2826
ctx := context.Background()
2927
b := &Builder{
3028
options: opts,
@@ -116,7 +114,7 @@ func TestFromScratch(t *testing.T) {
116114
err := initializeStage(sb, cmd)
117115

118116
if runtime.GOOS == "windows" && !system.LCOWSupported() {
119-
assert.Check(t, is.Error(err, "Windows does not support FROM scratch"))
117+
assert.Check(t, is.Error(err, "Linux containers are not supported on this system"))
120118
return
121119
}
122120

‎builder/dockerfile/internals.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error {
169169
return err
170170
}
171171

172-
imageMount, err := b.imageSources.Get(state.imageID, true, req.builder.options.Platform)
172+
imageMount, err := b.imageSources.Get(state.imageID, true, req.builder.platform)
173173
if err != nil {
174174
return errors.Wrapf(err, "failed to get destination image %q", state.imageID)
175175
}
@@ -416,7 +416,9 @@ func (b *Builder) probeAndCreate(dispatchState *dispatchState, runConfig *contai
416416

417417
func (b *Builder) create(runConfig *container.Config) (string, error) {
418418
logrus.Debugf("[BUILDER] Command to be executed: %v", runConfig.Cmd)
419-
hostConfig := hostConfigFromOptions(b.options)
419+
420+
isWCOW := runtime.GOOS == "windows" && b.platform != nil && b.platform.OS == "windows"
421+
hostConfig := hostConfigFromOptions(b.options, isWCOW)
420422
container, err := b.containerManager.Create(runConfig, hostConfig)
421423
if err != nil {
422424
return "", err
@@ -429,7 +431,7 @@ func (b *Builder) create(runConfig *container.Config) (string, error) {
429431
return container.ID, nil
430432
}
431433

432-
func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConfig {
434+
func hostConfigFromOptions(options *types.ImageBuildOptions, isWCOW bool) *container.HostConfig {
433435
resources := container.Resources{
434436
CgroupParent: options.CgroupParent,
435437
CPUShares: options.CPUShares,
@@ -457,7 +459,7 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf
457459
// is too small for builder scenarios where many users are
458460
// using RUN statements to install large amounts of data.
459461
// Use 127GB as that's the default size of a VHD in Hyper-V.
460-
if runtime.GOOS == "windows" && options.Platform != nil && options.Platform.OS == "windows" {
462+
if isWCOW {
461463
hc.StorageOpt = make(map[string]string)
462464
hc.StorageOpt["size"] = "127GB"
463465
}

‎client/image_build.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
"net/http"
99
"net/url"
1010
"strconv"
11+
"strings"
1112

12-
"github.com/containerd/containerd/platforms"
1313
"github.com/docker/docker/api/types"
1414
"github.com/docker/docker/api/types/container"
1515
)
@@ -30,12 +30,6 @@ func (cli *Client) ImageBuild(ctx context.Context, buildContext io.Reader, optio
3030
}
3131
headers.Add("X-Registry-Config", base64.URLEncoding.EncodeToString(buf))
3232

33-
if options.Platform != nil {
34-
if err := cli.NewVersionError("1.32", "platform"); err != nil {
35-
return types.ImageBuildResponse{}, err
36-
}
37-
query.Set("platform", platforms.Format(*options.Platform))
38-
}
3933
headers.Set("Content-Type", "application/x-tar")
4034

4135
serverResp, err := cli.postRaw(ctx, "/build", query, buildContext, headers)
@@ -130,8 +124,11 @@ func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (ur
130124
if options.SessionID != "" {
131125
query.Set("session", options.SessionID)
132126
}
133-
if options.Platform != nil {
134-
query.Set("platform", platforms.Format(*options.Platform))
127+
if options.Platform != "" {
128+
if err := cli.NewVersionError("1.32", "platform"); err != nil {
129+
return query, err
130+
}
131+
query.Set("platform", strings.ToLower(options.Platform))
135132
}
136133
if options.BuildID != "" {
137134
query.Set("buildid", options.BuildID)

0 commit comments

Comments
 (0)