Skip to content

bundle: keep resources.json in step with DMS, marked by a state feature - #6475

Open
shreyas-goenka wants to merge 10 commits into
isaac/pr6052-fixesfrom
isaac/dms-dual-state
Open

bundle: keep resources.json in step with DMS, marked by a state feature#6475
shreyas-goenka wants to merge 10 commits into
isaac/pr6052-fixesfrom
isaac/dms-dual-state

Conversation

@shreyas-goenka

@shreyas-goenka shreyas-goenka commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Stacked on #6094. Under recording, resources.json becomes a header-only marker (state_version: 3 plus features: {record_deployment_history: {}}) and the service is the state — the file's resources are never written or read back.

Every check that used to key off "has any resources" or the config setting now keys off that marker. That is what makes turning recording off, destroy, and bind/unbind behave: without it they silently created a second copy of every resource, wedged a destroyed bundle, or reported a write that was discarded.

bundle/dms/state-from-service deletes the local cache and the remote state file, and the redeploy is still a no-op on all four resources; stale-deployment and requires-recording cover the refusals. Acceptance state helpers read ids and state from the service via new acceptance/bin/dms_resources.py.

This pull request and its description were written by Isaac.

@shreyas-goenka
shreyas-goenka force-pushed the isaac/dms-dual-state branch 2 times, most recently from 7047d22 to 1fd16ac Compare September 2, 2026 02:03
@shreyas-goenka
shreyas-goenka force-pushed the isaac/pr6052-fixes branch 2 times, most recently from a5b1a71 to 58257c8 Compare September 2, 2026 02:30
@shreyas-goenka
shreyas-goenka force-pushed the isaac/dms-dual-state branch 5 times, most recently from cee981f to f7d92ee Compare September 2, 2026 13:03
@shreyas-goenka
shreyas-goenka force-pushed the isaac/dms-dual-state branch 3 times, most recently from 855650e to 0af021f Compare September 2, 2026 14:16
@shreyas-goenka shreyas-goenka mentioned this pull request Sep 2, 2026
Closed
@shreyas-goenka
shreyas-goenka force-pushed the isaac/pr6052-fixes branch 2 times, most recently from 26ad86b to c98c8e1 Compare September 2, 2026 23:39
@shreyas-goenka
shreyas-goenka marked this pull request as ready for review September 2, 2026 23:50
@github-actions

github-actions Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

35 files changed
Suggested: @denik
Also eligible: @janniklasrose, @pietern, @andrewnester, @lennartkats-db, @anton-107

/bundle/ - needs approval

5 files changed
Suggested: @denik
Also eligible: @janniklasrose, @pietern, @andrewnester, @lennartkats-db, @anton-107

/cmd/bundle/ - needs approval

Files: cmd/bundle/generate/dashboard.go, cmd/bundle/generate/genie_space.go, cmd/bundle/utils/process.go
Suggested: @denik
Also eligible: @janniklasrose, @pietern, @andrewnester, @lennartkats-db, @anton-107

General files (require maintainer)

5 files changed
Based on git history:

  • @denik -- recent work in cmd/bundle/utils/, bundle/direct/dstate/, acceptance/bin/

Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db, @rugpanov, @rclarey) can approve all areas.
See OWNERS for ownership rules.

@shreyas-goenka Isaac
bundle: stamp DMS fields at deploy time and reject stale plan replays
84f6960 
A saved plan no longer carries the DMS deployment id or version. They are stamped onto
jobs and pipelines at deploy time - InitForApply for deploy --plan, the deploy phase
otherwise - so bundle plan output is the same whether or not deployment-history
recording is on. deploy --plan rejects a first-plan replay once the deployment has
recorded a version. Also removes the now-unneeded deployment-version heartbeat goroutine.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka shreyas-goenka reopened this Sep 3, 2026
shreyas-goenka and others added 3 commits September 3, 2026 00:30
@shreyas-goenka Isaac
dstate: keep resources.json in step with DMS, marked by a state feature
5206a26 
Recording made the service the source of truth for resource state, but the
state file was still written on every deploy. Mark it as depending on the
recording feature so an unaware CLI refuses it instead of deploying over a
deployment it would leave the service behind on, and use the marker rather
than "has any resources" to decide whether an existing state can be recorded.

