Conversation
Collaborator
Integration test reportCommit: e435fc4
Top 3 slowest tests (at least 2 minutes):
|
denik
force-pushed
the
denik/migration-before-deploy
branch
from
September 22, 2026 11:08
32fe6bb to
14e8c46
Compare
When the direct engine is requested (the default) and the existing state
still uses terraform, convert the state to the direct engine before the
run proceeds, instead of after deploy. deploy/destroy commit the migration
(resources.json written and pushed, terraform.tfstate backed up); plan and
summary keep it in memory. The migration runs after phases.Build so its
conversion and plan check see library and ${artifacts.*} references
resolved (matching what a direct deploy records), and it falls back to the
terraform engine if the plan check fails. Terraform-state cleanup after the
commit is best-effort: resources.json already outranks the terraform state
by serial, so a failed backup/delete only warns.
Co-authored-by: Isaac <no-reply@databricks.com>
Now that the migration runs before deploy, a hard error blocks a deploy that would otherwise succeed on terraform. Treat every failure before resources.json is pushed (parse, conversion, empty-state sweep) as non-fatal: warn and deploy on terraform this time, retrying the migration next run - matching what plan-check and push failures already did. Only a failure after the push (placing/opening the local state) stays an error, since the workspace is already committed to direct; reword those messages to say the migration succeeded and re-running recovers. Co-authored-by: Isaac <no-reply@databricks.com>
The pre-deploy migration plan-checks the converted state. If that plan would recreate (destroy + create) an existing resource, do not commit the migration: a recreate risks data loss, whether it comes from a conversion that did not faithfully reproduce an immutable field or from a real pending config change (which terraform would recreate too). Fall back to terraform this run and retry the migration next run, once the recreate is applied. Records direct_migrate_recreate_planned. checkPlanOnTempState now returns the plan so the caller can inspect the planned actions. Co-authored-by: Isaac <no-reply@databricks.com>
denik
force-pushed
the
denik/migration-before-deploy
branch
from
September 22, 2026 12:21
6c1fd6c to
7b7aabe
Compare
The pre-deploy migration flips the engine terraform->direct, but the SDK user agent only appends dimensions, so tagging engine/terraform in PullResourcesState and then engine/direct after a successful migration left both on a migrated deploy's requests. PullResourcesState now skips the tag on the auto-migration path (direct requested, state still terraform); the caller sets engine from the resolved stateDesc.Engine once the migration has run, been skipped, or fallen back - so a migrated deploy is engine/direct, a fallback is engine/terraform, and there is never a stale second tag. The two generate commands, which never migrate, tag the resolved engine too. auto-migrate-envvar now records the deploy's engine tags to lock this in. Co-authored-by: Isaac <no-reply@databricks.com>
Extract the auto-migration condition to a named local instead of negating a conjunction inline. Co-authored-by: Isaac <no-reply@databricks.com>
Records (no fix) how the pre-deploy migration treats a schema's named id fields when they change or are backend-normalized: - name backend-lowercased (config MySchema vs deployed myschema): spurious warnOnIDFieldRename warning (also emitted by the terraform deploy's dry-run telemetry), but the follow-up plan converges. - catalog_name legitimately changed (immutable id): only a warning, no recreate - the migration records the config value and the moved-catalog drift is silently stable. - storage_root trailing slash normalized: absorbed by normalize_slash, no warning, converges. Co-authored-by: Isaac <no-reply@databricks.com>
Its golden had captured the buggy engine/terraform engine/direct double tag on the migrating plan/deploy commands; the user-agent fix makes those a single engine/direct. Regenerated (the only golden that recorded the double tag). Co-authored-by: Isaac <no-reply@databricks.com>
…odels, pipelines Extends the schema characterization (no fix) across more resources: - volume name backend-lowercased: spurious rename warning, converges (same as schema name). - volume_type changed (both a provided id field AND recreate_on_changes): the recreate classification wins - the recreate guard fires and the run stays on terraform (contrast with catalog_name, a pure provided id field, which only warns). - registered_model catalog_name changed (immutable id): warn + silently stable drift, no recreate (same as schema catalog_name). - pipeline storage changed (pure recreate_on_changes): recreate guard fires. Catalogs are direct-only (no terraform converter), so they are never a migration scenario and are intentionally not covered. Co-authored-by: Isaac <no-reply@databricks.com>
The migration seeded id-composing fields (provided_id_fields, updatable_id_fields) from config, which snapshotted a pending id change as already applied: a genuinely-moved/renamed resource then plan-converged and silently drifted from the backend, while a backend-normalized value (identifier case, trailing slash) produced a spurious "rename not applied" warning - also leaked onto plain terraform deploys via the dry-run telemetry. reconcileIDFields now seeds each id field from the deployed terraform value unless it differs from config only by backend normalization (case-insensitive + trailing-slash), in which case the config value is kept. So a real id change surfaces in the plan (recreate for provided_id -> caught by the recreate guard; rename update for updatable_id), and a normalized value converges with no warning. warnOnIDFieldRename is removed (its genuine case is now the recreate guard, its false-positive case is gone). Regenerated the id-field characterization goldens, which now assert the fixed behavior (recreate on genuine change, clean converge on normalization). Co-authored-by: Isaac <no-reply@databricks.com>
The terraform fallback leaves two terraform.tfstate* files, and find lists them in filesystem order, which differs on Windows. Pipe the finds through sort so the golden is deterministic across platforms. Co-authored-by: Isaac <no-reply@databricks.com>
First cloud coverage for the pre-deploy migration: renaming a schema (an immutable provided id field) must recreate. Verifies against a real workspace that the recreate guard detects the recreate, falls back to terraform (which recreates the schema under the new name), and the following deploy migrates the now-matching state to direct. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
Companion to cloud-recreate-schema. A volume name is an updatable id field, so unlike a schema (which recreates) it is renamed in place. Verifies against a real workspace that the migration seeds the deployed name, the plan renames the volume (UpdateWithID) rather than snapshotting the new name as applied, the migration succeeds (no fallback), and the schema alongside it is untouched. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
Extend the two schema recreate tests (local auto-migrate-recreate and cloud cloud-recreate-schema) with a step that deploys the recreate WITHOUT --auto-approve first: the migration guard falls back to terraform, which refuses the destructive recreate (exit 1) and changes nothing. The following --auto-approve step then applies it. The cloud step passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
Third cloud resource type. A registered model name is a provided id field, so a rename recreates - the migration guard detects it against the real backend and falls back to terraform, which recreates the model; the retry then migrates. Unlike schemas/volumes, registered models are not in the destructive-approval group, so the recreate applies without --auto-approve. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
…nario> Drop the redundant cloud-/idfield-/recreate-/auto-migrate- prefixes: migration and running on both testserver and cloud are the defaults, and Cloud=true only adds a cloud run. Name by resource and scenario instead (schema-rename, volume-type-change, ...). Remove auto-migrate-rename, now fully covered by the more thorough schema-rename (which also runs on cloud). Co-authored-by: Isaac <no-reply@databricks.com>
…lume) Upgrade schema-name-normalized and volume-name-normalized to Cloud=true (unique names + cleanup trap). This covers the last behavior class on cloud: a backend-normalized id (UC lowercasing) converges through the migration with no warning or recreate. Verified on aws-cli. Together with schema-rename, volume-rename and registered-model-rename, all id-field/recreate classes now run against a real workspace. Co-authored-by: Isaac <no-reply@databricks.com>
…alog catalog_name is an immutable provided id field; moving a schema to another catalog recreates it. Create the destination catalog out of band (catalogs are direct-only, so they cannot be a bundle resource here), then verify on a real workspace: the recreate is refused without --auto-approve and, with it, moves the schema to the new catalog; the retry migrates. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
…f-band catalog Moving a registered model to another catalog recreates it (catalog_name is a provided id field). Create the destination catalog out of band (it auto-creates a default schema the moved model reuses). Registered models are not in the destructive-approval group, so the recreate applies without --auto-approve; the retry migrates. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
storage is a recreate_on_changes field on pipelines. Give the pipeline a notebook library and a DBFS storage path (no external location needed) so it deploys on a real workspace, then verify: changing storage plans a recreate; pipelines are in the destructive-approval group, so it is refused without --auto-approve and applied with it; the retry migrates. Passed on aws-cli. Co-authored-by: Isaac <no-reply@databricks.com>
…only schema-storage-normalized, schema-storage-change and volume-type-change need a registered external location on a real workspace, so they are not Cloud-enabled. Add a note in each pointing to the cloud-enabled tests that cover the same class. 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
2 participants
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.
Moves the terraform→direct state migration to run before plan/deploy instead of after a terraform deploy. On deploy the migration is committed once its plan check passes (resources.json pushed, terraform.tfstate backed up); if the check or commit fails it falls back to the terraform engine, which is still available for an explicit
engine: terraformopt-in. The terraform-engine removal is stacked on top of this PR.This pull request and its description were written by Isaac.