WIP: attach: pass container as argument

Trying to make "runAttach" not depend on attachOptions, and to see if we can make it accept container.AttachOptions instead relates to docker#4637 Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah · Nov 7, 2023 · 976f399 · 976f399
1 parent b336124
commit 976f399
Showing 1 changed file with 28 additions and 19 deletions.
diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go
@@ -21,8 +21,6 @@ type attachOptions struct {
 	noStdin    bool
 	proxy      bool
 	detachKeys string
-
-	container string
 }
 
 func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, args string) (*types.ContainerJSON, error) {
@@ -52,8 +50,9 @@ func NewAttachCommand(dockerCli command.Cli) *cobra.Command {
 		Short: "Attach local standard input, output, and error streams to a running container",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			opts.container = args[0]
-			return runAttach(dockerCli, &opts)
+			containerID := args[0]
+			ctx := context.Background()
+			return runAttach(ctx, dockerCli, containerID, &opts)
 		},
 		Annotations: map[string]string{
 			"aliases": "docker container attach, docker attach",
@@ -70,30 +69,38 @@ func NewAttachCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runAttach(dockerCli command.Cli, opts *attachOptions) error {
-	ctx := context.Background()
+func runAttach(ctx context.Context, dockerCli command.Cli, containerID string, opts *attachOptions) error {
 	apiClient := dockerCli.Client()
 
-	// request channel to wait for client
-	resultC, errC := apiClient.ContainerWait(ctx, opts.container, "")
+	attachStdIn := true
+	if opts.noStdin {
+		// TODO(thaJeztah): this is the tricky one: can we use container.AttachOptions for this one without it being ambiguous?
+		attachStdIn = false
+	}
 
-	c, err := inspectContainerAndCheckState(ctx, apiClient, opts.container)
+	c, err := inspectContainerAndCheckState(ctx, apiClient, containerID)
 	if err != nil {
 		return err
 	}
 
-	if err := dockerCli.In().CheckTty(!opts.noStdin, c.Config.Tty); err != nil {
-		return err
+	if attachStdIn {
+		if err := dockerCli.In().CheckTty(attachStdIn, c.Config.Tty); err != nil {
+			return err
+		}
+		if !c.Config.OpenStdin {
+			// TODO(thaJeztah): should this produce an error?
+			attachStdIn = false
+		}
 	}
 
-	detachKeys := dockerCli.ConfigFile().DetachKeys
-	if opts.detachKeys != "" {
-		detachKeys = opts.detachKeys
+	detachKeys := opts.detachKeys
+	if opts.detachKeys == "" {
+		detachKeys = dockerCli.ConfigFile().DetachKeys
 	}
 
 	options := container.AttachOptions{
 		Stream:     true,
-		Stdin:      !opts.noStdin && c.Config.OpenStdin,
+		Stdin:      attachStdIn,
 		Stdout:     true,
 		Stderr:     true,
 		DetachKeys: detachKeys,
@@ -106,11 +113,11 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 
 	if opts.proxy && !c.Config.Tty {
 		sigc := notifyAllSignals()
-		go ForwardAllSignals(ctx, dockerCli, opts.container, sigc)
+		go ForwardAllSignals(ctx, dockerCli, containerID, sigc)
 		defer signal.StopCatch(sigc)
 	}
 
-	resp, errAttach := apiClient.ContainerAttach(ctx, opts.container, options)
+	resp, errAttach := apiClient.ContainerAttach(ctx, containerID, options)
 	if errAttach != nil {
 		return errAttach
 	}
@@ -124,13 +131,13 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 	// the container and not exit.
 	//
 	// Recheck the container's state to avoid attach block.
-	_, err = inspectContainerAndCheckState(ctx, apiClient, opts.container)
+	_, err = inspectContainerAndCheckState(ctx, apiClient, containerID)
 	if err != nil {
 		return err
 	}
 
 	if c.Config.Tty && dockerCli.Out().IsTerminal() {
-		resizeTTY(ctx, dockerCli, opts.container)
+		resizeTTY(ctx, dockerCli, containerID)
 	}
 
 	streamer := hijackedIOStreamer{
@@ -147,6 +154,8 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 		return err
 	}
 
+	// request channel to wait for client
+	resultC, errC := apiClient.ContainerWait(ctx, containerID, "")
 	return getExitStatus(errC, resultC)
 }