While recording, the file's resource set is never read back: the service is
asked on every open, and holding no deployment means the resources are created
again. Turning recording off for a marked deployment is refused. The WAL needs
no special handling - whatever a replay rebuilds is replaced by what the
service holds.

Two tests that seed a state file now run with recording on; the ones that
cannot are opted out with their real reasons recorded.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka Isaac
acc: read recorded ids and state from the deployment metadata service
04c259b 
WIP. The id helpers read the direct-engine state file, which is the wrong source
while a bundle records deployment history: the service owns the resource set. Add
dms_resources.get_resources, which resolves the deployment from the workspace node
the service registers and lists its resources, and have read_id, replace_ids and
read_state use it when recording is on.

Not ready: resolving the remote state path runs "bundle validate", which fails for
tests that deploy with --var or other flags the helper cannot know (see
bundle/resources/secrets/*). And a hollow state file - the reason for reading from
the service at all - breaks "bundle generate", bind and configsync, which open the
state with no DMS client and need the resource ids from it.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka Isaac
dstate: make the recorded state file a header-only marker
dbed355 
The service owns the resource set while recording, so writing the resources
into the state file as well left a second copy that nothing read. Write only
the header - above all the feature, which is what an unaware CLI refuses.

Every consumer of resource state therefore has to read it through the service.
bundle generate now opens the state with a client (new utils.DmsClientForState,
which also removes the construction process.go had inline), and bind/unbind
refuse a recorded state outright rather than silently binding against one that
looks empty. The acceptance helpers that read the state file - read_id,
read_state, replace_ids and print_state - go to the service too, so both DMS
variants of a test still assert one golden.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka Isaac
acc: follow the base's per-command recording switch
e949edc 
The DMS tests now enable recording through the env var per command rather than
by editing databricks.yml, so the tests that turn it off had to do the same.
failed-delete drains the reads print_state.py makes against the service, which
would otherwise be left for the harness to diff.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka Isaac
acc: cover the service standing in for the state file on its own
4f581d2 
Wipes every local trace of a deployment and redeploys, so the only thing the
CLI can know comes from the service. Covers a dependency edge round-tripping
(depends_on is not something the service has a field for) and a resource type
that carries no deployment stamp, where an unchanged redeploy is only possible
if the recorded state really is the state.

Co-authored-by: Isaac <no-reply@databricks.com>
shreyas-goenka and others added 4 commits September 3, 2026 08:30
@shreyas-goenka Isaac
dstate: key the recording guards off the state marker
b7bc24d 
The header-only write leaves a recorded state with no resources, which defeated
three checks that read len(state) or the config setting instead of the marker:
turning recording on over an unrecorded state was silently allowed whenever a
deployment happened to resolve (dropping those resources and recreating them),
destroy left the marker behind with nothing to protect, and bind/unbind wrote
into a recorded state and reported success while the write was discarded.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka Isaac
Say what to do about a state that requires recording
6d7639e 
Also puts back the doc comment my earlier change pushed off
fetchDeploymentFromStatePath, and reverts a comment rewrite.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka Isaac
acc: read the recorded state in the secrets test too
3b665ee 
print_state.py piped through "jq .state" drops the header the two DMS variants
disagree on, so the test runs in both instead of opting out of recording. Also
pages ListResources, which read only the first 50 resources on cloud runs.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka Isaac
acc: prove the service alone can rebuild the deployment
4717c02 
Deleting the remote state file as well as the local cache leaves no state file
anywhere, and the redeploy is still a no-op on all four resources. It writes no
state file back either, since nothing changed.

Co-authored-by: Isaac <no-reply@databricks.com>
@shreyas-goenka
shreyas-goenka force-pushed the isaac/pr6052-fixes branch 6 times, most recently from 9dd21de to 8804196 Compare September 3, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@shreyas-goenka