diff --git a/components/cli/cli/command/container/attach.go b/components/cli/cli/command/container/attach.go index 6e4628d95d..dca84d5ddb 100644 --- a/components/cli/cli/command/container/attach.go +++ b/components/cli/cli/command/container/attach.go @@ -1,12 +1,14 @@ package container import ( + "fmt" "io" "net/http/httputil" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" "github.com/docker/docker/pkg/signal" "github.com/pkg/errors" @@ -66,6 +68,9 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { ctx := context.Background() client := dockerCli.Client() + // request channel to wait for client + resultC, errC := client.ContainerWait(ctx, opts.container, "") + c, err := inspectContainerAndCheckState(ctx, client, opts.container) if err != nil { return err @@ -140,7 +145,24 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { if errAttach != nil { return errAttach } - return getExitStatus(ctx, dockerCli.Client(), opts.container) + + return getExitStatus(errC, resultC) +} + +func getExitStatus(errC <-chan error, resultC <-chan container.ContainerWaitOKBody) error { + select { + case result := <-resultC: + if result.Error != nil { + return fmt.Errorf(result.Error.Message) + } + if result.StatusCode != 0 { + return cli.StatusError{StatusCode: int(result.StatusCode)} + } + case err := <-errC: + return err + } + + return nil } func resizeTTY(ctx context.Context, dockerCli command.Cli, containerID string) { @@ -157,19 +179,3 @@ func resizeTTY(ctx context.Context, dockerCli command.Cli, containerID string) { logrus.Debugf("Error monitoring TTY size: %s", err) } } - -func getExitStatus(ctx context.Context, apiclient client.ContainerAPIClient, containerID string) error { - container, err := apiclient.ContainerInspect(ctx, containerID) - if err != nil { - // If we can't connect, then the daemon probably died. - if !client.IsErrConnectionFailed(err) { - return err - } - return cli.StatusError{StatusCode: -1} - } - status := container.State.ExitCode - if status != 0 { - return cli.StatusError{StatusCode: status} - } - return nil -} diff --git a/components/cli/cli/command/container/attach_test.go b/components/cli/cli/command/container/attach_test.go index 1ca775c6d5..1c305aa7a1 100644 --- a/components/cli/cli/command/container/attach_test.go +++ b/components/cli/cli/command/container/attach_test.go @@ -1,6 +1,7 @@ package container import ( + "fmt" "io/ioutil" "testing" @@ -8,9 +9,9 @@ import ( "github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test/testutil" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" "github.com/pkg/errors" "github.com/stretchr/testify/assert" - "golang.org/x/net/context" ) func TestNewAttachCommandErrors(t *testing.T) { @@ -78,40 +79,50 @@ func TestNewAttachCommandErrors(t *testing.T) { } func TestGetExitStatus(t *testing.T) { - containerID := "the exec id" - expecatedErr := errors.New("unexpected error") + var ( + expectedErr = fmt.Errorf("unexpected error") + errC = make(chan error, 1) + resultC = make(chan container.ContainerWaitOKBody, 1) + ) testcases := []struct { - inspectError error - exitCode int + result *container.ContainerWaitOKBody + err error expectedError error }{ { - inspectError: nil, - exitCode: 0, + result: &container.ContainerWaitOKBody{ + StatusCode: 0, + }, }, { - inspectError: expecatedErr, - expectedError: expecatedErr, + err: expectedErr, + expectedError: expectedErr, }, { - exitCode: 15, + result: &container.ContainerWaitOKBody{ + Error: &container.ContainerWaitOKBodyError{ + expectedErr.Error(), + }, + }, + expectedError: expectedErr, + }, + { + result: &container.ContainerWaitOKBody{ + StatusCode: 15, + }, expectedError: cli.StatusError{StatusCode: 15}, }, } for _, testcase := range testcases { - client := &fakeClient{ - inspectFunc: func(id string) (types.ContainerJSON, error) { - assert.Equal(t, containerID, id) - return types.ContainerJSON{ - ContainerJSONBase: &types.ContainerJSONBase{ - State: &types.ContainerState{ExitCode: testcase.exitCode}, - }, - }, testcase.inspectError - }, + if testcase.err != nil { + errC <- testcase.err } - err := getExitStatus(context.Background(), client, containerID) + if testcase.result != nil { + resultC <- *testcase.result + } + err := getExitStatus(errC, resultC) assert.Equal(t, testcase.expectedError, err) } } diff --git a/components/cli/cli/command/service/client_test.go b/components/cli/cli/command/service/client_test.go index 896892b943..b1349844c9 100644 --- a/components/cli/cli/command/service/client_test.go +++ b/components/cli/cli/command/service/client_test.go @@ -16,6 +16,7 @@ type fakeClient struct { serviceListFunc func(context.Context, types.ServiceListOptions) ([]swarm.Service, error) taskListFunc func(context.Context, types.TaskListOptions) ([]swarm.Task, error) infoFunc func(ctx context.Context) (types.Info, error) + networkInspectFunc func(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) } func (f *fakeClient) NodeList(ctx context.Context, options types.NodeListOptions) ([]swarm.Node, error) { @@ -60,6 +61,13 @@ func (f *fakeClient) Info(ctx context.Context) (types.Info, error) { return f.infoFunc(ctx) } +func (f *fakeClient) NetworkInspect(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) { + if f.networkInspectFunc != nil { + return f.networkInspectFunc(ctx, networkID, options) + } + return types.NetworkResource{}, nil +} + func newService(id string, name string) swarm.Service { return swarm.Service{ ID: id, diff --git a/components/cli/cli/command/service/opts.go b/components/cli/cli/command/service/opts.go index b57c5c8c4e..19915050b9 100644 --- a/components/cli/cli/command/service/opts.go +++ b/components/cli/cli/command/service/opts.go @@ -353,22 +353,21 @@ func (c *credentialSpecOpt) Value() *swarm.CredentialSpec { return c.value } -func convertNetworks(ctx context.Context, apiClient client.NetworkAPIClient, networks opts.NetworkOpt) ([]swarm.NetworkAttachmentConfig, error) { +func resolveNetworkID(ctx context.Context, apiClient client.NetworkAPIClient, networkIDOrName string) (string, error) { + nw, err := apiClient.NetworkInspect(ctx, networkIDOrName, types.NetworkInspectOptions{Scope: "swarm"}) + return nw.ID, err +} + +func convertNetworks(networks opts.NetworkOpt) []swarm.NetworkAttachmentConfig { var netAttach []swarm.NetworkAttachmentConfig for _, net := range networks.Value() { - networkIDOrName := net.Target - _, err := apiClient.NetworkInspect(ctx, networkIDOrName, types.NetworkInspectOptions{Scope: "swarm"}) - if err != nil { - return nil, err - } netAttach = append(netAttach, swarm.NetworkAttachmentConfig{ Target: net.Target, Aliases: net.Aliases, DriverOpts: net.DriverOpts, }) } - sort.Sort(byNetworkTarget(netAttach)) - return netAttach, nil + return netAttach } type endpointOptions struct { @@ -590,10 +589,15 @@ func (options *serviceOptions) ToService(ctx context.Context, apiClient client.N return service, err } - networks, err := convertNetworks(ctx, apiClient, options.networks) - if err != nil { - return service, err + networks := convertNetworks(options.networks) + for i, net := range networks { + nwID, err := resolveNetworkID(ctx, apiClient, net.Target) + if err != nil { + return service, err + } + networks[i].Target = nwID } + sort.Sort(byNetworkTarget(networks)) resources, err := options.resources.ToResourceRequirements() if err != nil { diff --git a/components/cli/cli/command/service/opts_test.go b/components/cli/cli/command/service/opts_test.go index e373c6479d..f68cbb3c5a 100644 --- a/components/cli/cli/command/service/opts_test.go +++ b/components/cli/cli/command/service/opts_test.go @@ -1,12 +1,17 @@ package service import ( + "context" + "fmt" "testing" "time" "github.com/docker/cli/opts" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/swarm" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMemBytesString(t *testing.T) { @@ -123,3 +128,37 @@ func TestResourceOptionsToResourceRequirements(t *testing.T) { } } + +func TestToServiceNetwork(t *testing.T) { + nws := []types.NetworkResource{ + {Name: "aaa-network", ID: "id555"}, + {Name: "mmm-network", ID: "id999"}, + {Name: "zzz-network", ID: "id111"}, + } + + client := &fakeClient{ + networkInspectFunc: func(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) { + for _, network := range nws { + if network.ID == networkID || network.Name == networkID { + return network, nil + } + } + return types.NetworkResource{}, fmt.Errorf("network not found: %s", networkID) + }, + } + + nwo := opts.NetworkOpt{} + nwo.Set("zzz-network") + nwo.Set("mmm-network") + nwo.Set("aaa-network") + + o := newServiceOptions() + o.mode = "replicated" + o.networks = nwo + + ctx := context.Background() + flags := newCreateCommand(nil).Flags() + service, err := o.ToService(ctx, client, flags) + require.NoError(t, err) + assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id111"}, {Target: "id555"}, {Target: "id999"}}, service.TaskTemplate.Networks) +} diff --git a/components/cli/cli/command/service/update.go b/components/cli/cli/command/service/update.go index d657871ff2..ca9752a656 100644 --- a/components/cli/cli/command/service/update.go +++ b/components/cli/cli/command/service/update.go @@ -1119,14 +1119,16 @@ func updateNetworks(ctx context.Context, apiClient client.NetworkAPIClient, flag if flags.Changed(flagNetworkAdd) { values := flags.Lookup(flagNetworkAdd).Value.(*opts.NetworkOpt) - networks, err := convertNetworks(ctx, apiClient, *values) - if err != nil { - return err - } + networks := convertNetworks(*values) for _, network := range networks { - if _, exists := existingNetworks[network.Target]; exists { + nwID, err := resolveNetworkID(ctx, apiClient, network.Target) + if err != nil { + return err + } + if _, exists := existingNetworks[nwID]; exists { return errors.Errorf("service is already attached to network %s", network.Target) } + network.Target = nwID newNetworks = append(newNetworks, network) existingNetworks[network.Target] = struct{}{} } diff --git a/components/cli/cli/command/service/update_test.go b/components/cli/cli/command/service/update_test.go index ba55269cb0..e7e428192f 100644 --- a/components/cli/cli/command/service/update_test.go +++ b/components/cli/cli/command/service/update_test.go @@ -1,6 +1,7 @@ package service import ( + "fmt" "reflect" "sort" "testing" @@ -586,3 +587,69 @@ func TestRemoveGenericResources(t *testing.T) { assert.NoError(t, removeGenericResources(flags, task)) assert.Len(t, task.Resources.Reservations.GenericResources, 1) } + +func TestUpdateNetworks(t *testing.T) { + ctx := context.Background() + nws := []types.NetworkResource{ + {Name: "aaa-network", ID: "id555"}, + {Name: "mmm-network", ID: "id999"}, + {Name: "zzz-network", ID: "id111"}, + } + + client := &fakeClient{ + networkInspectFunc: func(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) { + for _, network := range nws { + if network.ID == networkID || network.Name == networkID { + return network, nil + } + } + return types.NetworkResource{}, fmt.Errorf("network not found: %s", networkID) + }, + } + + svc := swarm.ServiceSpec{ + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: &swarm.ContainerSpec{}, + Networks: []swarm.NetworkAttachmentConfig{ + {Target: "id999"}, + }, + }, + } + + flags := newUpdateCommand(nil).Flags() + err := flags.Set(flagNetworkAdd, "aaa-network") + require.NoError(t, err) + err = updateService(ctx, client, flags, &svc) + require.NoError(t, err) + assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}, {Target: "id999"}}, svc.TaskTemplate.Networks) + + flags = newUpdateCommand(nil).Flags() + err = flags.Set(flagNetworkAdd, "aaa-network") + require.NoError(t, err) + err = updateService(ctx, client, flags, &svc) + assert.EqualError(t, err, "service is already attached to network aaa-network") + assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}, {Target: "id999"}}, svc.TaskTemplate.Networks) + + flags = newUpdateCommand(nil).Flags() + err = flags.Set(flagNetworkAdd, "id555") + require.NoError(t, err) + err = updateService(ctx, client, flags, &svc) + assert.EqualError(t, err, "service is already attached to network id555") + assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}, {Target: "id999"}}, svc.TaskTemplate.Networks) + + flags = newUpdateCommand(nil).Flags() + err = flags.Set(flagNetworkRemove, "id999") + require.NoError(t, err) + err = updateService(ctx, client, flags, &svc) + assert.NoError(t, err) + assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}}, svc.TaskTemplate.Networks) + + flags = newUpdateCommand(nil).Flags() + err = flags.Set(flagNetworkAdd, "mmm-network") + require.NoError(t, err) + err = flags.Set(flagNetworkRemove, "aaa-network") + require.NoError(t, err) + err = updateService(ctx, client, flags, &svc) + assert.NoError(t, err) + assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id999"}}, svc.TaskTemplate.Networks) +} diff --git a/components/cli/docker.Makefile b/components/cli/docker.Makefile index a881593dda..a94b81a81d 100644 --- a/components/cli/docker.Makefile +++ b/components/cli/docker.Makefile @@ -5,6 +5,7 @@ # DEV_DOCKER_IMAGE_NAME = docker-cli-dev$(IMAGE_TAG) +BINARY_NATIVE_IMAGE_NAME = docker-cli-native$(IMAGE_TAG) LINTER_IMAGE_NAME = docker-cli-lint$(IMAGE_TAG) CROSS_IMAGE_NAME = docker-cli-cross$(IMAGE_TAG) VALIDATE_IMAGE_NAME = docker-cli-shell-validate$(IMAGE_TAG) @@ -30,12 +31,18 @@ build_cross_image: build_shell_validate_image: docker build -t $(VALIDATE_IMAGE_NAME) -f ./dockerfiles/Dockerfile.shellcheck . +.PHONY: build_binary_native_image +build_binary_native_image: + docker build -t $(BINARY_NATIVE_IMAGE_NAME) -f ./dockerfiles/Dockerfile.binary-native . + + # build executable using a container -binary: build_docker_image - docker run --rm $(ENVVARS) $(MOUNTS) $(DEV_DOCKER_IMAGE_NAME) make binary +binary: build_binary_native_image + docker run --rm $(ENVVARS) $(MOUNTS) $(BINARY_NATIVE_IMAGE_NAME) build: binary + # clean build artifacts using a container .PHONY: clean clean: build_docker_image diff --git a/components/cli/dockerfiles/Dockerfile.binary-native b/components/cli/dockerfiles/Dockerfile.binary-native new file mode 100644 index 0000000000..e130e8240e --- /dev/null +++ b/components/cli/dockerfiles/Dockerfile.binary-native @@ -0,0 +1,8 @@ +FROM golang:1.9.2-alpine3.6 + +RUN apk add -U git bash coreutils gcc musl-dev + +ENV CGO_ENABLED=0 \ + DISABLE_WARN_OUTSIDE_CONTAINER=1 +WORKDIR /go/src/github.com/docker/cli +CMD ./scripts/build/binary diff --git a/components/cli/e2e/container/attach_test.go b/components/cli/e2e/container/attach_test.go new file mode 100644 index 0000000000..3e7d0ed244 --- /dev/null +++ b/components/cli/e2e/container/attach_test.go @@ -0,0 +1,30 @@ +package container + +import ( + "strings" + "testing" + + "github.com/docker/cli/e2e/internal/fixtures" + "github.com/gotestyourself/gotestyourself/icmd" +) + +func TestAttachExitCode(t *testing.T) { + containerID := runBackgroundContainsWithExitCode(t, 21) + + result := icmd.RunCmd( + icmd.Command("docker", "attach", containerID), + withStdinNewline) + + result.Assert(t, icmd.Expected{ExitCode: 21}) +} + +func runBackgroundContainsWithExitCode(t *testing.T, exitcode int) string { + result := icmd.RunCmd(shell(t, + "docker run -d -i --rm %s sh -c 'read; exit %d'", fixtures.AlpineImage, exitcode)) + result.Assert(t, icmd.Success) + return strings.TrimSpace(result.Stdout()) +} + +func withStdinNewline(cmd *icmd.Cmd) { + cmd.Stdin = strings.NewReader("\n") +}