Skip to content

Commit d55ee6d

Browse files
BenehikoBenehiko
committed
pkg/command: wrap jsonmessage.DisplayJSONMessagesStream and
`jsonmessage.DisplayJSONMessagesToStream` Allows for the `jsonmessage.DisplayJSONMessagesStream` and `jsonmessage.DisplayJSONMessagesToStream` to correctly return when the context is cancelled with the appropriate reason (`ctx.Error()`) instead of just a nil error. Follow-up to 30a73ff Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
1 parent 5afa739 commit d55ee6d

File tree

13 files changed

+104
-37
lines changed

13 files changed

+104
-37
lines changed

‎cli/command/container/create.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ import (
1313
"github.com/docker/cli/cli/command"
1414
"github.com/docker/cli/cli/command/completion"
1515
"github.com/docker/cli/cli/command/image"
16+
"github.com/docker/cli/cli/internal/jsonmessage"
1617
"github.com/docker/cli/cli/streams"
1718
"github.com/docker/cli/opts"
1819
"github.com/docker/docker/api/types/container"
1920
imagetypes "github.com/docker/docker/api/types/image"
2021
"github.com/docker/docker/api/types/versions"
2122
"github.com/docker/docker/errdefs"
22-
"github.com/docker/docker/pkg/jsonmessage"
2323
specs "github.com/opencontainers/image-spec/specs-go/v1"
2424
"github.com/pkg/errors"
2525
"github.com/spf13/cobra"
@@ -148,7 +148,7 @@ func pullImage(ctx context.Context, dockerCli command.Cli, img string, options *
148148
if options.quiet {
149149
out = streams.NewOut(io.Discard)
150150
}
151-
return jsonmessage.DisplayJSONMessagesToStream(responseBody, out, nil)
151+
return jsonmessage.DisplayStream(ctx, responseBody, out, nil)
152152
}
153153

154154
type cidFile struct {

‎cli/command/container/run_test.go

+2-7
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,7 @@ func TestRunPullTermination(t *testing.T) {
230230
createContainerFunc: func(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig,
231231
platform *specs.Platform, containerName string,
232232
) (container.CreateResponse, error) {
233-
select {
234-
case <-ctx.Done():
235-
return container.CreateResponse{}, ctx.Err()
236-
default:
237-
}
238-
return container.CreateResponse{}, fakeNotFound{}
233+
return container.CreateResponse{}, errors.New("shouldn't try to create a container")
239234
},
240235
containerAttachFunc: func(ctx context.Context, containerID string, options container.AttachOptions) (types.HijackedResponse, error) {
241236
return types.HijackedResponse{}, errors.New("shouldn't try to attach to a container")
@@ -277,7 +272,7 @@ func TestRunPullTermination(t *testing.T) {
277272
cmd := NewRunCommand(fakeCLI)
278273
cmd.SetOut(io.Discard)
279274
cmd.SetErr(io.Discard)
280-
cmd.SetArgs([]string{"foobar:latest"})
275+
cmd.SetArgs([]string{"--pull", "always", "foobar:latest"})
281276

282277
cmdErrC := make(chan error, 1)
283278
go func() {

‎cli/command/image/build.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/docker/cli/cli/command"
2121
"github.com/docker/cli/cli/command/completion"
2222
"github.com/docker/cli/cli/command/image/build"
23+
"github.com/docker/cli/cli/internal/jsonmessage"
2324
"github.com/docker/cli/opts"
2425
"github.com/docker/docker/api"
2526
"github.com/docker/docker/api/types"
@@ -28,7 +29,6 @@ import (
2829
"github.com/docker/docker/builder/remotecontext/urlutil"
2930
"github.com/docker/docker/pkg/archive"
3031
"github.com/docker/docker/pkg/idtools"
31-
"github.com/docker/docker/pkg/jsonmessage"
3232
"github.com/docker/docker/pkg/progress"
3333
"github.com/docker/docker/pkg/streamformatter"
3434
"github.com/pkg/errors"
@@ -361,7 +361,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
361361
}
362362
}
363363

364-
err = jsonmessage.DisplayJSONMessagesStream(response.Body, buildBuff, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), aux)
364+
err = jsonmessage.DisplayStreamWithTerminal(ctx, response.Body, buildBuff, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), aux)
365365
if err != nil {
366366
if jerr, ok := err.(*jsonmessage.JSONError); ok {
367367
// If no error code is set, default to 1

‎cli/command/image/import.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import (
88
"github.com/docker/cli/cli"
99
"github.com/docker/cli/cli/command"
1010
"github.com/docker/cli/cli/command/completion"
11+
"github.com/docker/cli/cli/internal/jsonmessage"
1112
dockeropts "github.com/docker/cli/opts"
1213
"github.com/docker/docker/api/types/image"
13-
"github.com/docker/docker/pkg/jsonmessage"
1414
"github.com/spf13/cobra"
1515
)
1616

@@ -90,5 +90,5 @@ func runImport(ctx context.Context, dockerCli command.Cli, options importOptions
9090
}
9191
defer responseBody.Close()
9292

93-
return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil)
93+
return jsonmessage.DisplayStream(ctx, responseBody, dockerCli.Out(), nil)
9494
}

‎cli/command/image/load.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
"github.com/docker/cli/cli"
99
"github.com/docker/cli/cli/command"
1010
"github.com/docker/cli/cli/command/completion"
11+
"github.com/docker/cli/cli/internal/jsonmessage"
1112
"github.com/docker/docker/api/types/image"
12-
"github.com/docker/docker/pkg/jsonmessage"
1313
"github.com/moby/sys/sequential"
1414
"github.com/pkg/errors"
1515
"github.com/spf13/cobra"
@@ -89,7 +89,7 @@ func runLoad(ctx context.Context, dockerCli command.Cli, opts loadOptions) error
8989
defer response.Body.Close()
9090

9191
if response.Body != nil && response.JSON {
92-
return jsonmessage.DisplayJSONMessagesToStream(response.Body, dockerCli.Out(), nil)
92+
return jsonmessage.DisplayStream(ctx, response.Body, dockerCli.Out(), nil)
9393
}
9494

9595
_, err = io.Copy(dockerCli.Out(), response.Body)

‎cli/command/image/push.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ import (
1515
"github.com/docker/cli/cli"
1616
"github.com/docker/cli/cli/command"
1717
"github.com/docker/cli/cli/command/completion"
18+
"github.com/docker/cli/cli/internal/jsonmessage"
1819
"github.com/docker/cli/cli/streams"
1920
"github.com/docker/docker/api/types/auxprogress"
2021
"github.com/docker/docker/api/types/image"
2122
registrytypes "github.com/docker/docker/api/types/registry"
22-
"github.com/docker/docker/pkg/jsonmessage"
2323
"github.com/docker/docker/registry"
2424
"github.com/morikuni/aec"
2525
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -140,22 +140,22 @@ To push the complete multi-platform image, remove the --platform flag.
140140
defer responseBody.Close()
141141
if !opts.untrusted {
142142
// TODO PushTrustedReference currently doesn't respect `--quiet`
143-
return PushTrustedReference(dockerCli, repoInfo, ref, authConfig, responseBody)
143+
return PushTrustedReference(ctx, dockerCli, repoInfo, ref, authConfig, responseBody)
144144
}
145145

146146
if opts.quiet {
147-
err = jsonmessage.DisplayJSONMessagesToStream(responseBody, streams.NewOut(io.Discard), handleAux(dockerCli))
147+
err = jsonmessage.DisplayStream(ctx, responseBody, streams.NewOut(io.Discard), handleAux())
148148
if err == nil {
149149
fmt.Fprintln(dockerCli.Out(), ref.String())
150150
}
151151
return err
152152
}
153-
return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), handleAux(dockerCli))
153+
return jsonmessage.DisplayStream(ctx, responseBody, dockerCli.Out(), handleAux())
154154
}
155155

156156
var notes []string
157157

158-
func handleAux(dockerCli command.Cli) func(jm jsonmessage.JSONMessage) {
158+
func handleAux() func(jm jsonmessage.JSONMessage) {
159159
return func(jm jsonmessage.JSONMessage) {
160160
b := []byte(*jm.Aux)
161161

‎cli/command/image/trust.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ import (
1010

1111
"github.com/distribution/reference"
1212
"github.com/docker/cli/cli/command"
13+
"github.com/docker/cli/cli/internal/jsonmessage"
1314
"github.com/docker/cli/cli/streams"
1415
"github.com/docker/cli/cli/trust"
1516
"github.com/docker/docker/api/types"
1617
"github.com/docker/docker/api/types/image"
1718
registrytypes "github.com/docker/docker/api/types/registry"
18-
"github.com/docker/docker/pkg/jsonmessage"
1919
"github.com/docker/docker/registry"
2020
"github.com/opencontainers/go-digest"
2121
"github.com/pkg/errors"
@@ -39,13 +39,13 @@ func TrustedPush(ctx context.Context, cli command.Cli, repoInfo *registry.Reposi
3939

4040
defer responseBody.Close()
4141

42-
return PushTrustedReference(cli, repoInfo, ref, authConfig, responseBody)
42+
return PushTrustedReference(ctx, cli, repoInfo, ref, authConfig, responseBody)
4343
}
4444

4545
// PushTrustedReference pushes a canonical reference to the trust server.
4646
//
4747
//nolint:gocyclo
48-
func PushTrustedReference(ioStreams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, in io.Reader) error {
48+
func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, in io.Reader) error {
4949
// If it is a trusted push we would like to find the target entry which match the
5050
// tag provided in the function and then do an AddTarget later.
5151
target := &client.Target{}
@@ -84,14 +84,14 @@ func PushTrustedReference(ioStreams command.Streams, repoInfo *registry.Reposito
8484
default:
8585
// We want trust signatures to always take an explicit tag,
8686
// otherwise it will act as an untrusted push.
87-
if err := jsonmessage.DisplayJSONMessagesToStream(in, ioStreams.Out(), nil); err != nil {
87+
if err := jsonmessage.DisplayStream(ctx, in, ioStreams.Out(), nil); err != nil {
8888
return err
8989
}
9090
fmt.Fprintln(ioStreams.Err(), "No tag specified, skipping trust metadata push")
9191
return nil
9292
}
9393

94-
if err := jsonmessage.DisplayJSONMessagesToStream(in, ioStreams.Out(), handleTarget); err != nil {
94+
if err := jsonmessage.DisplayStream(ctx, in, ioStreams.Out(), handleTarget); err != nil {
9595
return err
9696
}
9797

@@ -283,7 +283,7 @@ func imagePullPrivileged(ctx context.Context, cli command.Cli, imgRefAndAuth tru
283283
if opts.quiet {
284284
out = streams.NewOut(io.Discard)
285285
}
286-
return jsonmessage.DisplayJSONMessagesToStream(responseBody, out, nil)
286+
return jsonmessage.DisplayStream(ctx, responseBody, out, nil)
287287
}
288288

289289
// TrustedReference returns the canonical trusted reference for an image reference

‎cli/command/plugin/install.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import (
99
"github.com/docker/cli/cli"
1010
"github.com/docker/cli/cli/command"
1111
"github.com/docker/cli/cli/command/image"
12+
"github.com/docker/cli/cli/internal/jsonmessage"
1213
"github.com/docker/docker/api/types"
1314
registrytypes "github.com/docker/docker/api/types/registry"
14-
"github.com/docker/docker/pkg/jsonmessage"
1515
"github.com/docker/docker/registry"
1616
"github.com/pkg/errors"
1717
"github.com/spf13/cobra"
@@ -129,7 +129,7 @@ func runInstall(ctx context.Context, dockerCli command.Cli, opts pluginOptions)
129129
return err
130130
}
131131
defer responseBody.Close()
132-
if err := jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil); err != nil {
132+
if err := jsonmessage.DisplayStream(ctx, responseBody, dockerCli.Out(), nil); err != nil {
133133
return err
134134
}
135135
fmt.Fprintf(dockerCli.Out(), "Installed plugin %s\n", opts.remote) // todo: return proper values from the API for this result

‎cli/command/plugin/push.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77
"github.com/docker/cli/cli"
88
"github.com/docker/cli/cli/command"
99
"github.com/docker/cli/cli/command/image"
10+
"github.com/docker/cli/cli/internal/jsonmessage"
1011
registrytypes "github.com/docker/docker/api/types/registry"
11-
"github.com/docker/docker/pkg/jsonmessage"
1212
"github.com/docker/docker/registry"
1313
"github.com/pkg/errors"
1414
"github.com/spf13/cobra"
@@ -66,8 +66,8 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOptions) error
6666
defer responseBody.Close()
6767

6868
if !opts.untrusted {
69-
return image.PushTrustedReference(dockerCli, repoInfo, named, authConfig, responseBody)
69+
return image.PushTrustedReference(ctx, dockerCli, repoInfo, named, authConfig, responseBody)
7070
}
7171

72-
return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil)
72+
return jsonmessage.DisplayStream(ctx, responseBody, dockerCli.Out(), nil)
7373
}

‎cli/command/plugin/upgrade.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
"github.com/distribution/reference"
99
"github.com/docker/cli/cli"
1010
"github.com/docker/cli/cli/command"
11+
"github.com/docker/cli/cli/internal/jsonmessage"
1112
"github.com/docker/docker/errdefs"
12-
"github.com/docker/docker/pkg/jsonmessage"
1313
"github.com/pkg/errors"
1414
"github.com/spf13/cobra"
1515
)
@@ -86,7 +86,7 @@ func runUpgrade(ctx context.Context, dockerCli command.Cli, opts pluginOptions)
8686
return err
8787
}
8888
defer responseBody.Close()
89-
if err := jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil); err != nil {
89+
if err := jsonmessage.DisplayStream(ctx, responseBody, dockerCli.Out(), nil); err != nil {
9090
return err
9191
}
9292
fmt.Fprintf(dockerCli.Out(), "Upgraded plugin %s to %s\n", opts.localName, opts.remote) // todo: return proper values from the API for this result

‎cli/command/service/helpers.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66

77
"github.com/docker/cli/cli/command"
88
"github.com/docker/cli/cli/command/service/progress"
9-
"github.com/docker/docker/pkg/jsonmessage"
9+
"github.com/docker/cli/cli/internal/jsonmessage"
1010
)
1111

1212
// WaitOnService waits for the service to converge. It outputs a progress bar,
@@ -24,7 +24,7 @@ func WaitOnService(ctx context.Context, dockerCli command.Cli, serviceID string,
2424
return <-errChan
2525
}
2626

27-
err := jsonmessage.DisplayJSONMessagesToStream(pipeReader, dockerCli.Out(), nil)
27+
err := jsonmessage.DisplayStream(ctx, pipeReader, dockerCli.Out(), nil)
2828
if err == nil {
2929
err = <-errChan
3030
}

‎cli/command/swarm/ca.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import (
1010
"github.com/docker/cli/cli/command"
1111
"github.com/docker/cli/cli/command/completion"
1212
"github.com/docker/cli/cli/command/swarm/progress"
13+
"github.com/docker/cli/cli/internal/jsonmessage"
1314
"github.com/docker/docker/api/types/swarm"
14-
"github.com/docker/docker/pkg/jsonmessage"
1515
"github.com/pkg/errors"
1616
"github.com/spf13/cobra"
1717
"github.com/spf13/pflag"
@@ -120,7 +120,7 @@ func attach(ctx context.Context, dockerCli command.Cli, opts caOptions) error {
120120
return <-errChan
121121
}
122122

123-
err := jsonmessage.DisplayJSONMessagesToStream(pipeReader, dockerCli.Out(), nil)
123+
err := jsonmessage.DisplayStream(ctx, pipeReader, dockerCli.Out(), nil)
124124
if err == nil {
125125
err = <-errChan
126126
}

‎cli/internal/jsonmessage/display.go

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package jsonmessage
2+
3+
import (
4+
"context"
5+
"io"
6+
7+
"github.com/docker/docker/pkg/jsonmessage"
8+
)
9+
10+
type (
11+
Stream = jsonmessage.Stream
12+
JSONMessage = jsonmessage.JSONMessage
13+
JSONError = jsonmessage.JSONError
14+
)
15+
16+
// DisplayStreamWithTerminal prints the JSON messages from the given reader to
17+
// the given stream.
18+
//
19+
// Presentation of the JSONMessage depends on whether a terminal is attached,
20+
// and on the terminal width. Progress bars ([JSONProgress]) are suppressed
21+
// on narrower terminals (< 110 characters).
22+
//
23+
// - isTerminal describes if out is a terminal, in which case it prints
24+
// a newline ("\n") at the end of each line and moves the cursor while
25+
// displaying.
26+
// - terminalFd is the fd of the current terminal (if any), and used
27+
// to get the terminal width.
28+
// - auxCallback allows handling the [JSONMessage.Aux] field. It is
29+
// called if a JSONMessage contains an Aux field, in which case
30+
// DisplayJSONMessagesStream does not present the JSONMessage.
31+
//
32+
// It wraps the [jsonmessage.DisplayJSONMessagesStream] function to make it
33+
// "context aware" and appropriately returns why the function was canceled.
34+
//
35+
// It returns an error if the context is canceled, but not if the input reader / stream is closed.
36+
func DisplayStreamWithTerminal(ctx context.Context, in io.Reader, out io.Writer, terminalFd uintptr, isTerminal bool, auxCallback func(JSONMessage)) error {
37+
select {
38+
case <-ctx.Done():
39+
return ctx.Err()
40+
default:
41+
err := jsonmessage.DisplayJSONMessagesStream(in, out, terminalFd, isTerminal, auxCallback)
42+
if err != nil {
43+
return err
44+
}
45+
if ctx.Done() != nil {
46+
return ctx.Err()
47+
}
48+
}
49+
return nil
50+
}
51+
52+
// DisplayStream prints the JSON messages from the given reader to the given stream.
53+
//
54+
// It wraps the [jsonmessage.DisplayJSONMessagesToStream] function to make it
55+
// "context aware" and appropriately returns why the function was canceled.
56+
//
57+
// It returns an error if the context is canceled, but not if the input reader / stream is closed.
58+
func DisplayStream(ctx context.Context, in io.Reader, stream Stream, auxCallback func(JSONMessage)) error {
59+
select {
60+
case <-ctx.Done():
61+
return ctx.Err()
62+
default:
63+
err := jsonmessage.DisplayJSONMessagesToStream(in, stream, auxCallback)
64+
if err != nil {
65+
return err
66+
}
67+
if ctx.Done() != nil {
68+
return ctx.Err()
69+
}
70+
}
71+
return nil
72+
}

0 commit comments

Comments
 (0)