Skip to content

acc: cover spurious recreate/update when referencing a recreated resource - #6895

Open
denik wants to merge 9 commits into
mainfrom
denik/downgrade-action
Open

denik wants to merge 9 commits into
mainfrom
denik/downgrade-action

Conversation

@denik

@denik denik commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

When a resource is recreated, the direct deploy engine cannot resolve a reference to its output fields at plan time (a recreate does not preserve the id), so the dependent field diffs against the literal ${...} placeholder and the dependent gets a spurious recreate (immutable/id field) or update (mutable field) — even though the referenced value is unchanged. (stable_output_fields, #6865, only covers in-place updates of the referent, not recreates.)

Three acceptance tests capturing this. They run the direct engine only — terraform is being removed, so its behavior is described in the comments for context rather than executed:

  • postgres_projects/recreate_ref_both_engines — spurious recreate: a pg_version bump recreates a project (name stable); the catalog that references the project name is recreated too — the recorded requests show the destructive DELETE+POST.
  • postgres_databases/recreate_ref_spurious_update — spurious update: a postgres_role change recreates a role (name stable); the database that references the role name gets a PATCH.
  • schemas/recreate_id_ref_spurious_update — the same bug on UC schemas (via a .id reference) and its boundary: UC's idiomatic name references are user-provided inputs that resolve from config, so only output (.id) references hit it.

The field-level proof — the saved value vs the still-unresolved reference, with the remote unchanged — shows the action is spurious. The fix is in the stacked PR #6914.

This pull request and its description were written by Isaac.

@github-actions github-actions Bot added the DABs DABs related issues label Oct 1, 2026
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 9be9e49

Run: 37362339736

Env ✅​pass 🙈​skip Time
✅​ aws linux 276 19 4:54
✅​ aws windows 278 17 3:30
✅​ azure linux 275 19 5:36
✅​ azure windows 277 17 3:55
✅​ gcp linux 276 19 6:10
✅​ gcp windows 278 17 3:42
Top 6 slowest tests (at least 2 minutes):
duration env testname
3:52 azure windows TestAccept
3:50 azure linux TestAccept
3:28 aws windows TestAccept
3:24 gcp windows TestAccept
2:59 aws linux TestAccept
2:58 gcp linux TestAccept

@denik
denik marked this pull request as ready for review October 2, 2026 09:31
@denik
denik requested review from a team as code owners October 2, 2026 09:31
denik and others added 7 commits October 5, 2026 11:53
…urce

The direct engine cannot resolve ${resources.X.name}/.id while X is being
recreated (a recreate does not preserve the id), so a dependent referencing X
diffs the saved value against the unresolved "${...}" placeholder and plans a
phantom change -- recreate for an immutable field, update for a mutable one --
even though the referenced value is unchanged.

- postgres_projects/recreate_ref_both_engines: a pg_version bump recreates the
  project (project_id, hence name, unchanged); both engines spuriously recreate a
  catalog whose branch references the project name. Apples-to-apples, same behavior.
- postgres_databases/recreate_ref_spurious_update: a postgres_role change recreates
  the role (role_id, hence name, unchanged); the direct engine spuriously updates a
  database whose mutable role field references the role name.

Co-authored-by: Isaac <no-reply@databricks.com>
Rename p/b/owner/d/cat to my_project/my_branch/my_role/my_database/my_catalog.

Co-authored-by: Isaac <no-reply@databricks.com>
…ference tests

Deploy the spurious action and record the API calls per engine, so the goldens show
not just the plan but what actually happens:
- recreate test: both engines DELETE + POST the catalog with an identical, unchanged spec.
- update test: direct issues a spurious PATCH on the database (writing back the unchanged
  role) while terraform issues no request for it at all.

Deploy stdout goes to LOG.deploy (parallel completion order is nondeterministic). Scripts
now explain what is planned vs. what happens, and the plan/request differences per engine.

Co-authored-by: Isaac <no-reply@databricks.com>
Adds schemas/recreate_id_ref_spurious_update. UC resources reference each other by name
(user-provided inputs that resolve from config), so idiomatic UC wiring is immune; the bug
only surfaces via an output-only .id reference. referent recreates on storage_root (keeping
its full_name/.id), and consumer references its id in a comment, so both engines PLAN a
spurious update of consumer.

The recorded requests show the engines diverge at deploy: the direct engine PATCHes consumer
(a spurious write), while terraform downgrades the planned update to a no-op once the id
resolves. In the recreate test both engines instead execute the replace -- terraform cannot
cancel it (separate destroy/create graph nodes), which is why the direct engine's apply-time
downgrade would be strictly more capable.

Co-authored-by: Isaac <no-reply@databricks.com>
Lead each databricks.yml with a standalone description of the bug (a reference to a
recreated resource cannot be resolved at plan time, so a dependent sees a phantom change)
instead of cross-referencing the sibling tests, so each file reads on its own.

Co-authored-by: Isaac <no-reply@databricks.com>
Terraform is being removed, so pin these tests to the direct engine and drop the terraform
goldens. The comments describe terraform's behavior for context rather than running it, and
the field-level proof (the saved value vs the still-unresolved reference, with the remote
unchanged) demonstrates the bug without a terraform comparison.

Co-authored-by: Isaac <no-reply@databricks.com>
The engine matrix has been removed from the acceptance suite, so the tests no longer pin
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] or carry an engine suffix on their golden
files; they run with the (only) direct engine and plain golden names.

Co-authored-by: Isaac <no-reply@databricks.com>
@denik
denik force-pushed the denik/downgrade-action branch from 3b34033 to 3de1a80 Compare October 5, 2026 09:58
denik and others added 2 commits October 5, 2026 15:46
With a single engine the plan no longer diverges, so there is no reason to route it to a
separate file -- record it inline in output.txt, matching the recreate and UC tests.

Co-authored-by: Isaac <no-reply@databricks.com>
RecordRequests is already set in acceptance/bundle/resources/test.toml, so the leaf configs
need not repeat it. Also reword the schemas test: the bug is hit by any output-only reference
(e.g. .id or full_name), not only .id.

Co-authored-by: Isaac <no-reply@databricks.com>
@denik
denik enabled auto-merge October 5, 2026 19:16
@denik
denik added this pull request to the merge queue Oct 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DABs DABs related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants