alextab93/buildx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Merge pull request #3934 from crazy-max/fix-rm-broken-builders

Browse Source 
rm: handle broken builders during removal
This commit is contained in:
Tõnis Tiigi
2026-07-06 16:29:24 -07:00
committed by GitHub
parent cd58eac2cb 8db022122d
commit 80b990a339
3 changed files with 256 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
builder/node.go
+16 -6
View File
@@ -47,9 +47,10 @@ func (b *Builder) Nodes() []Node {
type LoadNodesOption func(*loadNodesOptions)
type loadNodesOptions struct {
data bool
dialMeta map[string][]string
clientOpt []client.ClientOpt
data bool
skipImageOpt bool
dialMeta map[string][]string
clientOpt []client.ClientOpt
}
func WithData() LoadNodesOption {
@@ -58,6 +59,12 @@ func WithData() LoadNodesOption {
}
}
func WithSkippedImageOpt() LoadNodesOption {
return func(o *loadNodesOptions) {
o.skipImageOpt = true
}
}
func WithDialMeta(dialMeta map[string][]string) LoadNodesOption {
return func(o *loadNodesOptions) {
o.dialMeta = dialMeta
@@ -94,9 +101,12 @@ func (b *Builder) LoadNodes(ctx context.Context, opts ...LoadNodesOption) (_ []N
return nil, err
}
imageopt, err := b.ImageOpt()
if err != nil {
return nil, err
var imageopt imagetools.Opt
if !lno.skipImageOpt {
imageopt, err = b.ImageOpt()
if err != nil {
return nil, err
}
}
for i, n := range b.NodeGroup.Nodes {
commands/rm.go
+42 -19
View File
@@ -2,6 +2,7 @@ package commands
import (
"context"
stderrors "errors"
"fmt"
"time"
@@ -78,12 +79,19 @@ func runRm(ctx context.Context, dockerCli command.Cli, in rmOptions) error {
return errors.Errorf("context builder cannot be removed, run `docker context rm %s` to remove this context", b.Name)
}
nodes, err := b.LoadNodes(timeoutCtx)
if in.keepDaemon {
return txn.Remove(b.Name)
}
nodes, err := b.LoadNodes(timeoutCtx, builder.WithSkippedImageOpt())
if err != nil {
if err1 := txn.Remove(b.Name); err1 != nil {
return err1
}
return err
}
err1 := rm(ctx, nodes, in)
err1 := rm(timeoutCtx, nodes, in)
if err := txn.Remove(b.Name); err != nil {
return err
}
@@ -133,24 +141,36 @@ func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
}
func rm(ctx context.Context, nodes []builder.Node, in rmOptions) (err error) {
errCh := make(chan error, len(nodes)*3)
var eg errgroup.Group
for _, node := range nodes {
if node.Driver == nil {
continue
}
// Do not stop the buildkitd daemon when --keep-daemon is provided
if !in.keepDaemon {
if err := node.Driver.Stop(ctx, true); err != nil {
return err
eg.Go(func() error {
if node.Err != nil {
errCh <- errors.Wrapf(node.Err, "failed to load node %s", node.Name)
}
}
if err := node.Driver.Rm(ctx, true, !in.keepState, !in.keepDaemon); err != nil {
return err
}
if node.Err != nil {
err = node.Err
}
if node.Driver == nil {
return nil
}
// Do not stop the buildkitd daemon when --keep-daemon is provided
if !in.keepDaemon {
if err := node.Driver.Stop(ctx, true); err != nil {
errCh <- errors.Wrapf(err, "failed to stop node %s", node.Name)
}
}
if err := node.Driver.Rm(ctx, true, !in.keepState, !in.keepDaemon); err != nil {
errCh <- errors.Wrapf(err, "failed to remove node %s", node.Name)
}
return nil
})
}
return err
_ = eg.Wait()
close(errCh)
var errs []error
for err := range errCh {
errs = append(errs, err)
}
return stderrors.Join(errs...)
}
func rmAllInactive(ctx context.Context, txn *store.Txn, dockerCli command.Cli, in rmOptions) error {
@@ -169,7 +189,10 @@ func rmAllInactive(ctx context.Context, txn *store.Txn, dockerCli command.Cli, i
for _, b := range builders {
func(b *builder.Builder) {
eg.Go(func() error {
nodes, err := b.LoadNodes(timeoutCtx, builder.WithData())
if b.DockerContext {
return nil
}
nodes, err := b.LoadNodes(timeoutCtx, builder.WithData(), builder.WithSkippedImageOpt())
if err != nil {
return errors.Wrapf(err, "cannot load %s", b.Name)
}
@@ -177,7 +200,7 @@ func rmAllInactive(ctx context.Context, txn *store.Txn, dockerCli command.Cli, i
return nil
}
if b.Inactive() {
rmerr := rm(ctx, nodes, in)
rmerr := rm(timeoutCtx, nodes, in)
if err := txn.Remove(b.Name); err != nil {
return err
}
tests/rm.go
+198
View File
@@ -1,10 +1,16 @@
package tests
import (
"os"
"strings"
"testing"
"github.com/docker/buildx/driver"
"github.com/docker/buildx/store"
"github.com/docker/buildx/util/confutil"
"github.com/moby/buildkit/identity"
"github.com/moby/buildkit/util/testutil/integration"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
)
@@ -18,6 +24,10 @@ func rmCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
var rmTests = []func(t *testing.T, sb integration.Sandbox){
testRm,
testRmMulti,
testRmInvalidBuildkitdConfig,
testRmAllInactiveInvalidBuildkitdConfig,
testRmUnreachableEndpoint,
testRmUnreachableRemoteEndpoint,
}
func testRm(t *testing.T, sb integration.Sandbox) {
@@ -27,6 +37,7 @@ func testRm(t *testing.T, sb integration.Sandbox) {
out, err := rmCmd(sb, withArgs("default"))
require.Error(t, err, out) // can't remove a docker builder
require.Contains(t, out, "context builder cannot be removed")
out, err = createCmd(sb, withArgs("--driver", "docker-container"))
require.NoError(t, err, out)
@@ -58,3 +69,190 @@ func testRmMulti(t *testing.T, sb integration.Sandbox) {
out, err := rmCmd(sb, withArgs(builderNames...))
require.NoError(t, err, out)
}
func testRmInvalidBuildkitdConfig(t *testing.T, sb integration.Sandbox) {
if !isDockerContainerWorker(sb) {
t.Skip("only testing with docker-container worker")
}
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)
var container string
t.Cleanup(func() {
if builderName != "" {
_, _ = rmCmd(sb, withArgs("--keep-daemon", builderName))
}
if container != "" {
_ = dockerCmd(sb, withArgs("container", "rm", "-f", container)).Run()
}
})
updateStoredBuilder(t, sb, builderName, func(ng *store.NodeGroup) {
require.NotEmpty(t, ng.Nodes)
container = driver.BuilderName(ng.Nodes[0].Name)
if ng.Nodes[0].Files == nil {
ng.Nodes[0].Files = map[string][]byte{}
}
ng.Nodes[0].Files["buildkitd.toml"] = []byte(`
[worker.oci]
gc = "maybe"
`)
})
out, err = rmCmd(sb, withArgs(builderName))
require.NoError(t, err, out)
require.Contains(t, out, builderName+" removed")
requireNoStoredBuilder(t, sb, builderName)
requireNoContainer(t, sb, container)
builderName = ""
}
func testRmAllInactiveInvalidBuildkitdConfig(t *testing.T, sb integration.Sandbox) {
if !isDockerContainerWorker(sb) {
t.Skip("only testing with docker-container worker")
}
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)
var container string
t.Cleanup(func() {
if builderName != "" {
_, _ = rmCmd(sb, withArgs("--keep-daemon", builderName))
}
if container != "" {
_ = dockerCmd(sb, withArgs("container", "rm", "-f", container)).Run()
}
})
updateStoredBuilder(t, sb, builderName, func(ng *store.NodeGroup) {
require.NotEmpty(t, ng.Nodes)
container = driver.BuilderName(ng.Nodes[0].Name)
if ng.Nodes[0].Files == nil {
ng.Nodes[0].Files = map[string][]byte{}
}
ng.Nodes[0].Files["buildkitd.toml"] = []byte(`
[worker.oci]
gc = "maybe"
`)
})
cmd := dockerCmd(sb, withArgs("container", "stop", container))
require.NoError(t, cmd.Run())
out, err = rmCmd(sb, withArgs("--all-inactive", "--force"))
require.NoError(t, err, out)
require.Contains(t, out, builderName+" removed")
requireNoStoredBuilder(t, sb, builderName)
requireNoContainer(t, sb, container)
builderName = ""
}
func testRmUnreachableEndpoint(t *testing.T, sb integration.Sandbox) {
if !isDockerContainerWorker(sb) {
t.Skip("only testing with docker-container worker")
}
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)
t.Cleanup(func() {
if builderName == "" {
return
}
_, _ = rmCmd(sb, withArgs("--keep-daemon", builderName))
})
var goodContainer string
updateStoredBuilder(t, sb, builderName, func(ng *store.NodeGroup) {
require.NotEmpty(t, ng.Nodes)
goodContainer = driver.BuilderName(ng.Nodes[0].Name)
badNode := ng.Nodes[0]
badNode.Name += "-unreachable"
badNode.Endpoint = "tcp://127.0.0.1:1"
ng.Nodes = append([]store.Node{badNode}, ng.Nodes...)
})
out, err = rmCmd(sb, withArgs("--timeout=2s", builderName))
require.Error(t, err, out)
require.Contains(t, out, "failed to remove "+builderName)
requireNoStoredBuilder(t, sb, builderName)
requireNoContainer(t, sb, goodContainer)
builderName = ""
}
func testRmUnreachableRemoteEndpoint(t *testing.T, sb integration.Sandbox) {
if !isRemoteWorker(sb) || isRemoteMultiNodeWorker(sb) {
t.Skip("only testing with remote worker")
}
builderName := "remote-" + identity.NewID()
out, err := createCmd(sb, withArgs("--driver", "remote", "--name", builderName, "--timeout=2s", "tcp://127.0.0.1:1"))
require.NoError(t, err, out)
t.Cleanup(func() {
if builderName != "" {
_, _ = rmCmd(sb, withArgs("--timeout=2s", builderName))
}
})
out, err = rmCmd(sb, withArgs("--timeout=2s", builderName))
require.NoError(t, err, out)
require.Contains(t, out, builderName+" removed")
requireNoStoredBuilder(t, sb, builderName)
builderName = ""
}
func updateStoredBuilder(t *testing.T, sb integration.Sandbox, name string, fn func(*store.NodeGroup)) {
t.Helper()
st, err := store.New(confutil.NewConfig(nil, confutil.WithDir(buildxConfig(sb))))
require.NoError(t, err)
txn, release, err := st.Txn()
require.NoError(t, err)
defer release()
ng, err := txn.NodeGroupByName(name)
require.NoError(t, err)
fn(ng)
require.NoError(t, txn.Save(ng))
}
func requireNoStoredBuilder(t *testing.T, sb integration.Sandbox, name string) {
t.Helper()
st, err := store.New(confutil.NewConfig(nil, confutil.WithDir(buildxConfig(sb))))
require.NoError(t, err)
txn, release, err := st.Txn()
require.NoError(t, err)
defer release()
_, err = txn.NodeGroupByName(name)
require.Error(t, err)
require.True(t, os.IsNotExist(errors.Cause(err)), "expected builder %q to be removed: %v", name, err)
}
func requireNoContainer(t *testing.T, sb integration.Sandbox, name string) {
t.Helper()
cmd := dockerCmd(sb, withArgs("container", "inspect", name))
require.Error(t, cmd.Run())
}