Repository navigation
Add lifecycle.bind to adopt existing workspace resources at deploy time - #7321
Draft
janniklasrose wants to merge 26 commits into
Draft
janniklasrose wants to merge 26 commits into
janniklasrose wants to merge 26 commits into
Conversation
Allow users to bring existing workspace resources under bundle management declaratively via `bind` blocks under a target. Adds: - `Bind` config type and `bind` field on `Target` in `bundle/config`, with a structural validator that rejects child resources (`*.permissions`, `*.grants`). - `Bind` and `BindAndUpdate` action types in the deploy plan, surfaced in `bundle plan` output (and in the summary as "N to bind"). - `DeploymentBundle.ApplyBindToPlan` (in `bundle/direct/bundle_plan_bind.go`), which runs after `CalculatePlan`, validates bind references, detects bind-ID/state-ID conflicts, and reads remote state to choose `Bind` vs `BindAndUpdate` per resource. - `DeploymentUnit.DeclarativeBind` apply path that persists the bound ID (and optionally invokes `DoUpdate` for `BindAndUpdate`). - Targeted error in `libs/dyn/convert/normalize.go` for `bind` placed at the root level. - Rejection of bind blocks in the terraform path (`RunPlan`), `bundle deployment migrate`, and `bundle destroy` when the bind matches a resource currently in state. Acceptance coverage in `acceptance/bundle/deploy/bind/` covers the happy path (basic, bind_and_update), validation errors (invalid resource type, orphaned bind, top-level bind, duplicate id, terraform engine, child resources), runtime errors (resource not found, recreate blocked), and the destroy / migrate guards. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Isaac
# Conflicts: # bundle/deployplan/plan.go # bundle/direct/bundle_apply.go # bundle/internal/schema/annotations.yml # bundle/phases/deploy.go # bundle/phases/destroy.go
- validateBindConfig now rejects two bind blocks pointing at the same workspace ID. Previously duplicate IDs were only checked against state, so two first-time binds could create two state entries managing one remote resource. - DeclarativeBind's BindAndUpdate path now defers to Update, so DoUpdate runs with transient retries and WaitAfterUpdate is invoked. Async resources rely on that hook; the previous inline path skipped it. - Add acceptance test duplicate-bind-id-config for the new validation. Co-authored-by: Isaac
…, drop root-bind error - Bind.Validate now checks resource types against the closed allowlist from config.SupportedResources() instead of an open blocklist of *.permissions / *.grants. Only types valid under the resources block are bindable; anything else (typo, child key, etc.) is rejected with a "not supported" message. Drops the child-resource terminology. - Split DeclarativeBind into Bind and BindAndUpdate DeploymentUnit methods, dispatched from the apply loop like the other action types. - Remove the special root-level bind error in normalize.go; a root bind block is just an ignored unknown-field warning now. - Move the changelog entry from NEXT_CHANGELOG.md to a .nextchanges/bundles/ fragment (new convention, #5534). - Regenerate affected acceptance goldens (bind-permissions, invalid-resource-type, top-level-bind). Co-authored-by: Isaac
Previously ApplyBindToPlan ran as a separate step after CalculatePlan, re-reading remote state for bound resources with its own error handling. Since binding a resource is just the "resource exists remotely under a known ID but isn't in state yet" variant of normal planning, fold it in: - CalculatePlan takes a bindConfig parameter. Cross-resource bind validation runs once up front; the per-resource bind flow (handleBindPlan) runs inside the DAG walk, in the not-in-state branch, instead of defaulting to Create. - Bound-resource remote reads now go through retryOnTransient and the parallel DAG walk like every other resource read. - The "already in state under a different ID" check moved into validateBindConfig (it is cross-resource, so it stays out of the parallel closure). - Remove ApplyBindToPlan; update all CalculatePlan callers. Bind planning errors now surface under the shared "planning failed" summary instead of "bind planning failed" (goldens regenerated); the detailed per-resource messages are unchanged. Co-authored-by: Isaac
staticcheck QF1002: prefer a tagged switch over switch-true with equality cases. Co-authored-by: Isaac
The design allows `bundle destroy` even when bind blocks reference resources in state: naming destroy explicitly captures the user's intent to tear the deployment down. The bind block guards against *accidental* deletion during a normal deploy, not against a deliberate destroy. Remove errIfBoundResourcesInState and the destroy-time guard, repurpose the destroy-blocked acceptance test as destroy-allowed, and drop the now-unnecessary "remove bind block before destroy" workaround from the basic and bind-and-update tests. Co-authored-by: Isaac
BindAndUpdate hard-coded d.Update, so a bound running cluster with only num_workers/autoscale drift was applied via clusters/edit (which restarts the cluster) instead of the graceful clusters/resize path a normal deploy would take. Dispatch on the planned action instead: Resize for a resize diff, Update otherwise. Recreate/UpdateWithID are already rejected at plan time, so only those two actions can reach here. Add a bind-and-resize acceptance test that binds a running cluster with worker-count-only drift and asserts the resize API is called. Co-authored-by: Isaac
The text plan printed bind actions via StringShort(), which truncates at the first underscore, so bind_and_update rendered as "bind" and was indistinguishable from a pure bind. Print the full action string for bind actions instead, matching the design's plan example and the JSON output. Co-authored-by: Isaac
The recreate/update_id guard only fired during the initial bind, when the resource was still absent from state. Once bound, a later config change that forced a recreate took the normal approval flow with no bind-block guard, so a bound resource could be recreated while its bind block was still defined. Extend the guard to the in-state deploy path: while an active bind block references a resource, a planned Recreate/UpdateWithID surfaces the same actionable error as the initial bind. Removing the bind block remains the explicit opt-out. Add a recreate-blocked-in-state acceptance test. Co-authored-by: Isaac
Main removed the terraform deployment engine (v1.20.0), so the terraform
rejection in RunPlan and the terraform-with-bind test are gone; bind now
runs on the only engine. Bind actions are wired into main's per-resource
deploy reporting ("Bound" / "Bound and updated" lines, "N bound" in the
summary), into plan.CountActions, and into the deployment history
operation types (OPERATION_ACTION_TYPE_BIND / _BIND_AND_UPDATE).
Co-authored-by: Isaac <no-reply@databricks.com>
A bind block whose resource is not in the resources section was only caught at plan time, by looking the key up in the plan map. State-only keys are in that map as delete entries, so removing a bound resource from config while keeping its bind block passed validation and 'bundle deploy' deleted the bound workspace resource. The agreed behaviour is that delete protection applies for as long as the bind block remains. Move the check into ValidateBindResources (initialize phase): it needs only config, so 'bundle validate' now reports it, and a delete can no longer be planned for a key that still has a bind block. Co-authored-by: Isaac <no-reply@databricks.com>
When a bound resource (in state, bind block present) no longer exists in the workspace, the in-state plan path mapped the 404 to a create. The deploy then created a replacement under a new ID, and every following plan failed with 'already bound to a different ID' until the user edited the bind block. A bind block pins the resource to one ID, so report the deletion and let the user decide: remove the block to create a new resource. Co-authored-by: Isaac <no-reply@databricks.com>
A bind block outside targets was dropped as an unknown field with only a warning, and the deploy then created the resources it named instead of binding the existing ones. Fail config loading with a pointer to targets.<target>.bind instead. Co-authored-by: Isaac <no-reply@databricks.com>
SelectTarget keeps the selected target as a typed snapshot and drops the
targets section from the config tree, so ${var.*} in a bind ID was never
interpolated: the plan tried to read a job with the literal ID
'${var.job_id}'. The generated schema already advertises the variable
pattern for this field.
Add a mutator that resolves bundle, workspace and variable references in
the target's bind IDs after the regular variable resolution, sharing the
${var.x} shorthand rewrite with it.
Co-authored-by: Isaac <no-reply@databricks.com>
- Scope the state ID collision check to the bind's own resource type: IDs
are only unique per type, so a job and an experiment may share one.
- Resolve ${resources.<key>.id} references to a resource being bound from
the bind block instead of deferring them to after apply; the ID is known.
- Reuse the remote state read at plan time in Bind, so dependents such as
permissions no longer trigger a second read of the bound resource.
- Reword the persistent recreate guard for resources already in state: they
are bound, not being bound, and 'make this a bind-only operation' does not
apply. Drop the trailing newline from the error text.
- Mirror the in-state DoUpdate check in first-time bind planning so a
resource without DoUpdate fails at plan time, not at apply time.
- Return a status from handleBindPlan instead of reading the shared error
flag, which is racy under parallel planning.
Co-authored-by: Isaac <no-reply@databricks.com>
Bind is state-less: nothing records that a resource was just unbound, so a bind block still in config re-adopts it on the next deploy. Say so when 'bundle deployment unbind' runs for a key that has a bind block in the selected target. The new pure-bind test also pins the re-bind round trip itself, the pure 'bind' action (the first deploy of a job is always a bind_and_update because the bundle sets deployment metadata on it), and the single remote read of a pure bind. Co-authored-by: Isaac <no-reply@databricks.com>
The command only converts an existing terraform state into a direct state now that the terraform engine is gone; it no longer plans anything, and 'bundle deploy' already migrates automatically with bind blocks present. Rejecting bind blocks here only forced users to remove and re-add them. Also note at the remote-sync call site why it plans without bind blocks. Co-authored-by: Isaac <no-reply@databricks.com>
- int-id: an unquoted (integer) ID, as written in the design doc, binds. - dev-mode: development mode transforms (rename, paused schedule, tags) apply to the adopted resource and are visible in the plan. - block-removed: removing the bind block is the opt-out; the same immutable change then plans a recreate. - rebind-different-id: a bind block for a resource already in state must carry the state ID. Co-authored-by: Isaac <no-reply@databricks.com>
Spell out in the schema descriptions what a bind block does on the first deploy (adopt, and update if the config differs, including target mode transforms), what it guards while it remains (delete, recreate), and what it does not (bundle destroy; no marker in state). The regenerated schema also picks up main's current variable pattern for the bind definitions. Co-authored-by: Isaac <no-reply@databricks.com>
…inter Co-authored-by: Isaac <no-reply@databricks.com>
Bind becomes a lifecycle setting of the resource it adopts:
resources:
jobs:
my_job:
lifecycle:
bind:
id: "12345"
with per-target IDs set through the usual target override of the resource.
This replaces the target-level 'bind' block (design decision DD2 reversed).
The block now lives on the resource, so a whole class of misplacements
cannot happen: a bind for an undefined resource, a root-level or target-level
misplacement that silently created a duplicate, and a stale block whose
resource was removed. Variable references, diagnostic locations and target
overrides come from the regular config pipeline, which removes the dedicated
resolver and validator and the typed target snapshot they worked around.
CalculatePlan derives the bind IDs from the config root again.
Semantics follow prevent_destroy: the guard against recreating the bound
resource holds while the block is in config; removing the resource removes
the block with it and the deploy deletes the resource, which the
removed-from-config test pins. Duplicate-ID detection is now scoped per
resource type, matching the state collision check.
Tests: configs moved to the new shape; orphaned-bind, invalid-resource-type,
bind-permissions and top-level-bind are gone (not expressible); new
target-override test; errors now point at resources.<type>.<name>.lifecycle.bind.id
with file locations.
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
|
Commit: Regression test reportSelected 20 changed tests (limit=20, 2 not selected) 40 tests: regression=40 unreleased=0 coverage=0 failing=0
TestAccept/bundle/deploy/bind/basic/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): TestAccept/bundle/deploy/bind/basic/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): TestAccept/bundle/deploy/bind/bind-and-resize/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/bind-and-resize/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): TestAccept/bundle/deploy/bind/bind-and-update/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/bind-and-update/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/block-removed/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/block-removed/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/delete-and-bind-conflict/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/delete-and-bind-conflict/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/destroy-allowed/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/destroy-allowed/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/dev-mode/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/dev-mode/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/duplicate-bind-id-config/DMS=: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) TestAccept/bundle/deploy/bind/duplicate-bind-id-config/DMS=true: branch ✅ | main (44f2eda) ❌ | latest (v1.23.0) ❌main (44f2eda): latest (v1.23.0): same output as main (44f2eda) 24 more failure details omitted, see the full report in the run. |
Collaborator
Integration test reportCommit: 0e9256f
Top 50 slowest tests (at least 2 minutes):
|
… test isolation - Regenerate the PyDABs models: Lifecycle gains bind (validate-generated). - List lifecycle.bind as a non-state field for the resources whose state keeps a lifecycle sub-struct (apps, clusters, sql_warehouses, job_runs), like prevent_destroy and depends_on (TestInputSubset). - Give every deploying bind test a per-run bundle name. The deployment-history variants and the OS runners run concurrently against one workspace, so a fixed name shared the root path and failed with 'Failed to acquire deployment lock' or with the other variant's state. - bind-and-resize: create the cluster with the instance pool only; the cloud API rejects node_type_id together with instance_pool_id. - dev-mode: assert only the planned new values; a real workspace reports schedule.pause_status: UNPAUSED where the test server omits it. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Git Bash on Windows rewrites a leading-slash argument as a Windows path
('C:/Program Files/Git/old/storage/path'), so update_file.py did not find the
value and the test failed on every Windows cloud runner. Match the path
without its leading slash instead.
Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Adds a
lifecycle.bindsetting to bundle resources. On the nextbundle deploy, the CLI adopts the existing workspace resource with that ID into the deployment state instead of creating a new one; from then on the bundle manages it like any other resource.This is the declarative counterpart of
databricks bundle deployment bind, which stays as is.Semantics
bindwhen the remote resource already matches the config,bind_and_updatewhen the config differs (the configured changes are applied through the same update/resize paths as a normal deploy). Both show up inbundle plan(text, and JSON withbind_id), in the deploy output (Bound ...,Bound and updated ...,N bound) and in deployment history.lifecycle.bindis present, a change that would recreate the resource or change its ID is an error that names the fields and the two ways out (revert the fields, or remove the block). The block is state-less: nothing in the state records the adoption, so the guard is exactly the presence of the block, likeprevent_destroy.prevent_destroy.bundle destroydeletes bound resources like any other.bundle deployment unbindwarns when the resource still haslifecycle.bind, since the next deploy re-binds it.idare resolved like any other resource field.Note for adopters: the bundle sets
deployment,edit_modeand similar fields on every job and pipeline, so the first bind of a resource that was never bundle-deployed is abind_and_update(for jobs a fullreset), andmode: developmenttransforms (name prefix, paused schedules, tags) apply to the adopted resource.bundle plan -o jsonlists the fields.Errors caught at plan time
Relation to #4630 and the design
Builds on #4630 and keeps its commits. Changes against the design as approved:
targets.<name>.bindto the resource'slifecycle, so misplaced or stale blocks cannot happen and the regular config pipeline handles variables, locations and overrides;bundle deployment migraterestrictions are moot since the terraform engine was removed in v1.20.0;bundle destroyis allowed, as the design states.Test plan
Acceptance tests under
acceptance/bundle/deploy/bind/(allCloud = true; run locally in both deployment-history variants): happy paths (basic,bind-and-update,bind-and-resize,pure-bind,variable-id,int-id,target-override,ref-to-bound), guards (recreate-blocked,recreate-blocked-in-state,gone-in-state,rebind-different-id,duplicate-bind-id,duplicate-bind-id-config,delete-and-bind-conflict,id-collision-different-type,resource-not-found), lifecycle semantics (removed-from-config,block-removed,destroy-allowed,dev-mode). Unit tests for the touched packages pass; lint is clean.This pull request and its description were written by Isaac.