Skip to content

Commit

Permalink
rm: support removing multiple builders at once
Browse files Browse the repository at this point in the history
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
  • Loading branch information
crazy-max committed Nov 24, 2023
1 parent cec4496 commit 98f6161
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 27 deletions.
61 changes: 35 additions & 26 deletions commands/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

type rmOptions struct {
builder string
builder []string
keepState bool
keepDaemon bool
allInactive bool
Expand Down Expand Up @@ -46,34 +46,43 @@ func runRm(dockerCli command.Cli, in rmOptions) error {
return rmAllInactive(ctx, txn, dockerCli, in)
}

b, err := builder.New(dockerCli,
builder.WithName(in.builder),
builder.WithStore(txn),
builder.WithSkippedValidation(),
)
if err != nil {
return err
}
eg, _ := errgroup.WithContext(ctx)
for _, name := range in.builder {
func(name string) {
eg.Go(func() error {
b, err := builder.New(dockerCli,
builder.WithName(name),
builder.WithStore(txn),
builder.WithSkippedValidation(),
)
if err != nil {
return err
}

nodes, err := b.LoadNodes(ctx)
if err != nil {
return err
}
nodes, err := b.LoadNodes(ctx)
if err != nil {
return err
}

if cb := b.ContextName(); cb != "" {
return errors.Errorf("context builder cannot be removed, run `docker context rm %s` to remove this context", cb)
}
if cb := b.ContextName(); cb != "" {
return errors.Errorf("context builder cannot be removed, run `docker context rm %s` to remove this context", cb)
}

err1 := rm(ctx, nodes, in)
if err := txn.Remove(b.Name); err != nil {
return err
}
if err1 != nil {
return err1
err1 := rm(ctx, nodes, in)
if err := txn.Remove(b.Name); err != nil {
return err
}
if err1 != nil {
return err1
}

_, _ = fmt.Fprintf(dockerCli.Err(), "%s removed\n", b.Name)
return nil
})
}(name)
}

_, _ = fmt.Fprintf(dockerCli.Err(), "%s removed\n", b.Name)
return nil
return eg.Wait()
}

func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
Expand All @@ -84,12 +93,12 @@ func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
Short: "Remove a builder instance",
Args: cli.RequiresMaxArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
options.builder = rootOpts.builder
options.builder = []string{rootOpts.builder}
if len(args) > 0 {
if options.allInactive {
return errors.New("cannot specify builder name when --all-inactive is set")
}
options.builder = args[0]
options.builder = args
}
return runRm(dockerCli, options)
},
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/buildx_rm.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# buildx rm

```text
docker buildx rm [NAME]
docker buildx rm [OPTIONS] [NAME] [NAME...]
```

<!---MARKER_GEN_START-->
Expand Down
12 changes: 12 additions & 0 deletions tests/create.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package tests

import (
"github.com/moby/buildkit/util/testutil/integration"
)

func createCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
opts = append([]cmdOpt{withArgs("create")}, opts...)
cmd := buildxCmd(sb, opts...)
out, err := cmd.CombinedOutput()
return string(out), err
}
1 change: 1 addition & 0 deletions tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestIntegration(t *testing.T) {
tests = append(tests, lsTests...)
tests = append(tests, imagetoolsTests...)
tests = append(tests, versionTests...)
tests = append(tests, rmTests...)
testIntegration(t, tests...)
}

Expand Down
60 changes: 60 additions & 0 deletions tests/rm.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package tests

import (
"strings"
"testing"

"github.com/moby/buildkit/util/testutil/integration"
"github.com/stretchr/testify/require"
)

func rmCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
opts = append([]cmdOpt{withArgs("rm")}, opts...)
cmd := buildxCmd(sb, opts...)
out, err := cmd.CombinedOutput()
return string(out), err
}

var rmTests = []func(t *testing.T, sb integration.Sandbox){
testRm,
testRmMulti,
}

func testRm(t *testing.T, sb integration.Sandbox) {
if sb.Name() != "docker-container" {
t.Skip("only testing for docker-container driver")
}

out, err := rmCmd(sb, withArgs("default"))
require.Error(t, err, out)

out, err = createCmd(sb, withArgs("--driver", "docker-container"))
require.NoError(t, err, out)
builderName := strings.TrimSpace(out)

out, err = inspectCmd(sb, withArgs(builderName, "--bootstrap"))
require.NoError(t, err, out)

out, err = rmCmd(sb, withArgs(builderName))
require.NoError(t, err, out)
}

func testRmMulti(t *testing.T, sb integration.Sandbox) {
if sb.Name() != "docker-container" {
t.Skip("only testing for docker-container driver")
}

var builderNames []string
for i := 0; i < 3; i++ {
out, err := createCmd(sb, withArgs("--driver", "docker-container"))
require.NoError(t, err, out)
builderName := strings.TrimSpace(out)

out, err = inspectCmd(sb, withArgs(builderName, "--bootstrap"))
require.NoError(t, err, out)
builderNames = append(builderNames, builderName)
}

out, err := rmCmd(sb, withArgs(builderNames...))
require.NoError(t, err, out)

Check failure on line 59 in tests/rm.go

View workflow job for this annotation

GitHub Actions / test (docker-container, ./tests)

Failed: tests/TestIntegration/TestRmMulti/worker=docker-container

=== RUN TestIntegration/TestRmMulti/worker=docker-container === PAUSE TestIntegration/TestRmMulti/worker=docker-container === CONT TestIntegration/TestRmMulti/worker=docker-container rm.go:59: Error Trace: /src/tests/rm.go:59 /src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:91 /src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:205 Error: Received unexpected error: exit status 1 Test: TestIntegration/TestRmMulti/worker=docker-container Messages: ERROR: "buildx rm" requires at most 1 argument. See 'buildx rm --help'. Usage: buildx rm [NAME] [flags] Remove a builder instance --- FAIL: TestIntegration/TestRmMulti/worker=docker-container (8.42s)
}

0 comments on commit 98f6161

Please sign in to comment.