From c257001e5aaf8d3c9a5efe192afba94ff40174fb Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 20 Jul 2021 11:39:29 +0200 Subject: [PATCH 1/3] Restore support for control sequence in `compose run` Signed-off-by: Nicolas De Loof --- pkg/compose/attach.go | 37 +++++++------ pkg/compose/run.go | 121 +++++++++++++++++++++++++++--------------- 2 files changed, 97 insertions(+), 61 deletions(-) diff --git a/pkg/compose/attach.go b/pkg/compose/attach.go index a06770545..01b8d9e9a 100644 --- a/pkg/compose/attach.go +++ b/pkg/compose/attach.go @@ -24,11 +24,11 @@ import ( "github.com/compose-spec/compose-go/types" "github.com/docker/cli/cli/streams" - "github.com/docker/compose-cli/pkg/api" moby "github.com/docker/docker/api/types" "github.com/docker/docker/pkg/stdcopy" "github.com/moby/term" + "github.com/docker/compose-cli/pkg/api" "github.com/docker/compose-cli/pkg/utils" ) @@ -85,27 +85,10 @@ func (s *composeService) attachContainer(ctx context.Context, container moby.Con func (s *composeService) attachContainerStreams(ctx context.Context, container string, tty bool, stdin io.ReadCloser, stdout, stderr io.Writer) (func(), chan bool, error) { detached := make(chan bool) var ( - in *streams.In restore = func() { /* noop */ } ) if stdin != nil { - in = streams.NewIn(stdin) - } - - streamIn, streamOut, err := s.getContainerStreams(ctx, container) - if err != nil { - return restore, detached, err - } - - go func() { - <-ctx.Done() - if in != nil { - in.Close() //nolint:errcheck - } - streamOut.Close() //nolint:errcheck - }() - - if in != nil && streamIn != nil { + in := streams.NewIn(stdin) if in.IsTerminal() { state, err := term.SetRawTerminal(in.FD()) if err != nil { @@ -115,6 +98,22 @@ func (s *composeService) attachContainerStreams(ctx context.Context, container s term.RestoreTerminal(in.FD(), state) //nolint:errcheck } } + } + + streamIn, streamOut, err := s.getContainerStreams(ctx, container) + if err != nil { + return restore, detached, err + } + + go func() { + <-ctx.Done() + if stdin != nil { + stdin.Close() //nolint:errcheck + } + streamOut.Close() //nolint:errcheck + }() + + if streamIn != nil && stdin != nil { go func() { _, err := io.Copy(streamIn, stdin) if _, ok := err.(term.EscapeError); ok { diff --git a/pkg/compose/run.go b/pkg/compose/run.go index c59234593..dc4fc84a7 100644 --- a/pkg/compose/run.go +++ b/pkg/compose/run.go @@ -24,9 +24,11 @@ import ( "github.com/docker/compose-cli/pkg/api" "github.com/compose-spec/compose-go/types" + "github.com/docker/cli/cli/streams" moby "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/pkg/ioutils" + "github.com/docker/docker/pkg/stdcopy" "github.com/docker/docker/pkg/stringid" "github.com/moby/term" ) @@ -37,11 +39,83 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types. return 0, err } - service, err := project.GetService(opts.Service) + containerID, err := s.prepareRun(ctx, project, observedState, opts) if err != nil { return 0, err } + if opts.Detach { + err := s.apiClient.ContainerStart(ctx, containerID, moby.ContainerStartOptions{}) + if err != nil { + return 0, err + } + fmt.Fprintln(opts.Stdout, containerID) + return 0, nil + } + + r, err := s.getEscapeKeyProxy(opts.Stdin) + if err != nil { + return 0, err + } + + stdin, stdout, err := s.getContainerStreams(ctx, containerID) + if err != nil { + return 0, err + } + defer stdin.Close() //nolint:errcheck + defer stdout.Close() //nolint:errcheck + + detached := make(chan bool) + + in := streams.NewIn(opts.Stdin) + if in.IsTerminal() { + state, err := term.SetRawTerminal(in.FD()) + if err != nil { + return 0, err + } + defer term.RestoreTerminal(in.FD(), state) //nolint:errcheck + } + + go func() { + if opts.Tty { + io.Copy(opts.Stdout, stdout) //nolint:errcheck + } else { + stdcopy.StdCopy(opts.Stdout, opts.Stderr, stdout) //nolint:errcheck + } + }() + + go func() { + _, err := io.Copy(stdin, r) + if _, ok := err.(term.EscapeError); ok { + detached <- true + } + }() + + err = s.apiClient.ContainerStart(ctx, containerID, moby.ContainerStartOptions{}) + if err != nil { + return 0, err + } + + s.monitorTTySize(ctx, containerID, s.apiClient.ContainerResize) + + statusC, errC := s.apiClient.ContainerWait(ctx, containerID, container.WaitConditionNextExit) + select { + case status := <-statusC: + return int(status.StatusCode), nil + case err := <-errC: + return 0, err + case <-detached: + return 0, err + } + +} + +func (s *composeService) prepareRun(ctx context.Context, project *types.Project, observedState Containers, opts api.RunOptions) (string, error) { + service, err := project.GetService(opts.Service) + if err != nil { + return "", err + } + applyRunOptions(project, &service, opts) slug := stringid.GenerateRandomID() @@ -58,54 +132,17 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types. service.Labels = service.Labels.Add(api.OneoffLabel, "True") if err := s.ensureImagesExists(ctx, project, observedState, false); err != nil { // all dependencies already checked, but might miss service img - return 0, err + return "", err } if err := s.waitDependencies(ctx, project, service); err != nil { - return 0, err + return "", err } created, err := s.createContainer(ctx, project, service, service.ContainerName, 1, opts.AutoRemove, opts.UseNetworkAliases) if err != nil { - return 0, err + return "", err } containerID := created.ID - - if opts.Detach { - err := s.apiClient.ContainerStart(ctx, containerID, moby.ContainerStartOptions{}) - if err != nil { - return 0, err - } - fmt.Fprintln(opts.Stdout, containerID) - return 0, nil - } - - r, err := s.getEscapeKeyProxy(opts.Stdin) - if err != nil { - return 0, err - } - restore, detachC, err := s.attachContainerStreams(ctx, containerID, service.Tty, r, opts.Stdout, opts.Stderr) - if err != nil { - return 0, err - } - defer restore() - - statusC, errC := s.apiClient.ContainerWait(context.Background(), containerID, container.WaitConditionNextExit) - - err = s.apiClient.ContainerStart(ctx, containerID, moby.ContainerStartOptions{}) - if err != nil { - return 0, err - } - - s.monitorTTySize(ctx, containerID, s.apiClient.ContainerResize) - - select { - case status := <-statusC: - return int(status.StatusCode), nil - case <-detachC: - return 0, nil - case err := <-errC: - return 0, err - } - + return containerID, nil } func (s *composeService) getEscapeKeyProxy(r io.ReadCloser) (io.ReadCloser, error) { From 0b72b502d3e086e853cfbc048d2a58f9c58fc581 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Fri, 23 Jul 2021 14:52:31 +0200 Subject: [PATCH 2/3] close container stream on os.stdin EOF close https://github.com/docker/compose-cli/issues/1944 Signed-off-by: Nicolas De Loof --- pkg/compose/exec.go | 4 ++-- pkg/compose/run.go | 55 +++++++++++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/pkg/compose/exec.go b/pkg/compose/exec.go index dd97babe7..eadb8d097 100644 --- a/pkg/compose/exec.go +++ b/pkg/compose/exec.go @@ -118,13 +118,13 @@ func (s *composeService) interactiveExec(ctx context.Context, opts api.RunOption _, err := stdcopy.StdCopy(opts.Stdout, opts.Stderr, stdout) outputDone <- err } - defer stdout.Close() //nolint:errcheck + stdout.Close() //nolint:errcheck }() go func() { _, err := io.Copy(stdin, r) inputDone <- err - defer stdin.Close() //nolint:errcheck + stdin.Close() //nolint:errcheck }() for { diff --git a/pkg/compose/run.go b/pkg/compose/run.go index dc4fc84a7..d6c64da98 100644 --- a/pkg/compose/run.go +++ b/pkg/compose/run.go @@ -26,7 +26,6 @@ import ( "github.com/compose-spec/compose-go/types" "github.com/docker/cli/cli/streams" moby "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/container" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/stdcopy" "github.com/docker/docker/pkg/stringid" @@ -62,10 +61,6 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types. if err != nil { return 0, err } - defer stdin.Close() //nolint:errcheck - defer stdout.Close() //nolint:errcheck - - detached := make(chan bool) in := streams.NewIn(opts.Stdin) if in.IsTerminal() { @@ -76,19 +71,24 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types. defer term.RestoreTerminal(in.FD(), state) //nolint:errcheck } + outputDone := make(chan error) + inputDone := make(chan error) + go func() { if opts.Tty { - io.Copy(opts.Stdout, stdout) //nolint:errcheck + _, err := io.Copy(opts.Stdout, stdout) //nolint:errcheck + outputDone <- err } else { - stdcopy.StdCopy(opts.Stdout, opts.Stderr, stdout) //nolint:errcheck + _, err := stdcopy.StdCopy(opts.Stdout, opts.Stderr, stdout) //nolint:errcheck + outputDone <- err } + stdout.Close() //nolint:errcheck }() go func() { _, err := io.Copy(stdin, r) - if _, ok := err.(term.EscapeError); ok { - detached <- true - } + inputDone <- err + stdin.Close() //nolint:errcheck }() err = s.apiClient.ContainerStart(ctx, containerID, moby.ContainerStartOptions{}) @@ -98,16 +98,33 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types. s.monitorTTySize(ctx, containerID, s.apiClient.ContainerResize) - statusC, errC := s.apiClient.ContainerWait(ctx, containerID, container.WaitConditionNextExit) - select { - case status := <-statusC: - return int(status.StatusCode), nil - case err := <-errC: - return 0, err - case <-detached: - return 0, err + for { + select { + case err := <-outputDone: + if err != nil { + return 0, err + } + inspect, err := s.apiClient.ContainerInspect(ctx, containerID) + if err != nil { + return 0, err + } + exitCode := 0 + if inspect.State != nil { + exitCode = inspect.State.ExitCode + } + return exitCode, nil + case err := <-inputDone: + if _, ok := err.(term.EscapeError); ok { + return 0, nil + } + if err != nil { + return 0, err + } + // Wait for output to complete streaming + case <-ctx.Done(): + return 0, ctx.Err() + } } - } func (s *composeService) prepareRun(ctx context.Context, project *types.Project, observedState Containers, opts api.RunOptions) (string, error) { From 86eadc7d67102e8e37e46ac7978839e3a442c6e9 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Fri, 23 Jul 2021 15:00:43 +0200 Subject: [PATCH 3/3] reduce complexity (linter) Signed-off-by: Nicolas De Loof --- pkg/compose/run.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/compose/run.go b/pkg/compose/run.go index d6c64da98..4da052e49 100644 --- a/pkg/compose/run.go +++ b/pkg/compose/run.go @@ -52,6 +52,10 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types. return 0, nil } + return s.runInteractive(ctx, containerID, opts) +} + +func (s *composeService) runInteractive(ctx context.Context, containerID string, opts api.RunOptions) (int, error) { r, err := s.getEscapeKeyProxy(opts.Stdin) if err != nil { return 0, err