Browse code

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 <tibor@docker.com>

Tibor Vass authored on 2018/07/03 11:31:05
Showing 9 changed files
... ...
@@ -14,7 +14,6 @@ import (
14 14
 	"strings"
15 15
 	"sync"
16 16
 
17
-	"github.com/containerd/containerd/platforms"
18 17
 	"github.com/docker/docker/api/server/httputils"
19 18
 	"github.com/docker/docker/api/types"
20 19
 	"github.com/docker/docker/api/types/backend"
... ...
@@ -24,8 +23,7 @@ import (
24 24
 	"github.com/docker/docker/pkg/ioutils"
25 25
 	"github.com/docker/docker/pkg/progress"
26 26
 	"github.com/docker/docker/pkg/streamformatter"
27
-	"github.com/docker/docker/pkg/system"
28
-	"github.com/docker/go-units"
27
+	units "github.com/docker/go-units"
29 28
 	"github.com/pkg/errors"
30 29
 	"github.com/sirupsen/logrus"
31 30
 )
... ...
@@ -72,17 +70,7 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui
72 72
 	options.Target = r.FormValue("target")
73 73
 	options.RemoteContext = r.FormValue("remote")
74 74
 	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
-		}
75
+		options.Platform = r.FormValue("platform")
86 76
 	}
87 77
 
