Skip to content

Commit

Permalink
pkg/command: pass context along to `jsonmessage.DisplayJSONMessagesSt…
Browse files Browse the repository at this point in the history
…ream`

Allows for the `jsonmessage.DisplayJSONMessagesStream` to
correctly return when the context is cancelled with the
appropriate error message, instead of just a nil error.

Follow-up to 30a73ff

Signed-off-by: Alano Terblanche <[email protected]>
  • Loading branch information
Benehiko committed Dec 2, 2024
1 parent 5afa739 commit c21438f
Show file tree
Hide file tree
Showing 12 changed files with 26 additions and 25 deletions.
2 changes: 1 addition & 1 deletion cli/command/container/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func pullImage(ctx context.Context, dockerCli command.Cli, img string, options *
if options.quiet {
out = streams.NewOut(io.Discard)
}
return jsonmessage.DisplayJSONMessagesToStream(responseBody, out, nil)
return jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, out, nil)
}

type cidFile struct {
Expand Down
15 changes: 8 additions & 7 deletions cli/command/container/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,13 @@ func TestRunPullTermination(t *testing.T) {
createContainerFunc: func(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig,
platform *specs.Platform, containerName string,
) (container.CreateResponse, error) {
select {
case <-ctx.Done():
return container.CreateResponse{}, ctx.Err()
default:
}
return container.CreateResponse{}, fakeNotFound{}
// select {
// case <-ctx.Done():
// return container.CreateResponse{}, ctx.Err()
// default:
// }
// return container.CreateResponse{}, fakeNotFound{}
return container.CreateResponse{}, errors.New("shouldn't try to create a container")
},
containerAttachFunc: func(ctx context.Context, containerID string, options container.AttachOptions) (types.HijackedResponse, error) {
return types.HijackedResponse{}, errors.New("shouldn't try to attach to a container")
Expand Down Expand Up @@ -277,7 +278,7 @@ func TestRunPullTermination(t *testing.T) {
cmd := NewRunCommand(fakeCLI)
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
cmd.SetArgs([]string{"foobar:latest"})
cmd.SetArgs([]string{"--pull", "always", "foobar:latest"})

cmdErrC := make(chan error, 1)
go func() {
Expand Down
2 changes: 1 addition & 1 deletion cli/command/image/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
}
}

err = jsonmessage.DisplayJSONMessagesStream(response.Body, buildBuff, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), aux)
err = jsonmessage.DisplayJSONMessagesStream(ctx, response.Body, buildBuff, dockerCli.Out().FD(), dockerCli.Out().IsTerminal(), aux)

Check failure on line 364 in cli/command/image/build.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesStream
if err != nil {
if jerr, ok := err.(*jsonmessage.JSONError); ok {
// If no error code is set, default to 1
Expand Down
2 changes: 1 addition & 1 deletion cli/command/image/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,5 @@ func runImport(ctx context.Context, dockerCli command.Cli, options importOptions
}
defer responseBody.Close()

return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil)
return jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, dockerCli.Out(), nil)

Check failure on line 93 in cli/command/image/import.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
}
2 changes: 1 addition & 1 deletion cli/command/image/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func runLoad(ctx context.Context, dockerCli command.Cli, opts loadOptions) error
defer response.Body.Close()

if response.Body != nil && response.JSON {
return jsonmessage.DisplayJSONMessagesToStream(response.Body, dockerCli.Out(), nil)
return jsonmessage.DisplayJSONMessagesToStream(ctx, response.Body, dockerCli.Out(), nil)

Check failure on line 92 in cli/command/image/load.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
}

_, err = io.Copy(dockerCli.Out(), response.Body)
Expand Down
6 changes: 3 additions & 3 deletions cli/command/image/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,17 +140,17 @@ To push the complete multi-platform image, remove the --platform flag.
defer responseBody.Close()
if !opts.untrusted {
// TODO PushTrustedReference currently doesn't respect `--quiet`
return PushTrustedReference(dockerCli, repoInfo, ref, authConfig, responseBody)
return PushTrustedReference(ctx, dockerCli, repoInfo, ref, authConfig, responseBody)
}

if opts.quiet {
err = jsonmessage.DisplayJSONMessagesToStream(responseBody, streams.NewOut(io.Discard), handleAux(dockerCli))
err = jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, streams.NewOut(io.Discard), handleAux(dockerCli))

Check failure on line 147 in cli/command/image/push.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
if err == nil {
fmt.Fprintln(dockerCli.Out(), ref.String())
}
return err
}
return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), handleAux(dockerCli))
return jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, dockerCli.Out(), handleAux(dockerCli))

Check failure on line 153 in cli/command/image/push.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
}

