diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go index dce6432846be..372fcbb34846 100644 --- a/cli/command/container/attach.go +++ b/cli/command/container/attach.go @@ -120,18 +120,7 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { } if c.Config.Tty && dockerCli.Out().IsTerminal() { - height, width := dockerCli.Out().GetTtySize() - // To handle the case where a user repeatedly attaches/detaches without resizing their - // terminal, the only way to get the shell prompt to display for attaches 2+ is to artificially - // resize it, then go back to normal. Without this, every attach after the first will - // require the user to manually resize or hit enter. - resizeTtyTo(ctx, client, opts.container, height+1, width+1, false) - - // After the above resizing occurs, the call to MonitorTtySize below will handle resetting back - // to the actual size. - if err := MonitorTtySize(ctx, dockerCli, opts.container, false); err != nil { - logrus.Debugf("Error monitoring TTY size: %s", err) - } + resizeTTY(ctx, dockerCli, opts.container) } streamer := hijackedIOStreamer{ @@ -151,14 +140,36 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { if errAttach != nil { return errAttach } + return getExitStatus(ctx, dockerCli.Client(), opts.container) +} + +func resizeTTY(ctx context.Context, dockerCli command.Cli, containerID string) { + height, width := dockerCli.Out().GetTtySize() + // To handle the case where a user repeatedly attaches/detaches without resizing their + // terminal, the only way to get the shell prompt to display for attaches 2+ is to artificially + // resize it, then go back to normal. Without this, every attach after the first will + // require the user to manually resize or hit enter. + resizeTtyTo(ctx, dockerCli.Client(), containerID, height+1, width+1, false) - _, status, err := getExitCode(ctx, dockerCli, opts.container) + // After the above resizing occurs, the call to MonitorTtySize below will handle resetting back + // to the actual size. + if err := MonitorTtySize(ctx, dockerCli, containerID, false); err != nil { + 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 { - return err + // 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/cli/command/container/client_test.go b/cli/command/container/client_test.go index 1e2d5d9cf876..caabe43880f9 100644 --- a/cli/command/container/client_test.go +++ b/cli/command/container/client_test.go @@ -1,7 +1,13 @@ package container import ( + "io" + "time" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" "golang.org/x/net/context" ) @@ -9,6 +15,7 @@ import ( type fakeClient struct { client.Client containerInspectFunc func(string) (types.ContainerJSON, error) + execInspectFunc func(ctx context.Context, execID string) (types.ContainerExecInspect, error) } func (cli *fakeClient) ContainerInspect(_ context.Context, containerID string) (types.ContainerJSON, error) { @@ -17,3 +24,132 @@ func (cli *fakeClient) ContainerInspect(_ context.Context, containerID string) ( } return types.ContainerJSON{}, nil } + +func (cli *fakeClient) ContainerAttach(ctx context.Context, container string, options types.ContainerAttachOptions) (types.HijackedResponse, error) { + return types.HijackedResponse{}, nil +} + +func (cli *fakeClient) ContainerCommit(ctx context.Context, container string, options types.ContainerCommitOptions) (types.IDResponse, error) { + return types.IDResponse{}, nil +} + +func (cli *fakeClient) ContainerCreate( + ctx context.Context, + config *container.Config, + hostConfig *container.HostConfig, + networkingConfig *network.NetworkingConfig, + containerName string, +) (container.ContainerCreateCreatedBody, error) { + return container.ContainerCreateCreatedBody{}, nil +} + +func (cli *fakeClient) ContainerDiff(ctx context.Context, container string) ([]container.ContainerChangeResponseItem, error) { + return nil, nil +} + +func (cli *fakeClient) ContainerExecAttach(ctx context.Context, execID string, config types.ExecConfig) (types.HijackedResponse, error) { + return types.HijackedResponse{}, nil +} + +func (cli *fakeClient) ContainerExecCreate(ctx context.Context, container string, config types.ExecConfig) (types.IDResponse, error) { + return types.IDResponse{}, nil +} + +func (cli *fakeClient) ContainerExecInspect(ctx context.Context, execID string) (types.ContainerExecInspect, error) { + if cli.execInspectFunc != nil { + return cli.execInspectFunc(ctx, execID) + } + return types.ContainerExecInspect{}, nil +} + +func (cli *fakeClient) ContainerExecResize(ctx context.Context, execID string, options types.ResizeOptions) error { + return nil +} + +func (cli *fakeClient) ContainerExecStart(ctx context.Context, execID string, config types.ExecStartCheck) error { + return nil +} + +func (cli *fakeClient) ContainerExport(ctx context.Context, container string) (io.ReadCloser, error) { + return nil, nil +} + +func (cli *fakeClient) ContainerInspectWithRaw(ctx context.Context, container string, getSize bool) (types.ContainerJSON, []byte, error) { + return types.ContainerJSON{}, nil, nil +} + +func (cli *fakeClient) ContainerKill(ctx context.Context, container, signal string) error { + return nil +} + +func (cli *fakeClient) ContainerList(ctx context.Context, options types.ContainerListOptions) ([]types.Container, error) { + return nil, nil +} + +func (cli *fakeClient) ContainerLogs(ctx context.Context, container string, options types.ContainerLogsOptions) (io.ReadCloser, error) { + return nil, nil +} + +func (cli *fakeClient) ContainerPause(ctx context.Context, container string) error { + return nil +} + +func (cli *fakeClient) ContainerRemove(ctx context.Context, container string, options types.ContainerRemoveOptions) error { + return nil +} + +func (cli *fakeClient) ContainerRename(ctx context.Context, container, newContainerName string) error { + return nil +} + +func (cli *fakeClient) ContainerResize(ctx context.Context, container string, options types.ResizeOptions) error { + return nil +} + +func (cli *fakeClient) ContainerRestart(ctx context.Context, container string, timeout *time.Duration) error { + return nil +} + +func (cli *fakeClient) ContainerStatPath(ctx context.Context, container, path string) (types.ContainerPathStat, error) { + return types.ContainerPathStat{}, nil +} + +func (cli *fakeClient) ContainerStats(ctx context.Context, container string, stream bool) (types.ContainerStats, error) { + return types.ContainerStats{}, nil +} + +func (cli *fakeClient) ContainerStart(ctx context.Context, container string, options types.ContainerStartOptions) error { + return nil +} + +func (cli *fakeClient) ContainerStop(ctx context.Context, container string, timeout *time.Duration) error { + return nil +} + +func (cli *fakeClient) ContainerTop(ctx context.Context, containerID string, arguments []string) (container.ContainerTopOKBody, error) { + return container.ContainerTopOKBody{}, nil +} + +func (cli *fakeClient) ContainerUnpause(ctx context.Context, container string) error { + return nil +} + +func (cli *fakeClient) ContainerUpdate(ctx context.Context, containerID string, updateConfig container.UpdateConfig) (container.ContainerUpdateOKBody, error) { + return container.ContainerUpdateOKBody{}, nil +} + +func (cli *fakeClient) ContainerWait(ctx context.Context, container string, condition container.WaitCondition) (<-chan container.ContainerWaitOKBody, <-chan error) { + return nil, nil +} + +func (cli *fakeClient) CopyFromContainer(ctx context.Context, container, srcPath string) (io.ReadCloser, types.ContainerPathStat, error) { + return nil, types.ContainerPathStat{}, nil +} + +func (cli *fakeClient) CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error { + return nil +} + +func (cli *fakeClient) ContainersPrune(ctx context.Context, pruneFilters filters.Args) (types.ContainersPruneReport, error) { + return types.ContainersPruneReport{}, nil +} diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 2795e4fed703..97bc1863f026 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -113,6 +113,9 @@ type cidFile struct { } func (cid *cidFile) Close() error { + if cid.file == nil { + return nil + } cid.file.Close() if cid.written { @@ -126,6 +129,9 @@ func (cid *cidFile) Close() error { } func (cid *cidFile) Write(id string) error { + if cid.file == nil { + return nil + } if _, err := cid.file.Write([]byte(id)); err != nil { return errors.Errorf("Failed to write the container ID to the file: %s", err) } @@ -134,6 +140,9 @@ func (cid *cidFile) Write(id string) error { } func newCIDFile(path string) (*cidFile, error) { + if path == "" { + return &cidFile{}, nil + } if _, err := os.Stat(path); err == nil { return nil, errors.Errorf("Container ID file found, make sure the other container isn't running or delete %s", path) } @@ -153,19 +162,15 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig stderr := dockerCli.Err() var ( - containerIDFile *cidFile - trustedRef reference.Canonical - namedRef reference.Named + trustedRef reference.Canonical + namedRef reference.Named ) - cidfile := hostConfig.ContainerIDFile - if cidfile != "" { - var err error - if containerIDFile, err = newCIDFile(cidfile); err != nil { - return nil, err - } - defer containerIDFile.Close() + containerIDFile, err := newCIDFile(hostConfig.ContainerIDFile) + if err != nil { + return nil, err } + defer containerIDFile.Close() ref, err := reference.ParseAnyReference(config.Image) if err != nil { @@ -207,18 +212,13 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig if retryErr != nil { return nil, retryErr } - } else { - return nil, err } + return nil, err } for _, warning := range response.Warnings { fmt.Fprintf(stderr, "WARNING: %s\n", warning) } - if containerIDFile != nil { - if err = containerIDFile.Write(response.ID); err != nil { - return nil, err - } - } - return &response, nil + err = containerIDFile.Write(response.ID) + return &response, err } diff --git a/cli/command/container/exec.go b/cli/command/container/exec.go index bbcd34ae0cfd..115dc1d73fb8 100644 --- a/cli/command/container/exec.go +++ b/cli/command/container/exec.go @@ -7,6 +7,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/opts" "github.com/docker/docker/api/types" apiclient "github.com/docker/docker/client" @@ -62,21 +63,12 @@ func NewExecCommand(dockerCli command.Cli) *cobra.Command { return cmd } -// nolint: gocyclo 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 { - return cli.StatusError{StatusCode: 1} + execConfig := parseExec(options, dockerCli.ConfigFile(), execCmd) + if container == "" { + return cli.StatusError{StatusCode: 1, Status: "No container id or name"} } - if options.detachKeys != "" { - dockerCli.ConfigFile().DetachKeys = options.detachKeys - } - - // Send client escape keys - execConfig.DetachKeys = dockerCli.ConfigFile().DetachKeys - ctx := context.Background() client := dockerCli.Client() @@ -104,16 +96,17 @@ func runExec(dockerCli command.Cli, options *execOptions, container string, exec return nil } - // Temp struct for execStart so that we don't need to transfer all the execConfig. if execConfig.Detach { execStartCheck := types.ExecStartCheck{ Detach: execConfig.Detach, Tty: execConfig.Tty, } - return client.ContainerExecStart(ctx, execID, execStartCheck) } + return interactiveExec(ctx, dockerCli, execConfig, execID) +} +func interactiveExec(ctx context.Context, dockerCli command.Cli, execConfig *types.ExecConfig, execID string) error { // Interactive exec requested. var ( out, stderr io.Writer @@ -135,6 +128,7 @@ func runExec(dockerCli command.Cli, options *execOptions, container string, exec } } + client := dockerCli.Client() resp, err := client.ContainerExecAttach(ctx, execID, *execConfig) if err != nil { return err @@ -165,36 +159,28 @@ func runExec(dockerCli command.Cli, options *execOptions, container string, exec return err } - var status int - if _, status, err = getExecExitCode(ctx, client, execID); err != nil { - return err - } - - if status != 0 { - return cli.StatusError{StatusCode: status} - } - - return nil + return getExecExitStatus(ctx, client, execID) } -// getExecExitCode perform an inspect on the exec command. It returns -// the running state and the exit code. -func getExecExitCode(ctx context.Context, client apiclient.ContainerAPIClient, execID string) (bool, int, error) { +func getExecExitStatus(ctx context.Context, client apiclient.ContainerAPIClient, execID string) error { resp, err := client.ContainerExecInspect(ctx, execID) if err != nil { // If we can't connect, then the daemon probably died. if !apiclient.IsErrConnectionFailed(err) { - return false, -1, err + return err } - return false, -1, nil + return cli.StatusError{StatusCode: -1} } - - return resp.Running, resp.ExitCode, nil + status := resp.ExitCode + if status != 0 { + return cli.StatusError{StatusCode: status} + } + return nil } // parseExec parses the specified args for the specified command and generates // an ExecConfig from it. -func parseExec(opts *execOptions, execCmd []string) (*types.ExecConfig, error) { +func parseExec(opts *execOptions, configFile *configfile.ConfigFile, execCmd []string) *types.ExecConfig { execConfig := &types.ExecConfig{ User: opts.user, Privileged: opts.privileged, @@ -216,5 +202,10 @@ func parseExec(opts *execOptions, execCmd []string) (*types.ExecConfig, error) { execConfig.Env = opts.env.GetAll() } - return execConfig, nil + if opts.detachKeys != "" { + execConfig.DetachKeys = opts.detachKeys + } else { + execConfig.DetachKeys = configFile.DetachKeys + } + return execConfig } diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go index 0eed2ff0f7db..ee28eca25bdf 100644 --- a/cli/command/container/exec_test.go +++ b/cli/command/container/exec_test.go @@ -4,33 +4,39 @@ import ( "io/ioutil" "testing" + "github.com/docker/cli/cli" + "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" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/net/context" ) -type arguments struct { - options execOptions - execCmd []string -} - func TestParseExec(t *testing.T) { - valids := map[*arguments]*types.ExecConfig{ + testcases := []struct { + options execOptions + execCmd []string + configFile configfile.ConfigFile + expected types.ExecConfig + }{ { execCmd: []string{"command"}, - }: { - Cmd: []string{"command"}, - AttachStdout: true, - AttachStderr: true, + expected: types.ExecConfig{ + Cmd: []string{"command"}, + AttachStdout: true, + AttachStderr: true, + }, }, { execCmd: []string{"command1", "command2"}, - }: { - Cmd: []string{"command1", "command2"}, - AttachStdout: true, - AttachStderr: true, + expected: types.ExecConfig{ + Cmd: []string{"command1", "command2"}, + AttachStdout: true, + AttachStderr: true, + }, }, { options: execOptions{ @@ -39,25 +45,24 @@ func TestParseExec(t *testing.T) { user: "uid", }, execCmd: []string{"command"}, - }: { - User: "uid", - AttachStdin: true, - AttachStdout: true, - AttachStderr: true, - Tty: true, - Cmd: []string{"command"}, + expected: types.ExecConfig{ + User: "uid", + AttachStdin: true, + AttachStdout: true, + AttachStderr: true, + Tty: true, + Cmd: []string{"command"}, + }, }, { options: execOptions{ detach: true, }, execCmd: []string{"command"}, - }: { - AttachStdin: false, - AttachStdout: false, - AttachStderr: false, - Detach: true, - Cmd: []string{"command"}, + expected: types.ExecConfig{ + Detach: true, + Cmd: []string{"command"}, + }, }, { options: execOptions{ @@ -66,56 +71,82 @@ func TestParseExec(t *testing.T) { detach: true, }, execCmd: []string{"command"}, - }: { - AttachStdin: false, - AttachStdout: false, - AttachStderr: false, - Detach: true, - Tty: true, - Cmd: []string{"command"}, + expected: types.ExecConfig{ + Detach: true, + Tty: true, + Cmd: []string{"command"}, + }, + }, + { + execCmd: []string{"command"}, + options: execOptions{detach: true}, + configFile: configfile.ConfigFile{DetachKeys: "de"}, + expected: types.ExecConfig{ + Cmd: []string{"command"}, + DetachKeys: "de", + Detach: true, + }, + }, + { + execCmd: []string{"command"}, + options: execOptions{detach: true, detachKeys: "ab"}, + configFile: configfile.ConfigFile{DetachKeys: "de"}, + expected: types.ExecConfig{ + Cmd: []string{"command"}, + DetachKeys: "ab", + Detach: true, + }, }, } - for valid, expectedExecConfig := range valids { - execConfig, err := parseExec(&valid.options, valid.execCmd) - require.NoError(t, err) - if !compareExecConfig(expectedExecConfig, execConfig) { - t.Fatalf("Expected [%v] for %v, got [%v]", expectedExecConfig, valid, execConfig) - } + for _, testcase := range testcases { + execConfig := parseExec(&testcase.options, &testcase.configFile, testcase.execCmd) + assert.Equal(t, testcase.expected, *execConfig) } } -func compareExecConfig(config1 *types.ExecConfig, config2 *types.ExecConfig) bool { - if config1.AttachStderr != config2.AttachStderr { - return false - } - if config1.AttachStdin != config2.AttachStdin { - return false - } - if config1.AttachStdout != config2.AttachStdout { - return false - } - if config1.Detach != config2.Detach { - return false - } - if config1.Privileged != config2.Privileged { - return false - } - if config1.Tty != config2.Tty { - return false - } - if config1.User != config2.User { - return false - } - if len(config1.Cmd) != len(config2.Cmd) { - return false +func TestRunExec(t *testing.T) { + client := &fakeClient{} + cli := test.NewFakeCli(client) + options := &execOptions{detach: true} + + err := runExec(cli, options, "cid", []string{"bash"}) + require.NoError(t, err) +} + +func TestGetExecExitStatus(t *testing.T) { + execID := "the exec id" + expecatedErr := errors.New("unexpected error") + + testcases := []struct { + inspectError error + exitCode int + expectedError error + }{ + { + inspectError: nil, + expectedError: nil, + }, + { + inspectError: expecatedErr, + expectedError: expecatedErr, + }, + { + exitCode: 15, + expectedError: cli.StatusError{StatusCode: 15}, + }, } - for index, value := range config1.Cmd { - if value != config2.Cmd[index] { - return false + + for _, testcase := range testcases { + client := &fakeClient{ + execInspectFunc: func(ctx context.Context, id string) (types.ContainerExecInspect, error) { + assert.Equal(t, execID, id) + return types.ContainerExecInspect{ExitCode: testcase.exitCode}, testcase.inspectError + }, } + err := getExecExitStatus(context.Background(), client, execID) + assert.Equal(t, testcase.expectedError, err) } - return true } func TestNewExecCommandErrors(t *testing.T) { diff --git a/cli/command/container/stats_helpers.go b/cli/command/container/stats_helpers.go index eb12cd0dec47..3b4bd66bba3f 100644 --- a/cli/command/container/stats_helpers.go +++ b/cli/command/container/stats_helpers.go @@ -52,6 +52,7 @@ func (s *stats) isKnownContainer(cid string) (int, bool) { return -1, false } +// nolint: gocyclo func collect(ctx context.Context, s *formatter.ContainerStats, cli client.APIClient, streamStats bool, waitFirst *sync.WaitGroup) { logrus.Debugf("collecting stats for %s", s.Container) var ( diff --git a/cli/command/container/utils.go b/cli/command/container/utils.go index d9afe241633b..1109b66b2139 100644 --- a/cli/command/container/utils.go +++ b/cli/command/container/utils.go @@ -10,7 +10,6 @@ import ( "github.com/docker/docker/api/types/events" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/versions" - clientapi "github.com/docker/docker/client" "golang.org/x/net/context" ) @@ -125,20 +124,6 @@ func legacyWaitExitOrRemoved(ctx context.Context, dockerCli *command.DockerCli, return statusChan } -// getExitCode performs an inspect on the container. It returns -// the running state and the exit code. -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. - if !clientapi.IsErrConnectionFailed(err) { - return false, -1, err - } - return false, -1, nil - } - return c.State.Running, c.State.ExitCode, nil -} - func parallelOperation(ctx context.Context, containers []string, op func(ctx context.Context, container string) error) chan error { if len(containers) == 0 { return nil diff --git a/cli/command/formatter/custom_test.go b/cli/command/formatter/custom_test.go index da42039dca2d..a9f6ccdac9cb 100644 --- a/cli/command/formatter/custom_test.go +++ b/cli/command/formatter/custom_test.go @@ -1,9 +1,10 @@ package formatter import ( - "reflect" "strings" "testing" + + "github.com/stretchr/testify/assert" ) func compareMultipleValues(t *testing.T, value, expected string) { @@ -22,7 +23,5 @@ func compareMultipleValues(t *testing.T, value, expected string) { keyval := strings.Split(expected, "=") expMap[keyval[0]] = keyval[1] } - if !reflect.DeepEqual(expMap, entriesMap) { - t.Fatalf("Expected entries: %v, got: %v", expected, value) - } + assert.Equal(t, expMap, entriesMap) } diff --git a/cli/command/formatter/image.go b/cli/command/formatter/image.go index cbd3edad3c08..00de7f08f12a 100644 --- a/cli/command/formatter/image.go +++ b/cli/command/formatter/image.go @@ -81,9 +81,9 @@ func ImageWrite(ctx ImageContext, images []types.ImageSummary) error { func imageFormat(ctx ImageContext, images []types.ImageSummary, format func(subContext subContext) error) error { for _, image := range images { - images := []*imageContext{} + formatted := []*imageContext{} if isDangling(image) { - images = append(images, &imageContext{ + formatted = append(formatted, &imageContext{ trunc: ctx.Trunc, i: image, repo: "", @@ -91,96 +91,91 @@ func imageFormat(ctx ImageContext, images []types.ImageSummary, format func(subC digest: "", }) } else { - repoTags := map[string][]string{} - repoDigests := map[string][]string{} - - for _, refString := range image.RepoTags { - ref, err := reference.ParseNormalizedNamed(refString) - if err != nil { - continue - } - if nt, ok := ref.(reference.NamedTagged); ok { - familiarRef := reference.FamiliarName(ref) - repoTags[familiarRef] = append(repoTags[familiarRef], nt.Tag()) - } - } - for _, refString := range image.RepoDigests { - ref, err := reference.ParseNormalizedNamed(refString) - if err != nil { - continue - } - if c, ok := ref.(reference.Canonical); ok { - familiarRef := reference.FamiliarName(ref) - repoDigests[familiarRef] = append(repoDigests[familiarRef], c.Digest().String()) - } + formatted = imageFormatTaggedAndDigest(ctx, image) + } + for _, imageCtx := range formatted { + if err := format(imageCtx); err != nil { + return err } + } + } + return nil +} - for repo, tags := range repoTags { - digests := repoDigests[repo] - - // Do not display digests as their own row - delete(repoDigests, repo) - - if !ctx.Digest { - // Ignore digest references, just show tag once - digests = nil - } - - for _, tag := range tags { - if len(digests) == 0 { - images = append(images, &imageContext{ - trunc: ctx.Trunc, - i: image, - repo: repo, - tag: tag, - digest: "", - }) - continue - } - // Display the digests for each tag - for _, dgst := range digests { - images = append(images, &imageContext{ - trunc: ctx.Trunc, - i: image, - repo: repo, - tag: tag, - digest: dgst, - }) - } - - } - } +func imageFormatTaggedAndDigest(ctx ImageContext, image types.ImageSummary) []*imageContext { + repoTags := map[string][]string{} + repoDigests := map[string][]string{} + images := []*imageContext{} + + for _, refString := range image.RepoTags { + ref, err := reference.ParseNormalizedNamed(refString) + if err != nil { + continue + } + if nt, ok := ref.(reference.NamedTagged); ok { + familiarRef := reference.FamiliarName(ref) + repoTags[familiarRef] = append(repoTags[familiarRef], nt.Tag()) + } + } + for _, refString := range image.RepoDigests { + ref, err := reference.ParseNormalizedNamed(refString) + if err != nil { + continue + } + if c, ok := ref.(reference.Canonical); ok { + familiarRef := reference.FamiliarName(ref) + repoDigests[familiarRef] = append(repoDigests[familiarRef], c.Digest().String()) + } + } - // Show rows for remaining digest only references - for repo, digests := range repoDigests { - // If digests are displayed, show row per digest - if ctx.Digest { - for _, dgst := range digests { - images = append(images, &imageContext{ - trunc: ctx.Trunc, - i: image, - repo: repo, - tag: "", - digest: dgst, - }) - } - } else { - images = append(images, &imageContext{ - trunc: ctx.Trunc, - i: image, - repo: repo, - tag: "", - }) - } + addImage := func(repo, tag, digest string) { + image := &imageContext{ + trunc: ctx.Trunc, + i: image, + repo: repo, + tag: tag, + digest: digest, + } + images = append(images, image) + } + + for repo, tags := range repoTags { + digests := repoDigests[repo] + + // Do not display digests as their own row + delete(repoDigests, repo) + + if !ctx.Digest { + // Ignore digest references, just show tag once + digests = nil + } + + for _, tag := range tags { + if len(digests) == 0 { + addImage(repo, tag, "") + continue + } + // Display the digests for each tag + for _, dgst := range digests { + addImage(repo, tag, dgst) } + } - for _, imageCtx := range images { - if err := format(imageCtx); err != nil { - return err + } + + // Show rows for remaining digest only references + for repo, digests := range repoDigests { + // If digests are displayed, show row per digest + if ctx.Digest { + for _, dgst := range digests { + addImage(repo, "", dgst) } + } else { + addImage(repo, "", "") + } } - return nil + return images } type imageContext struct { diff --git a/cli/command/formatter/image_test.go b/cli/command/formatter/image_test.go index b3c4cc8094b9..ee22fe669556 100644 --- a/cli/command/formatter/image_test.go +++ b/cli/command/formatter/image_test.go @@ -55,6 +55,26 @@ func TestImageContext(t *testing.T) { i: types.ImageSummary{}, digest: "sha256:d149ab53f8718e987c3a3024bb8aa0e2caadf6c0328f1d9d850b2a2a67f2819a", }, "sha256:d149ab53f8718e987c3a3024bb8aa0e2caadf6c0328f1d9d850b2a2a67f2819a", ctx.Digest}, + { + imageContext{ + i: types.ImageSummary{Containers: 10}, + }, "10", ctx.Containers, + }, + { + imageContext{ + i: types.ImageSummary{VirtualSize: 10000}, + }, "10kB", ctx.VirtualSize, + }, + { + imageContext{ + i: types.ImageSummary{SharedSize: 10000}, + }, "10kB", ctx.SharedSize, + }, + { + imageContext{ + i: types.ImageSummary{SharedSize: 5000, VirtualSize: 20000}, + }, "15kB", ctx.UniqueSize, + }, } for _, c := range cases { @@ -62,8 +82,8 @@ func TestImageContext(t *testing.T) { v := c.call() if strings.Contains(v, ",") { compareMultipleValues(t, v, c.expValue) - } else if v != c.expValue { - t.Fatalf("Expected %s, was %s\n", c.expValue, v) + } else { + assert.Equal(t, c.expValue, v) } } } diff --git a/cli/command/image/pull.go b/cli/command/image/pull.go index e60e5a434883..9fa5f3a049cf 100644 --- a/cli/command/image/pull.go +++ b/cli/command/image/pull.go @@ -6,6 +6,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/trust" "github.com/docker/distribution/reference" "github.com/docker/docker/registry" "github.com/pkg/errors" @@ -70,16 +71,29 @@ func runPull(dockerCli command.Cli, opts pullOptions) error { // Check if reference has a digest _, isCanonical := distributionRef.(reference.Canonical) if command.IsTrusted() && !isCanonical { - err = trustedPull(ctx, dockerCli, repoInfo, distributionRef, authConfig, requestPrivilege) - } else { - err = imagePullPrivileged(ctx, dockerCli, authConfig, reference.FamiliarString(distributionRef), requestPrivilege, opts.all) - } - if err != nil { - if strings.Contains(err.Error(), "when fetching 'plugin'") { - return errors.New(err.Error() + " - Use `docker plugin install`") + notaryRepo, err := trust.GetNotaryRepository(dockerCli, repoInfo, authConfig, "pull") + if err != nil { + fmt.Fprintf(dockerCli.Out(), "Error establishing connection to trust repository: %s\n", err) + return err } - return err + targets, err := getTargetsForTrustedPull(dockerCli, distributionRef, notaryRepo) + if err != nil { + return err + } + err = trustedPull(ctx, dockerCli, targets, distributionRef, authConfig, requestPrivilege) + return handlePullError(err) } - return nil + err = imagePullPrivileged(ctx, dockerCli, authConfig, reference.FamiliarString(distributionRef), requestPrivilege, opts.all) + return handlePullError(err) +} + +func handlePullError(err error) error { + if err == nil { + return nil + } + if strings.Contains(err.Error(), "when fetching 'plugin'") { + return errors.New(err.Error() + " - Use `docker plugin install`") + } + return err } diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index dd0d12c4c519..2c77f6378216 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -216,26 +216,19 @@ func imagePushPrivileged(ctx context.Context, cli command.Cli, authConfig types. return cli.Client().ImagePush(ctx, reference.FamiliarString(ref), options) } -// trustedPull handles content trust pulling of an image -func trustedPull(ctx context.Context, cli command.Cli, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig types.AuthConfig, requestPrivilege types.RequestPrivilegeFunc) error { - var refs []target - - notaryRepo, err := trust.GetNotaryRepository(cli, repoInfo, authConfig, "pull") - if err != nil { - fmt.Fprintf(cli.Out(), "Error establishing connection to trust repository: %s\n", err) - return err - } - - if tagged, isTagged := ref.(reference.NamedTagged); !isTagged { +func getTargetsForTrustedPull(streams command.Streams, ref reference.Named, notaryRepo *client.NotaryRepository) ([]target, error) { + tagged, isTagged := ref.(reference.NamedTagged) + if !isTagged { + var refs []target // List all targets targets, err := notaryRepo.ListTargets(trust.ReleasesRole, data.CanonicalTargetsRole) if err != nil { - return trust.NotaryError(ref.Name(), err) + return nil, trust.NotaryError(ref.Name(), err) } for _, tgt := range targets { t, err := convertTarget(tgt.Target) if err != nil { - fmt.Fprintf(cli.Out(), "Skipping target for %q\n", reference.FamiliarName(ref)) + fmt.Fprintf(streams.Out(), "Skipping target for %q\n", reference.FamiliarName(ref)) continue } // Only list tags in the top level targets role or the releases delegation role - ignore @@ -246,28 +239,32 @@ func trustedPull(ctx context.Context, cli command.Cli, repoInfo *registry.Reposi refs = append(refs, t) } if len(refs) == 0 { - return trust.NotaryError(ref.Name(), errors.Errorf("No trusted tags for %s", ref.Name())) - } - } else { - t, err := notaryRepo.GetTargetByName(tagged.Tag(), trust.ReleasesRole, data.CanonicalTargetsRole) - if err != nil { - return trust.NotaryError(ref.Name(), err) - } - // Only get the tag if it's in the top level targets role or the releases delegation role - // ignore it if it's in any other delegation roles - if t.Role != trust.ReleasesRole && t.Role != data.CanonicalTargetsRole { - return trust.NotaryError(ref.Name(), errors.Errorf("No trust data for %s", tagged.Tag())) + return nil, trust.NotaryError(ref.Name(), errors.Errorf("No trusted tags for %s", ref.Name())) } + return refs, nil + } - logrus.Debugf("retrieving target for %s role\n", t.Role) - r, err := convertTarget(t.Target) - if err != nil { - return err + t, err := notaryRepo.GetTargetByName(tagged.Tag(), trust.ReleasesRole, data.CanonicalTargetsRole) + if err != nil { + return nil, trust.NotaryError(ref.Name(), err) + } + // Only get the tag if it's in the top level targets role or the releases delegation role + // ignore it if it's in any other delegation roles + if t.Role != trust.ReleasesRole && t.Role != data.CanonicalTargetsRole { + return nil, trust.NotaryError(ref.Name(), errors.Errorf("No trust data for %s", tagged.Tag())) + } + + logrus.Debugf("retrieving target for %s role\n", t.Role) + r, err := convertTarget(t.Target) + if err != nil { + return nil, err - } - refs = append(refs, r) } + return []target{r}, nil +} +// trustedPull handles content trust pulling of an image +func trustedPull(ctx context.Context, cli command.Cli, refs []target, ref reference.Named, authConfig types.AuthConfig, requestPrivilege types.RequestPrivilegeFunc) error { for i, r := range refs { displayTag := r.name if displayTag != "" { diff --git a/cli/command/service/ps.go b/cli/command/service/ps.go index 07dbba72302d..e441f9e1aaa8 100644 --- a/cli/command/service/ps.go +++ b/cli/command/service/ps.go @@ -56,6 +56,9 @@ func runPS(dockerCli command.Cli, options psOptions) error { if err != nil { return err } + if err := updateNodeFilter(ctx, client, filter); err != nil { + return err + } tasks, err := client.TaskList(ctx, types.TaskListOptions{Filters: filter}) if err != nil { @@ -130,16 +133,20 @@ loop: if serviceCount == 0 { return filter, nil, errors.New(strings.Join(notfound, "\n")) } + return filter, notfound, err +} + +func updateNodeFilter(ctx context.Context, client client.APIClient, filter filters.Args) error { if filter.Include("node") { nodeFilters := filter.Get("node") for _, nodeFilter := range nodeFilters { nodeReference, err := node.Reference(ctx, client, nodeFilter) if err != nil { - return filter, nil, err + return err } filter.Del("node", nodeFilter) filter.Add("node", nodeReference) } } - return filter, notfound, err + return nil } diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index b456e1824dc1..09aaf95c8e25 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -93,11 +93,7 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) { } cfg.Configs, err = LoadConfigObjs(config["configs"], configDetails.WorkingDir) - if err != nil { - return nil, err - } - - return &cfg, nil + return &cfg, err } func interpolateConfig(configDict map[string]interface{}, lookupEnv template.Mapping) (map[string]map[string]interface{}, error) { @@ -570,7 +566,6 @@ func transformServiceVolumeConfig(data interface{}) (interface{}, error) { default: return data, errors.Errorf("invalid type %T for service volume", value) } - } func transformServiceNetworkMap(value interface{}) (interface{}, error) { diff --git a/cli/compose/loader/volume_test.go b/cli/compose/loader/volume_test.go index f1b90fe8eace..1e5b73a9c50b 100644 --- a/cli/compose/loader/volume_test.go +++ b/cli/compose/loader/volume_test.go @@ -200,3 +200,13 @@ func TestParseVolumeSplitCases(t *testing.T) { assert.Equal(t, expected, parsed.Source != "", msg) } } + +func TestParseVolumeInvalidEmptySpec(t *testing.T) { + _, err := ParseVolume("") + testutil.ErrorContains(t, err, "invalid empty volume spec") +} + +func TestParseVolumeInvalidSections(t *testing.T) { + _, err := ParseVolume("/foo::rw") + testutil.ErrorContains(t, err, "invalid spec") +} diff --git a/gometalinter.json b/gometalinter.json index b5956dca602b..853d797687ea 100644 --- a/gometalinter.json +++ b/gometalinter.json @@ -22,6 +22,6 @@ "vet" ], - "Cyclo": 19, + "Cyclo": 16, "LineLength": 200 }