Merge pull request moby#51289 from vvoland/client-container-exec2

client/container_exec: Separate structs for Start and Attach

Author: thaJeztah

client/container_exec.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 
+	"github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/container"
 )
 
@@ -61,36 +62,45 @@ func (cli *Client) ExecCreate(ctx context.Context, containerID string, options E
 	return ExecCreateResult{ID: response.ID}, err
 }
 
-type execStartAttachOptions struct {
+type ConsoleSize struct {
+	Height, Width uint
+}
+
+// ExecStartOptions holds options for starting a container exec.
+type ExecStartOptions struct {
 	// ExecStart will first check if it's detached
 	Detach bool
 	// Check if there's a tty
-	Tty bool
-	// Terminal size [height, width], unused if Tty == false
-	ConsoleSize *[2]uint `json:",omitempty"`
+	TTY bool
+	// Terminal size [height, width], unused if TTY == false
+	ConsoleSize ConsoleSize `json:",omitzero"`
 }
 
-// ExecStartOptions holds options for starting a container exec.
-type ExecStartOptions execStartAttachOptions
-
 // ExecStartResult holds the result of starting a container exec.
 type ExecStartResult struct {
 }
 
 // ExecStart starts an exec process already created in the docker host.
 func (cli *Client) ExecStart(ctx context.Context, execID string, options ExecStartOptions) (ExecStartResult, error) {
 	req := container.ExecStartRequest{
-		Detach:      options.Detach,
-		Tty:         options.Tty,
-		ConsoleSize: options.ConsoleSize,
+		Detach: options.Detach,
+		Tty:    options.TTY,
+	}
+	if err := applyConsoleSize(&req, &options.ConsoleSize); err != nil {
+		return ExecStartResult{}, err
 	}
 	resp, err := cli.post(ctx, "/exec/"+execID+"/start", nil, req, nil)
 	defer ensureReaderClosed(resp)
 	return ExecStartResult{}, err
 }
 
 // ExecAttachOptions holds options for attaching to a container exec.
-type ExecAttachOptions execStartAttachOptions
+type ExecAttachOptions struct {
+	// Check if there's a tty
+	TTY bool
+	// Terminal size [height, width], unused if TTY == false
+	ConsoleSize ConsoleSize `json:",omitzero"`
+}
 
 // ExecAttachResult holds the result of attaching to a container exec.
 type ExecAttachResult struct {
@@ -117,16 +127,28 @@ type ExecAttachResult struct {
 // [stdcopy.StdCopy]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#StdCopy
 func (cli *Client) ExecAttach(ctx context.Context, execID string, options ExecAttachOptions) (ExecAttachResult, error) {
 	req := container.ExecStartRequest{
-		Detach:      options.Detach,
-		Tty:         options.Tty,
-		ConsoleSize: options.ConsoleSize,
+		Detach: false,
+		Tty:    options.TTY,
+	}
+	if err := applyConsoleSize(&req, &options.ConsoleSize); err != nil {
+		return ExecAttachResult{}, err
 	}
 	response, err := cli.postHijacked(ctx, "/exec/"+execID+"/start", nil, req, http.Header{
 		"Content-Type": {"application/json"},
 	})
 	return ExecAttachResult{HijackedResponse: response}, err
 }
 
+func applyConsoleSize(req *container.ExecStartRequest, consoleSize *ConsoleSize) error {
+	if consoleSize.Height != 0 || consoleSize.Width != 0 {
+		if !req.Tty {
+			return errdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled")
+		}
+		req.ConsoleSize = &[2]uint{consoleSize.Height, consoleSize.Width}
+	}
+	return nil
+}
+
 // ExecInspectOptions holds options for inspecting a container exec.
 type ExecInspectOptions struct {
 }

client/container_exec_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/containerd/errdefs"
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/container"
 	"gotest.tools/v3/assert"
@@ -109,11 +110,28 @@ func TestExecStart(t *testing.T) {
 
 	_, err = client.ExecStart(context.Background(), "exec_id", ExecStartOptions{
 		Detach: true,
-		Tty:    false,
+		TTY:    false,
 	})
 	assert.NilError(t, err)
 }
 
+func TestExecStartConsoleSize(t *testing.T) {
+	client, err := NewClientWithOpts(
+		WithMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, nil
+		}),
+	)
+	assert.NilError(t, err)
+
+	_, err = client.ExecStart(context.Background(), "exec_id", ExecStartOptions{
+		Detach:      true,
+		TTY:         false,
+		ConsoleSize: ConsoleSize{Height: 100, Width: 100},
+	})
+	assert.Check(t, is.ErrorType(err, errdefs.IsInvalidArgument))
+	assert.Check(t, is.ErrorContains(err, "console size is only supported when TTY is enabled"))
+}
+
 func TestExecInspectError(t *testing.T) {
 	client, err := NewClientWithOpts(
 		WithMockClient(errorMock(http.StatusInternalServerError, "Server error")),

integration/system/event_test.go

@@ -36,7 +36,7 @@ func TestEventsExecDie(t *testing.T) {
 
 	_, err = apiClient.ExecStart(ctx, res.ID, client.ExecStartOptions{
 		Detach: true,
-		Tty:    false,
+		TTY:    false,
 	})
 	assert.NilError(t, err)