var notes []string
Expand Down
10 changes: 5 additions & 5 deletions cli/command/image/trust.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ func TrustedPush(ctx context.Context, cli command.Cli, repoInfo *registry.Reposi

defer responseBody.Close()

return PushTrustedReference(cli, repoInfo, ref, authConfig, responseBody)
return PushTrustedReference(ctx, cli, repoInfo, ref, authConfig, responseBody)
}

// PushTrustedReference pushes a canonical reference to the trust server.
//
//nolint:gocyclo
func PushTrustedReference(ioStreams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, in io.Reader) error {
func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, in io.Reader) error {
// If it is a trusted push we would like to find the target entry which match the
// tag provided in the function and then do an AddTarget later.
target := &client.Target{}
Expand Down Expand Up @@ -84,14 +84,14 @@ func PushTrustedReference(ioStreams command.Streams, repoInfo *registry.Reposito
default:
// We want trust signatures to always take an explicit tag,
// otherwise it will act as an untrusted push.
if err := jsonmessage.DisplayJSONMessagesToStream(in, ioStreams.Out(), nil); err != nil {
if err := jsonmessage.DisplayJSONMessagesToStream(ctx, in, ioStreams.Out(), nil); err != nil {

Check failure on line 87 in cli/command/image/trust.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
return err
}
fmt.Fprintln(ioStreams.Err(), "No tag specified, skipping trust metadata push")
return nil
}

if err := jsonmessage.DisplayJSONMessagesToStream(in, ioStreams.Out(), handleTarget); err != nil {
if err := jsonmessage.DisplayJSONMessagesToStream(ctx, in, ioStreams.Out(), handleTarget); err != nil {

Check failure on line 94 in cli/command/image/trust.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
return err
}

Expand Down Expand Up @@ -283,7 +283,7 @@ func imagePullPrivileged(ctx context.Context, cli command.Cli, imgRefAndAuth tru
if opts.quiet {
out = streams.NewOut(io.Discard)
}
return jsonmessage.DisplayJSONMessagesToStream(responseBody, out, nil)
return jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, out, nil)

Check failure on line 286 in cli/command/image/trust.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
}

// TrustedReference returns the canonical trusted reference for an image reference
Expand Down
2 changes: 1 addition & 1 deletion cli/command/plugin/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func runInstall(ctx context.Context, dockerCli command.Cli, opts pluginOptions)
return err
}
defer responseBody.Close()
if err := jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil); err != nil {
if err := jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, dockerCli.Out(), nil); err != nil {
return err
}
fmt.Fprintf(dockerCli.Out(), "Installed plugin %s\n", opts.remote) // todo: return proper values from the API for this result
Expand Down
4 changes: 2 additions & 2 deletions cli/command/plugin/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOptions) error
defer responseBody.Close()

if !opts.untrusted {
return image.PushTrustedReference(dockerCli, repoInfo, named, authConfig, responseBody)
return image.PushTrustedReference(ctx, dockerCli, repoInfo, named, authConfig, responseBody)
}

return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil)
return jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, dockerCli.Out(), nil)
}
2 changes: 1 addition & 1 deletion cli/command/plugin/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func runUpgrade(ctx context.Context, dockerCli command.Cli, opts pluginOptions)
return err
}
defer responseBody.Close()
if err := jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil); err != nil {
if err := jsonmessage.DisplayJSONMessagesToStream(ctx, responseBody, dockerCli.Out(), nil); err != nil {
return err
}
fmt.Fprintf(dockerCli.Out(), "Upgraded plugin %s to %s\n", opts.localName, opts.remote) // todo: return proper values from the API for this result
Expand Down
2 changes: 1 addition & 1 deletion cli/command/service/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func WaitOnService(ctx context.Context, dockerCli command.Cli, serviceID string,
return <-errChan
}

err := jsonmessage.DisplayJSONMessagesToStream(pipeReader, dockerCli.Out(), nil)
err := jsonmessage.DisplayJSONMessagesToStream(ctx, pipeReader, dockerCli.Out(), nil)

Check failure on line 27 in cli/command/service/helpers.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
if err == nil {
err = <-errChan
}
Expand Down
2 changes: 1 addition & 1 deletion cli/command/swarm/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func attach(ctx context.Context, dockerCli command.Cli, opts caOptions) error {
return <-errChan
}

err := jsonmessage.DisplayJSONMessagesToStream(pipeReader, dockerCli.Out(), nil)
err := jsonmessage.DisplayJSONMessagesToStream(ctx, pipeReader, dockerCli.Out(), nil)

Check failure on line 123 in cli/command/swarm/ca.go

View workflow job for this annotation

GitHub Actions / codeql

too many arguments in call to jsonmessage.DisplayJSONMessagesToStream
if err == nil {
err = <-errChan
}
Expand Down

0 comments on commit c21438f

Please sign in to comment.