From 444caaa59b7748e35b0d244a1a8a82db4892ffb9 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 23 Jul 2026 13:17:47 +0200 Subject: [PATCH 1/2] Treat DELETING apps as gone during plan; enable app idempotency tests App DELETE is fire-and-forget: the app sits in ComputeState=DELETING for up to ~20 minutes and a GET returns it (not 404), so a second delete returns 400. Add an optional IResource.IsGone(remote) hook, consulted in the plan Delete branch, and implement it for apps to mark the entry Gone so apply cleans up state without re-issuing the delete. Re-enables app.yml.tmpl in the delete_idempotent/destroy_idempotent invariant tests. Co-authored-by: Isaac --- .nextchanges/bundles/app-delete-idempotent.md | 1 + .../invariant/delete_idempotent/out.test.toml | 1 + .../invariant/delete_idempotent/test.toml | 17 ---------- .../destroy_idempotent/out.test.toml | 1 + .../invariant/destroy_idempotent/test.toml | 17 ---------- bundle/direct/bundle_plan.go | 5 +++ bundle/direct/dresources/adapter.go | 31 +++++++++++++++++++ bundle/direct/dresources/all_test.go | 6 ++-- bundle/direct/dresources/app.go | 12 +++++++ 9 files changed, 54 insertions(+), 37 deletions(-) create mode 100644 .nextchanges/bundles/app-delete-idempotent.md diff --git a/.nextchanges/bundles/app-delete-idempotent.md b/.nextchanges/bundles/app-delete-idempotent.md new file mode 100644 index 00000000000..72ca7a3f4da --- /dev/null +++ b/.nextchanges/bundles/app-delete-idempotent.md @@ -0,0 +1 @@ +Fixed `bundle deploy`/`bundle destroy` failing when an app is still in the transient DELETING state; the delete is now treated as complete instead of erroring (direct engine only). diff --git a/acceptance/bundle/invariant/delete_idempotent/out.test.toml b/acceptance/bundle/invariant/delete_idempotent/out.test.toml index 357ff82e029..1b0b5d6585a 100644 --- a/acceptance/bundle/invariant/delete_idempotent/out.test.toml +++ b/acceptance/bundle/invariant/delete_idempotent/out.test.toml @@ -4,6 +4,7 @@ RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] EnvMatrix.INPUT_CONFIG = [ "alert.yml.tmpl", + "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "cluster_apply_policy_default_values.yml.tmpl", diff --git a/acceptance/bundle/invariant/delete_idempotent/test.toml b/acceptance/bundle/invariant/delete_idempotent/test.toml index dfdc44ebf96..3f5bb92afad 100644 --- a/acceptance/bundle/invariant/delete_idempotent/test.toml +++ b/acceptance/bundle/invariant/delete_idempotent/test.toml @@ -3,20 +3,3 @@ EnvMatrix.READPLAN = ["", "1"] # Snapshot of pre-delete state used to re-run the delete on state that still # references the (now-gone) resources; may linger if the test fails mid-run. Ignore = [".databricks.backup"] - -# Badness: apps DoDelete is fire-and-forget (bundle/direct/dresources/app.go): after the -# first DELETE the app sits in ComputeState=DELETING for up to ~20 minutes, and -# a second DELETE against it returns 400 rather than 404. The CLI's plan/apply -# does not special-case this transient state -- DoRead sees DELETING (not 404), -# entry.Gone stays false, apply calls DoDelete, and the deploy errors out with: -# -# Error: cannot delete resources.apps.foo: deleting id=app-: Cannot -# delete app app- as it is not terminal with state DELETING, and was -# updated less than 20 minutes ago. Please wait before trying again. -# (400 BAD_REQUEST) -# -# Reproduces 100% on aws-prod-ucws / azure-prod-ucws / gcp-prod-ucws and -# locally with the DELETING transition modeled in libs/testserver/apps.go. -# Re-enable once app delete becomes idempotent (either WaitAfterDelete polls to -# terminal, or the plan/apply layer treats DELETING as effectively-Gone). -EnvMatrixExclude.no_app = ["INPUT_CONFIG=app.yml.tmpl"] diff --git a/acceptance/bundle/invariant/destroy_idempotent/out.test.toml b/acceptance/bundle/invariant/destroy_idempotent/out.test.toml index 357ff82e029..1b0b5d6585a 100644 --- a/acceptance/bundle/invariant/destroy_idempotent/out.test.toml +++ b/acceptance/bundle/invariant/destroy_idempotent/out.test.toml @@ -4,6 +4,7 @@ RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] EnvMatrix.INPUT_CONFIG = [ "alert.yml.tmpl", + "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "cluster_apply_policy_default_values.yml.tmpl", diff --git a/acceptance/bundle/invariant/destroy_idempotent/test.toml b/acceptance/bundle/invariant/destroy_idempotent/test.toml index f578e26ff69..16cf0797a77 100644 --- a/acceptance/bundle/invariant/destroy_idempotent/test.toml +++ b/acceptance/bundle/invariant/destroy_idempotent/test.toml @@ -3,20 +3,3 @@ EnvMatrix.READPLAN = ["", "1"] # Snapshot of pre-destroy state used to re-run destroy on state that still # references the (now-gone) resources; may linger if the test fails mid-run. Ignore = [".databricks.backup"] - -# apps DoDelete is fire-and-forget (bundle/direct/dresources/app.go): after the -# first DELETE the app sits in ComputeState=DELETING for up to ~20 minutes, and -# a second DELETE against it returns 400 rather than 404. The CLI's plan/apply -# does not special-case this transient state -- DoRead sees DELETING (not 404), -# entry.Gone stays false, apply calls DoDelete, and destroy errors out with: -# -# Error: cannot delete resources.apps.foo: deleting id=app-: Cannot -# delete app app- as it is not terminal with state DELETING, and was -# updated less than 20 minutes ago. Please wait before trying again. -# (400 BAD_REQUEST) -# -# Reproduces 100% on all clouds and locally with the DELETING transition -# modeled in libs/testserver/apps.go. -# Re-enable once app delete becomes idempotent (either WaitAfterDelete polls to -# terminal, or the plan/apply layer treats DELETING as effectively-Gone). -EnvMatrixExclude.no_app = ["INPUT_CONFIG=app.yml.tmpl"] diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 139012423ea..fda8180cfc4 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -191,6 +191,11 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks log.Warnf(ctx, "reading %s id=%q: %s", resourceKey, id, err) // This is not an error during deletion, so don't return false here } + } else if adapter.IsGone(remoteState) { + // The resource is in a transient terminal-teardown state (e.g. an app in + // DELETING) that a GET still returns but a second delete would reject. + // Treat it as gone: apply cleans up state without re-issuing the delete. + entry.Gone = true } entry.RemoteState = remoteState diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index 70a9f1f5e3d..fdaa15bfcea 100644 --- a/bundle/direct/dresources/adapter.go +++ b/bundle/direct/dresources/adapter.go @@ -83,6 +83,13 @@ type IResource interface { // [Optional] KeyedSlices returns a map from path patterns to KeyFunc for comparing slices by key instead of by index. // Example: func (*ResourcePermissions) KeyedSlices(state *PermissionsState) map[string]any KeyedSlices() map[string]any + + // [Optional] IsGone reports whether a remote resource should be treated as + // already-deleted when planning a delete. Use for backends whose DELETE is + // asynchronous and leaves the resource in a transient terminal-teardown state + // (returned by GET, not 404) that rejects a second DELETE. + // Example: func (*ResourceApp) IsGone(remote *AppRemote) bool + IsGone(remoteState any) bool } // Adapter wraps resource implementation, validates signatures and type consistency across methods @@ -103,6 +110,7 @@ type Adapter struct { waitAfterDelete *calladapt.BoundCaller overrideChangeDesc *calladapt.BoundCaller doResize *calladapt.BoundCaller + isGone *calladapt.BoundCaller resourceConfig *ResourceLifecycleConfig generatedResourceConfig *ResourceLifecycleConfig @@ -135,6 +143,7 @@ func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceC waitAfterUpdate: nil, waitAfterDelete: nil, overrideChangeDesc: nil, + isGone: nil, resourceConfig: GetResourceConfig(resourceType), generatedResourceConfig: GetGeneratedResourceConfig(resourceType), keyedSlices: nil, @@ -231,6 +240,11 @@ func (a *Adapter) initMethods(resource any) error { return err } + a.isGone, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "IsGone") + if err != nil { + return err + } + keyedSlicesCall, err := calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "KeyedSlices") if err != nil { return err @@ -312,6 +326,10 @@ func (a *Adapter) validate() error { validations = append(validations, "DoResize newState", a.doResize.InTypes[2], stateType) } + if a.isGone != nil { + validations = append(validations, "IsGone remoteState", a.isGone.InTypes[0], remoteType) + } + if a.doUpdateWithID != nil { validations = append(validations, "DoUpdateWithID newState", a.doUpdateWithID.InTypes[2], stateType) // DoUpdateWithID must return (string, remoteType, error) @@ -564,6 +582,19 @@ func (a *Adapter) KeyedSlices() map[string]any { return a.keyedSlices } +// IsGone reports whether the remote state represents an already-deleted resource +// for planning purposes. Resources that don't implement IsGone are never gone. +func (a *Adapter) IsGone(remoteState any) bool { + if a.isGone == nil { + return false + } + outs, err := a.isGone.Call(remoteState) + if err != nil { + return false + } + return outs[0].(bool) +} + // prepareCallRequired prepares a call and ensures the method is found. func prepareCallRequired(resource any, methodName string) (*calladapt.BoundCaller, error) { caller, err := calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), methodName) diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 80e186ca9b8..2a83b408918 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1079,9 +1079,9 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W // Apps DoDelete is fire-and-forget: the API returns success while the app // sits in DELETING state for up to ~20 minutes before the record is removed. // A GET on the DELETING app returns the app, not 404 -- the testserver - // mirrors that in libs/testserver/apps.go. The CLI does not yet special-case - // this transient state (see acceptance/bundle/invariant/{delete,destroy} - // _idempotent tests for the resulting idempotency gap). + // mirrors that in libs/testserver/apps.go. DoRead therefore still succeeds + // here; the planner treats this transient state as gone via ResourceApp.IsGone + // so delete/destroy stay idempotent (acceptance/bundle/invariant/{delete,destroy}_idempotent). deleteLeavesDeleting := group == "apps" remoteAfterDelete, err := adapter.DoRead(ctx, createdID) diff --git a/bundle/direct/dresources/app.go b/bundle/direct/dresources/app.go index c3928755818..1e8a92dd88c 100644 --- a/bundle/direct/dresources/app.go +++ b/bundle/direct/dresources/app.go @@ -308,6 +308,18 @@ func (r *ResourceApp) DoDelete(ctx context.Context, id string, _ *AppState) erro return err } +// IsGone treats a DELETING app as already-deleted when planning a delete. The +// Apps DELETE is fire-and-forget: it returns success while the app sits in +// ComputeState=DELETING for up to ~20 minutes, and a GET during that window +// returns the app (not 404), so the planner cannot rely on IsMissing. A second +// DELETE while DELETING is rejected with 400, so without this the delete/destroy +// path is not idempotent (acceptance/bundle/invariant/{delete,destroy}_idempotent). +// This only covers the delete path; a DELETING app hit during a normal deploy still +// produces a spurious update/skip because plan/apply do not special-case it there. +func (*ResourceApp) IsGone(remote *AppRemote) bool { + return remote.ComputeStatus != nil && remote.ComputeStatus.State == apps.ComputeStateDeleting +} + func (r *ResourceApp) WaitAfterCreate(ctx context.Context, id string, config *AppState) (*AppRemote, error) { remote, err := r.waitForApp(ctx, r.client, config.Name) if err != nil { From cdf6cfbd1fb47381f7a4a0d5b323b1e8edaaab61 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 23 Jul 2026 17:27:24 +0200 Subject: [PATCH 2/2] Assert IsGone contract in testCRUD Assert IsGone directly on the remote returned after delete: true for a DELETING app (the state the planner short-circuits on), false for the noop permissions/grants case where the resource genuinely still exists. Co-authored-by: Isaac --- bundle/direct/dresources/all_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 2a83b408918..54541d94ba9 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1088,9 +1088,15 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W switch { case deleteIsNoop: require.NoError(t, err) + // The resource genuinely still exists, so it must not report as gone. + assert.False(t, adapter.IsGone(remoteAfterDelete)) case deleteLeavesDeleting: require.NoError(t, err) require.NotNil(t, remoteAfterDelete) + // IsGone lets the planner short-circuit the second delete on this + // transient DELETING state; this is what keeps the delete/destroy + // invariant tests idempotent, so assert the contract directly. + assert.True(t, adapter.IsGone(remoteAfterDelete)) default: require.Error(t, err) require.Nil(t, remoteAfterDelete)