Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .nextchanges/bundles/app-delete-deleting-apply.md
Original file line number Diff line number Diff line change
@@ -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).
1 change: 1 addition & 0 deletions acceptance/bundle/apps/delete_deleting/app/app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print("Simple test app")
1 change: 1 addition & 0 deletions acceptance/bundle/apps/delete_deleting/app/app.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
command: ["python", "app.py"]
11 changes: 11 additions & 0 deletions acceptance/bundle/apps/delete_deleting/databricks.yml.tmpl
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions acceptance/bundle/apps/delete_deleting/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions acceptance/bundle/apps/delete_deleting/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

=== Deploy the app

=== Apply the stale delete plan against the DELETING app
DELETE_OK
41 changes: 41 additions & 0 deletions acceptance/bundle/apps/delete_deleting/script
Original file line number Diff line number Diff line change
@@ -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 <<EOF
bundle:
name: app-delete-deleting-$UNIQUE_NAME

workspace:
root_path: "~/.bundle/app-delete-deleting-$UNIQUE_NAME"

resources: {}
EOF

trace $CLI bundle plan -o json > 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
8 changes: 8 additions & 0 deletions acceptance/bundle/apps/delete_deleting/test.toml
Original file line number Diff line number Diff line change
@@ -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"]
26 changes: 25 additions & 1 deletion bundle/direct/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand Down
13 changes: 8 additions & 5 deletions bundle/direct/dresources/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading