Skip to content

Commit

Permalink
Merge pull request #4774 from thaJeztah/pass_context
Browse files Browse the repository at this point in the history
improve passing context
  • Loading branch information
Benehiko committed Apr 25, 2024
2 parents 8651906 + 86162f7 commit 7f15dfa
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 12 deletions.
8 changes: 1 addition & 7 deletions cli-plugins/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,7 @@ func RunPlugin(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager
PersistentPreRunE = func(cmd *cobra.Command, _ []string) error {
var err error
persistentPreRunOnce.Do(func() {
cmdContext := cmd.Context()
// TODO: revisit and make sure this check makes sense
// see: https://github.com/docker/cli/pull/4599#discussion_r1422487271
if cmdContext == nil {
cmdContext = context.TODO()
}
ctx, cancel := context.WithCancel(cmdContext)
ctx, cancel := context.WithCancel(cmd.Context())
cmd.SetContext(ctx)
// Set up the context to cancel based on signalling via CLI socket.
socket.ConnectAndWait(cancel)
Expand Down
18 changes: 18 additions & 0 deletions cmd/docker/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
var pluginFilename = "docker-buildx"

func TestBuildWithBuilder(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

testcases := []struct {
name string
context string
Expand Down Expand Up @@ -64,12 +67,16 @@ echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortD
for _, tt := range testcases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ctx2, cancel2 := context.WithCancel(ctx)
defer cancel2()

if tt.builder != "" {
t.Setenv("BUILDX_BUILDER", tt.builder)
}

var b bytes.Buffer
dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx2),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(&b),
Expand Down Expand Up @@ -126,6 +133,9 @@ func (c *fakeClient) Ping(_ context.Context) (types.Ping, error) {
}

func TestBuildkitDisabled(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

t.Setenv("DOCKER_BUILDKIT", "0")

dir := fs.NewDir(t, t.Name(),
Expand All @@ -136,6 +146,7 @@ func TestBuildkitDisabled(t *testing.T) {
b := bytes.NewBuffer(nil)

dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(b),
Expand Down Expand Up @@ -163,6 +174,9 @@ func TestBuildkitDisabled(t *testing.T) {
}

func TestBuilderBroken(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

dir := fs.NewDir(t, t.Name(),
fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0o777)),
)
Expand All @@ -171,6 +185,7 @@ func TestBuilderBroken(t *testing.T) {
b := bytes.NewBuffer(nil)

dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(b),
Expand Down Expand Up @@ -199,6 +214,8 @@ func TestBuilderBroken(t *testing.T) {

func TestBuilderBrokenEnforced(t *testing.T) {
t.Setenv("DOCKER_BUILDKIT", "1")
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

dir := fs.NewDir(t, t.Name(),
fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0o777)),
Expand All @@ -208,6 +225,7 @@ func TestBuilderBrokenEnforced(t *testing.T) {
b := bytes.NewBuffer(nil)

dockerCli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithAPIClient(&fakeClient{}),
command.WithInputStream(discard),
command.WithCombinedStreams(b),
Expand Down
3 changes: 2 additions & 1 deletion cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

func main() {
ctx := context.Background()

dockerCli, err := command.NewDockerCli(command.WithBaseContext(ctx))
if err != nil {
fmt.Fprintln(os.Stderr, err)
Expand Down Expand Up @@ -352,7 +353,7 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error {
// We've parsed global args already, so reset args to those
// which remain.
cmd.SetArgs(args)
err = cmd.Execute()
err = cmd.ExecuteContext(ctx)

// If the command is being executed in an interactive terminal
// and hook are enabled, run the plugin hooks.
Expand Down
13 changes: 11 additions & 2 deletions cmd/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"bytes"
"context"
"io"
"os"
"testing"
Expand All @@ -15,8 +16,10 @@ import (

func TestClientDebugEnabled(t *testing.T) {
defer debug.Disable()
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

cli, err := command.NewDockerCli()
cli, err := command.NewDockerCli(command.WithBaseContext(ctx))
assert.NilError(t, err)
tcmd := newDockerCommand(cli)
tcmd.SetFlag("debug", "true")
Expand All @@ -39,7 +42,13 @@ func runCliCommand(t *testing.T, r io.ReadCloser, w io.Writer, args ...string) e
if w == nil {
w = io.Discard
}
cli, err := command.NewDockerCli(command.WithInputStream(r), command.WithCombinedStreams(w))
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

cli, err := command.NewDockerCli(
command.WithBaseContext(ctx),
command.WithInputStream(r),
command.WithCombinedStreams(w))
assert.NilError(t, err)
tcmd := newDockerCommand(cli)

Expand Down
1 change: 1 addition & 0 deletions docs/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func gen(opts *options) error {
Use: "docker [OPTIONS] COMMAND [ARG...]",
Short: "The base command for the Docker CLI.",
}

clientOpts, _ := cli.SetupRootCommand(cmd)
if err := dockerCLI.Initialize(clientOpts); err != nil {
return err
Expand Down
3 changes: 1 addition & 2 deletions e2e/cli-plugins/plugins/nopersistentprerun/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

func main() {
plugin.Run(func(dockerCli command.Cli) *cobra.Command {
cmd := &cobra.Command{
return &cobra.Command{
Use: "nopersistentprerun",
Short: "Testing without PersistentPreRun hooks",
// PersistentPreRunE: Not specified, we need to test that it works in the absence of an explicit call
Expand All @@ -25,7 +25,6 @@ func main() {
return nil
},
}
return cmd
},
manager.Metadata{
SchemaVersion: "0.1.0",
Expand Down

0 comments on commit 7f15dfa

Please sign in to comment.