rm: clean up all nodes before returning errors

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

tests/rm.go

@@ -8,6 +8,7 @@ import (
 	"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"
@@ -25,6 +26,8 @@ var rmTests = []func(t *testing.T, sb integration.Sandbox){
 	testRmMulti,
 	testRmInvalidBuildkitdConfig,
 	testRmAllInactiveInvalidBuildkitdConfig,
+	testRmUnreachableEndpoint,
+	testRmUnreachableRemoteEndpoint,
 }
 
 func testRm(t *testing.T, sb integration.Sandbox) {
@@ -34,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)
@@ -155,6 +159,65 @@ func testRmAllInactiveInvalidBuildkitdConfig(t *testing.T, sb integration.Sandbo
 	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()