Skip to content

Commit

Permalink
Run classic builder with BuildConfig, not buildx.Options
Browse files Browse the repository at this point in the history
Signed-off-by: Nicolas De Loof <[email protected]>
  • Loading branch information
ndeloof committed Mar 17, 2023
1 parent 2fefd6e commit 58d5796
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 129 deletions.
33 changes: 33 additions & 0 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"time"

"github.com/compose-spec/compose-go/types"
"github.com/docker/compose/v2/pkg/utils"
)

// Service manages a compose project
Expand Down Expand Up @@ -107,6 +108,38 @@ type BuildOptions struct {
SSHs []types.SSHKey
}

// Apply mutates project according to build options
func (o BuildOptions) Apply(project *types.Project) error {
platform := project.Environment["DOCKER_DEFAULT_PLATFORM"]
for i, service := range project.Services {
if service.Image == "" && service.Build == nil {
return fmt.Errorf("invalid service %q. Must specify either image or build", service.Name)
}

if service.Build == nil {
continue
}
service.Image = GetImageNameOrDefault(service, project.Name)
if platform != "" {
if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, platform) {
return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", service.Name, platform)
}
service.Platform = platform
}
if service.Platform != "" {
if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, service.Platform) {
return fmt.Errorf("service %q build configuration does not support platform: %s", service.Name, service.Platform)
}
}

service.Build.Pull = service.Build.Pull || o.Pull
service.Build.NoCache = service.Build.NoCache || o.NoCache

project.Services[i] = service
}
return nil
}

// CreateOptions group options of the Create API
type CreateOptions struct {
// Services defines the services user interacts with
Expand Down
76 changes: 34 additions & 42 deletions pkg/compose/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,18 @@ import (
)

func (s *composeService) Build(ctx context.Context, project *types.Project, options api.BuildOptions) error {
err := options.Apply(project)
if err != nil {
return err
}
return progress.Run(ctx, func(ctx context.Context) error {
_, err := s.build(ctx, project, options)
return err
}, s.stderr())
}

func (s *composeService) build(ctx context.Context, project *types.Project, options api.BuildOptions) (map[string]string, error) {
args := flatten(options.Args.Resolve(envResolver(project.Environment)))
args := options.Args.Resolve(envResolver(project.Environment))

builtIDs := make([]string, len(project.Services))
err := InDependencyOrder(ctx, project, func(ctx context.Context, name string) error {
Expand All @@ -61,25 +65,37 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti
if service.Name != name {
continue
}
service, err := project.GetService(name)
if err != nil {
return err
}

if service.Build == nil {
return nil
}
imageName := api.GetImageNameOrDefault(service, project.Name)
buildOptions, err := s.toBuildOptions(project, service, imageName, options)

if buildkitEnabled, err := s.dockerCli.BuildKitEnabled(); err != nil || !buildkitEnabled {
service.Build.Args = service.Build.Args.OverrideBy(args)
id, err := s.doBuildClassic(ctx, service)
if err != nil {
return err
}
builtIDs[i] = id

if options.Push {
return s.push(ctx, project, api.PushOptions{})
}
return nil
}
buildOptions, err := s.toBuildOptions(project, service, options)
if err != nil {
return err
}
buildOptions.BuildArgs = mergeArgs(buildOptions.BuildArgs, args)
opts := map[string]build.Options{imageName: buildOptions}
ids, err := s.doBuild(ctx, project, opts, options.Progress)
buildOptions.BuildArgs = mergeArgs(buildOptions.BuildArgs, flatten(args))
opts := map[string]build.Options{service.Name: buildOptions}

ids, err := s.doBuildBuildkit(ctx, opts, options.Progress)
if err != nil {
return err
}
builtIDs[i] = ids[imageName]
builtIDs[i] = ids[service.Name]
return nil
}
return nil
}, func(traversal *graphTraversal) {
Expand Down Expand Up @@ -146,39 +162,26 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types.
}

func (s *composeService) prepareProjectForBuild(project *types.Project, images map[string]string) error {
platform := project.Environment["DOCKER_DEFAULT_PLATFORM"]
err := api.BuildOptions{}.Apply(project)
if err != nil {
return err
}
for i, service := range project.Services {
if service.Image == "" && service.Build == nil {
return fmt.Errorf("invalid service %q. Must specify either image or build", service.Name)
}
if service.Build == nil {
continue
}

imageName := api.GetImageNameOrDefault(service, project.Name)
service.Image = imageName

_, localImagePresent := images[imageName]
_, localImagePresent := images[service.Image]
if localImagePresent && service.PullPolicy != types.PullPolicyBuild {
service.Build = nil
project.Services[i] = service
continue
}

if platform != "" {
if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, platform) {
return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", service.Name, platform)
}
service.Platform = platform
}

if service.Platform == "" {
// let builder to build for default platform
service.Build.Platforms = nil
} else {
if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, service.Platform) {
return fmt.Errorf("service %q build configuration does not support platform: %s", service.Name, platform)
}
service.Build.Platforms = []string{service.Platform}
}
project.Services[i] = service
Expand Down Expand Up @@ -214,19 +217,8 @@ func (s *composeService) getLocalImagesDigests(ctx context.Context, project *typ
return images, nil
}

func (s *composeService) doBuild(ctx context.Context, project *types.Project, opts map[string]build.Options, mode string) (map[string]string, error) {
if len(opts) == 0 {
return nil, nil
}
if buildkitEnabled, err := s.dockerCli.BuildKitEnabled(); err != nil || !buildkitEnabled {
return s.doBuildClassic(ctx, project, opts)
}
return s.doBuildBuildkit(ctx, opts, mode)
}

func (s *composeService) toBuildOptions(project *types.Project, service types.ServiceConfig, imageTag string, options api.BuildOptions) (build.Options, error) {
var tags []string
tags = append(tags, imageTag)
func (s *composeService) toBuildOptions(project *types.Project, service types.ServiceConfig, options api.BuildOptions) (build.Options, error) {
tags := []string{service.Image}

buildArgs := flatten(service.Build.Args.Resolve(envResolver(project.Environment)))

Expand Down
112 changes: 32 additions & 80 deletions pkg/compose/build_classic.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,55 +27,23 @@ import (
"strings"

"github.com/compose-spec/compose-go/types"
buildx "github.com/docker/buildx/build"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command/image/build"
"github.com/docker/compose/v2/pkg/utils"
dockertypes "github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/builder/remotecontext/urlutil"
"github.com/docker/docker/pkg/archive"
"github.com/docker/docker/pkg/idtools"
"github.com/docker/docker/pkg/jsonmessage"
"github.com/docker/docker/pkg/progress"
"github.com/docker/docker/pkg/streamformatter"
"github.com/hashicorp/go-multierror"
"github.com/moby/buildkit/util/entitlements"
"github.com/pkg/errors"

"github.com/docker/compose/v2/pkg/api"
)

func (s *composeService) doBuildClassic(ctx context.Context, project *types.Project, opts map[string]buildx.Options) (map[string]string, error) {
nameDigests := make(map[string]string)
var errs error
err := project.WithServices(nil, func(service types.ServiceConfig) error {
imageName := api.GetImageNameOrDefault(service, project.Name)
o, ok := opts[imageName]
if !ok {
return nil
}
digest, err := s.doBuildClassicSimpleImage(ctx, o)
if err != nil {
errs = multierror.Append(errs, err).ErrorOrNil()
}
nameDigests[imageName] = digest
if errs != nil {
return nil
}
if len(o.Exports) != 0 && o.Exports[0].Attrs["push"] == "true" {
return s.push(ctx, project, api.PushOptions{})
}
return nil
})
if err != nil {
return nil, err
}

return nameDigests, errs
}

//nolint:gocyclo
func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options buildx.Options) (string, error) {
func (s *composeService) doBuildClassic(ctx context.Context, service types.ServiceConfig) (string, error) {
var (
buildCtx io.ReadCloser
dockerfileCtx io.ReadCloser
Expand All @@ -86,31 +54,31 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
err error
)

dockerfileName := options.Inputs.DockerfilePath
specifiedContext := options.Inputs.ContextPath
dockerfileName := dockerFilePath(service.Build.Context, service.Build.Dockerfile)
specifiedContext := service.Build.Context
progBuff := s.stdout()
buildBuff := s.stdout()
if options.ImageIDFile != "" {
// Avoid leaving a stale file if we eventually fail
if err := os.Remove(options.ImageIDFile); err != nil && !os.IsNotExist(err) {
return "", errors.Wrap(err, "removing image ID file")
}
}

if len(options.Platforms) > 1 {
return "", errors.Errorf("this builder doesn't support multi-arch build, set DOCKER_BUILDKIT=1 to use multi-arch builder")
if len(service.Build.Platforms) > 1 {
return "", errors.Errorf("the classic builder doesn't support multi-arch build, set DOCKER_BUILDKIT=1 to use BuildKit")
}
if service.Build.Privileged {
return "", errors.Errorf("the classic builder doesn't support privileged mode, set DOCKER_BUILDKIT=1 to use BuildKit")
}
if len(service.Build.AdditionalContexts) > 0 {
return "", errors.Errorf("the classic builder doesn't support additional contexts, set DOCKER_BUILDKIT=1 to use BuildKit")
}
if utils.Contains(options.Allow, entitlements.EntitlementSecurityInsecure) {
return "", errors.Errorf("this builder doesn't support privileged mode, set DOCKER_BUILDKIT=1 to use builder supporting privileged mode")
if len(service.Build.SSH) > 0 {
return "", errors.Errorf("the classic builder doesn't support SSH keys, set DOCKER_BUILDKIT=1 to use BuildKit")
}
if len(options.Inputs.NamedContexts) > 0 {
return "", errors.Errorf("this builder doesn't support additional contexts, set DOCKER_BUILDKIT=1 to use BuildKit which does")
if len(service.Build.Secrets) > 0 {
return "", errors.Errorf("the classic builder doesn't support Secrets, set DOCKER_BUILDKIT=1 to use BuildKit")
}

if options.Labels == nil {
options.Labels = make(map[string]string)
if service.Build.Labels == nil {
service.Build.Labels = make(map[string]string)
}
options.Labels[api.ImageBuilderLabel] = "classic"
service.Build.Labels[api.ImageBuilderLabel] = "classic"

switch {
case isLocalDir(specifiedContext):
Expand Down Expand Up @@ -189,8 +157,8 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
for k, auth := range creds {
authConfigs[k] = dockertypes.AuthConfig(auth)
}
buildOptions := imageBuildOptions(options)
buildOptions.Version = dockertypes.BuilderV1
buildOptions := imageBuildOptions(service.Build)
buildOptions.Tags = append(buildOptions.Tags, service.Image)
buildOptions.Dockerfile = relDockerfile
buildOptions.AuthConfigs = authConfigs

Expand Down Expand Up @@ -235,15 +203,6 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
"files and directories.")
}

if options.ImageIDFile != "" {
if imageID == "" {
return "", errors.Errorf("Server did not provide an image ID. Cannot write %s", options.ImageIDFile)
}
if err := os.WriteFile(options.ImageIDFile, []byte(imageID), 0o666); err != nil {
return "", err
}
}

return imageID, nil
}

Expand All @@ -252,25 +211,18 @@ func isLocalDir(c string) bool {
return err == nil
}

func imageBuildOptions(options buildx.Options) dockertypes.ImageBuildOptions {
func imageBuildOptions(config *types.BuildConfig) dockertypes.ImageBuildOptions {
return dockertypes.ImageBuildOptions{
Tags: options.Tags,
NoCache: options.NoCache,
Version: dockertypes.BuilderV1,
Tags: config.Tags,
NoCache: config.NoCache,
Remove: true,
PullParent: options.Pull,
BuildArgs: toMapStringStringPtr(options.BuildArgs),
Labels: options.Labels,
NetworkMode: options.NetworkMode,
ExtraHosts: options.ExtraHosts,
Target: options.Target,
}
}

func toMapStringStringPtr(source map[string]string) map[string]*string {
dest := make(map[string]*string)
for k, v := range source {
v := v
dest[k] = &v
PullParent: config.Pull,
BuildArgs: config.Args,
Labels: config.Labels,
NetworkMode: config.Network,
ExtraHosts: config.ExtraHosts.AsList(),
Target: config.Target,
Isolation: container.Isolation(config.Isolation),
}
return dest
}
8 changes: 4 additions & 4 deletions pkg/e2e/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func TestBuildPlatformsStandardErrors(t *testing.T) {
})
res.Assert(t, icmd.Expected{
ExitCode: 1,
Err: "this builder doesn't support multi-arch build, set DOCKER_BUILDKIT=1 to use multi-arch builder",
Err: "the classic builder doesn't support multi-arch build, set DOCKER_BUILDKIT=1 to use BuildKit",
})
})

Expand All @@ -391,7 +391,7 @@ func TestBuildPlatformsStandardErrors(t *testing.T) {
"-f", "fixtures/build-test/platforms/compose-service-platform-not-in-build-platforms.yaml", "build")
res.Assert(t, icmd.Expected{
ExitCode: 1,
Err: `service.platform "linux/riscv64" should be part of the service.build.platforms: ["linux/amd64" "linux/arm64"]`,
Err: `service "platforms" build configuration does not support platform: linux/riscv64`,
})
})

Expand All @@ -402,7 +402,7 @@ func TestBuildPlatformsStandardErrors(t *testing.T) {
})
res.Assert(t, icmd.Expected{
ExitCode: 1,
Err: `DOCKER_DEFAULT_PLATFORM "windows/amd64" value should be part of the service.build.platforms: ["linux/amd64" "linux/arm64"]`,
Err: `service "platforms" build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: windows/amd64`,
})
})

Expand All @@ -414,7 +414,7 @@ func TestBuildPlatformsStandardErrors(t *testing.T) {
})
res.Assert(t, icmd.Expected{
ExitCode: 1,
Err: "this builder doesn't support privileged mode, set DOCKER_BUILDKIT=1 to use builder supporting privileged mode",
Err: "the classic builder doesn't support privileged mode, set DOCKER_BUILDKIT=1 to use BuildKit",
})
})

Expand Down
4 changes: 1 addition & 3 deletions pkg/e2e/fixtures/build-dependencies/service.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

FROM alpine

COPY --from=base /hello.txt /hello.txt
FROM base

CMD [ "cat", "/hello.txt" ]

0 comments on commit 58d5796

Please sign in to comment.