88 78
 	if r.Form.Get("shmsize") != "" {
... ...
@@ -7,8 +7,7 @@ import (
7 7
 
8 8
 	"github.com/docker/docker/api/types/container"
9 9
 	"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"
12 11
 )
13 12
 
14 13
 // CheckpointCreateOptions holds parameters to create a checkpoint from a container
... ...
@@ -181,7 +180,7 @@ type ImageBuildOptions struct {
181 181
 	ExtraHosts  []string // List of extra hosts
182 182
 	Target      string
183 183
 	SessionID   string
184
-	Platform    *specs.Platform
184
+	Platform    string
185 185
 	// Version specifies the version of the unerlying builder to use
186 186
 	Version BuilderVersion
187 187
 	// BuildID is an optional identifier that can be passed together with the
... ...
@@ -14,6 +14,7 @@ import (
14 14
 	"github.com/docker/docker/builder"
15 15
 	"github.com/docker/docker/daemon/images"
16 16
 	"github.com/docker/docker/pkg/streamformatter"
17
+	"github.com/docker/docker/pkg/system"
17 18
 	controlapi "github.com/moby/buildkit/api/services/control"
18 19
 	"github.com/moby/buildkit/control"
19 20
 	"github.com/moby/buildkit/identity"
... ...
@@ -208,8 +209,17 @@ func (b *Builder) Build(ctx context.Context, opt backend.BuildConfig) (*builder.
208 208
 		frontendAttrs["no-cache"] = ""
209 209
 	}
210 210
 
211
-	if opt.Options.Platform != nil {
212
-		frontendAttrs["platform"] = platforms.Format(*opt.Options.Platform)
211
+	if opt.Options.Platform != "" {
212
+		// same as in newBuilder in builder/dockerfile.builder.go
213
+		// TODO: remove once opt.Options.Platform is of type specs.Platform
214
+		sp, err := platforms.Parse(opt.Options.Platform)
215
+		if err != nil {
216
+			return nil, err
217
+		}
218
+		if err := system.ValidatePlatform(sp); err != nil {
219
+			return nil, err
220
+		}
221
+		frontendAttrs["platform"] = opt.Options.Platform
213 222
 	}
214 223
 
215 224
 	exporterAttrs := map[string]string{}
... ...
@@ -10,6 +10,7 @@ import (
10 10
 	"strings"
11 11
 	"time"
12 12
 
13
+	"github.com/containerd/containerd/platforms"
13 14
 	"github.com/docker/docker/api/types"
14 15
 	"github.com/docker/docker/api/types/backend"
15 16
 	"github.com/docker/docker/api/types/container"
... ...
@@ -25,6 +26,7 @@ import (
25 25
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
26 26
 	"github.com/moby/buildkit/frontend/dockerfile/shell"
27 27
 	"github.com/moby/buildkit/session"
28
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
28 29
 	"github.com/pkg/errors"
29 30
 	"github.com/sirupsen/logrus"
30 31
 	"golang.org/x/sync/syncmap"
... ...
@@ -111,7 +113,11 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) (
111 111
 		PathCache:      bm.pathCache,
112 112
 		IDMappings:     bm.idMappings,
113 113
 	}
114
-	return newBuilder(ctx, builderOptions).build(source, dockerfile)
114
+	b, err := newBuilder(ctx, builderOptions)
115
+	if err != nil {
116
+		return nil, err
117
+	}
118
+	return b.build(source, dockerfile)
115 119
 }
116 120
 
117 121
 func (bm *BuildManager) initializeClientSession(ctx context.Context, cancel func(), options *types.ImageBuildOptions) (builder.Source, error) {
... ...
@@ -175,10 +181,11 @@ type Builder struct {
175 175
 	pathCache        pathCache
176 176
 	containerManager *containerManager
177 177
 	imageProber      ImageProber
178
+	platform         *specs.Platform
178 179
 }
179 180
 
180 181
 // newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options.
181
-func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
182
+func newBuilder(clientCtx context.Context, options builderOptions) (*Builder, error) {
182 183
 	config := options.Options
183 184
 	if config == nil {
184 185
 		config = new(types.ImageBuildOptions)
... ...
@@ -199,7 +206,20 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
199 199
 		containerManager: newContainerManager(options.Backend),
200 200
 	}
201 201
 
202
-	return b
202
+	// same as in Builder.Build in builder/builder-next/builder.go
203
+	// TODO: remove once config.Platform is of type specs.Platform
204
+	if config.Platform != "" {
205
+		sp, err := platforms.Parse(config.Platform)
206
+		if err != nil {
207
+			return nil, err
208
+		}
209
+		if err := system.ValidatePlatform(sp); err != nil {
210
+			return nil, err
211
+		}
212
+		b.platform = &sp
213
+	}
214
+
215
+	return b, nil
203 216
 }
204 217
 
205 218
 // 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
365 365
 		return nil, errdefs.InvalidParameter(err)
366 366
 	}
367 367
 
368
-	b := newBuilder(context.Background(), builderOptions{
368
+	b, err := newBuilder(context.Background(), builderOptions{
369 369
 		Options: &types.ImageBuildOptions{NoCache: true},
370 370
 	})
371
+	if err != nil {
372
+		return nil, err
373
+	}
371 374
 
372 375
 	// ensure that the commands are valid
373 376
 	for _, n := range dockerfile.AST.Children {
... ...
@@ -87,7 +87,7 @@ func copierFromDispatchRequest(req dispatchRequest, download sourceDownloader, i
87 87
 		pathCache:   req.builder.pathCache,
88 88
 		download:    download,
89 89
 		imageSource: imageSource,
90
-		platform:    req.builder.options.Platform,
90
+		platform:    req.builder.platform,
91 91
 	}
92 92
 }
93 93
 
... ...
@@ -146,7 +146,7 @@ func (d *dispatchRequest) getImageMount(imageRefOrID string) (*imageMount, error
146 146
 		imageRefOrID = stage.Image
147 147
 		localOnly = true
148 148
 	}
149
-	return d.builder.imageSources.Get(imageRefOrID, localOnly, d.builder.options.Platform)
149
+	return d.builder.imageSources.Get(imageRefOrID, localOnly, d.builder.platform)
150 150
 }
151 151
 
152 152
 // FROM [--platform=platform] imagename[:tag | @digest] [AS build-stage-name]
... ...
@@ -238,7 +238,7 @@ func (d *dispatchRequest) getImageOrStage(name string, platform *specs.Platform)
238 238
 	}
239 239
 
240 240
 	if platform == nil {
241
-		platform = d.builder.options.Platform
241
+		platform = d.builder.platform
242 242
 	}
243 243
 
244 244
 	// Windows cannot support a container with no base image unless it is LCOW.
... ...
@@ -6,7 +6,6 @@ import (
6 6
 	"runtime"
7 7
 	"testing"
8 8
 
9
-	"github.com/containerd/containerd/platforms"
10 9
 	"github.com/docker/docker/api/types"
11 10
 	"github.com/docker/docker/api/types/backend"
12 11
 	"github.com/docker/docker/api/types/container"
... ...
@@ -23,8 +22,7 @@ import (
23 23
 
24 24
 func newBuilderWithMockBackend() *Builder {
25 25
 	mockBackend := &MockBackend{}
26
-	defaultPlatform := platforms.DefaultSpec()
27
-	opts := &types.ImageBuildOptions{Platform: &defaultPlatform}
26
+	opts := &types.ImageBuildOptions{}
28 27
 	ctx := context.Background()
29 28
 	b := &Builder{
30 29
 		options:       opts,
... ...
@@ -116,7 +114,7 @@ func TestFromScratch(t *testing.T) {
116 116
 	err := initializeStage(sb, cmd)
117 117
 
118 118
 	if runtime.GOOS == "windows" && !system.LCOWSupported() {
119
-		assert.Check(t, is.Error(err, "Windows does not support FROM scratch"))
119
+		assert.Check(t, is.Error(err, "Linux containers are not supported on this system"))
120 120
 		return
121 121
 	}
122 122
 
... ...
@@ -169,7 +169,7 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error {
169 169
 		return err
170 170
 	}
171 171
 
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)
173 173
 	if err != nil {
174 174
 		return errors.Wrapf(err, "failed to get destination image %q", state.imageID)
175 175
 	}
... ...
@@ -416,7 +416,9 @@ func (b *Builder) probeAndCreate(dispatchState *dispatchState, runConfig *contai
416 416
 
417 417
 func (b *Builder) create(runConfig *container.Config) (string, error) {
418 418
 	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)
420 422
 	container, err := b.containerManager.Create(runConfig, hostConfig)
421 423
 	if err != nil {
422 424
 		return "", err
... ...
@@ -429,7 +431,7 @@ func (b *Builder) create(runConfig *container.Config) (string, error) {
429 429
 	return container.ID, nil
430 430
 }
431 431
 
432
-func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConfig {
432
+func hostConfigFromOptions(options *types.ImageBuildOptions, isWCOW bool) *container.HostConfig {
433 433
 	resources := container.Resources{
434 434
 		CgroupParent: options.CgroupParent,
435 435
 		CPUShares:    options.CPUShares,
... ...
@@ -457,7 +459,7 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf
457 457
 	// is too small for builder scenarios where many users are
458 458
 	// using RUN statements to install large amounts of data.
459 459
 	// 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" {
460
+	if isWCOW {
461 461
 		hc.StorageOpt = make(map[string]string)
462 462
 		hc.StorageOpt["size"] = "127GB"
463 463
 	}
... ...
@@ -8,8 +8,8 @@ import (
8 8
 	"net/http"
9 9
 	"net/url"
10 10
 	"strconv"
11
+	"strings"
11 12
 
12
-	"github.com/containerd/containerd/platforms"
13 13
 	"github.com/docker/docker/api/types"
14 14
 	"github.com/docker/docker/api/types/container"
15 15
 )
... ...
@@ -30,12 +30,6 @@ func (cli *Client) ImageBuild(ctx context.Context, buildContext io.Reader, optio
30 30
 	}
31 31
 	headers.Add("X-Registry-Config", base64.URLEncoding.EncodeToString(buf))
32 32
 
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
-	}
39 33
 	headers.Set("Content-Type", "application/x-tar")
40 34
 
41 35
 	serverResp, err := cli.postRaw(ctx, "/build", query, buildContext, headers)
... ...
@@ -130,8 +124,11 @@ func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (ur
130 130
 	if options.SessionID != "" {
131 131
 		query.Set("session", options.SessionID)
132 132
 	}
133
-	if options.Platform != nil {
134
-		query.Set("platform", platforms.Format(*options.Platform))
133
+	if options.Platform != "" {
134
+		if err := cli.NewVersionError("1.32", "platform"); err != nil {
135
+			return query, err
136
+		}
137
+		query.Set("platform", strings.ToLower(options.Platform))
135 138
 	}
136 139
 	if options.BuildID != "" {
137 140
 		query.Set("buildid", options.BuildID)