From ffab323c325f2bcd5771a720866b0b342cce2ea5 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 21:58:15 +0200 Subject: [PATCH 1/4] direct: clean up state entries for remotely deleted resources removed from config When a resource was deleted remotely and also removed from the bundle config, CalculatePlan dropped the Delete entry from the plan, so the stale state entry was never removed and reappeared on every deploy. Keep the Delete entry instead; Delete() already tolerates resources that are gone and removes the entry from state. Also add the missing location guard in metadata.Compute for pipelines and dashboards (jobs got it in #3710), which made the same scenario hard-fail the deploy. Co-authored-by: Isaac --- .../destroy/out.destroy.direct.txt | 3 ++ .../remote_delete/removed_from_config/bar.py | 1 + .../removed_from_config/databricks.yml | 14 ++++++++ .../removed_from_config/empty.yml | 4 +++ .../out.deploy_removed.direct.txt | 12 +++++++ .../out.deploy_removed.terraform.txt | 6 ++++ .../out.plan_removed.direct.txt | 6 ++++ .../out.plan_removed.terraform.txt | 3 ++ .../out.summary.direct.txt | 8 +++++ .../out.summary.terraform.txt | 16 +++++++++ .../removed_from_config/out.test.toml | 3 ++ .../removed_from_config/output.txt | 21 ++++++++++++ .../remote_delete/removed_from_config/script | 18 ++++++++++ .../removed_from_config/test.toml | 1 + bundle/deploy/metadata/compute.go | 10 ++++++ bundle/deploy/metadata/compute_test.go | 34 +++++++++++++++++++ bundle/deployplan/plan.go | 6 ---- bundle/direct/bundle_plan.go | 15 ++++---- 18 files changed, 167 insertions(+), 14 deletions(-) create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/bar.py create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/databricks.yml create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/empty.yml create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.terraform.txt create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.direct.txt create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.terraform.txt create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.direct.txt create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.terraform.txt create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.test.toml create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script create mode 100644 acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml diff --git a/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt b/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt index 1cea8f06419..00b8678e0e3 100644 --- a/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt +++ b/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt @@ -1,5 +1,8 @@ >>> errcode [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.jobs.foo + All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default Deleting files... diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/bar.py b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/bar.py new file mode 100644 index 00000000000..11b15b1a458 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/bar.py @@ -0,0 +1 @@ +print("hello") diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/databricks.yml b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/databricks.yml new file mode 100644 index 00000000000..fc261e691b6 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/databricks.yml @@ -0,0 +1,14 @@ +bundle: + name: test-bundle + +resources: + jobs: + foo: + name: foo + + pipelines: + bar: + name: bar + libraries: + - file: + path: ./bar.py diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/empty.yml b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/empty.yml new file mode 100644 index 00000000000..f12a8955927 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/empty.yml @@ -0,0 +1,4 @@ +bundle: + name: test-bundle + +resources: {} diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt new file mode 100644 index 00000000000..a77e9251eba --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt @@ -0,0 +1,12 @@ + +>>> [CLI] bundle deploy --auto-approve +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... + +This action will result in the deletion or recreation of the following Lakeflow Spark Declarative Pipelines along with the +Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the pipelines will +restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline +properties such as the 'catalog' or 'storage' are changed: + delete resources.pipelines.bar +Deploying resources... +Updating deployment state... +Deployment complete! diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.terraform.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.terraform.txt new file mode 100644 index 00000000000..0a628d4cb69 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.terraform.txt @@ -0,0 +1,6 @@ + +>>> [CLI] bundle deploy --auto-approve +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.direct.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.direct.txt new file mode 100644 index 00000000000..acce6c2fee5 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.direct.txt @@ -0,0 +1,6 @@ + +>>> [CLI] bundle plan +delete jobs.foo +delete pipelines.bar + +Plan: 0 to add, 0 to change, 2 to delete, 0 unchanged diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.terraform.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.terraform.txt new file mode 100644 index 00000000000..ed49fd178e0 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.plan_removed.terraform.txt @@ -0,0 +1,3 @@ + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 0 unchanged diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.direct.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.direct.txt new file mode 100644 index 00000000000..e5b5bae68fe --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.direct.txt @@ -0,0 +1,8 @@ + +>>> [CLI] bundle summary +Name: test-bundle +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default +Resources: diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.terraform.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.terraform.txt new file mode 100644 index 00000000000..1f15be287ce --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.summary.terraform.txt @@ -0,0 +1,16 @@ + +>>> [CLI] bundle summary +Name: test-bundle +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default +Resources: + Jobs: + foo: + Name: + URL: [DATABRICKS_URL]/jobs/[NUMID]?w=[NUMID] + Pipelines: + bar: + Name: + URL: [DATABRICKS_URL]/pipelines/[UUID]?w=[NUMID] diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.test.toml b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt new file mode 100644 index 00000000000..38da7a938a0 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt @@ -0,0 +1,21 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] jobs delete [NUMID] + +>>> [CLI] pipelines delete [UUID] + +=== Remove resources from config, plan and deploy +=== Plan and deploy again, then summary +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 0 unchanged + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script new file mode 100644 index 00000000000..ecf66d8c3e0 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script @@ -0,0 +1,18 @@ +trace $CLI bundle deploy +job_id=`$CLI bundle summary -o json | jq -r .resources.jobs.foo.id` +pipeline_id=`$CLI bundle summary -o json | jq -r .resources.pipelines.bar.id` +trace $CLI jobs delete $job_id +trace $CLI pipelines delete $pipeline_id + +title "Remove resources from config, plan and deploy" +# The engines diverge here: direct plans a delete for the stale state entries and +# removes them from state; terraform drops the gone resources from the plan but +# keeps them in its state, so the summary below still lists them. +cp empty.yml databricks.yml +trace $CLI bundle plan &> out.plan_removed.$DATABRICKS_BUNDLE_ENGINE.txt +trace $CLI bundle deploy --auto-approve &> out.deploy_removed.$DATABRICKS_BUNDLE_ENGINE.txt + +title "Plan and deploy again, then summary" +trace $CLI bundle plan +trace $CLI bundle deploy +trace $CLI bundle summary &> out.summary.$DATABRICKS_BUNDLE_ENGINE.txt diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml new file mode 100644 index 00000000000..a030353d571 --- /dev/null +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml @@ -0,0 +1 @@ +RecordRequests = false diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index cb7be9811c4..cf8c37f03c6 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -72,6 +72,11 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics // Compute config file path the pipeline is defined in, relative to the bundle // root l := b.Config.GetLocation("resources.pipelines." + name) + if l.File == "" { + // b.Config.Resources.Pipelines may include a pipeline that only exists in state but not in config + continue + } + relativePath, err := filepath.Rel(b.BundleRootPath, l.File) if err != nil { return diag.Errorf("failed to compute relative path for pipeline %s: %v", name, err) @@ -90,6 +95,11 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics // Compute config file path the dashboard is defined in, relative to the bundle // root l := b.Config.GetLocation("resources.dashboards." + name) + if l.File == "" { + // b.Config.Resources.Dashboards may include a dashboard that only exists in state but not in config + continue + } + relativePath, err := filepath.Rel(b.BundleRootPath, l.File) if err != nil { return diag.Errorf("failed to compute relative path for dashboard %s: %v", name, err) diff --git a/bundle/deploy/metadata/compute_test.go b/bundle/deploy/metadata/compute_test.go index a10f65d72e5..85212bfe77b 100644 --- a/bundle/deploy/metadata/compute_test.go +++ b/bundle/deploy/metadata/compute_test.go @@ -148,6 +148,40 @@ func TestComputeMetadataMutator(t *testing.T) { assert.Equal(t, expectedMetadata, b.Metadata) } +func TestComputeMetadataMutatorStateOnlyResources(t *testing.T) { + // Resources that exist in state but not in config (e.g. deleted remotely and + // removed from config) have no location. They must be skipped, not error. + b := &bundle.Bundle{ + BundleRootPath: "/tmp/some/root", + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "state-only-job": { + BaseResource: resources.BaseResource{ID: "1111"}, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "state-only-pipeline": { + BaseResource: resources.BaseResource{ID: "2222"}, + }, + }, + Dashboards: map[string]*resources.Dashboard{ + "state-only-dashboard": { + BaseResource: resources.BaseResource{ID: "3333"}, + }, + }, + }, + }, + } + + diags := bundle.Apply(t.Context(), b, Compute()) + require.NoError(t, diags.Error()) + + assert.Empty(t, b.Metadata.Config.Resources.Jobs) + assert.Empty(t, b.Metadata.Config.Resources.Pipelines) + assert.Empty(t, b.Metadata.Config.Resources.Dashboards) +} + func TestComputeMetadataMutatorSourceLinked(t *testing.T) { syncRootPath := "/Users/shreyas.goenka@databricks.com/source" enabled := true diff --git a/bundle/deployplan/plan.go b/bundle/deployplan/plan.go index 2fb5d38c806..cc3d8435333 100644 --- a/bundle/deployplan/plan.go +++ b/bundle/deployplan/plan.go @@ -196,12 +196,6 @@ func (p *Plan) ReadUnlockEntry(resourceKey string) { p.lockmap.RUnlock(resourceKey) } -func (p *Plan) RemoveEntry(resourceKey string) { - p.mutex.Lock() - defer p.mutex.Unlock() - delete(p.Plan, resourceKey) -} - // FilterToSelected reduces the plan to the nodes in selected (format "type.name", // e.g. "jobs.my_job") plus their transitive dependencies as recorded in each // entry's DependsOn field. Nodes not reachable from the selected set are removed. diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 5591626cd75..6f9cea6cbf9 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -160,14 +160,13 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks remoteState, err := retryOnTransient(ctx, func() (any, error) { return adapter.DoRead(ctx, id) }) - if err != nil { - if isResourceGone(err) { - // no such resource - plan.RemoveEntry(resourceKey) - } else { - log.Warnf(ctx, "reading %s id=%q: %s", resourceKey, id, err) - // This is not an error during deletion, so don't return false here - } + // If the resource is already gone remotely, keep the Delete entry: + // Delete() tolerates missing resources, and applying the entry removes + // it from the state. Dropping the entry here would leave the stale + // state entry behind forever. + if err != nil && !isResourceGone(err) { + log.Warnf(ctx, "reading %s id=%q: %s", resourceKey, id, err) + // This is not an error during deletion, so don't return false here } entry.RemoteState = remoteState From 941baf91ed2c52adcd96240451a1dc2eb4ab28f0 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 23:41:40 +0200 Subject: [PATCH 2/4] Remove redundant comments Co-authored-by: Isaac --- .../resources/jobs/remote_delete/removed_from_config/script | 4 +--- bundle/deploy/metadata/compute_test.go | 3 +-- bundle/direct/bundle_plan.go | 6 ++---- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script index ecf66d8c3e0..5ffd5763998 100644 --- a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script @@ -5,9 +5,7 @@ trace $CLI jobs delete $job_id trace $CLI pipelines delete $pipeline_id title "Remove resources from config, plan and deploy" -# The engines diverge here: direct plans a delete for the stale state entries and -# removes them from state; terraform drops the gone resources from the plan but -# keeps them in its state, so the summary below still lists them. +# Engines diverge: direct plans deletes for the stale state entries; terraform keeps them in state, so its summary still lists them. cp empty.yml databricks.yml trace $CLI bundle plan &> out.plan_removed.$DATABRICKS_BUNDLE_ENGINE.txt trace $CLI bundle deploy --auto-approve &> out.deploy_removed.$DATABRICKS_BUNDLE_ENGINE.txt diff --git a/bundle/deploy/metadata/compute_test.go b/bundle/deploy/metadata/compute_test.go index 85212bfe77b..834a81a9358 100644 --- a/bundle/deploy/metadata/compute_test.go +++ b/bundle/deploy/metadata/compute_test.go @@ -149,8 +149,7 @@ func TestComputeMetadataMutator(t *testing.T) { } func TestComputeMetadataMutatorStateOnlyResources(t *testing.T) { - // Resources that exist in state but not in config (e.g. deleted remotely and - // removed from config) have no location. They must be skipped, not error. + // State-only resources (in state but not in config) have no location and must be skipped, not error. b := &bundle.Bundle{ BundleRootPath: "/tmp/some/root", Config: config.Root{ diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 6f9cea6cbf9..66ada0780ac 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -160,10 +160,8 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks remoteState, err := retryOnTransient(ctx, func() (any, error) { return adapter.DoRead(ctx, id) }) - // If the resource is already gone remotely, keep the Delete entry: - // Delete() tolerates missing resources, and applying the entry removes - // it from the state. Dropping the entry here would leave the stale - // state entry behind forever. + // If the resource is already gone remotely, keep the Delete entry: Delete() + // tolerates missing resources and applying the entry removes it from the state. if err != nil && !isResourceGone(err) { log.Warnf(ctx, "reading %s id=%q: %s", resourceKey, id, err) // This is not an error during deletion, so don't return false here From 24c3538893f75e7a7536aa37dd620e3e0ef331c4 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Wed, 10 Jun 2026 07:25:58 +0200 Subject: [PATCH 3/4] Record direct-engine output for delete-trashed-out-of-band Co-authored-by: Isaac --- .../delete-trashed-out-of-band/out.destroy.direct.txt | 9 +++++++++ .../delete-trashed-out-of-band/out.destroy.terraform.txt | 6 ++++++ .../dashboards/delete-trashed-out-of-band/output.txt | 6 ------ .../dashboards/delete-trashed-out-of-band/script | 6 ++++-- 4 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt create mode 100644 acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt new file mode 100644 index 00000000000..c40637b4b2a --- /dev/null +++ b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt @@ -0,0 +1,9 @@ + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.dashboards.dashboard1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/delete-trashed-out-of-band-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt new file mode 100644 index 00000000000..433e4b37a59 --- /dev/null +++ b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt @@ -0,0 +1,6 @@ + +>>> [CLI] bundle destroy --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/delete-trashed-out-of-band-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt index 1511a1c1187..53b98fd3a70 100644 --- a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt +++ b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt @@ -19,9 +19,3 @@ create dashboards.dashboard1 Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged >>> [CLI] bundle plan -o json - ->>> [CLI] bundle destroy --auto-approve -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/delete-trashed-out-of-band-[UNIQUE_NAME]/default - -Deleting files... -Destroy complete! diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script index 0868adbc13b..a2ba2576bd9 100755 --- a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script +++ b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script @@ -25,5 +25,7 @@ trace $CLI bundle plan trace $CLI bundle plan -o json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json -# Run destroy - should succeed even though dashboard is already trashed -trace $CLI bundle destroy --auto-approve +# Run destroy - should succeed even though dashboard is already trashed. +# The direct engine keeps the delete entry for the trashed dashboard to clean up +# its state entry, so the destroy output diverges per engine. +trace $CLI bundle destroy --auto-approve &> out.destroy.$DATABRICKS_BUNDLE_ENGINE.txt From a01eb5d40eb27d3474496236c789809491426615 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Wed, 10 Jun 2026 18:43:35 +0200 Subject: [PATCH 4/4] Mark gone resources in the plan instead of calling delete on them Per review feedback: planning now sets gone on a Delete entry when the remote read returns 404. Applying such an entry removes it from state without calling the delete API, and deploy/destroy approval prompts no longer list it as a deletion or ask for approval, since nothing is actually deleted. With that the destroy output for remotely deleted resources matches the terraform engine again, so the per-engine out.destroy split is reverted for delete-trashed-out-of-band and the removed_from_config test now also asserts that no delete API calls are made for gone resources. Also reword the state-only resource comments in metadata.Compute to explain the statemgmt.Load injection that makes the guards necessary. Co-authored-by: Isaac --- .../out.destroy.direct.txt | 9 --------- .../out.destroy.terraform.txt | 6 ------ .../delete-trashed-out-of-band/output.txt | 6 ++++++ .../dashboards/delete-trashed-out-of-band/script | 6 ++---- .../remote_delete/destroy/out.destroy.direct.txt | 3 --- .../out.deploy_removed.direct.txt | 6 ------ .../remote_delete/removed_from_config/output.txt | 3 +++ .../jobs/remote_delete/removed_from_config/script | 5 +++++ .../remote_delete/removed_from_config/test.toml | 2 +- bundle/deploy/metadata/compute.go | 12 +++++++++--- bundle/deployplan/action.go | 3 +++ bundle/deployplan/plan.go | 11 ++++++++--- bundle/direct/bundle_apply.go | 8 +++++++- bundle/direct/bundle_plan.go | 15 ++++++++++----- bundle/phases/deploy.go | 5 +++++ bundle/phases/destroy.go | 5 +++++ 16 files changed, 64 insertions(+), 41 deletions(-) delete mode 100644 acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt delete mode 100644 acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt deleted file mode 100644 index c40637b4b2a..00000000000 --- a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.direct.txt +++ /dev/null @@ -1,9 +0,0 @@ - ->>> [CLI] bundle destroy --auto-approve -The following resources will be deleted: - delete resources.dashboards.dashboard1 - -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/delete-trashed-out-of-band-[UNIQUE_NAME]/default - -Deleting files... -Destroy complete! diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt deleted file mode 100644 index 433e4b37a59..00000000000 --- a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.destroy.terraform.txt +++ /dev/null @@ -1,6 +0,0 @@ - ->>> [CLI] bundle destroy --auto-approve -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/delete-trashed-out-of-band-[UNIQUE_NAME]/default - -Deleting files... -Destroy complete! diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt index 53b98fd3a70..1511a1c1187 100644 --- a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt +++ b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/output.txt @@ -19,3 +19,9 @@ create dashboards.dashboard1 Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged >>> [CLI] bundle plan -o json + +>>> [CLI] bundle destroy --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/delete-trashed-out-of-band-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script index a2ba2576bd9..0868adbc13b 100755 --- a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script +++ b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/script @@ -25,7 +25,5 @@ trace $CLI bundle plan trace $CLI bundle plan -o json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json -# Run destroy - should succeed even though dashboard is already trashed. -# The direct engine keeps the delete entry for the trashed dashboard to clean up -# its state entry, so the destroy output diverges per engine. -trace $CLI bundle destroy --auto-approve &> out.destroy.$DATABRICKS_BUNDLE_ENGINE.txt +# Run destroy - should succeed even though dashboard is already trashed +trace $CLI bundle destroy --auto-approve diff --git a/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt b/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt index 00b8678e0e3..1cea8f06419 100644 --- a/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt +++ b/acceptance/bundle/resources/jobs/remote_delete/destroy/out.destroy.direct.txt @@ -1,8 +1,5 @@ >>> errcode [CLI] bundle destroy --auto-approve -The following resources will be deleted: - delete resources.jobs.foo - All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default Deleting files... diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt index a77e9251eba..0a628d4cb69 100644 --- a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/out.deploy_removed.direct.txt @@ -1,12 +1,6 @@ >>> [CLI] bundle deploy --auto-approve Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... - -This action will result in the deletion or recreation of the following Lakeflow Spark Declarative Pipelines along with the -Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the pipelines will -restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline -properties such as the 'catalog' or 'storage' are changed: - delete resources.pipelines.bar Deploying resources... Updating deployment state... Deployment complete! diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt index 38da7a938a0..c97c93273cb 100644 --- a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/output.txt @@ -19,3 +19,6 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/defaul Deploying resources... Updating deployment state... Deployment complete! + +=== No delete API calls for resources that are already gone remotely +>>> print_requests.py //jobs/delete //pipelines/ diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script index 5ffd5763998..2b032406a00 100644 --- a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/script @@ -7,6 +7,8 @@ trace $CLI pipelines delete $pipeline_id title "Remove resources from config, plan and deploy" # Engines diverge: direct plans deletes for the stale state entries; terraform keeps them in state, so its summary still lists them. cp empty.yml databricks.yml +# Drain requests recorded so far; only requests made after the config removal matter below. +print_requests.py --get // &> LOG.requests_setup trace $CLI bundle plan &> out.plan_removed.$DATABRICKS_BUNDLE_ENGINE.txt trace $CLI bundle deploy --auto-approve &> out.deploy_removed.$DATABRICKS_BUNDLE_ENGINE.txt @@ -14,3 +16,6 @@ title "Plan and deploy again, then summary" trace $CLI bundle plan trace $CLI bundle deploy trace $CLI bundle summary &> out.summary.$DATABRICKS_BUNDLE_ENGINE.txt + +title "No delete API calls for resources that are already gone remotely" +trace print_requests.py //jobs/delete //pipelines/ diff --git a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml index a030353d571..159efe02696 100644 --- a/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml +++ b/acceptance/bundle/resources/jobs/remote_delete/removed_from_config/test.toml @@ -1 +1 @@ -RecordRequests = false +RecordRequests = true diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index cf8c37f03c6..1fa4c3bf2d9 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -48,7 +48,9 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics // root l := b.Config.GetLocation("resources.jobs." + name) if l.File == "" { - // b.Config.Resources.Jobs may include a job that only exists in state but not in config + // Skip resources that exist only in the deployment state: statemgmt.Load, + // which runs before this mutator, injects them into the config without a + // file location. continue } @@ -73,7 +75,9 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics // root l := b.Config.GetLocation("resources.pipelines." + name) if l.File == "" { - // b.Config.Resources.Pipelines may include a pipeline that only exists in state but not in config + // Skip resources that exist only in the deployment state: statemgmt.Load, + // which runs before this mutator, injects them into the config without a + // file location. continue } @@ -96,7 +100,9 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics // root l := b.Config.GetLocation("resources.dashboards." + name) if l.File == "" { - // b.Config.Resources.Dashboards may include a dashboard that only exists in state but not in config + // Skip resources that exist only in the deployment state: statemgmt.Load, + // which runs before this mutator, injects them into the config without a + // file location. continue } diff --git a/bundle/deployplan/action.go b/bundle/deployplan/action.go index e8e14279567..e855dbb2197 100644 --- a/bundle/deployplan/action.go +++ b/bundle/deployplan/action.go @@ -9,6 +9,9 @@ type Action struct { // Full resource key, e.g. "resources.jobs.foo" or "resources.jobs.foo.permissions" ResourceKey string ActionType ActionType + // Gone mirrors PlanEntry.Gone: the delete is a state-only cleanup because the + // resource no longer exists remotely. + Gone bool } func (a Action) String() string { diff --git a/bundle/deployplan/plan.go b/bundle/deployplan/plan.go index cc3d8435333..fc03c055986 100644 --- a/bundle/deployplan/plan.go +++ b/bundle/deployplan/plan.go @@ -73,9 +73,13 @@ func LoadPlanFromFile(path string) (*Plan, error) { } type PlanEntry struct { - ID string `json:"id,omitempty"` - DependsOn []DependsOnEntry `json:"depends_on,omitempty"` - Action ActionType `json:"action,omitempty"` + ID string `json:"id,omitempty"` + DependsOn []DependsOnEntry `json:"depends_on,omitempty"` + Action ActionType `json:"action,omitempty"` + // Gone is set on Delete entries when planning confirmed the resource no longer + // exists remotely. Applying such an entry only removes it from the state, without + // calling the delete API, and approval prompts do not list it as a deletion. + Gone bool `json:"gone,omitempty"` NewState *structvar.StructVarJSON `json:"new_state,omitempty"` RemoteState any `json:"remote_state,omitempty"` Changes Changes `json:"changes,omitempty"` @@ -153,6 +157,7 @@ func (p *Plan) GetActions() []Action { actions = append(actions, Action{ ResourceKey: key, ActionType: entry.Action, + Gone: entry.Gone, }) } diff --git a/bundle/direct/bundle_apply.go b/bundle/direct/bundle_apply.go index a9981ee63d8..b42988956df 100644 --- a/bundle/direct/bundle_apply.go +++ b/bundle/direct/bundle_apply.go @@ -96,7 +96,13 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa } return true } - err = d.Destroy(ctx, &b.StateDB) + if entry.Gone { + // Planning confirmed the resource is already deleted remotely; only + // remove it from the state, without calling the delete API. + err = b.StateDB.DeleteState(resourceKey) + } else { + err = d.Destroy(ctx, &b.StateDB) + } if err != nil { logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err)) return false diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 66ada0780ac..11132ec5836 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -160,11 +160,16 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks remoteState, err := retryOnTransient(ctx, func() (any, error) { return adapter.DoRead(ctx, id) }) - // If the resource is already gone remotely, keep the Delete entry: Delete() - // tolerates missing resources and applying the entry removes it from the state. - if err != nil && !isResourceGone(err) { - log.Warnf(ctx, "reading %s id=%q: %s", resourceKey, id, err) - // This is not an error during deletion, so don't return false here + if err != nil { + if isResourceGone(err) { + // The resource is already deleted remotely. Keep the Delete entry so + // that applying it removes the stale state entry, but mark it Gone so + // apply skips the delete call and prompts don't list it as a deletion. + entry.Gone = true + } else { + log.Warnf(ctx, "reading %s id=%q: %s", resourceKey, id, err) + // This is not an error during deletion, so don't return false here + } } entry.RemoteState = remoteState diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 92e5950ee96..0938ff0c36d 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -3,6 +3,7 @@ package phases import ( "context" "errors" + "slices" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/artifacts" @@ -42,6 +43,10 @@ var deployApprovalGroups = []approvalGroup{ func approvalForDeploy(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) (bool, error) { actions := plan.GetActions() + // Deletes of resources that are already gone remotely only clean up the state, + // so they don't count as destructive actions and need no approval. + actions = slices.DeleteFunc(actions, func(a deployplan.Action) bool { return a.Gone }) + err := checkForPreventDestroy(b, actions) if err != nil { return false, err diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index 95eec600dc2..8997299c10c 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -4,6 +4,7 @@ import ( "context" "errors" "net/http" + "slices" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/engine" @@ -46,6 +47,10 @@ var destroyApprovalGroups = []approvalGroup{ func approvalForDestroy(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) (bool, error) { deleteActions := plan.GetActions() + // Deletes of resources that are already gone remotely only clean up the state, + // so they don't count as destructive actions and are not listed as deletions. + deleteActions = slices.DeleteFunc(deleteActions, func(a deployplan.Action) bool { return a.Gone }) + err := checkForPreventDestroy(b, deleteActions) if err != nil { return false, err