Skip to content

Commit 699df7c

Browse files
authored
acc: Destroy leaked bundles after cloud test runs (#6092)
## Why Cloud acceptance tests deploy bundles under the shared test workspace's ~/.bundle directory. Tests that crash before their cleanup trap fires leak their deployments, which accumulate and eventually exhaust the workspace child-node limit. ## Changes A suite-level cleanup runs after all tests finish and destroys every bundle this run deployed, keyed off the "ci<runid>x" prefix that ciUniqueName embeds into $UNIQUE_NAME (synthesizing a run id for non-CI cloud runs). It sweeps only this run's deployments, so it is safe while other runs deploy concurrently. Local runs are unaffected — each gets its own in-memory fake workspace. ## Tests Verified on gcp-cli: a bundle deployed without a destroy step is found and destroyed by the cleanup.
1 parent 8100d87 commit 699df7c

3 files changed

Lines changed: 267 additions & 30 deletions

File tree

acceptance/acceptance_test.go

Lines changed: 64 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"io"
1313
"io/fs"
1414
"maps"
15+
"math/rand/v2"
1516
"net/http"
1617
"os"
1718
"os/exec"
@@ -427,6 +428,15 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int {
427428
t.Setenv("NODE_TYPE_ID", nodeTypeID)
428429
repls.Set(nodeTypeID, "[NODE_TYPE_ID]")
429430

431+
// On cloud, tag every $UNIQUE_NAME with a per-process prefix (see
432+
// newBundleNamePrefix) so this leg's bundles can be attributed and swept, and
433+
// destroy them once all tests finish. Registered before the tests are spawned
434+
// so it runs after they complete. Off cloud names need no attribution.
435+
if cloudEnv != "" {
436+
bundleNamePrefix = newBundleNamePrefix()
437+
setupBundleCleanup(t, execPath, bundleNamePrefix)
438+
}
439+
430440
testDirs := getTests(t)
431441
require.NotEmpty(t, testDirs)
432442

@@ -721,27 +731,61 @@ func getSkipReason(config *internal.TestConfig, configPath, dir, skipLocalMode s
721731
return ""
722732
}
723733

724-
var ciRunID = regexp.MustCompile(`^[0-9]{1,16}$`)
725-
726-
// ciUniqueName embeds a CI run id into the random unique name as "ci<runID>x<random>".
727-
// The result stays purely lowercase-alphanumeric like the base32 name it replaces, so it
728-
// remains valid everywhere $UNIQUE_NAME is used: app names (no hyphens would be fine but
729-
// underscores/uppercase are not), Python and Unity Catalog identifiers (no hyphens). No
730-
// punctuation separator works for all of them, so the run id (all digits) is delimited by
731-
// the letter "x", which also keeps the sweep prefix "ci<runID>x" collision-free between
732-
// runs whose ids share a prefix. Length is preserved ("app-$UNIQUE_NAME" is exactly the
733-
// 30-char app name maximum). Returns random unchanged when runID is absent, malformed, or
734-
// too long to leave at least 8 random characters.
735-
func ciUniqueName(runID, random string) string {
734+
// Cap at 11 digits: the prefix "ci<runID>x<suffix>" plus the 8-char random
735+
// minimum must fit the 26-char unique name (26 - 8 - len("ci")-len("x") -
736+
// bundleLegSuffixLen = 11), so a longer GITHUB_RUN_ID falls through to a random
737+
// id rather than building a prefix ciUniqueName would silently drop.
738+
var ciRunID = regexp.MustCompile(`^[0-9]{1,11}$`)
739+
740+
// bundleLegSuffixLen is the length of the per-process random suffix. 36^4 values
741+
// keep an accidental collision between the few matrix legs that share a workspace
742+
// within one run (which would let one leg destroy another's live bundles)
743+
// negligible, while still leaving >=8 random characters after an 11-digit run id.
744+
const bundleLegSuffixLen = 4
745+
746+
// bundleNamePrefix is the sweepable prefix embedded into every $UNIQUE_NAME on
747+
// cloud runs (see newBundleNamePrefix / ciUniqueName). Set once in testAccept,
748+
// empty off cloud where names need no attribution.
749+
var bundleNamePrefix string
750+
751+
// newBundleNamePrefix builds the "ci<runID>x<suffix>" prefix that attributes
752+
// deployed bundles to this test process so cleanup can sweep them.
753+
//
754+
// runID is the GitHub run id, or a random numeric id when it is unset/malformed
755+
// (e.g. a local `deco env run`). All matrix legs of a CI run share one GitHub run
756+
// id and legs of different OSes share a workspace, so a run-id-only prefix would
757+
// let one leg's cleanup destroy another leg's live bundles; the random
758+
// lowercase-alphanumeric suffix (bundleLegSuffixLen chars) makes each leg's prefix
759+
// distinct while keeping "ci<runID>x" a matchable substring for the run-wide
760+
// sweeper (sweep_test_resources.py). The run id (all digits) is delimited by "x"
761+
// so that prefix stays collision-free between runs whose ids share a prefix.
762+
func newBundleNamePrefix() string {
763+
runID := os.Getenv("GITHUB_RUN_ID")
736764
if !ciRunID.MatchString(runID) {
737-
return random
765+
runID = strconv.Itoa(rand.IntN(1_000_000_000))
738766
}
739-
prefix := "ci" + runID + "x"
740-
randLen := len(random) - len(prefix)
741-
if randLen < 8 {
742-
return random
767+
const alphabet = "0123456789abcdefghijklmnopqrstuvwxyz"
768+
suffix := make([]byte, bundleLegSuffixLen)
769+
for i := range suffix {
770+
suffix[i] = alphabet[rand.IntN(len(alphabet))]
743771
}
744-
return prefix + random[:randLen]
772+
return "ci" + runID + "x" + string(suffix)
773+
}
774+
775+
// ciUniqueName prepends prefix to the random unique name, preserving its length
776+
// (e.g. "app-$UNIQUE_NAME" is exactly the 30-char app name maximum) and its
777+
// lowercase-alphanumeric shape so it stays valid everywhere $UNIQUE_NAME is used
778+
// (app names, Python and Unity Catalog identifiers). Returns random unchanged
779+
// when prefix is empty or too long to leave at least 8 random characters.
780+
func ciUniqueName(prefix, random string) string {
781+
// newBundleNamePrefix keeps the prefix short enough to leave at least this
782+
// many random characters (empty prefix trivially fits); a longer one is a bug.
783+
const minRandom = 8
784+
cut := len(random) - len(prefix)
785+
if cut < minRandom {
786+
panic(fmt.Sprintf("bundle name prefix %q leaves only %d of %d random chars, need %d", prefix, cut, len(random), minRandom))
787+
}
788+
return prefix + random[:cut]
745789
}
746790

747791
func runTest(t *testing.T,
@@ -778,8 +822,8 @@ func runTest(t *testing.T,
778822

779823
id := uuid.New()
780824
uniqueName := strings.ToLower(strings.Trim(base32.StdEncoding.EncodeToString(id[:]), "="))
781-
// Embed the CI run id, when present, so leaked resources can be attributed to a run and swept by prefix.
782-
uniqueName = ciUniqueName(os.Getenv("GITHUB_RUN_ID"), uniqueName)
825+
// Embed the run prefix, when present, so leaked resources can be attributed to this leg and swept.
826+
uniqueName = ciUniqueName(bundleNamePrefix, uniqueName)
783827
repls.Set(uniqueName, "[UNIQUE_NAME]")
784828

785829
var tmpDir string

acceptance/bundle_clean_test.go

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
package acceptance_test
2+
3+
import (
4+
"context"
5+
"errors"
6+
"os"
7+
"os/exec"
8+
"path"
9+
"path/filepath"
10+
"slices"
11+
"strings"
12+
"sync"
13+
"testing"
14+
"time"
15+
16+
"github.com/databricks/databricks-sdk-go"
17+
"github.com/databricks/databricks-sdk-go/apierr"
18+
"github.com/databricks/databricks-sdk-go/service/iam"
19+
"github.com/databricks/databricks-sdk-go/service/workspace"
20+
)
21+
22+
// setupBundleCleanup arranges for every bundle deployed by this run to be
23+
// destroyed once the suite finishes. The caller invokes it only on cloud: all
24+
// cloud tests share one real workspace, whereas local tests each get a
25+
// throwaway in-memory fake workspace with nothing to clean up.
26+
//
27+
// prefix is the leg-specific "ci<runID>x<legSuffix>" that ciUniqueName stamps
28+
// into every $UNIQUE_NAME, so the cleanup sweeps exactly the deployments this
29+
// leg created and nothing else. That is what makes destroying against the shared
30+
// workspace safe even while sibling matrix legs (which share the run id and may
31+
// share the workspace) deploy concurrently.
32+
func setupBundleCleanup(t *testing.T, execPath, prefix string) {
33+
// t.Context() is canceled once the test finishes, before cleanups run, so
34+
// derive a context that survives cancellation for the cleanup's API calls.
35+
ctx := context.WithoutCancel(t.Context())
36+
t.Cleanup(func() {
37+
cleanBundles(ctx, t, execPath, prefix)
38+
})
39+
}
40+
41+
// cleanBundles finds every bundle this run deployed under the current user's
42+
// ~/.bundle directory (identified by the run's prefix) and destroys each one,
43+
// logging each deployment and the total time taken.
44+
func cleanBundles(ctx context.Context, t *testing.T, execPath, prefix string) {
45+
start := time.Now()
46+
47+
// Cleanup never fails the test (see the WARNING note below), so on any error
48+
// that prevents sweeping, log loudly and return rather than require-failing.
49+
w, err := databricks.NewWorkspaceClient()
50+
if err != nil {
51+
t.Logf("WARNING: bundle cleanup skipped, cannot create client: %s", err)
52+
return
53+
}
54+
55+
me, err := w.CurrentUser.Me(ctx, iam.MeRequest{})
56+
if err != nil {
57+
t.Logf("WARNING: bundle cleanup skipped, cannot resolve current user: %s", err)
58+
return
59+
}
60+
61+
// Tests deploy under the user's home .bundle by default, but some set
62+
// workspace.root_path under /Shared (e.g. resources/jobs/shared-root-path),
63+
// so sweep both. A "/Shared/..." root_path is normalized to "/Workspace/Shared/..."
64+
// by prependWorkspacePrefix, so the swept path uses that form.
65+
bundleRoots := []string{
66+
"/Workspace/Users/" + me.UserName + "/.bundle",
67+
"/Workspace/Shared/" + me.UserName + "/.bundle",
68+
}
69+
70+
// The run's prefix always appears in the first path segment under .bundle
71+
// (the bundle name, or the leaf of a workspace.root_path override), so match
72+
// there and only descend into this run's subtrees. This avoids walking the
73+
// thousands of directories other runs may have leaked under .bundle.
74+
var roots []string
75+
for _, bundleRoot := range bundleRoots {
76+
for _, child := range listChildDirs(ctx, t, w, bundleRoot) {
77+
if strings.Contains(path.Base(child), prefix) {
78+
roots = append(roots, findDeploymentRoots(ctx, t, w, child)...)
79+
}
80+
}
81+
}
82+
slices.Sort(roots)
83+
84+
t.Logf("%s bundle cleanup: found %d deployment(s) with prefix %q", time.Now().Format(time.RFC3339), len(roots), prefix)
85+
86+
// Each destroy shells out to a separate `bundle destroy` (auth + state pull +
87+
// deletes), so run them concurrently. Each is network-bound (not CPU-bound),
88+
// so cap at a fixed 20 rather than by GOMAXPROCS: it parallelizes the API
89+
// waits while staying well under the workspace rate limit. This is
90+
// best-effort, not fail-fast: a failed destroy is recorded and the rest still
91+
// run, so a plain WaitGroup with a semaphore fits better than errgroup (whose
92+
// error short-circuit we would not use).
93+
const maxConcurrentDestroys = 20
94+
sem := make(chan struct{}, maxConcurrentDestroys)
95+
var wg sync.WaitGroup
96+
var mu sync.Mutex
97+
var failed []string
98+
for _, root := range roots {
99+
sem <- struct{}{}
100+
wg.Go(func() {
101+
defer func() { <-sem }()
102+
t.Logf("%s destroying %s", time.Now().Format(time.RFC3339), root)
103+
if out, err := destroyBundle(execPath, root); err != nil {
104+
t.Logf("%s destroy failed: %s\n%s", time.Now().Format(time.RFC3339), root, out)
105+
mu.Lock()
106+
failed = append(failed, root)
107+
mu.Unlock()
108+
}
109+
})
110+
}
111+
wg.Wait()
112+
113+
slices.Sort(failed)
114+
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))
115+
116+
// Do not fail the test on a cleanup failure: this runs in a t.Cleanup on the
117+
// root TestAccept, so failing here marks the root test failed with no failed
118+
// subtest, which makes gotestsum --rerun-fails (used by the integration task)
119+
// rerun the entire cloud suite. Cleanup is best-effort housekeeping and the
120+
// product tests already passed, so log loudly instead; leaked deployments are
121+
// reclaimed by the periodic prefix sweep (sweep_test_resources.py).
122+
if len(failed) > 0 {
123+
t.Logf("WARNING: bundle cleanup failed to destroy %d deployment(s), leaked until swept: %s", len(failed), strings.Join(failed, ", "))
124+
}
125+
}
126+
127+
// findDeploymentRoots walks the workspace tree under dir and returns the paths
128+
// of bundle deployment roots. A directory is a deployment root when it contains
129+
// a "state" or "files" child, which the bundle deploy writes beneath the
130+
// resolved workspace.root_path. This works regardless of whether the root is
131+
// the default ~/.bundle/<name>/<target> or a custom ~/.bundle/<...> override.
132+
func findDeploymentRoots(ctx context.Context, t *testing.T, w *databricks.WorkspaceClient, dir string) []string {
133+
var childDirs []string
134+
for _, child := range listChildDirs(ctx, t, w, dir) {
135+
if base := path.Base(child); base == "state" || base == "files" {
136+
// dir is a deployment root; don't descend into its internals.
137+
return []string{dir}
138+
}
139+
childDirs = append(childDirs, child)
140+
}
141+
142+
var roots []string
143+
for _, child := range childDirs {
144+
roots = append(roots, findDeploymentRoots(ctx, t, w, child)...)
145+
}
146+
return roots
147+
}
148+
149+
// listChildDirs returns the immediate subdirectory paths of dir. A missing dir
150+
// (nothing was deployed under it) yields nil silently; any other listing error
151+
// is logged loudly (it means the sweep under dir is incomplete) but does not
152+
// fail the test, since cleanup runs in the root t.Cleanup.
153+
func listChildDirs(ctx context.Context, t *testing.T, w *databricks.WorkspaceClient, dir string) []string {
154+
objects, err := w.Workspace.ListAll(ctx, workspace.ListWorkspaceRequest{Path: dir})
155+
if err != nil {
156+
if !errors.Is(err, apierr.ErrNotFound) {
157+
t.Logf("WARNING: bundle cleanup incomplete, cannot list %s: %s", dir, err)
158+
}
159+
return nil
160+
}
161+
var dirs []string
162+
for _, o := range objects {
163+
if o.ObjectType == workspace.ObjectTypeDirectory {
164+
dirs = append(dirs, o.Path)
165+
}
166+
}
167+
return dirs
168+
}
169+
170+
// destroyBundle destroys the bundle deployed at rootPath using the CLI binary,
171+
// returning the combined output and any error. It writes a throwaway
172+
// databricks.yml pinning workspace.root_path to the deployment root; destroy
173+
// pulls the remote state from there (auto-detecting the engine) and deletes the
174+
// resources and files. The bundle name and target are placeholders because
175+
// root_path fully determines the deployment location. --force-lock overrides a
176+
// stale deployment lock left by a test that was killed mid-deploy; these are
177+
// known-leaked bundles, so there is no concurrent deployment to conflict with.
178+
func destroyBundle(execPath, rootPath string) ([]byte, error) {
179+
dir, err := os.MkdirTemp("", "bundle-clean") //nolint:usetesting // runs in a cleanup, where t.TempDir is already removed
180+
if err != nil {
181+
return nil, err
182+
}
183+
defer os.RemoveAll(dir)
184+
185+
databricksYML := "bundle:\n name: cleanup\nworkspace:\n root_path: " + rootPath + "\ntargets:\n default: {}\n"
186+
if err := os.WriteFile(filepath.Join(dir, "databricks.yml"), []byte(databricksYML), 0o644); err != nil {
187+
return nil, err
188+
}
189+
190+
cmd := exec.Command(execPath, "bundle", "destroy", "--target", "default", "--auto-approve", "--force-lock")
191+
cmd.Dir = dir
192+
cmd.Env = os.Environ()
193+
194+
return cmd.CombinedOutput()
195+
}

acceptance/unique_name_test.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,16 @@ func TestCIUniqueName(t *testing.T) {
1010
// 26 lowercase base32 characters, like the generated unique name.
1111
random := "osr5mzrrvzb73juixjoviti24y"
1212

13-
// Run id embedded, same length as input, lowercase-alphanumeric, sweepable prefix.
14-
assert.Equal(t, "ci15799017600xosr5mzrrvzb7", ciUniqueName("15799017600", random))
15-
assert.Equal(t, "ci1xosr5mzrrvzb73juixjovit", ciUniqueName("1", random))
13+
// Prefix prepended, same length as input, lowercase-alphanumeric.
14+
assert.Equal(t, "ci15799017600xabcdosr5mzrr", ciUniqueName("ci15799017600xabcd", random))
15+
assert.Equal(t, "ci1xabcdosr5mzrrvzb73juixj", ciUniqueName("ci1xabcd", random))
1616

17-
// No or invalid run id: unchanged.
17+
// Empty prefix (off cloud): unchanged.
1818
assert.Equal(t, random, ciUniqueName("", random))
19-
assert.Equal(t, random, ciUniqueName("abc123", random))
20-
assert.Equal(t, random, ciUniqueName("123 456", random))
2119

22-
// 15-digit run id still leaves exactly the 8-char random minimum: prefixed.
23-
assert.Equal(t, "ci123456789012345xosr5mzrr", ciUniqueName("123456789012345", random))
20+
// An 11-digit run id plus the 4-char suffix leaves exactly the 8-char random minimum: prepended.
21+
assert.Equal(t, "ci15799017600xabcdosr5mzrr", ciUniqueName("ci15799017600xabcd", random))
2422

25-
// 16-digit run id is too long to leave enough randomness: unchanged.
26-
assert.Equal(t, random, ciUniqueName("1234567890123456", random))
23+
// A prefix too long to leave 8 random chars is a bug in newBundleNamePrefix, so it panics.
24+
assert.Panics(t, func() { ciUniqueName("ci159990176001xabcd", random) })
2725
}

0 commit comments

Comments
 (0)