diff --git a/.gitignore b/.gitignore index 4b82c6d1521..fc0194ad3a4 100644 --- a/.gitignore +++ b/.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 diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 281fa699f6e..8e813c2c94d 100644 --- a/acceptance/acceptance_test.go +++ b/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,14 +717,14 @@ func getSkipReason(config *internal.TestConfig, configPath string) string { // Cap at 11 digits: the prefix "cix" 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 "cix" 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 @@ -733,28 +732,34 @@ const bundleLegSuffixLen = 4 // empty off cloud where names need no attribution. var bundleNamePrefix string -// newBundleNamePrefix builds the "cix" 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 "cix" 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 "cix" 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 "cix" 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 diff --git a/acceptance/bundle_clean_test.go b/acceptance/cleanup/main.go similarity index 58% rename from acceptance/bundle_clean_test.go rename to acceptance/cleanup/main.go index bb1392173f1..d7e914a5f98 100644 --- a/acceptance/bundle_clean_test.go +++ b/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 "cix" 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,17 +119,11 @@ 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 @@ -129,9 +131,9 @@ func cleanBundles(ctx context.Context, t *testing.T, execPath, prefix string) { // 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// 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 }