Merge pull request #1767 from ndeloof/exec_no_tty

automatically disable TTY if shell isn't a terminal

Author: ndeloof

cli/cmd/compose/exec.go

@@ -68,7 +68,7 @@ func execCommand(p *projectOptions, backend compose.Service) *cobra.Command {
 	runCmd.Flags().IntVar(&opts.index, "index", 1, "index of the container if there are multiple instances of a service [default: 1].")
 	runCmd.Flags().BoolVarP(&opts.privileged, "privileged", "", false, "Give extended privileges to the process.")
 	runCmd.Flags().StringVarP(&opts.user, "user", "u", "", "Run the command as this user.")
-	runCmd.Flags().BoolVarP(&opts.noTty, "", "T", false, "Disable pseudo-TTY allocation. By default `docker compose exec` allocates a TTY.")
+	runCmd.Flags().BoolVarP(&opts.noTty, "no-TTY", "T", notAtTTY(), "Disable pseudo-TTY allocation. By default `docker compose exec` allocates a TTY.")
 	runCmd.Flags().StringVarP(&opts.workingDir, "workdir", "w", "", "Path to workdir directory for this command.")
 
 	runCmd.Flags().SetInterspersed(false)

cli/cmd/compose/run.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/compose-spec/compose-go/loader"
 	"github.com/compose-spec/compose-go/types"
+	"github.com/mattn/go-isatty"
 	"github.com/mattn/go-shellwords"
 	"github.com/spf13/cobra"
 
@@ -132,7 +133,7 @@ func runCommand(p *projectOptions, backend compose.Service) *cobra.Command {
 	flags.StringArrayVarP(&opts.environment, "env", "e", []string{}, "Set environment variables")
 	flags.StringArrayVarP(&opts.labels, "labels", "l", []string{}, "Add or override a label")
 	flags.BoolVar(&opts.Remove, "rm", false, "Automatically remove the container when it exits")
-	flags.BoolVarP(&opts.noTty, "no-TTY", "T", false, "Disable pseudo-noTty allocation. By default docker compose run allocates a TTY")
+	flags.BoolVarP(&opts.noTty, "no-TTY", "T", notAtTTY(), "Disable pseudo-noTty allocation. By default docker compose run allocates a TTY")
 	flags.StringVar(&opts.name, "name", "", " Assign a name to the container")
 	flags.StringVarP(&opts.user, "user", "u", "", "Run as specified username or uid")
 	flags.StringVarP(&opts.workdir, "workdir", "w", "", "Working directory inside the container")
@@ -147,6 +148,10 @@ func runCommand(p *projectOptions, backend compose.Service) *cobra.Command {
 	return cmd
 }
 
+func notAtTTY() bool {
+	return !isatty.IsTerminal(os.Stdout.Fd())
+}
+
 func runRun(ctx context.Context, backend compose.Service, project *types.Project, opts runOptions) error {
 	err := opts.apply(project)
 	if err != nil {

local/compose/exec.go

@@ -22,8 +22,9 @@ import (
 	"io"
 
 	"github.com/compose-spec/compose-go/types"
-	apitypes "github.com/docker/docker/api/types"
+	moby "github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/pkg/stdcopy"
 
 	"github.com/docker/compose-cli/api/compose"
 )
@@ -34,34 +35,14 @@ func (s *composeService) Exec(ctx context.Context, project *types.Project, opts
 		return 0, err
 	}
 
-	containers, err := s.apiClient.ContainerList(ctx, apitypes.ContainerListOptions{
-		Filters: filters.NewArgs(
-			projectFilter(project.Name),
-			serviceFilter(service.Name),
-			filters.Arg("label", fmt.Sprintf("%s=%d", containerNumberLabel, opts.Index)),
-		),
-	})
+	container, err := s.getExecTarget(ctx, project, service, opts)
 	if err != nil {
 		return 0, err
 	}
-	if len(containers) < 1 {
-		return 0, fmt.Errorf("container %s not running", getContainerName(project.Name, service, opts.Index))
-	}
-	container := containers[0]
-
-	var env []string
-	for k, v := range service.Environment.OverrideBy(types.NewMappingWithEquals(opts.Environment)).
-		Resolve(func(s string) (string, bool) {
-			v, ok := project.Environment[s]
-			return v, ok
-		}).
-		RemoveEmpty() {
-		env = append(env, fmt.Sprintf("%s=%s", k, *v))
-	}
 
-	exec, err := s.apiClient.ContainerExecCreate(ctx, container.ID, apitypes.ExecConfig{
+	exec, err := s.apiClient.ContainerExecCreate(ctx, container.ID, moby.ExecConfig{
 		Cmd:        opts.Command,
-		Env:        env,
+		Env:        s.getExecEnvironment(project, service, opts),
 		User:       opts.User,
 		Privileged: opts.Privileged,
 		Tty:        opts.Tty,
@@ -77,15 +58,14 @@ func (s *composeService) Exec(ctx context.Context, project *types.Project, opts
 	}
 
 	if opts.Detach {
-		return 0, s.apiClient.ContainerExecStart(ctx, exec.ID, apitypes.ExecStartCheck{
+		return 0, s.apiClient.ContainerExecStart(ctx, exec.ID, moby.ExecStartCheck{
 			Detach: true,
 			Tty:    opts.Tty,
 		})
 	}
 
-	resp, err := s.apiClient.ContainerExecAttach(ctx, exec.ID, apitypes.ExecStartCheck{
-		Detach: false,
-		Tty:    opts.Tty,
+	resp, err := s.apiClient.ContainerExecAttach(ctx, exec.ID, moby.ExecStartCheck{
+		Tty: opts.Tty,
 	})
 	if err != nil {
 		return 0, err
@@ -99,30 +79,78 @@ func (s *composeService) Exec(ctx context.Context, project *types.Project, opts
 		}
 	}
 
-	readChannel := make(chan error)
-	writeChannel := make(chan error)
+	err = s.interactiveExec(ctx, opts, resp)
+	if err != nil {
+		return 0, err
+	}
+
+	return s.getExecExitStatus(ctx, exec.ID)
+}
+
+// inspired by https://github.com/docker/cli/blob/master/cli/command/container/exec.go#L116
+func (s *composeService) interactiveExec(ctx context.Context, opts compose.RunOptions, resp moby.HijackedResponse) error {
+	outputDone := make(chan error)
+	inputDone := make(chan error)
 
 	go func() {
-		_, err := io.Copy(opts.Writer, resp.Reader)
-		readChannel <- err
+		if opts.Tty {
+			_, err := io.Copy(opts.Writer, resp.Reader)
+			outputDone <- err
+		} else {
+			_, err := stdcopy.StdCopy(opts.Writer, opts.Writer, resp.Reader)
+			outputDone <- err
+		}
 	}()
 
 	go func() {
 		_, err := io.Copy(resp.Conn, opts.Reader)
-		writeChannel <- err
+		inputDone <- err
 	}()
 
-	select {
-	case err = <-readChannel:
-		break
-	case err = <-writeChannel:
-		break
+	for {
+		select {
+		case err := <-outputDone:
+			return err
+		case err := <-inputDone:
+			if err != nil {
+				return err
+			}
+			// Wait for output to complete streaming
+		case <-ctx.Done():
+			return ctx.Err()
+		}
 	}
+}
 
+func (s *composeService) getExecTarget(ctx context.Context, project *types.Project, service types.ServiceConfig, opts compose.RunOptions) (moby.Container, error) {
+	containers, err := s.apiClient.ContainerList(ctx, moby.ContainerListOptions{
+		Filters: filters.NewArgs(
+			projectFilter(project.Name),
+			serviceFilter(service.Name),
+			filters.Arg("label", fmt.Sprintf("%s=%d", containerNumberLabel, opts.Index)),
+		),
+	})
 	if err != nil {
-		return 0, err
+		return moby.Container{}, err
 	}
-	return s.getExecExitStatus(ctx, exec.ID)
+	if len(containers) < 1 {
+		return moby.Container{}, fmt.Errorf("container %s not running", getContainerName(project.Name, service, opts.Index))
+	}
+	container := containers[0]
+	return container, nil
+}
+
+func (s *composeService) getExecEnvironment(project *types.Project, service types.ServiceConfig, opts compose.RunOptions) []string {
+	var env []string
+	for k, v := range service.Environment.OverrideBy(types.NewMappingWithEquals(opts.Environment)).
+		Resolve(func(s string) (string, bool) {
+			v, ok := project.Environment[s]
+			return v, ok
+		}).
+		RemoveEmpty() {
+		env = append(env, fmt.Sprintf("%s=%s", k, *v))
+	}
+	return env
 }
 
 func (s *composeService) getExecExitStatus(ctx context.Context, execID string) (int, error) {

local/e2e/compose/networks_test.go

@@ -82,7 +82,7 @@ func TestNetworkAliasses(t *testing.T) {
 
 	t.Run("curl", func(t *testing.T) {
 		res := c.RunDockerCmd("compose", "-f", "./fixtures/network-alias/compose.yaml", "--project-name", projectName, "exec", "-T", "container1", "curl", "http://alias-of-container2/")
-		assert.Assert(t, !strings.Contains(res.Stdout(), "Welcome to nginx!"), res.Stdout())
+		assert.Assert(t, strings.Contains(res.Stdout(), "Welcome to nginx!"), res.Stdout())
 	})
 
 	t.Run("down", func(t *testing.T) {