From a3eacfa48bb2dbcfe1b563bad04fcf4950fd9f22 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 19 Aug 2026 09:46:29 +0000 Subject: [PATCH 1/2] Remove the empty .bundle/ directory on destroy `bundle destroy` deleted `~/.bundle//` but never the `` parent, so every destroy left an empty directory behind. Deployments that use a fresh bundle name each time never reuse those directories, so they accumulate and count against the workspace child-node limit; the CLI's own CI workspace reached ~27k of them per day. Delete the parent too, non-recursively so it survives while another target of the bundle is still deployed there. The delete is restricted to the `~/.bundle//` layout the CLI generates, since `root_path` is user-configurable and the parent of an arbitrary path is not ours to remove. The fake workspace accepted a non-recursive delete of a non-empty directory, which the real API rejects with DIRECTORY_NOT_EMPTY. Model that, so the sibling-target case is covered by the local suite. Co-authored-by: Isaac --- .../destroy-removes-bundle-directory.md | 1 + .../bundle/destroy/all-resources/output.txt | 6 ++++ .../bundle/destroy/all-resources/script | 5 +++ .../destroy/sibling-target/databricks.yml | 7 ++++ .../destroy/sibling-target/out.test.toml | 2 ++ .../bundle/destroy/sibling-target/output.txt | 35 +++++++++++++++++++ .../bundle/destroy/sibling-target/script | 17 +++++++++ .../resource_deps/remote_app_url/output.txt | 7 ++++ .../simple/out.requests.destroy.direct.json | 12 +++++++ .../out.requests.destroy.terraform.json | 12 +++++++ bundle/deploy/files/delete.go | 26 ++++++++++++++ libs/testserver/fake_workspace.go | 32 ++++++++++++++++- libs/testserver/handlers.go | 3 +- libs/testserver/workspace_test.go | 28 +++++++++++++++ 14 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 .nextchanges/bundles/destroy-removes-bundle-directory.md create mode 100644 acceptance/bundle/destroy/sibling-target/databricks.yml create mode 100644 acceptance/bundle/destroy/sibling-target/out.test.toml create mode 100644 acceptance/bundle/destroy/sibling-target/output.txt create mode 100644 acceptance/bundle/destroy/sibling-target/script diff --git a/.nextchanges/bundles/destroy-removes-bundle-directory.md b/.nextchanges/bundles/destroy-removes-bundle-directory.md new file mode 100644 index 00000000000..fca3bc5db04 --- /dev/null +++ b/.nextchanges/bundles/destroy-removes-bundle-directory.md @@ -0,0 +1 @@ +`bundle destroy` now also removes the `~/.bundle/` directory the deployment lived under, instead of only `~/.bundle//`. The directory is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit. diff --git a/acceptance/bundle/destroy/all-resources/output.txt b/acceptance/bundle/destroy/all-resources/output.txt index f8eb4d90e0d..207a85f8c8b 100644 --- a/acceptance/bundle/destroy/all-resources/output.txt +++ b/acceptance/bundle/destroy/all-resources/output.txt @@ -21,3 +21,9 @@ Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascad All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default Destroy: 2 deleted + +=== Assert the bundle directory is deleted +>>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle) doesn't exist. + +Exit code: 1 diff --git a/acceptance/bundle/destroy/all-resources/script b/acceptance/bundle/destroy/all-resources/script index ceb8acee91a..71160f7c3f7 100644 --- a/acceptance/bundle/destroy/all-resources/script +++ b/acceptance/bundle/destroy/all-resources/script @@ -1,2 +1,7 @@ trace $CLI bundle deploy trace $CLI bundle destroy --auto-approve + +# Destroy removes the target directory, and the .bundle/ parent goes with it once +# nothing else is deployed under it. Double slash keeps Windows from rewriting the path. +title "Assert the bundle directory is deleted" +trace errcode $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" diff --git a/acceptance/bundle/destroy/sibling-target/databricks.yml b/acceptance/bundle/destroy/sibling-target/databricks.yml new file mode 100644 index 00000000000..a12a0457944 --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/databricks.yml @@ -0,0 +1,7 @@ +bundle: + name: test-bundle + +targets: + dev: + default: true + prod: diff --git a/acceptance/bundle/destroy/sibling-target/out.test.toml b/acceptance/bundle/destroy/sibling-target/out.test.toml new file mode 100644 index 00000000000..98ea5040486 --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/destroy/sibling-target/output.txt b/acceptance/bundle/destroy/sibling-target/output.txt new file mode 100644 index 00000000000..48cbff888ce --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/output.txt @@ -0,0 +1,35 @@ + +>>> [CLI] bundle deploy -t dev +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/dev/files... +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 0 unchanged + +>>> [CLI] bundle deploy -t prod +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/prod/files... +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 0 unchanged + +=== Destroy one target while the other is still deployed +>>> [CLI] bundle destroy -t dev --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/dev + +Destroy: 0 deleted + +=== Assert the bundle directory is kept for the remaining target +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +{ + "path": "/Users/[USERNAME]/.bundle/test-bundle", + "object_type": "DIRECTORY" +} + +=== Destroy the remaining target +>>> [CLI] bundle destroy -t prod --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/prod + +Destroy: 0 deleted + +=== Assert the bundle directory is deleted +>>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle) doesn't exist. + +Exit code: 1 diff --git a/acceptance/bundle/destroy/sibling-target/script b/acceptance/bundle/destroy/sibling-target/script new file mode 100644 index 00000000000..1f2db9fec6c --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/script @@ -0,0 +1,17 @@ +# Double slash keeps Windows from rewriting the path. +BUNDLE_DIR="//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" + +trace $CLI bundle deploy -t dev +trace $CLI bundle deploy -t prod + +title "Destroy one target while the other is still deployed" +trace $CLI bundle destroy -t dev --auto-approve + +title "Assert the bundle directory is kept for the remaining target" +trace $CLI workspace get-status "${BUNDLE_DIR}" | jq '{path, object_type}' + +title "Destroy the remaining target" +trace $CLI bundle destroy -t prod --auto-approve + +title "Assert the bundle directory is deleted" +trace errcode $CLI workspace get-status "${BUNDLE_DIR}" diff --git a/acceptance/bundle/resource_deps/remote_app_url/output.txt b/acceptance/bundle/resource_deps/remote_app_url/output.txt index 84d8da92e35..9301329af12 100644 --- a/acceptance/bundle/resource_deps/remote_app_url/output.txt +++ b/acceptance/bundle/resource_deps/remote_app_url/output.txt @@ -107,6 +107,13 @@ Destroy: 2 deleted "method": "DELETE", "path": "/api/2.0/pipelines/[UUID]" } +{ + "method": "POST", + "path": "/api/2.0/workspace/delete", + "body": { + "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle" + } +} { "method": "POST", "path": "/api/2.0/workspace/delete", diff --git a/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json b/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json index c67f22e3730..1e97537e4d6 100644 --- a/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json +++ b/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json @@ -132,6 +132,18 @@ "User": "[USERNAME]" } } +{ + "headers": { + "User-Agent": [ + "cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_destroy cmd-exec-id/[UUID] interactive/none engine/direct auth/pat" + ] + }, + "method": "POST", + "path": "/api/2.0/workspace/delete", + "body": { + "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle" + } +} { "headers": { "User-Agent": [ diff --git a/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json b/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json index 09b923be4dd..325ff5b60aa 100644 --- a/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json +++ b/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json @@ -111,6 +111,18 @@ "User": "[USERNAME]" } } +{ + "headers": { + "User-Agent": [ + "cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_destroy cmd-exec-id/[UUID] interactive/none engine/terraform auth/pat" + ] + }, + "method": "POST", + "path": "/api/2.0/workspace/delete", + "body": { + "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle" + } +} { "headers": { "User-Agent": [ diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index 562437e5432..7f3502cd061 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -6,13 +6,17 @@ import ( "fmt" "io/fs" "os" + "path" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/sync" "github.com/databricks/databricks-sdk-go/service/workspace" ) +const bundleDirName = ".bundle" + type delete struct{} func (m *delete) Name() string { @@ -28,6 +32,8 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return diag.FromErr(err) } + removeEmptyBundleDir(ctx, b) + // Clean up sync snapshot file err = deleteSnapshotFile(ctx, b) if err != nil { @@ -36,6 +42,26 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return nil } +// removeEmptyBundleDir removes the ~/.bundle/ directory left behind once the +// target subdirectory under it is gone. The delete is not recursive, so it only +// succeeds while no other target of the bundle is still deployed there; both a +// remaining sibling target and an already-removed directory surface as an error that +// is expected and ignored. +func removeEmptyBundleDir(ctx context.Context, b *bundle.Bundle) { + dir := path.Dir(b.Config.Workspace.RootPath) + + // root_path is user-configurable, so only clean up the layout the CLI generates + // (~/.bundle//) instead of deleting the parent of an arbitrary path. + if path.Base(path.Dir(dir)) != bundleDirName { + return + } + + err := b.WorkspaceClient(ctx).Workspace.Delete(ctx, workspace.Delete{Path: dir}) + if err != nil { + log.Debugf(ctx, "Leaving %s in place: %s", dir, err) + } +} + func deleteSnapshotFile(ctx context.Context, b *bundle.Bundle) error { opts, err := GetSyncOptions(ctx, b) if err != nil { diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 1caec84d1e4..7625c4d8546 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -698,9 +698,21 @@ func (s *FakeWorkspace) WorkspaceExport(path string) []byte { return s.files[path].Data } -func (s *FakeWorkspace) WorkspaceDelete(path string, recursive bool) { +// WorkspaceDelete implements POST /api/2.0/workspace/delete. As in the real API, a +// non-recursive delete of a directory that still has children fails instead of removing +// it, which is what lets a caller delete a directory only if it is empty. +func (s *FakeWorkspace) WorkspaceDelete(path string, recursive bool) Response { defer s.LockUnlock()() if !recursive { + if _, isDir := s.directories[path]; isDir && s.hasChildren(path) { + return Response{ + StatusCode: 400, + Body: map[string]string{ + "error_code": "DIRECTORY_NOT_EMPTY", + "message": "Directory " + path + " is not empty", + }, + } + } delete(s.files, path) delete(s.directories, path) } else { @@ -715,6 +727,24 @@ func (s *FakeWorkspace) WorkspaceDelete(path string, recursive bool) { } } } + return Response{} +} + +// hasChildren reports whether any file or directory lives under dirPath. Callers must +// hold the lock. +func (s *FakeWorkspace) hasChildren(dirPath string) bool { + prefix := dirPath + "/" + for key := range s.files { + if strings.HasPrefix(key, prefix) { + return true + } + } + for key := range s.directories { + if strings.HasPrefix(key, prefix) { + return true + } + } + return false } func (s *FakeWorkspace) WorkspaceFilesImportFile(filePath string, body []byte, overwrite bool) Response { diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index 099230ad94e..1cc037dcb6b 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -143,8 +143,7 @@ func AddDefaultHandlers(server *Server) { StatusCode: 500, } } - req.Workspace.WorkspaceDelete(request.Path, request.Recursive) - return "" + return req.Workspace.WorkspaceDelete(request.Path, request.Recursive) }) server.Handle("POST", "/api/2.0/workspace-files/import-file/{path...}", func(req Request) any { diff --git a/libs/testserver/workspace_test.go b/libs/testserver/workspace_test.go index 5e4753246cf..e8d8c4a12a8 100644 --- a/libs/testserver/workspace_test.go +++ b/libs/testserver/workspace_test.go @@ -31,6 +31,18 @@ func mkdirs(t *testing.T, baseURL, path string) { require.Equal(t, 200, resp.StatusCode) } +func workspaceDelete(t *testing.T, baseURL, path string, recursive bool) int { + t.Helper() + body, err := json.Marshal(map[string]any{"path": path, "recursive": recursive}) + require.NoError(t, err) + req, _ := http.NewRequest(http.MethodPost, baseURL+"/api/2.0/workspace/delete", strings.NewReader(string(body))) + req.Header.Set("Authorization", "Bearer test-token") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + return resp.StatusCode +} + func getStatus(t *testing.T, baseURL, path string) int { t.Helper() req, _ := http.NewRequest(http.MethodGet, baseURL+"/api/2.0/workspace/get-status?path="+path, nil) @@ -53,6 +65,22 @@ func TestWorkspaceImportRejectsMissingParent(t *testing.T) { assert.Equal(t, 200, importFile(t, server.URL, "/test-dir/file.py", "content")) } +// A non-recursive delete only removes an empty directory, so a caller can use it to +// clean up a parent directory without touching one that still holds a sibling. +func TestWorkspaceDeleteNonRecursiveRequiresEmptyDirectory(t *testing.T) { + server := testserver.New(t) + testserver.AddDefaultHandlers(server) + + mkdirs(t, server.URL, "/a/b/c") + + assert.Equal(t, 400, workspaceDelete(t, server.URL, "/a/b", false)) + assert.Equal(t, 200, getStatus(t, server.URL, "/a/b")) + + assert.Equal(t, 200, workspaceDelete(t, server.URL, "/a/b/c", false)) + assert.Equal(t, 200, workspaceDelete(t, server.URL, "/a/b", false)) + assert.Equal(t, 404, getStatus(t, server.URL, "/a/b")) +} + // mkdirs creates all intermediate directories, matching "mkdir -p". func TestWorkspaceMkdirsRecursive(t *testing.T) { server := testserver.New(t) From ccf0ff7510f124e6327532b9ebf1d02803aaa725 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 19 Aug 2026 09:55:09 +0000 Subject: [PATCH 2/2] Link the changelog fragment to the PR Co-authored-by: Isaac --- .nextchanges/bundles/destroy-removes-bundle-directory.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/bundles/destroy-removes-bundle-directory.md b/.nextchanges/bundles/destroy-removes-bundle-directory.md index fca3bc5db04..5bbba58514e 100644 --- a/.nextchanges/bundles/destroy-removes-bundle-directory.md +++ b/.nextchanges/bundles/destroy-removes-bundle-directory.md @@ -1 +1 @@ -`bundle destroy` now also removes the `~/.bundle/` directory the deployment lived under, instead of only `~/.bundle//`. The directory is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit. +`bundle destroy` now also removes the `~/.bundle/` directory the deployment lived under, instead of only `~/.bundle//`. The directory is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit ([#6317](https://github.com/databricks/cli/pull/6317)).