acc: Extract bundle cleanup in a separate test

.gitignore

@@ -9,6 +9,7 @@
 # Root binary from a bare `go build`; anchored so it doesn't also ignore
 # nested paths like .nextchanges/cli/.
 /cli
+/cleanup
 
 # Test binary, built with `go test -c`
 *.test

acceptance/acceptance_test.go

@@ -459,13 +459,12 @@ func testAccept(t *testing.T, inprocessMode bool, selectedTests []string, skipTo
 	t.Setenv("NODE_TYPE_ID", nodeTypeID)
 	repls.Set(nodeTypeID, "[NODE_TYPE_ID]")
 
-	// On cloud, tag every $UNIQUE_NAME with a per-process prefix (see
-	// newBundleNamePrefix) so this leg's bundles can be attributed and swept, and
-	// destroy them once all tests finish. Registered before the tests are spawned
-	// so it runs after they complete. Off cloud names need no attribution.
+	// On cloud, tag every $UNIQUE_NAME with a per-run prefix (see newBundleNamePrefix)
+	// so this run's bundles can be attributed and swept by the post-run cleanup step
+	// (TestCleanupLeakedBundles) after every matrix leg finishes. Off cloud names need
+	// no attribution: each local test gets a throwaway in-memory fake workspace.
 	if cloudEnv != "" {
 		bundleNamePrefix = newBundleNamePrefix()
-		setupBundleCleanup(t, execPath, bundleNamePrefix)
 	}
 
 	testDirs := getTests(t)
@@ -718,43 +717,49 @@ func getSkipReason(config *internal.TestConfig, configPath string) string {
 
 // Cap at 11 digits: the prefix "ci<runID>x<suffix>" plus the 8-char random
 // minimum must fit the 26-char unique name (26 - 8 - len("ci")-len("x") -
-// bundleLegSuffixLen = 11), so a longer GITHUB_RUN_ID falls through to a random
-// id rather than building a prefix ciUniqueName would silently drop.
+// bundleLegSuffixLen = 11), so a longer GITHUB_RUN_ID is treated as absent
+// rather than building a prefix ciUniqueName would silently drop.
 var ciRunID = regexp.MustCompile(`^[0-9]{1,11}$`)
 
-// bundleLegSuffixLen is the length of the per-process random suffix. 36^4 values
-// keep an accidental collision between the few matrix legs that share a workspace
-// within one run (which would let one leg destroy another's live bundles)
-// negligible, while still leaving >=8 random characters after an 11-digit run id.
+// bundleLegSuffixLen is the length of the per-process random suffix that keeps
+// each matrix leg's bundle names distinct within a run (useful when eyeballing
+// leaked deployments). The run-wide cleanup matches on the "ci<runID>x" prefix
+// alone, so it sweeps every leg regardless of the suffix.
 const bundleLegSuffixLen = 4
 
 // bundleNamePrefix is the sweepable prefix embedded into every $UNIQUE_NAME on
 // cloud runs (see newBundleNamePrefix / ciUniqueName). Set once in testAccept,
 // empty off cloud where names need no attribution.
 var bundleNamePrefix string
 
-// newBundleNamePrefix builds the "ci<runID>x<suffix>" prefix that attributes
-// deployed bundles to this test process so cleanup can sweep them.
-//
-// runID is the GitHub run id, or a random numeric id when it is unset/malformed
-// (e.g. a local `deco env run`). All matrix legs of a CI run share one GitHub run
-// id and legs of different OSes share a workspace, so a run-id-only prefix would
-// let one leg's cleanup destroy another leg's live bundles; the random
-// lowercase-alphanumeric suffix (bundleLegSuffixLen chars) makes each leg's prefix
-// distinct while keeping "ci<runID>x" a matchable substring for the run-wide
-// sweeper (sweep_test_resources.py). The run id (all digits) is delimited by "x"
-// so that prefix stays collision-free between runs whose ids share a prefix.
-func newBundleNamePrefix() string {
+// ciRunPrefix returns the run-wide "ci<runID>x" prefix that attributes every
+// bundle a cloud run deploys to its GitHub run, so they can be swept by
+// acceptance/cleanup and tools/sweep_test_resources.py. The run id (all digits)
+// is delimited by "x" so the prefix stays collision-free between runs whose ids
+// share a leading substring. Returns "" when GITHUB_RUN_ID is unset or not a
+// valid numeric id (e.g. a local `deco env run`).
+func ciRunPrefix() string {
 	runID := os.Getenv("GITHUB_RUN_ID")
 	if !ciRunID.MatchString(runID) {
-		runID = strconv.Itoa(rand.IntN(1_000_000_000))
+		return ""
+	}
+	return "ci" + runID + "x"
+}
+
+// newBundleNamePrefix builds the "ci<runID>x<suffix>" prefix that ciUniqueName
+// stamps into every $UNIQUE_NAME so deployed bundles can be attributed and swept.
+// Returns "" on non-CI runs (no GITHUB_RUN_ID), where cleanup is not automatic.
+func newBundleNamePrefix() string {
+	prefix := ciRunPrefix()
+	if prefix == "" {
+		return ""
 	}
 	const alphabet = "0123456789abcdefghijklmnopqrstuvwxyz"
 	suffix := make([]byte, bundleLegSuffixLen)
 	for i := range suffix {
 		suffix[i] = alphabet[rand.IntN(len(alphabet))]
 	}
-	return "ci" + runID + "x" + string(suffix)
+	return prefix + string(suffix)
 }
 
 // ciUniqueName prepends prefix to the random unique name, preserving its length

acceptance/bundle_clean_test.go → acceptance/cleanup/main.go

@@ -1,61 +1,69 @@
-package acceptance_test
+// Package main implements a standalone bundle cleanup program. It is invoked as a
+// separate always()-triggered workflow job (see cli-isolated-tests.yml in
+// databricks-eng/eng-dev-ecosystem) so it runs even when a test job times out or
+// is cancelled — unlike a t.Cleanup, which go test skips in those cases.
+package main
 
 import (
 	"context"
 	"errors"
+	"flag"
+	"fmt"
 	"os"
 	"os/exec"
 	"path"
 	"path/filepath"
+	"regexp"
 	"slices"
 	"strings"
 	"sync"
-	"testing"
 	"time"
 
+	"github.com/databricks/cli/libs/env"
+	"github.com/databricks/cli/libs/log"
 	"github.com/databricks/databricks-sdk-go"
 	"github.com/databricks/databricks-sdk-go/apierr"
 	"github.com/databricks/databricks-sdk-go/service/iam"
 	"github.com/databricks/databricks-sdk-go/service/workspace"
 )
 
-// setupBundleCleanup arranges for every bundle deployed by this run to be
-// destroyed once the suite finishes. The caller invokes it only on cloud: all
-// cloud tests share one real workspace, whereas local tests each get a
-// throwaway in-memory fake workspace with nothing to clean up.
-//
-// prefix is the leg-specific "ci<runID>x<legSuffix>" that ciUniqueName stamps
-// into every $UNIQUE_NAME, so the cleanup sweeps exactly the deployments this
-// leg created and nothing else. That is what makes destroying against the shared
-// workspace safe even while sibling matrix legs (which share the run id and may
-// share the workspace) deploy concurrently.
-func setupBundleCleanup(t *testing.T, execPath, prefix string) {
-	// t.Context() is canceled once the test finishes, before cleanups run, so
-	// derive a context that survives cancellation for the cleanup's API calls.
-	ctx := context.WithoutCancel(t.Context())
-	t.Cleanup(func() {
-		cleanBundles(ctx, t, execPath, prefix)
-	})
+// ciRunID matches a valid GITHUB_RUN_ID (same cap as acceptance_test.go).
+var ciRunID = regexp.MustCompile(`^[0-9]{1,11}$`)
+
+func main() {
+	ctx := context.Background()
+	var cliPath string
+	flag.StringVar(&cliPath, "cli", "", "path to databricks CLI binary (required)")
+	flag.Parse()
+
+	if cliPath == "" {
+		log.Errorf(ctx, "-cli: path to databricks CLI binary is required")
+	}
+
+	runID := env.Get(ctx, "GITHUB_RUN_ID")
+	if !ciRunID.MatchString(runID) {
+		log.Errorf(ctx, "GITHUB_RUN_ID %q is not a valid run id (must be 1-11 digits)", runID)
+	}
+	prefix := "ci" + runID + "x"
+
+	if err := cleanBundles(ctx, cliPath, prefix); err != nil {
+		log.Errorf(ctx, "failed to clean bundles: %s", err)
+	}
 }
 
 // cleanBundles finds every bundle this run deployed under the current user's
-// ~/.bundle directory (identified by the run's prefix) and destroys each one,
-// logging each deployment and the total time taken.
-func cleanBundles(ctx context.Context, t *testing.T, execPath, prefix string) {
+// ~/.bundle directory (identified by the run's prefix) and destroys each one.
+func cleanBundles(ctx context.Context, execPath, prefix string) error {
 	start := time.Now()
 
-	// Cleanup never fails the test (see the WARNING note below), so on any error
-	// that prevents sweeping, log loudly and return rather than require-failing.
 	w, err := databricks.NewWorkspaceClient()
 	if err != nil {
-		t.Logf("WARNING: bundle cleanup skipped, cannot create client: %s", err)
-		return
+		return fmt.Errorf("cannot create workspace client: %w", err)
 	}
 
 	me, err := w.CurrentUser.Me(ctx, iam.MeRequest{})
 	if err != nil {
-		t.Logf("WARNING: bundle cleanup skipped, cannot resolve current user: %s", err)
-		return
+		return fmt.Errorf("cannot resolve current user: %w", err)
 	}
 
 	// Tests deploy under the user's home .bundle by default, but some set
@@ -73,15 +81,15 @@ func cleanBundles(ctx context.Context, t *testing.T, execPath, prefix string) {
 	// thousands of directories other runs may have leaked under .bundle.
 	var roots []string
 	for _, bundleRoot := range bundleRoots {
-		for _, child := range listChildDirs(ctx, t, w, bundleRoot) {
+		for _, child := range listChildDirs(ctx, w, bundleRoot) {
 			if strings.Contains(path.Base(child), prefix) {
-				roots = append(roots, findDeploymentRoots(ctx, t, w, child)...)
+				roots = append(roots, findDeploymentRoots(ctx, w, child)...)
 			}
 		}
 	}
 	slices.Sort(roots)
 
-	t.Logf("%s bundle cleanup: found %d deployment(s) with prefix %q", time.Now().Format(time.RFC3339), len(roots), prefix)
+	log.Infof(ctx, "bundle cleanup: found %d deployment(s) with prefix %q", len(roots), prefix)
 
 	// Each destroy shells out to a separate `bundle destroy` (auth + state pull +
 	// deletes), so run them concurrently. Each is network-bound (not CPU-bound),
@@ -99,9 +107,9 @@ func cleanBundles(ctx context.Context, t *testing.T, execPath, prefix string) {
 		sem <- struct{}{}
 		wg.Go(func() {
 			defer func() { <-sem }()
-			t.Logf("%s destroying %s", time.Now().Format(time.RFC3339), root)
+			log.Infof(ctx, "destroying %s", root)
 			if out, err := destroyBundle(execPath, root); err != nil {
-				t.Logf("%s destroy failed: %s\n%s", time.Now().Format(time.RFC3339), root, out)
+				log.Infof(ctx, "destroy failed: %s\n%s", root, out)
 				mu.Lock()
 				failed = append(failed, root)
 				mu.Unlock()
@@ -111,27 +119,21 @@ func cleanBundles(ctx context.Context, t *testing.T, execPath, prefix string) {
 	wg.Wait()
 
 	slices.Sort(failed)
-	t.Logf("%s bundle cleanup: destroyed %d/%d deployment(s) in %s", time.Now().Format(time.RFC3339), len(roots)-len(failed), len(roots), time.Since(start))
-
-	// Do not fail the test on a cleanup failure: this runs in a t.Cleanup on the
-	// root TestAccept, so failing here marks the root test failed with no failed
-	// subtest, which makes gotestsum --rerun-fails (used by the integration task)
-	// rerun the entire cloud suite. Cleanup is best-effort housekeeping and the
-	// product tests already passed, so log loudly instead; leaked deployments are
-	// reclaimed by the periodic prefix sweep (sweep_test_resources.py).
+	log.Infof(ctx, "bundle cleanup: destroyed %d/%d deployment(s) in %s", len(roots)-len(failed), len(roots), time.Since(start))
 	if len(failed) > 0 {
-		t.Logf("WARNING: bundle cleanup failed to destroy %d deployment(s), leaked until swept: %s", len(failed), strings.Join(failed, ", "))
+		return fmt.Errorf("failed to destroy %d deployment(s): %s", len(failed), strings.Join(failed, ", "))
 	}
+	return nil
 }
 
 // findDeploymentRoots walks the workspace tree under dir and returns the paths
 // of bundle deployment roots. A directory is a deployment root when it contains
 // a "state" or "files" child, which the bundle deploy writes beneath the
 // resolved workspace.root_path. This works regardless of whether the root is
 // the default ~/.bundle/<name>/<target> or a custom ~/.bundle/<...> override.
-func findDeploymentRoots(ctx context.Context, t *testing.T, w *databricks.WorkspaceClient, dir string) []string {
+func findDeploymentRoots(ctx context.Context, w *databricks.WorkspaceClient, dir string) []string {
 	var childDirs []string
-	for _, child := range listChildDirs(ctx, t, w, dir) {
+	for _, child := range listChildDirs(ctx, w, dir) {
 		if base := path.Base(child); base == "state" || base == "files" {
 			// dir is a deployment root; don't descend into its internals.
 			return []string{dir}
@@ -141,20 +143,19 @@ func findDeploymentRoots(ctx context.Context, t *testing.T, w *databricks.Worksp
 
 	var roots []string
 	for _, child := range childDirs {
-		roots = append(roots, findDeploymentRoots(ctx, t, w, child)...)
+		roots = append(roots, findDeploymentRoots(ctx, w, child)...)
 	}
 	return roots
 }
 
 // listChildDirs returns the immediate subdirectory paths of dir. A missing dir
 // (nothing was deployed under it) yields nil silently; any other listing error
-// is logged loudly (it means the sweep under dir is incomplete) but does not
-// fail the test, since cleanup runs in the root t.Cleanup.
-func listChildDirs(ctx context.Context, t *testing.T, w *databricks.WorkspaceClient, dir string) []string {
+// is logged loudly but does not stop the overall sweep.
+func listChildDirs(ctx context.Context, w *databricks.WorkspaceClient, dir string) []string {
 	objects, err := w.Workspace.ListAll(ctx, workspace.ListWorkspaceRequest{Path: dir})
 	if err != nil {
 		if !errors.Is(err, apierr.ErrNotFound) {
-			t.Logf("WARNING: bundle cleanup incomplete, cannot list %s: %s", dir, err)
+			log.Infof(ctx, "WARNING: bundle cleanup incomplete, cannot list %s: %s", dir, err)
 		}
 		return nil
 	}
@@ -176,7 +177,7 @@ func listChildDirs(ctx context.Context, t *testing.T, w *databricks.WorkspaceCli
 // stale deployment lock left by a test that was killed mid-deploy; these are
 // known-leaked bundles, so there is no concurrent deployment to conflict with.
 func destroyBundle(execPath, rootPath string) ([]byte, error) {
-	dir, err := os.MkdirTemp("", "bundle-clean") //nolint:usetesting // runs in a cleanup, where t.TempDir is already removed
+	dir, err := os.MkdirTemp("", "bundle-clean")
 	if err != nil {
 		return nil, err
 	}