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..54541d94ba9 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1079,18 +1079,24 @@ 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) 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) 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 {