diff --git a/.nextchanges/bundles/app-delete-deleting-apply.md b/.nextchanges/bundles/app-delete-deleting-apply.md new file mode 100644 index 00000000000..104e278de2b --- /dev/null +++ b/.nextchanges/bundles/app-delete-deleting-apply.md @@ -0,0 +1 @@ +Fixed `bundle deploy`/`bundle destroy` failing when an app enters the transient DELETING state between plan and apply (e.g. with a saved plan); the delete is now treated as complete instead of erroring (direct engine only). diff --git a/acceptance/bundle/apps/delete_deleting/app/app.py b/acceptance/bundle/apps/delete_deleting/app/app.py new file mode 100644 index 00000000000..271596cec04 --- /dev/null +++ b/acceptance/bundle/apps/delete_deleting/app/app.py @@ -0,0 +1 @@ +print("Simple test app") diff --git a/acceptance/bundle/apps/delete_deleting/app/app.yml b/acceptance/bundle/apps/delete_deleting/app/app.yml new file mode 100644 index 00000000000..45b242d4064 --- /dev/null +++ b/acceptance/bundle/apps/delete_deleting/app/app.yml @@ -0,0 +1 @@ +command: ["python", "app.py"] diff --git a/acceptance/bundle/apps/delete_deleting/databricks.yml.tmpl b/acceptance/bundle/apps/delete_deleting/databricks.yml.tmpl new file mode 100644 index 00000000000..7dc2e096926 --- /dev/null +++ b/acceptance/bundle/apps/delete_deleting/databricks.yml.tmpl @@ -0,0 +1,11 @@ +bundle: + name: app-delete-deleting-$UNIQUE_NAME + +workspace: + root_path: "~/.bundle/app-delete-deleting-$UNIQUE_NAME" + +resources: + apps: + my_app: + name: app-$UNIQUE_NAME + source_code_path: ./app diff --git a/acceptance/bundle/apps/delete_deleting/out.test.toml b/acceptance/bundle/apps/delete_deleting/out.test.toml new file mode 100644 index 00000000000..e90b6d5d1ba --- /dev/null +++ b/acceptance/bundle/apps/delete_deleting/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/apps/delete_deleting/output.txt b/acceptance/bundle/apps/delete_deleting/output.txt new file mode 100644 index 00000000000..3b2474e01ad --- /dev/null +++ b/acceptance/bundle/apps/delete_deleting/output.txt @@ -0,0 +1,5 @@ + +=== Deploy the app + +=== Apply the stale delete plan against the DELETING app +DELETE_OK diff --git a/acceptance/bundle/apps/delete_deleting/script b/acceptance/bundle/apps/delete_deleting/script new file mode 100644 index 00000000000..063e763c644 --- /dev/null +++ b/acceptance/bundle/apps/delete_deleting/script @@ -0,0 +1,41 @@ +#!/bin/bash + +# Regression: a saved delete-plan computed while the app is ACTIVE has Gone=false. +# If the app enters DELETING before the plan is applied (concurrent delete, or an +# interrupted destroy), apply must still succeed instead of failing with the +# backend's "Cannot delete app ... not terminal with state DELETING" 400. + +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve &> LOG.destroy +} +trap cleanup EXIT + +title "Deploy the app\n" +trace $CLI bundle deploy &> LOG.deploy +cat LOG.deploy | contains.py '!panic' '!internal error' > /dev/null + +# Save a delete plan while the app is still ACTIVE, so the plan records Gone=false. +cat > databricks.yml < plan_delete.json 2>LOG.plan_delete.err +cat LOG.plan_delete.err | contains.py '!panic' '!internal error' > /dev/null + +# Out-of-band delete flips the app into DELETING: a second delete now returns 400. +trace $CLI apps delete "app-$UNIQUE_NAME" &> LOG.apps_delete +cat LOG.apps_delete | contains.py '!panic' '!internal error' > /dev/null + +title "Apply the stale delete plan against the DELETING app\n" +trace $CLI bundle deploy --auto-approve --plan plan_delete.json &> LOG.deploy_delete +cat LOG.deploy_delete | contains.py '!panic' '!internal error' > /dev/null + +echo DELETE_OK diff --git a/acceptance/bundle/apps/delete_deleting/test.toml b/acceptance/bundle/apps/delete_deleting/test.toml new file mode 100644 index 00000000000..67577a6d822 --- /dev/null +++ b/acceptance/bundle/apps/delete_deleting/test.toml @@ -0,0 +1,8 @@ +Local = true +Cloud = false +RecordRequests = false + +# Saved-plan deploy (`--plan`) and the IsGone gone-detection are direct-engine only. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [".databricks", "databricks.yml", "plan_delete.json"] diff --git a/bundle/direct/apply.go b/bundle/direct/apply.go index d227d7f564c..b3c46036c53 100644 --- a/bundle/direct/apply.go +++ b/bundle/direct/apply.go @@ -107,7 +107,10 @@ func (d *DeploymentUnit) Recreate(ctx context.Context, db *dstate.DeploymentStat // place, matching the Terraform provider's recreate behaviour. err = retryOnTransientErr(ctx, func() error { return d.Adapter.DoDelete(ctx, oldID, oldState) }) if err != nil && !apierr.IsMissing(err) && !isManagedByParent(err) { - return fmt.Errorf("deleting old id=%s: %w", oldID, err) + if !d.deleteConfirmedGone(ctx, oldID) { + return fmt.Errorf("deleting old id=%s: %w", oldID, err) + } + log.Warnf(ctx, "Treating %s id=%s as already deleted despite delete error: %s", d.ResourceKey, oldID, err) } // Drop the state entry so a subsequent failure of Create or WaitAfterDelete @@ -225,6 +228,8 @@ func (d *DeploymentUnit) Delete(ctx context.Context, db *dstate.DeploymentState, // mean configuration error that user is trying to fix by removing resource from their bundle. if errors.Is(err, apierr.ErrPermissionDenied) { log.Warnf(ctx, "Ignoring permission error when deleting %s id=%s: %s", d.ResourceKey, oldID, err) + } else if d.deleteConfirmedGone(ctx, oldID) { + log.Warnf(ctx, "Treating %s id=%s as already deleted despite delete error: %s", d.ResourceKey, oldID, err) } else { return fmt.Errorf("deleting id=%s: %w", oldID, err) } @@ -246,6 +251,25 @@ func (d *DeploymentUnit) Delete(ctx context.Context, db *dstate.DeploymentState, return nil } +// deleteConfirmedGone reports whether a failed DoDelete can be treated as +// complete: the backend already removed the resource, or it is in a transient +// terminal-teardown state (e.g. an app in DELETING) that IsGone recognises and a +// retried delete would keep rejecting. This closes the plan/apply gap for saved +// plans: IsGone is consulted at plan time, but with `deploy --plan` the resource +// may enter that state only after the plan is saved. We re-read rather than match +// the delete error because the backend returns a generic 400 BAD_REQUEST (see +// apps/src/utils/AppsStatusUtils.scala) that carries no distinct SDK sentinel. +func (d *DeploymentUnit) deleteConfirmedGone(ctx context.Context, id string) bool { + remote, err := d.Adapter.DoRead(ctx, id) + if apierr.IsMissing(err) { + return true + } + if err != nil { + return false + } + return d.Adapter.IsGone(remote) +} + func (d *DeploymentUnit) Resize(ctx context.Context, db *dstate.DeploymentState, id string, newState any, entry *deployplan.PlanEntry) error { err := retryOnTransientErr(ctx, func() error { return d.Adapter.DoResize(ctx, id, newState, entry) }) if err != nil { diff --git a/bundle/direct/dresources/app.go b/bundle/direct/dresources/app.go index 1e8a92dd88c..29e1110513c 100644 --- a/bundle/direct/dresources/app.go +++ b/bundle/direct/dresources/app.go @@ -308,14 +308,17 @@ 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 +// IsGone treats a DELETING app as already-deleted. 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 +// returns the app (not 404), so callers 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. +// Consulted both at plan time (bundle_plan.go) and after a failed apply-time +// delete (apply.go deleteConfirmedGone), which covers saved-plan deploys where +// the app enters DELETING only after the plan was computed. +// Still uncovered: a DELETING app hit during a normal deploy/update (not a +// delete) 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 }