From b2cd089bae8baad1c18419247ccc922eb4e06da6 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Wed, 22 Jun 2022 16:13:08 -0400 Subject: [PATCH 1/2] build: respect dependency order for classic builder When using the "classic" (non-BuildKit) builder, ensure that services are iterated in dependency order for a build so that it's possible to guarantee the presence of a base image that's been added as a dependency with `depends_on`. This is a very common pattern when using base images with Compose. A fix for BuildKit is blocked currently until we can rely on a newer version of the engine (see docker/compose#9324)[^1]. [^1]: https://github.com/docker/compose/issues/9232#issuecomment-1060389808 Signed-off-by: Milas Bowman --- pkg/compose/build.go | 2 +- pkg/compose/build_classic.go | 16 ++++++-- .../{compose_build_test.go => build_test.go} | 37 +++++++++++++++++++ .../build-dependencies/base.dockerfile | 5 +++ .../fixtures/build-dependencies/compose.yaml | 12 ++++++ pkg/e2e/fixtures/build-dependencies/hello.txt | 1 + .../build-dependencies/service.dockerfile | 5 +++ 7 files changed, 74 insertions(+), 4 deletions(-) rename pkg/e2e/{compose_build_test.go => build_test.go} (87%) create mode 100644 pkg/e2e/fixtures/build-dependencies/base.dockerfile create mode 100644 pkg/e2e/fixtures/build-dependencies/compose.yaml create mode 100644 pkg/e2e/fixtures/build-dependencies/hello.txt create mode 100644 pkg/e2e/fixtures/build-dependencies/service.dockerfile diff --git a/pkg/compose/build.go b/pkg/compose/build.go index eeb2dba2e6..689a6cd844 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -205,7 +205,7 @@ func (s *composeService) doBuild(ctx context.Context, project *types.Project, op return nil, nil } if buildkitEnabled, err := s.dockerCli.BuildKitEnabled(); err != nil || !buildkitEnabled { - return s.doBuildClassic(ctx, opts) + return s.doBuildClassic(ctx, project, opts) } return s.doBuildBuildkit(ctx, project, opts, mode) } diff --git a/pkg/compose/build_classic.go b/pkg/compose/build_classic.go index 6b578dd7e9..302d55fa2e 100644 --- a/pkg/compose/build_classic.go +++ b/pkg/compose/build_classic.go @@ -27,6 +27,7 @@ import ( "runtime" "strings" + "github.com/compose-spec/compose-go/types" buildx "github.com/docker/buildx/build" "github.com/docker/cli/cli/command/image/build" dockertypes "github.com/docker/docker/api/types" @@ -41,15 +42,24 @@ import ( "github.com/pkg/errors" ) -func (s *composeService) doBuildClassic(ctx context.Context, opts map[string]buildx.Options) (map[string]string, error) { +func (s *composeService) doBuildClassic(ctx context.Context, project *types.Project, opts map[string]buildx.Options) (map[string]string, error) { var nameDigests = make(map[string]string) var errs error - for name, o := range opts { + err := project.WithServices(nil, func(service types.ServiceConfig) error { + imageName := getImageName(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[name] = digest + nameDigests[imageName] = digest + return nil + }) + if err != nil { + return nil, err } return nameDigests, errs diff --git a/pkg/e2e/compose_build_test.go b/pkg/e2e/build_test.go similarity index 87% rename from pkg/e2e/compose_build_test.go rename to pkg/e2e/build_test.go index 76a27f0321..c7ea531bd3 100644 --- a/pkg/e2e/compose_build_test.go +++ b/pkg/e2e/build_test.go @@ -201,3 +201,40 @@ func TestBuildTags(t *testing.T) { res.Assert(t, icmd.Expected{Out: expectedOutput}) }) } + +func TestBuildImageDependencies(t *testing.T) { + doTest := func(t *testing.T, cli *CLI) { + resetState := func() { + cli.RunDockerComposeCmd(t, "down", "--rmi=all", "-t=0") + } + resetState() + t.Cleanup(resetState) + + // the image should NOT exist now + res := cli.RunDockerOrExitError(t, "image", "inspect", "build-dependencies_service") + res.Assert(t, icmd.Expected{ + ExitCode: 1, + Err: "Error: No such image: build-dependencies_service", + }) + + res = cli.RunDockerComposeCmd(t, "build") + t.Log(res.Combined()) + + res = cli.RunDockerCmd(t, + "image", "inspect", "--format={{ index .RepoTags 0 }}", + "build-dependencies_service") + res.Assert(t, icmd.Expected{Out: "build-dependencies_service:latest"}) + } + + t.Run("ClassicBuilder", func(t *testing.T) { + cli := NewParallelCLI(t, WithEnv( + "DOCKER_BUILDKIT=0", + "COMPOSE_FILE=./fixtures/build-dependencies/compose.yaml", + )) + doTest(t, cli) + }) + + t.Run("BuildKit", func(t *testing.T) { + t.Skip("See https://github.com/docker/compose/issues/9232") + }) +} diff --git a/pkg/e2e/fixtures/build-dependencies/base.dockerfile b/pkg/e2e/fixtures/build-dependencies/base.dockerfile new file mode 100644 index 0000000000..6828fe52ac --- /dev/null +++ b/pkg/e2e/fixtures/build-dependencies/base.dockerfile @@ -0,0 +1,5 @@ +FROM alpine + +COPY hello.txt /hello.txt + +CMD [ "/bin/true" ] diff --git a/pkg/e2e/fixtures/build-dependencies/compose.yaml b/pkg/e2e/fixtures/build-dependencies/compose.yaml new file mode 100644 index 0000000000..7de1960b4e --- /dev/null +++ b/pkg/e2e/fixtures/build-dependencies/compose.yaml @@ -0,0 +1,12 @@ +services: + base: + image: base + build: + context: . + dockerfile: base.dockerfile + service: + depends_on: + - base + build: + context: . + dockerfile: service.dockerfile diff --git a/pkg/e2e/fixtures/build-dependencies/hello.txt b/pkg/e2e/fixtures/build-dependencies/hello.txt new file mode 100644 index 0000000000..810e7ba64a --- /dev/null +++ b/pkg/e2e/fixtures/build-dependencies/hello.txt @@ -0,0 +1 @@ +this file was copied from base -> service diff --git a/pkg/e2e/fixtures/build-dependencies/service.dockerfile b/pkg/e2e/fixtures/build-dependencies/service.dockerfile new file mode 100644 index 0000000000..4c27a635c6 --- /dev/null +++ b/pkg/e2e/fixtures/build-dependencies/service.dockerfile @@ -0,0 +1,5 @@ +FROM alpine + +COPY --from=base /hello.txt /hello.txt + +CMD [ "cat", "/hello.txt" ] From ec0efec839e5bcef67a49cf4c2312ee8af466771 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Fri, 24 Jun 2022 16:16:53 -0400 Subject: [PATCH 2/2] test: add copyright notice Signed-off-by: Milas Bowman --- .../fixtures/build-dependencies/base.dockerfile | 14 ++++++++++++++ .../fixtures/build-dependencies/service.dockerfile | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/pkg/e2e/fixtures/build-dependencies/base.dockerfile b/pkg/e2e/fixtures/build-dependencies/base.dockerfile index 6828fe52ac..9dce0b74f4 100644 --- a/pkg/e2e/fixtures/build-dependencies/base.dockerfile +++ b/pkg/e2e/fixtures/build-dependencies/base.dockerfile @@ -1,3 +1,17 @@ +# Copyright 2020 Docker Compose CLI authors + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + FROM alpine COPY hello.txt /hello.txt diff --git a/pkg/e2e/fixtures/build-dependencies/service.dockerfile b/pkg/e2e/fixtures/build-dependencies/service.dockerfile index 4c27a635c6..95abc433d7 100644 --- a/pkg/e2e/fixtures/build-dependencies/service.dockerfile +++ b/pkg/e2e/fixtures/build-dependencies/service.dockerfile @@ -1,3 +1,17 @@ +# Copyright 2020 Docker Compose CLI authors + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + FROM alpine COPY --from=base /hello.txt /hello.txt