Merge pull request #111 from keloyang/attach-restarting-check

Add a restarting check to runAttach

cli/command/container/attach.go

@@ -34,11 +34,15 @@ func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, ar
 	if c.State.Paused {
 		return nil, errors.New("You cannot attach to a paused container, unpause it first")
 	}
+	if c.State.Restarting {
+		return nil, errors.New("You cannot attach to a restarting container, wait until it is running")
+	}
+
 	return &c, nil
 }
 
 // NewAttachCommand creates a new cobra.Command for `docker attach`
-func NewAttachCommand(dockerCli *command.DockerCli) *cobra.Command {
+func NewAttachCommand(dockerCli command.Cli) *cobra.Command {
 	var opts attachOptions
 
 	cmd := &cobra.Command{
@@ -58,7 +62,7 @@ func NewAttachCommand(dockerCli *command.DockerCli) *cobra.Command {
 	return cmd
 }
 
-func runAttach(dockerCli *command.DockerCli, opts *attachOptions) error {
+func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 	ctx := context.Background()
 	client := dockerCli.Client()
 

cli/command/container/attach_test.go

@@ -0,0 +1,77 @@
+package container
+
+import (
+	"bytes"
+	"io/ioutil"
+	"testing"
+
+	"github.com/docker/cli/cli/internal/test"
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/pkg/testutil"
+	"github.com/pkg/errors"
+)
+
+func TestNewAttachCommandErrors(t *testing.T) {
+	testCases := []struct {
+		name                 string
+		args                 []string
+		expectedError        string
+		containerInspectFunc func(img string) (types.ContainerJSON, error)
+	}{
+		{
+			name:          "client-error",
+			args:          []string{"5cb5bb5e4a3b"},
+			expectedError: "something went wrong",
+			containerInspectFunc: func(containerID string) (types.ContainerJSON, error) {
+				return types.ContainerJSON{}, errors.Errorf("something went wrong")
+			},
+		},
+		{
+			name:          "client-stopped",
+			args:          []string{"5cb5bb5e4a3b"},
+			expectedError: "You cannot attach to a stopped container",
+			containerInspectFunc: func(containerID string) (types.ContainerJSON, error) {
+				c := types.ContainerJSON{}
+				c.ContainerJSONBase = &types.ContainerJSONBase{}
+				c.ContainerJSONBase.State = &types.ContainerState{Running: false}
+				return c, nil
+			},
+		},
+		{
+			name:          "client-paused",
+			args:          []string{"5cb5bb5e4a3b"},
+			expectedError: "You cannot attach to a paused container",
+			containerInspectFunc: func(containerID string) (types.ContainerJSON, error) {
+				c := types.ContainerJSON{}
+				c.ContainerJSONBase = &types.ContainerJSONBase{}
+				c.ContainerJSONBase.State = &types.ContainerState{
+					Running: true,
+					Paused:  true,
+				}
+				return c, nil
+			},
+		},
+		{
+			name:          "client-restarting",
+			args:          []string{"5cb5bb5e4a3b"},
+			expectedError: "You cannot attach to a restarting container",
+			containerInspectFunc: func(containerID string) (types.ContainerJSON, error) {
+				c := types.ContainerJSON{}
+				c.ContainerJSONBase = &types.ContainerJSONBase{}
+				c.ContainerJSONBase.State = &types.ContainerState{
+					Running:    true,
+					Paused:     false,
+					Restarting: true,
+				}
+				return c, nil
+			},
+		},
+	}
+	for _, tc := range testCases {
+		buf := new(bytes.Buffer)
+		cmd := NewAttachCommand(test.NewFakeCli(&fakeClient{containerInspectFunc: tc.containerInspectFunc}, buf))
+		cmd.SetOutput(ioutil.Discard)
+		cmd.SetArgs(tc.args)
+		testutil.ErrorContains(t, cmd.Execute(), tc.expectedError)
+	}
+}

cli/command/container/client_test.go

@@ -0,0 +1,19 @@
+package container
+
+import (
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/client"
+	"golang.org/x/net/context"
+)
+
+type fakeClient struct {
+	client.Client
+	containerInspectFunc func(string) (types.ContainerJSON, error)
+}
+
+func (cli *fakeClient) ContainerInspect(_ context.Context, containerID string) (types.ContainerJSON, error) {
+	if cli.containerInspectFunc != nil {
+		return cli.containerInspectFunc(containerID)
+	}
+	return types.ContainerJSON{}, nil
+}

cli/command/container/exec.go

@@ -33,7 +33,7 @@ func newExecOptions() *execOptions {
 }
 
 // NewExecCommand creates a new cobra.Command for `docker exec`
-func NewExecCommand(dockerCli *command.DockerCli) *cobra.Command {
+func NewExecCommand(dockerCli command.Cli) *cobra.Command {
 	options := newExecOptions()
 
 	cmd := &cobra.Command{
@@ -63,7 +63,7 @@ func NewExecCommand(dockerCli *command.DockerCli) *cobra.Command {
 }
 
 // nolint: gocyclo
-func runExec(dockerCli *command.DockerCli, options *execOptions, container string, execCmd []string) error {
+func runExec(dockerCli command.Cli, options *execOptions, container string, execCmd []string) error {
 	execConfig, err := parseExec(options, execCmd)
 	// just in case the ParseExec does not exit
 	if container == "" || err != nil {

cli/command/container/exec_test.go

@@ -1,9 +1,15 @@
 package container
 
 import (
+	"bytes"
+	"io/ioutil"
 	"testing"
 
+	"github.com/docker/cli/cli/config/configfile"
+	"github.com/docker/cli/cli/internal/test"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/pkg/testutil"
+	"github.com/pkg/errors"
 )
 
 type arguments struct {
@@ -114,3 +120,31 @@ func compareExecConfig(config1 *types.ExecConfig, config2 *types.ExecConfig) boo
 	}
 	return true
 }
+
+func TestNewExecCommandErrors(t *testing.T) {
+	testCases := []struct {
+		name                 string
+		args                 []string
+		expectedError        string
+		containerInspectFunc func(img string) (types.ContainerJSON, error)
+	}{
+		{
+			name:          "client-error",
+			args:          []string{"5cb5bb5e4a3b", "-t", "-i", "bash"},
+			expectedError: "something went wrong",
+			containerInspectFunc: func(containerID string) (types.ContainerJSON, error) {
+				return types.ContainerJSON{}, errors.Errorf("something went wrong")
+			},
+		},
+	}
+	for _, tc := range testCases {
+		buf := new(bytes.Buffer)
+		conf := configfile.ConfigFile{}
+		cli := test.NewFakeCli(&fakeClient{containerInspectFunc: tc.containerInspectFunc}, buf)
+		cli.SetConfigfile(&conf)
+		cmd := NewExecCommand(cli)
+		cmd.SetOutput(ioutil.Discard)
+		cmd.SetArgs(tc.args)
+		testutil.ErrorContains(t, cmd.Execute(), tc.expectedError)
+	}
+}

cli/command/container/tty.go

@@ -39,7 +39,7 @@ func resizeTtyTo(ctx context.Context, client client.ContainerAPIClient, id strin
 }
 
 // MonitorTtySize updates the container tty size when the terminal tty changes size
-func MonitorTtySize(ctx context.Context, cli *command.DockerCli, id string, isExec bool) error {
+func MonitorTtySize(ctx context.Context, cli command.Cli, id string, isExec bool) error {
 	resizeTty := func() {
 		height, width := cli.Out().GetTtySize()
 		resizeTtyTo(ctx, cli.Client(), id, height, width, isExec)
@@ -74,7 +74,7 @@ func MonitorTtySize(ctx context.Context, cli *command.DockerCli, id string, isEx
 }
 
 // ForwardAllSignals forwards signals to the container
-func ForwardAllSignals(ctx context.Context, cli *command.DockerCli, cid string) chan os.Signal {
+func ForwardAllSignals(ctx context.Context, cli command.Cli, cid string) chan os.Signal {
 	sigc := make(chan os.Signal, 128)
 	signal.CatchAll(sigc)
 	go func() {

cli/command/container/utils.go

@@ -127,7 +127,7 @@ func legacyWaitExitOrRemoved(ctx context.Context, dockerCli *command.DockerCli,
 
 // getExitCode performs an inspect on the container. It returns
 // the running state and the exit code.
-func getExitCode(ctx context.Context, dockerCli *command.DockerCli, containerID string) (bool, int, error) {
+func getExitCode(ctx context.Context, dockerCli command.Cli, containerID string) (bool, int, error) {
 	c, err := dockerCli.Client().ContainerInspect(ctx, containerID)
 	if err != nil {
 		// If we can't connect, then the daemon probably died.