From 13115468d52ef0f94396c012bb8d40b702c056b0 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Fri, 8 Sep 2023 11:35:57 -0400 Subject: [PATCH] cli: fix `--build` flag for `create` (#10982) I missed this during a refactor and there wasn't test coverage. Instead of adding more heavy-weight integration tests, I tried to use `gomock` here to assert on the options objects after CLI flag parsing. I think with a few more helpers, this could be a good way to get a lot more combinations covered without adding a ton of slow E2E tests. Signed-off-by: Milas Bowman --- cmd/compose/create.go | 47 +++++++--- cmd/compose/create_test.go | 170 +++++++++++++++++++++++++++++++++++++ go.mod | 4 +- 3 files changed, 205 insertions(+), 16 deletions(-) create mode 100644 cmd/compose/create_test.go diff --git a/cmd/compose/create.go b/cmd/compose/create.go index e19d4fcea..f47fa3719 100644 --- a/cmd/compose/create.go +++ b/cmd/compose/create.go @@ -49,6 +49,9 @@ type createOptions struct { func createCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service) *cobra.Command { opts := createOptions{} + buildOpts := buildOptions{ + ProjectOptions: p, + } cmd := &cobra.Command{ Use: "create [OPTIONS] [SERVICE...]", Short: "Creates containers for a service.", @@ -62,20 +65,9 @@ func createCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service } return nil }), - RunE: p.WithProject(func(ctx context.Context, project *types.Project) error { - if err := opts.Apply(project); err != nil { - return err - } - return backend.Create(ctx, project, api.CreateOptions{ - RemoveOrphans: opts.removeOrphans, - IgnoreOrphans: opts.ignoreOrphans, - Recreate: opts.recreateStrategy(), - RecreateDependencies: opts.dependenciesRecreateStrategy(), - Inherit: !opts.noInherit, - Timeout: opts.GetTimeout(), - QuietPull: false, - }) - }, dockerCli), + RunE: p.WithServices(dockerCli, func(ctx context.Context, project *types.Project, services []string) error { + return runCreate(ctx, dockerCli, backend, opts, buildOpts, project, services) + }), ValidArgsFunction: completeServiceNames(dockerCli, p), } flags := cmd.Flags() @@ -89,6 +81,33 @@ func createCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service return cmd } +func runCreate(ctx context.Context, _ command.Cli, backend api.Service, createOpts createOptions, buildOpts buildOptions, project *types.Project, services []string) error { + if err := createOpts.Apply(project); err != nil { + return err + } + + var build *api.BuildOptions + if !createOpts.noBuild { + bo, err := buildOpts.toAPIBuildOptions(services) + if err != nil { + return err + } + build = &bo + } + + return backend.Create(ctx, project, api.CreateOptions{ + Build: build, + Services: services, + RemoveOrphans: createOpts.removeOrphans, + IgnoreOrphans: createOpts.ignoreOrphans, + Recreate: createOpts.recreateStrategy(), + RecreateDependencies: createOpts.dependenciesRecreateStrategy(), + Inherit: !createOpts.noInherit, + Timeout: createOpts.GetTimeout(), + QuietPull: false, + }) +} + func (opts createOptions) recreateStrategy() string { if opts.noRecreate { return api.RecreateNever diff --git a/cmd/compose/create_test.go b/cmd/compose/create_test.go new file mode 100644 index 000000000..95a2a9614 --- /dev/null +++ b/cmd/compose/create_test.go @@ -0,0 +1,170 @@ +/* + Copyright 2023 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. +*/ + +package compose + +import ( + "context" + "fmt" + "testing" + + "github.com/compose-spec/compose-go/types" + "github.com/davecgh/go-spew/spew" + "github.com/docker/compose/v2/pkg/api" + "github.com/docker/compose/v2/pkg/mocks" + "github.com/golang/mock/gomock" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" +) + +func TestRunCreate(t *testing.T) { + ctrl, ctx := gomock.WithContext(context.Background(), t) + backend := mocks.NewMockService(ctrl) + backend.EXPECT().Create( + gomock.Eq(ctx), + pullPolicy(""), + deepEqual(defaultCreateOptions(true)), + ) + + createOpts := createOptions{} + buildOpts := buildOptions{} + project := sampleProject() + err := runCreate(ctx, nil, backend, createOpts, buildOpts, project, nil) + require.NoError(t, err) +} + +func TestRunCreate_Build(t *testing.T) { + ctrl, ctx := gomock.WithContext(context.Background(), t) + backend := mocks.NewMockService(ctrl) + backend.EXPECT().Create( + gomock.Eq(ctx), + pullPolicy("build"), + deepEqual(defaultCreateOptions(true)), + ) + + createOpts := createOptions{ + Build: true, + } + buildOpts := buildOptions{} + project := sampleProject() + err := runCreate(ctx, nil, backend, createOpts, buildOpts, project, nil) + require.NoError(t, err) +} + +func TestRunCreate_NoBuild(t *testing.T) { + ctrl, ctx := gomock.WithContext(context.Background(), t) + backend := mocks.NewMockService(ctrl) + backend.EXPECT().Create( + gomock.Eq(ctx), + pullPolicy(""), + deepEqual(defaultCreateOptions(false)), + ) + + createOpts := createOptions{ + noBuild: true, + } + buildOpts := buildOptions{} + project := sampleProject() + err := runCreate(ctx, nil, backend, createOpts, buildOpts, project, nil) + require.NoError(t, err) +} + +func sampleProject() *types.Project { + return &types.Project{ + Name: "test", + Services: types.Services{ + { + Name: "svc", + Build: &types.BuildConfig{ + Context: ".", + }, + }, + }, + } +} + +func defaultCreateOptions(includeBuild bool) api.CreateOptions { + var build *api.BuildOptions + if includeBuild { + bo := defaultBuildOptions() + build = &bo + } + return api.CreateOptions{ + Build: build, + Services: nil, + RemoveOrphans: false, + IgnoreOrphans: false, + Recreate: "diverged", + RecreateDependencies: "diverged", + Inherit: true, + Timeout: nil, + QuietPull: false, + } +} + +func defaultBuildOptions() api.BuildOptions { + return api.BuildOptions{ + Args: make(types.MappingWithEquals), + Progress: "auto", + } +} + +// deepEqual returns a nice diff on failure vs gomock.Eq when used +// on structs. +func deepEqual(x interface{}) gomock.Matcher { + return gomock.GotFormatterAdapter( + gomock.GotFormatterFunc(func(got interface{}) string { + return cmp.Diff(x, got) + }), + gomock.Eq(x), + ) +} + +func spewAdapter(m gomock.Matcher) gomock.Matcher { + return gomock.GotFormatterAdapter( + gomock.GotFormatterFunc(func(got interface{}) string { + return spew.Sdump(got) + }), + m, + ) +} + +type withPullPolicy struct { + policy string +} + +func pullPolicy(policy string) gomock.Matcher { + return spewAdapter(withPullPolicy{policy: policy}) +} + +func (w withPullPolicy) Matches(x interface{}) bool { + proj, ok := x.(*types.Project) + if !ok || proj == nil || len(proj.Services) == 0 { + return false + } + + for _, svc := range proj.Services { + if svc.PullPolicy != w.policy { + return false + } + } + + return true +} + +func (w withPullPolicy) String() string { + return fmt.Sprintf("has pull policy %q for all services", w.policy) +} diff --git a/go.mod b/go.mod index 69f8c51bc..55de039a5 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/containerd/console v1.0.3 github.com/containerd/containerd v1.7.3 github.com/cucumber/godog v0.0.0-00010101000000-000000000000 // replaced; see replace for the actual version used + github.com/davecgh/go-spew v1.1.1 github.com/distribution/reference v0.5.0 github.com/docker/buildx v0.11.2 github.com/docker/cli v24.0.5+incompatible @@ -20,6 +21,7 @@ require ( github.com/docker/go-units v0.5.0 github.com/fsnotify/fsevents v0.1.1 github.com/golang/mock v1.6.0 + github.com/google/go-cmp v0.5.9 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-version v1.6.0 github.com/jonboulle/clockwork v0.4.0 @@ -75,7 +77,6 @@ require ( github.com/cucumber/gherkin-go/v19 v19.0.3 // indirect github.com/cucumber/messages-go/v16 v16.0.1 // indirect github.com/cyphar/filepath-securejoin v0.2.3 // indirect - github.com/davecgh/go-spew v1.1.1 // indirect github.com/docker/distribution v2.8.2+incompatible // indirect github.com/docker/docker-credential-helpers v0.7.0 // indirect github.com/docker/go v1.5.1-1.0.20160303222718-d30aec9fd63c // indirect @@ -94,7 +95,6 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/gnostic v0.5.7-v3refs // indirect - github.com/google/go-cmp v0.5.9 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect github.com/gorilla/mux v1.8.0 // indirect