From bbe1b77a67dd0ae30dde6eac0f60b6c75f219021 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Wed, 8 Mar 2023 15:11:32 +0000 Subject: [PATCH] progress writer uses dockercli.Err stream Signed-off-by: Nicolas De Loof --- cmd/compose/compose.go | 4 ++-- cmd/compose/run.go | 6 +++--- cmd/compose/version.go | 13 +++++++------ pkg/compose/build.go | 2 +- pkg/compose/create.go | 2 +- pkg/compose/down.go | 2 +- pkg/compose/down_test.go | 31 ++++++++++++++++--------------- pkg/compose/kill.go | 2 +- pkg/compose/kill_test.go | 9 ++------- pkg/compose/logs_test.go | 9 ++------- pkg/compose/pause.go | 4 ++-- pkg/compose/ps_test.go | 5 +---- pkg/compose/pull.go | 4 ++-- pkg/compose/push.go | 2 +- pkg/compose/remove.go | 2 +- pkg/compose/restart.go | 2 +- pkg/compose/start.go | 2 +- pkg/compose/stop.go | 2 +- pkg/compose/stop_test.go | 5 +---- pkg/compose/up.go | 4 ++-- pkg/progress/writer.go | 16 ++++++++-------- 21 files changed, 57 insertions(+), 71 deletions(-) diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 51a74bdb..4c59666c 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -271,7 +271,7 @@ func RootCommand(streams command.Cli, backend api.Service) *cobra.Command { //no return cmd.Help() } if version { - return versionCommand().Execute() + return versionCommand(streams).Execute() } _ = cmd.Help() return dockercli.StatusError{ @@ -371,7 +371,7 @@ func RootCommand(streams command.Cli, backend api.Service) *cobra.Command { //no eventsCommand(&opts, streams, backend), portCommand(&opts, streams, backend), imagesCommand(&opts, streams, backend), - versionCommand(), + versionCommand(streams), buildCommand(&opts, streams, backend), pushCommand(&opts, backend), pullCommand(&opts, backend), diff --git a/cmd/compose/run.go b/cmd/compose/run.go index 37084041..5fbbac76 100644 --- a/cmd/compose/run.go +++ b/cmd/compose/run.go @@ -147,7 +147,7 @@ func runCommand(p *ProjectOptions, streams api.Streams, backend api.Service) *co return err } opts.ignoreOrphans = utils.StringToBool(project.Environment["COMPOSE_IGNORE_ORPHANS"]) - return runRun(ctx, backend, project, opts, createOpts) + return runRun(ctx, backend, project, opts, createOpts, streams) }), ValidArgsFunction: completeServiceNames(p), } @@ -189,7 +189,7 @@ func normalizeRunFlags(f *pflag.FlagSet, name string) pflag.NormalizedName { return pflag.NormalizedName(name) } -func runRun(ctx context.Context, backend api.Service, project *types.Project, opts runOptions, createOpts createOptions) error { +func runRun(ctx context.Context, backend api.Service, project *types.Project, opts runOptions, createOpts createOptions, streams api.Streams) error { err := opts.apply(project) if err != nil { return err @@ -202,7 +202,7 @@ func runRun(ctx context.Context, backend api.Service, project *types.Project, op err = progress.Run(ctx, func(ctx context.Context) error { return startDependencies(ctx, backend, *project, opts.Service, opts.ignoreOrphans) - }) + }, streams.Err()) if err != nil { return err } diff --git a/cmd/compose/version.go b/cmd/compose/version.go index 18c7b359..d48e5781 100644 --- a/cmd/compose/version.go +++ b/cmd/compose/version.go @@ -20,6 +20,7 @@ import ( "fmt" "strings" + "github.com/docker/cli/cli/command" "github.com/docker/compose/v2/cmd/formatter" "github.com/spf13/cobra" @@ -32,14 +33,14 @@ type versionOptions struct { short bool } -func versionCommand() *cobra.Command { +func versionCommand(streams command.Cli) *cobra.Command { opts := versionOptions{} cmd := &cobra.Command{ Use: "version [OPTIONS]", Short: "Show the Docker Compose version information", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - runVersion(opts) + runVersion(opts, streams) return nil }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { @@ -56,14 +57,14 @@ func versionCommand() *cobra.Command { return cmd } -func runVersion(opts versionOptions) { +func runVersion(opts versionOptions, streams command.Cli) { if opts.short { - fmt.Println(strings.TrimPrefix(internal.Version, "v")) + fmt.Fprintln(streams.Out(), strings.TrimPrefix(internal.Version, "v")) return } if opts.format == formatter.JSON { - fmt.Printf("{\"version\":%q}\n", internal.Version) + fmt.Fprintf(streams.Out(), "{\"version\":%q}\n", internal.Version) return } - fmt.Println("Docker Compose version", internal.Version) + fmt.Fprintln(streams.Out(), "Docker Compose version", internal.Version) } diff --git a/pkg/compose/build.go b/pkg/compose/build.go index 1c2b0455..25526810 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -46,7 +46,7 @@ func (s *composeService) Build(ctx context.Context, project *types.Project, opti 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) { diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 49cd960d..1a16a51c 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -51,7 +51,7 @@ import ( func (s *composeService) Create(ctx context.Context, project *types.Project, options api.CreateOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.create(ctx, project, options) - }) + }, s.stderr()) } func (s *composeService) create(ctx context.Context, project *types.Project, options api.CreateOptions) error { diff --git a/pkg/compose/down.go b/pkg/compose/down.go index 174a1cde..55b5b068 100644 --- a/pkg/compose/down.go +++ b/pkg/compose/down.go @@ -41,7 +41,7 @@ type downOp func() error func (s *composeService) Down(ctx context.Context, projectName string, options api.DownOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.down(ctx, strings.ToLower(projectName), options) - }) + }, s.stderr()) } func (s *composeService) down(ctx context.Context, projectName string, options api.DownOptions) error { diff --git a/pkg/compose/down_test.go b/pkg/compose/down_test.go index a03fee3a..6c7da97c 100644 --- a/pkg/compose/down_test.go +++ b/pkg/compose/down_test.go @@ -19,10 +19,12 @@ package compose import ( "context" "fmt" + "os" "strings" "testing" "github.com/compose-spec/compose-go/types" + "github.com/docker/cli/cli/streams" moby "github.com/docker/docker/api/types" containerType "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" @@ -39,12 +41,10 @@ func TestDown(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return( []moby.Container{ @@ -90,12 +90,10 @@ func TestDownRemoveOrphans(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(true)).Return( []moby.Container{ @@ -130,12 +128,10 @@ func TestDownRemoveVolumes(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return( []moby.Container{testContainer("service1", "123", false)}, nil) @@ -173,12 +169,10 @@ func TestDownRemoveImages(t *testing.T) { }, } - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)). Return([]moby.Container{ @@ -264,12 +258,10 @@ func TestDownRemoveImages_NoLabel(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() container := testContainer("service1", "123", false) @@ -303,3 +295,12 @@ func TestDownRemoveImages_NoLabel(t *testing.T) { err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{Images: "local"}) assert.NilError(t, err) } + +func prepareMocks(mockCtrl *gomock.Controller) (*mocks.MockAPIClient, *mocks.MockCli) { + api := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(api).AnyTimes() + cli.EXPECT().Err().Return(os.Stderr).AnyTimes() + cli.EXPECT().Out().Return(streams.NewOut(os.Stdout)).AnyTimes() + return api, cli +} diff --git a/pkg/compose/kill.go b/pkg/compose/kill.go index 6460c4b4..7a9aa43f 100644 --- a/pkg/compose/kill.go +++ b/pkg/compose/kill.go @@ -31,7 +31,7 @@ import ( func (s *composeService) Kill(ctx context.Context, projectName string, options api.KillOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.kill(ctx, strings.ToLower(projectName), options) - }) + }, s.stderr()) } func (s *composeService) kill(ctx context.Context, projectName string, options api.KillOptions) error { diff --git a/pkg/compose/kill_test.go b/pkg/compose/kill_test.go index f17845f7..7f06a5d2 100644 --- a/pkg/compose/kill_test.go +++ b/pkg/compose/kill_test.go @@ -30,7 +30,6 @@ import ( "gotest.tools/v3/assert" compose "github.com/docker/compose/v2/pkg/api" - "github.com/docker/compose/v2/pkg/mocks" ) const testProject = "testProject" @@ -39,12 +38,10 @@ func TestKillAll(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() name := strings.ToLower(testProject) @@ -72,12 +69,10 @@ func TestKillSignal(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() name := strings.ToLower(testProject) listOptions := moby.ContainerListOptions{ diff --git a/pkg/compose/logs_test.go b/pkg/compose/logs_test.go index de80a987..21e30f2c 100644 --- a/pkg/compose/logs_test.go +++ b/pkg/compose/logs_test.go @@ -33,19 +33,16 @@ import ( "github.com/stretchr/testify/require" compose "github.com/docker/compose/v2/pkg/api" - "github.com/docker/compose/v2/pkg/mocks" ) func TestComposeService_Logs_Demux(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() name := strings.ToLower(testProject) @@ -113,12 +110,10 @@ func TestComposeService_Logs_ServiceFiltering(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() name := strings.ToLower(testProject) diff --git a/pkg/compose/pause.go b/pkg/compose/pause.go index 02434b4f..d094fc8a 100644 --- a/pkg/compose/pause.go +++ b/pkg/compose/pause.go @@ -30,7 +30,7 @@ import ( func (s *composeService) Pause(ctx context.Context, projectName string, options api.PauseOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.pause(ctx, strings.ToLower(projectName), options) - }) + }, s.stderr()) } func (s *composeService) pause(ctx context.Context, projectName string, options api.PauseOptions) error { @@ -62,7 +62,7 @@ func (s *composeService) pause(ctx context.Context, projectName string, options func (s *composeService) UnPause(ctx context.Context, projectName string, options api.PauseOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.unPause(ctx, strings.ToLower(projectName), options) - }) + }, s.stderr()) } func (s *composeService) unPause(ctx context.Context, projectName string, options api.PauseOptions) error { diff --git a/pkg/compose/ps_test.go b/pkg/compose/ps_test.go index c5e7fdb9..015660a2 100644 --- a/pkg/compose/ps_test.go +++ b/pkg/compose/ps_test.go @@ -25,7 +25,6 @@ import ( "gotest.tools/v3/assert" compose "github.com/docker/compose/v2/pkg/api" - "github.com/docker/compose/v2/pkg/mocks" moby "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" ) @@ -34,12 +33,10 @@ func TestPs(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() ctx := context.Background() args := filters.NewArgs(projectFilter(strings.ToLower(testProject)), hasConfigHashLabel()) diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index 10828541..93337b79 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -44,7 +44,7 @@ func (s *composeService) Pull(ctx context.Context, project *types.Project, optio } return progress.Run(ctx, func(ctx context.Context) error { return s.pull(ctx, project, options) - }) + }, s.stderr()) } func (s *composeService) pull(ctx context.Context, project *types.Project, opts api.PullOptions) error { //nolint:gocyclo @@ -327,7 +327,7 @@ func (s *composeService) pullRequiredImages(ctx context.Context, project *types. } } return err - }) + }, s.stderr()) } func isServiceImageToBuild(service types.ServiceConfig, services []types.ServiceConfig) bool { diff --git a/pkg/compose/push.go b/pkg/compose/push.go index a5a5bb75..5e7446ef 100644 --- a/pkg/compose/push.go +++ b/pkg/compose/push.go @@ -42,7 +42,7 @@ func (s *composeService) Push(ctx context.Context, project *types.Project, optio } return progress.Run(ctx, func(ctx context.Context) error { return s.push(ctx, project, options) - }) + }, s.stderr()) } func (s *composeService) push(ctx context.Context, project *types.Project, options api.PushOptions) error { diff --git a/pkg/compose/remove.go b/pkg/compose/remove.go index f5eeb829..7b1b3d14 100644 --- a/pkg/compose/remove.go +++ b/pkg/compose/remove.go @@ -71,7 +71,7 @@ func (s *composeService) Remove(ctx context.Context, projectName string, options } return progress.Run(ctx, func(ctx context.Context) error { return s.remove(ctx, stoppedContainers, options) - }) + }, s.stderr()) } func (s *composeService) remove(ctx context.Context, containers Containers, options api.RemoveOptions) error { diff --git a/pkg/compose/restart.go b/pkg/compose/restart.go index 2f41aa6b..c1f4ee74 100644 --- a/pkg/compose/restart.go +++ b/pkg/compose/restart.go @@ -31,7 +31,7 @@ import ( func (s *composeService) Restart(ctx context.Context, projectName string, options api.RestartOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.restart(ctx, strings.ToLower(projectName), options) - }) + }, s.stderr()) } func (s *composeService) restart(ctx context.Context, projectName string, options api.RestartOptions) error { diff --git a/pkg/compose/start.go b/pkg/compose/start.go index 265a448d..4627259d 100644 --- a/pkg/compose/start.go +++ b/pkg/compose/start.go @@ -36,7 +36,7 @@ import ( func (s *composeService) Start(ctx context.Context, projectName string, options api.StartOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.start(ctx, strings.ToLower(projectName), options, nil) - }) + }, s.stderr()) } func (s *composeService) start(ctx context.Context, projectName string, options api.StartOptions, listener api.ContainerEventListener) error { diff --git a/pkg/compose/stop.go b/pkg/compose/stop.go index 971b7e99..7c54d19d 100644 --- a/pkg/compose/stop.go +++ b/pkg/compose/stop.go @@ -28,7 +28,7 @@ import ( func (s *composeService) Stop(ctx context.Context, projectName string, options api.StopOptions) error { return progress.Run(ctx, func(ctx context.Context) error { return s.stop(ctx, strings.ToLower(projectName), options) - }) + }, s.stderr()) } func (s *composeService) stop(ctx context.Context, projectName string, options api.StopOptions) error { diff --git a/pkg/compose/stop_test.go b/pkg/compose/stop_test.go index 01449b40..ea22c3a0 100644 --- a/pkg/compose/stop_test.go +++ b/pkg/compose/stop_test.go @@ -25,7 +25,6 @@ import ( "github.com/docker/compose/v2/pkg/utils" compose "github.com/docker/compose/v2/pkg/api" - "github.com/docker/compose/v2/pkg/mocks" containerType "github.com/docker/docker/api/types/container" moby "github.com/docker/docker/api/types" @@ -39,12 +38,10 @@ func TestStopTimeout(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - api := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) + api, cli := prepareMocks(mockCtrl) tested := composeService{ dockerCli: cli, } - cli.EXPECT().Client().Return(api).AnyTimes() ctx := context.Background() api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return( diff --git a/pkg/compose/up.go b/pkg/compose/up.go index 19fb9dfe..611ee8e4 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -40,7 +40,7 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options return s.start(ctx, project.Name, options.Start, nil) } return nil - }) + }, s.stderr()) if err != nil { return err } @@ -70,7 +70,7 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options Services: options.Create.Services, Project: project, }) - }) + }, s.stderr()) } go func() { <-signalChan diff --git a/pkg/progress/writer.go b/pkg/progress/writer.go index 62c4bd20..67c2cbcd 100644 --- a/pkg/progress/writer.go +++ b/pkg/progress/writer.go @@ -18,7 +18,7 @@ package progress import ( "context" - "os" + "io" "sync" "github.com/docker/compose/v2/pkg/api" @@ -58,17 +58,17 @@ type progressFunc func(context.Context) error type progressFuncWithStatus func(context.Context) (string, error) // Run will run a writer and the progress function in parallel -func Run(ctx context.Context, pf progressFunc) error { +func Run(ctx context.Context, pf progressFunc, out io.Writer) error { _, err := RunWithStatus(ctx, func(ctx context.Context) (string, error) { return "", pf(ctx) - }) + }, out) return err } // RunWithStatus will run a writer and the progress function in parallel and return a status -func RunWithStatus(ctx context.Context, pf progressFuncWithStatus) (string, error) { +func RunWithStatus(ctx context.Context, pf progressFuncWithStatus, out io.Writer) (string, error) { eg, _ := errgroup.WithContext(ctx) - w, err := NewWriter(ctx, os.Stderr) + w, err := NewWriter(ctx, out) var result string if err != nil { return "", err @@ -105,17 +105,17 @@ const ( var Mode = ModeAuto // NewWriter returns a new multi-progress writer -func NewWriter(ctx context.Context, out console.File) (Writer, error) { +func NewWriter(ctx context.Context, out io.Writer) (Writer, error) { _, isTerminal := term.GetFdInfo(out) dryRun, ok := ctx.Value(api.DryRunKey{}).(bool) if !ok { dryRun = false } if Mode == ModeAuto && isTerminal { - return newTTYWriter(out, dryRun) + return newTTYWriter(out.(console.File), dryRun) } if Mode == ModeTTY { - return newTTYWriter(out, dryRun) + return newTTYWriter(out.(console.File), dryRun) } return &plainWriter{ out: out,