Skip to content

Commit 1fdbcb6

Browse files
milasndeloof
authored andcommitted
build: pass BuildOptions around explicitly & fix multi-platform issues
The big change here is to pass around an explicit `*BuildOptions` object as part of Compose operations like `up` & `run` that may or may not do builds. If the options object is `nil`, no builds whatsoever will be attempted. Motivation is to allow for partial rebuilds in the context of an `up` for watch. This was broken and tricky to accomplish because various parts of the Compose APIs mutate the `*Project` for convenience in ways that make it unusable afterwards. (For example, it might set `service.Build = nil` because it's not going to build that service right _then_. But we might still want to build it later!) NOTE: This commit does not actually touch the watch logic. This is all in preparation to make it possible. As part of this, a bunch of code moved around and I eliminated a bunch of partially redundant logic, mostly around multi-platform. Several edge cases have been addressed as part of this: * `DOCKER_DEFAULT_PLATFORM` was _overriding_ explicitly set platforms in some cases, this is no longer true, and it behaves like the Docker CLI now * It was possible for Compose to build an image for one platform and then try to run it for a different platform (and fail) * Errors are no longer returned if a local image exists but for the wrong platform - the correct platform will be fetched/built (if possible). Because there's a LOT of subtlety and tricky logic here, I've also tried to add an excessive amount of explanatory comments. Signed-off-by: Milas Bowman <[email protected]>
1 parent 407a0d5 commit 1fdbcb6

File tree

13 files changed

+340
-242
lines changed

13 files changed

+340
-242
lines changed

cmd/compose/build.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import (
3535

3636
type buildOptions struct {
3737
*ProjectOptions
38-
composeOptions
3938
quiet bool
4039
pull bool
4140
push bool
@@ -73,7 +72,7 @@ func (opts buildOptions) toAPIBuildOptions(services []string) (api.BuildOptions,
7372
}, nil
7473
}
7574

76-
func buildCommand(p *ProjectOptions, progress *string, backend api.Service) *cobra.Command {
75+
func buildCommand(p *ProjectOptions, backend api.Service) *cobra.Command {
7776
opts := buildOptions{
7877
ProjectOptions: p,
7978
}
@@ -118,7 +117,7 @@ func buildCommand(p *ProjectOptions, progress *string, backend api.Service) *cob
118117
cmd.Flags().Bool("no-rm", false, "Do not remove intermediate containers after a successful build. DEPRECATED")
119118
cmd.Flags().MarkHidden("no-rm") //nolint:errcheck
120119
cmd.Flags().VarP(&opts.memory, "memory", "m", "Set memory limit for the build container. Not supported by BuildKit.")
121-
cmd.Flags().StringVar(progress, "progress", buildx.PrinterModeAuto, fmt.Sprintf(`Set type of ui output (%s)`, strings.Join(printerModes, ", ")))
120+
cmd.Flags().StringVar(&p.Progress, "progress", buildx.PrinterModeAuto, fmt.Sprintf(`Set type of ui output (%s)`, strings.Join(printerModes, ", ")))
122121
cmd.Flags().MarkHidden("progress") //nolint:errcheck
123122

124123
return cmd
@@ -130,6 +129,10 @@ func runBuild(ctx context.Context, backend api.Service, opts buildOptions, servi
130129
return err
131130
}
132131

132+
if err := applyPlatforms(project, false); err != nil {
133+
return err
134+
}
135+
133136
apiBuildOptions, err := opts.toAPIBuildOptions(services)
134137
if err != nil {
135138
return err

cmd/compose/compose.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ import (
2626
"strings"
2727
"syscall"
2828

29-
"github.com/compose-spec/compose-go/dotenv"
3029
buildx "github.com/docker/buildx/util/progress"
30+
31+
"github.com/compose-spec/compose-go/dotenv"
3132
"github.com/docker/cli/cli/command"
3233
"github.com/docker/compose/v2/pkg/remote"
3334

@@ -117,6 +118,7 @@ type ProjectOptions struct {
117118
ProjectDir string
118119
EnvFiles []string
119120
Compatibility bool
121+
Progress string
120122
}
121123

122124
// ProjectFunc does stuff within a types.Project
@@ -170,6 +172,7 @@ func (o *ProjectOptions) addProjectFlags(f *pflag.FlagSet) {
170172
f.StringVar(&o.ProjectDir, "project-directory", "", "Specify an alternate working directory\n(default: the path of the, first specified, Compose file)")
171173
f.StringVar(&o.WorkDir, "workdir", "", "DEPRECATED! USE --project-directory INSTEAD.\nSpecify an alternate working directory\n(default: the path of the, first specified, Compose file)")
172174
f.BoolVar(&o.Compatibility, "compatibility", false, "Run compose in backward compatibility mode")
175+
f.StringVar(&o.Progress, "progress", buildx.PrinterModeAuto, fmt.Sprintf(`Set type of progress output (%s)`, strings.Join(printerModes, ", ")))
173176
_ = f.MarkHidden("workdir")
174177
}
175178

@@ -294,7 +297,6 @@ func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { //
294297
version bool
295298
parallel int
296299
dryRun bool
297-
progress string
298300
)
299301
c := &cobra.Command{
300302
Short: "Docker Compose",
@@ -359,7 +361,7 @@ func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { //
359361
ui.Mode = ui.ModeTTY
360362
}
361363

362-
switch progress {
364+
switch opts.Progress {
363365
case ui.ModeAuto:
364366
ui.Mode = ui.ModeAuto
365367
case ui.ModeTTY:
@@ -375,7 +377,7 @@ func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { //
375377
case ui.ModeQuiet, "none":
376378
ui.Mode = ui.ModeQuiet
377379
default:
378-
return fmt.Errorf("unsupported --progress value %q", progress)
380+
return fmt.Errorf("unsupported --progress value %q", opts.Progress)
379381
}
380382

381383
if opts.WorkDir != "" {
@@ -446,7 +448,7 @@ func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { //
446448
portCommand(&opts, dockerCli, backend),
447449
imagesCommand(&opts, dockerCli, backend),
448450
versionCommand(dockerCli),
449-
buildCommand(&opts, &progress, backend),
451+
buildCommand(&opts, backend),
450452
pushCommand(&opts, backend),
451453
pullCommand(&opts, backend),
452454
createCommand(&opts, backend),
@@ -478,8 +480,6 @@ func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { //
478480
completeProfileNames(&opts),
479481
)
480482

481-
c.Flags().StringVar(&progress, "progress", buildx.PrinterModeAuto, fmt.Sprintf(`Set type of progress output (%s)`, strings.Join(printerModes, ", ")))
482-
483483
c.Flags().StringVar(&ansi, "ansi", "auto", `Control when to print ANSI control characters ("never"|"always"|"auto")`)
484484
c.Flags().IntVar(&parallel, "parallel", -1, `Control max parallelism, -1 for unlimited`)
485485
c.Flags().BoolVarP(&version, "version", "v", false, "Show the Docker Compose version information")

cmd/compose/create.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ func (opts createOptions) Apply(project *types.Project) error {
123123
project.Services[i] = service
124124
}
125125
}
126+
// N.B. opts.Build means "force build all", but images can still be built
127+
// when this is false
128+
// e.g. if a service has pull_policy: build or its local image is missing
126129
if opts.Build {
127130
for i, service := range project.Services {
128131
if service.Build == nil {
@@ -132,6 +135,7 @@ func (opts createOptions) Apply(project *types.Project) error {
132135
project.Services[i] = service
133136
}
134137
}
138+
// opts.noBuild, however, means do not perform ANY builds
135139
if opts.noBuild {
136140
for i, service := range project.Services {
137141
service.Build = nil
@@ -141,6 +145,11 @@ func (opts createOptions) Apply(project *types.Project) error {
141145
project.Services[i] = service
142146
}
143147
}
148+
149+
if err := applyPlatforms(project, true); err != nil {
150+
return err
151+
}
152+
144153
for _, scale := range opts.scale {
145154
split := strings.Split(scale, "=")
146155
if len(split) != 2 {

cmd/compose/options.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
Copyright 2023 Docker Compose CLI authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package compose
18+
19+
import (
20+
"fmt"
21+
22+
"github.com/compose-spec/compose-go/types"
23+
"github.com/docker/compose/v2/pkg/utils"
24+
)
25+
26+
func applyPlatforms(project *types.Project, buildForSinglePlatform bool) error {
27+
defaultPlatform := project.Environment["DOCKER_DEFAULT_PLATFORM"]
28+
for i := range project.Services {
29+
// mutable reference so platform fields can be updated
30+
service := &project.Services[i]
31+
32+
if service.Build == nil {
33+
continue
34+
}
35+
36+
// default platform only applies if the service doesn't specify
37+
if defaultPlatform != "" && service.Platform == "" {
38+
if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, defaultPlatform) {
39+
return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", service.Name, defaultPlatform)
40+
}
41+
service.Platform = defaultPlatform
42+
}
43+
44+
if service.Platform != "" {
45+
if len(service.Build.Platforms) > 0 {
46+
if !utils.StringContains(service.Build.Platforms, service.Platform) {
47+
return fmt.Errorf("service %q build configuration does not support platform: %s", service.Name, service.Platform)
48+
}
49+
}
50+
51+
if buildForSinglePlatform || len(service.Build.Platforms) == 0 {
52+
// if we're building for a single platform, we want to build for the platform we'll use to run the image
53+
// similarly, if no build platforms were explicitly specified, it makes sense to build for the platform
54+
// the image is designed for rather than allowing the builder to infer the platform
55+
service.Build.Platforms = []string{service.Platform}
56+
}
57+
}
58+
59+
// services can specify that they should be built for multiple platforms, which can be used
60+
// with `docker compose build` to produce a multi-arch image
61+
// other cases, such as `up` and `run`, need a single architecture to actually run
62+
// if there is only a single platform present (which might have been inferred
63+
// from service.Platform above), it will be used, even if it requires emulation.
64+
// if there's more than one platform, then the list is cleared so that the builder
65+
// can decide.
66+
// TODO(milas): there's no validation that the platform the builder will pick is actually one
67+
// of the supported platforms from the build definition
68+
// e.g. `build.platforms: [linux/arm64, linux/amd64]` on a `linux/ppc64` machine would build
69+
// for `linux/ppc64` instead of returning an error that it's not a valid platform for the service.
70+
if buildForSinglePlatform && len(service.Build.Platforms) > 1 {
71+
// empty indicates that the builder gets to decide
72+
service.Build.Platforms = nil
73+
}
74+
}
75+
return nil
76+
}

cmd/compose/options_test.go

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
Copyright 2023 Docker Compose CLI authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package compose
18+
19+
import (
20+
"testing"
21+
22+
"github.com/compose-spec/compose-go/types"
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
func TestApplyPlatforms_InferFromRuntime(t *testing.T) {
27+
makeProject := func() *types.Project {
28+
return &types.Project{
29+
Services: []types.ServiceConfig{
30+
{
31+
Name: "test",
32+
Image: "foo",
33+
Build: &types.BuildConfig{
34+
Context: ".",
35+
Platforms: []string{
36+
"linux/amd64",
37+
"linux/arm64",
38+
"alice/32",
39+
},
40+
},
41+
Platform: "alice/32",
42+
},
43+
},
44+
}
45+
}
46+
47+
t.Run("SinglePlatform", func(t *testing.T) {
48+
project := makeProject()
49+
require.NoError(t, applyPlatforms(project, true))
50+
require.EqualValues(t, []string{"alice/32"}, project.Services[0].Build.Platforms)
51+
})
52+
53+
t.Run("MultiPlatform", func(t *testing.T) {
54+
project := makeProject()
55+
require.NoError(t, applyPlatforms(project, false))
56+
require.EqualValues(t, []string{"linux/amd64", "linux/arm64", "alice/32"},
57+
project.Services[0].Build.Platforms)
58+
})
59+
}
60+
61+
func TestApplyPlatforms_DockerDefaultPlatform(t *testing.T) {
62+
makeProject := func() *types.Project {
63+
return &types.Project{
64+
Environment: map[string]string{
65+
"DOCKER_DEFAULT_PLATFORM": "linux/amd64",
66+
},
67+
Services: []types.ServiceConfig{
68+
{
69+
Name: "test",
70+
Image: "foo",
71+
Build: &types.BuildConfig{
72+
Context: ".",
73+
Platforms: []string{
74+
"linux/amd64",
75+
"linux/arm64",
76+
},
77+
},
78+
},
79+
},
80+
}
81+
}
82+
83+
t.Run("SinglePlatform", func(t *testing.T) {
84+
project := makeProject()
85+
require.NoError(t, applyPlatforms(project, true))
86+
require.EqualValues(t, []string{"linux/amd64"}, project.Services[0].Build.Platforms)
87+
})
88+
89+
t.Run("MultiPlatform", func(t *testing.T) {
90+
project := makeProject()
91+
require.NoError(t, applyPlatforms(project, false))
92+
require.EqualValues(t, []string{"linux/amd64", "linux/arm64"},
93+
project.Services[0].Build.Platforms)
94+
})
95+
}
96+
97+
func TestApplyPlatforms_UnsupportedPlatform(t *testing.T) {
98+
makeProject := func() *types.Project {
99+
return &types.Project{
100+
Environment: map[string]string{
101+
"DOCKER_DEFAULT_PLATFORM": "commodore/64",
102+
},
103+
Services: []types.ServiceConfig{
104+
{
105+
Name: "test",
106+
Image: "foo",
107+
Build: &types.BuildConfig{
108+
Context: ".",
109+
Platforms: []string{
110+
"linux/amd64",
111+
"linux/arm64",
112+
},
113+
},
114+
},
115+
},
116+
}
117+
}
118+
119+
t.Run("SinglePlatform", func(t *testing.T) {
120+
project := makeProject()
121+
require.EqualError(t, applyPlatforms(project, true),
122+
`service "test" build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: commodore/64`)
123+
})
124+
125+
t.Run("MultiPlatform", func(t *testing.T) {
126+
project := makeProject()
127+
require.EqualError(t, applyPlatforms(project, false),
128+
`service "test" build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: commodore/64`)
129+
})
130+
}

0 commit comments

Comments
 (0)