From 1455d4e068e728cdb0b4e1fc98b5c079f4fbb1e4 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 9 Sep 2026 15:36:13 +0000 Subject: [PATCH 1/5] Support bundle deployment bind/unbind for DMS-recorded deployments Bind and unbind now record their change with the deployment metadata service instead of refusing when experimental.deployment_history is on, using bind/unbind operation action types. Enables the DMS variant for the bind/unbind acceptance tests; the few that assert job/pipeline deployment stamps or dumped state stay opted out. Co-authored-by: Isaac --- .../deployment/bind/alert/out.test.toml | 2 +- .../deployment/bind/catalog/out.test.toml | 2 +- .../deployment/bind/cluster/out.test.toml | 2 +- .../deployment/bind/dashboard/out.test.toml | 2 +- .../bind/dashboard/recreation/test.toml | 5 + .../bind/database_instance/out.test.toml | 2 +- .../deployment/bind/experiment/out.test.toml | 2 +- .../bind/external_location/out.test.toml | 2 +- .../deployment/bind/genie_space/out.test.toml | 2 +- .../already-managed-different/out.test.toml | 2 +- .../job/already-managed-same/out.test.toml | 2 +- .../bind/job/engine-from-config/out.test.toml | 2 +- .../bind/job/generate-and-bind/out.test.toml | 2 +- .../bind/job/job-abort-bind/out.test.toml | 2 +- .../job/job-spark-python-task/out.test.toml | 2 +- .../deployment/bind/job/noop-job/test.toml | 5 + .../deployment/bind/job/python-job/test.toml | 5 + .../bind/job/stale-state/out.test.toml | 2 +- .../bind/model-serving-endpoint/out.test.toml | 2 +- .../deployment/bind/pipelines/test.toml | 5 + .../bind/postgres_database/out.test.toml | 2 +- .../bind/postgres_role/out.test.toml | 2 +- .../postgres_snapshot_schedule/out.test.toml | 2 +- .../bind/quality-monitor/out.test.toml | 2 +- .../bind/registered-model/out.test.toml | 2 +- .../deployment/bind/schema/out.test.toml | 2 +- .../bind/secret-scope/out.test.toml | 2 +- .../bind/sql_warehouse/out.test.toml | 2 +- acceptance/bundle/deployment/bind/test.toml | 2 - .../bind/vector_search_endpoint/out.test.toml | 2 +- .../bind/vector_search_index/out.test.toml | 2 +- .../deployment/bind/volume/out.test.toml | 2 +- .../unbind/engine-from-config/out.test.toml | 2 +- .../deployment/unbind/grants/out.test.toml | 2 +- .../bundle/deployment/unbind/job/test.toml | 5 + .../unbind/permissions/out.test.toml | 2 +- .../deployment/unbind/python-job/test.toml | 5 + acceptance/bundle/deployment/unbind/test.toml | 2 - .../dms/bind-unbind-unsupported/output.txt | 16 -- .../bundle/dms/bind-unbind-unsupported/script | 16 -- .../dms/bind-unbind-unsupported/test.toml | 3 - .../databricks.yml.tmpl | 0 .../out.test.toml | 2 +- acceptance/bundle/dms/bind-unbind/output.txt | 60 ++++ acceptance/bundle/dms/bind-unbind/script | 12 + bundle/direct/dstate/state.go | 24 ++ bundle/phases/bind.go | 124 +++++---- bundle/phases/bind_dms.go | 262 ++++++++++++++++++ cmd/bundle/utils/process.go | 35 +-- libs/dms/client.go | 38 +++ libs/dms/client_test.go | 7 + 51 files changed, 539 insertions(+), 154 deletions(-) create mode 100644 acceptance/bundle/deployment/bind/dashboard/recreation/test.toml delete mode 100644 acceptance/bundle/deployment/bind/test.toml delete mode 100644 acceptance/bundle/deployment/unbind/test.toml delete mode 100644 acceptance/bundle/dms/bind-unbind-unsupported/output.txt delete mode 100644 acceptance/bundle/dms/bind-unbind-unsupported/script delete mode 100644 acceptance/bundle/dms/bind-unbind-unsupported/test.toml rename acceptance/bundle/dms/{bind-unbind-unsupported => bind-unbind}/databricks.yml.tmpl (100%) rename acceptance/bundle/dms/{bind-unbind-unsupported => bind-unbind}/out.test.toml (83%) create mode 100644 acceptance/bundle/dms/bind-unbind/output.txt create mode 100644 acceptance/bundle/dms/bind-unbind/script create mode 100644 bundle/phases/bind_dms.go diff --git a/acceptance/bundle/deployment/bind/alert/out.test.toml b/acceptance/bundle/deployment/bind/alert/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/alert/out.test.toml +++ b/acceptance/bundle/deployment/bind/alert/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/catalog/out.test.toml b/acceptance/bundle/deployment/bind/catalog/out.test.toml index d73c45e3119..ae5c7bd798f 100644 --- a/acceptance/bundle/deployment/bind/catalog/out.test.toml +++ b/acceptance/bundle/deployment/bind/catalog/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/cluster/out.test.toml b/acceptance/bundle/deployment/bind/cluster/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/cluster/out.test.toml +++ b/acceptance/bundle/deployment/bind/cluster/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/dashboard/out.test.toml b/acceptance/bundle/deployment/bind/dashboard/out.test.toml index 2acec25aece..b1bef937a27 100644 --- a/acceptance/bundle/deployment/bind/dashboard/out.test.toml +++ b/acceptance/bundle/deployment/bind/dashboard/out.test.toml @@ -1,4 +1,4 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] EnvMatrix.READPLAN = ["", "1"] diff --git a/acceptance/bundle/deployment/bind/dashboard/recreation/test.toml b/acceptance/bundle/deployment/bind/dashboard/recreation/test.toml new file mode 100644 index 00000000000..a9e9f0c3488 --- /dev/null +++ b/acceptance/bundle/deployment/bind/dashboard/recreation/test.toml @@ -0,0 +1,5 @@ +# This dumps the state file after bind, whose serial counts local state writes; under DMS the +# service owns the version number instead, so the dumped serial differs. That is an engine-level +# difference, not a bind one, and DMS bind is covered by bundle/dms/bind-unbind, so skip the DMS +# variant here. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/database_instance/out.test.toml b/acceptance/bundle/deployment/bind/database_instance/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/database_instance/out.test.toml +++ b/acceptance/bundle/deployment/bind/database_instance/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/experiment/out.test.toml b/acceptance/bundle/deployment/bind/experiment/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/experiment/out.test.toml +++ b/acceptance/bundle/deployment/bind/experiment/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/external_location/out.test.toml b/acceptance/bundle/deployment/bind/external_location/out.test.toml index 27ec2a7fcd6..59b56a2037c 100644 --- a/acceptance/bundle/deployment/bind/external_location/out.test.toml +++ b/acceptance/bundle/deployment/bind/external_location/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/genie_space/out.test.toml b/acceptance/bundle/deployment/bind/genie_space/out.test.toml index 27ec2a7fcd6..59b56a2037c 100644 --- a/acceptance/bundle/deployment/bind/genie_space/out.test.toml +++ b/acceptance/bundle/deployment/bind/genie_space/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml b/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml b/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml b/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml index 27ec2a7fcd6..59b56a2037c 100644 --- a/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml b/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml b/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml b/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/job/noop-job/test.toml b/acceptance/bundle/deployment/bind/job/noop-job/test.toml index 339a7a29f54..ffb9a92d09b 100644 --- a/acceptance/bundle/deployment/bind/job/noop-job/test.toml +++ b/acceptance/bundle/deployment/bind/job/noop-job/test.toml @@ -1 +1,6 @@ Cloud = false # test leaves deployed job + +# `jobs get` shows the job's deployment block, which under DMS also carries the +# deployment_id/version_id stamp - an engine-level difference, not a bind one. DMS bind is covered +# by bundle/dms/bind-unbind, so skip the DMS variant here. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/python-job/test.toml b/acceptance/bundle/deployment/bind/job/python-job/test.toml index 39101095ee6..f7d685ac5a9 100644 --- a/acceptance/bundle/deployment/bind/job/python-job/test.toml +++ b/acceptance/bundle/deployment/bind/job/python-job/test.toml @@ -1,2 +1,7 @@ Cloud = false # test leaves deployed job Timeout = '40s' # python can be slow in CI likely due to caching problems + +# `jobs get` shows the job's deployment block, which under DMS also carries the +# deployment_id/version_id stamp - an engine-level difference, not a bind one. DMS bind is covered +# by bundle/dms/bind-unbind, so skip the DMS variant here. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml b/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml b/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml +++ b/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/pipelines/test.toml b/acceptance/bundle/deployment/bind/pipelines/test.toml index 18b1a88417e..3f801a08b9a 100644 --- a/acceptance/bundle/deployment/bind/pipelines/test.toml +++ b/acceptance/bundle/deployment/bind/pipelines/test.toml @@ -1 +1,6 @@ Cloud = false + +# These assert the deploy's request bodies, which under DMS also carry the pipeline's +# deployment_id/version_id stamp - an engine-level difference, not a bind one. DMS bind is covered +# by bundle/dms/bind-unbind, so skip the DMS variant here. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/postgres_database/out.test.toml b/acceptance/bundle/deployment/bind/postgres_database/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/postgres_database/out.test.toml +++ b/acceptance/bundle/deployment/bind/postgres_database/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/postgres_role/out.test.toml b/acceptance/bundle/deployment/bind/postgres_role/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/postgres_role/out.test.toml +++ b/acceptance/bundle/deployment/bind/postgres_role/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml b/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml index 27ec2a7fcd6..59b56a2037c 100644 --- a/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml +++ b/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml b/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml +++ b/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/registered-model/out.test.toml b/acceptance/bundle/deployment/bind/registered-model/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/registered-model/out.test.toml +++ b/acceptance/bundle/deployment/bind/registered-model/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/schema/out.test.toml b/acceptance/bundle/deployment/bind/schema/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/schema/out.test.toml +++ b/acceptance/bundle/deployment/bind/schema/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/secret-scope/out.test.toml b/acceptance/bundle/deployment/bind/secret-scope/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/secret-scope/out.test.toml +++ b/acceptance/bundle/deployment/bind/secret-scope/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml b/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml index ae800809893..e1af1a235ad 100644 --- a/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml +++ b/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/test.toml b/acceptance/bundle/deployment/bind/test.toml deleted file mode 100644 index b693e4ded26..00000000000 --- a/acceptance/bundle/deployment/bind/test.toml +++ /dev/null @@ -1,2 +0,0 @@ -# Bind operations are not yet supported by the Deployment Metadata Service (DMS) -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml b/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml index d73c45e3119..ae5c7bd798f 100644 --- a/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml +++ b/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml b/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml index 12effa8e96d..f9f4880725d 100644 --- a/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml +++ b/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml @@ -1,4 +1,4 @@ Cloud = true CloudSlow = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/bind/volume/out.test.toml b/acceptance/bundle/deployment/bind/volume/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/bind/volume/out.test.toml +++ b/acceptance/bundle/deployment/bind/volume/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml b/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml index 27ec2a7fcd6..59b56a2037c 100644 --- a/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml +++ b/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/unbind/grants/out.test.toml b/acceptance/bundle/deployment/unbind/grants/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/unbind/grants/out.test.toml +++ b/acceptance/bundle/deployment/unbind/grants/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/unbind/job/test.toml b/acceptance/bundle/deployment/unbind/job/test.toml index 5c721ad6e79..91e1ea914d5 100644 --- a/acceptance/bundle/deployment/unbind/job/test.toml +++ b/acceptance/bundle/deployment/unbind/job/test.toml @@ -1,2 +1,7 @@ Cloud = false # test leaves deployed job Ignore = [".databricks"] + +# The final `jobs get` shows the job's deployment block, which under DMS also carries the +# deployment_id/version_id stamp; that is an engine-level difference, not an unbind one, and DMS +# unbind is covered by bundle/dms/bind-unbind. So skip the DMS variant here. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/permissions/out.test.toml b/acceptance/bundle/deployment/unbind/permissions/out.test.toml index 9b8ddfcd2bd..cef45fe553a 100644 --- a/acceptance/bundle/deployment/unbind/permissions/out.test.toml +++ b/acceptance/bundle/deployment/unbind/permissions/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = [""] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deployment/unbind/python-job/test.toml b/acceptance/bundle/deployment/unbind/python-job/test.toml index 22a0568dce8..5b93f3eb3cf 100644 --- a/acceptance/bundle/deployment/unbind/python-job/test.toml +++ b/acceptance/bundle/deployment/unbind/python-job/test.toml @@ -1,3 +1,8 @@ Cloud = false # test leaves deployed job Ignore = [".databricks"] Timeout = '40s' # python can be slow in CI likely due to caching problems + +# The final `jobs get` shows the job's deployment block, which under DMS also carries the +# deployment_id/version_id stamp; that is an engine-level difference, not an unbind one, and DMS +# unbind is covered by bundle/dms/bind-unbind. So skip the DMS variant here. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/test.toml b/acceptance/bundle/deployment/unbind/test.toml deleted file mode 100644 index 9f5f5d49a05..00000000000 --- a/acceptance/bundle/deployment/unbind/test.toml +++ /dev/null @@ -1,2 +0,0 @@ -# Unbind operations are not yet supported by the Deployment Metadata Service (DMS) -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/dms/bind-unbind-unsupported/output.txt b/acceptance/bundle/dms/bind-unbind-unsupported/output.txt deleted file mode 100644 index 14686850b03..00000000000 --- a/acceptance/bundle/dms/bind-unbind-unsupported/output.txt +++ /dev/null @@ -1,16 +0,0 @@ - -=== bind is refused while recording deployment history ->>> musterr [CLI] bundle deployment bind foo [JOB_ID] -Error: bind is not supported for a bundle target that records deployment history - - -=== Deploy so the bundle records state, then unbind is refused too ->>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dms-bind-unbind-[UNIQUE_NAME]/default/files... -Created jobs.foo -Files: 6 uploaded, 0 deleted -Resources: 1 created, 0 changed, 0 deleted, 0 unchanged - ->>> musterr [CLI] bundle deployment unbind foo -Error: unbind is not supported for a bundle target that records deployment history - diff --git a/acceptance/bundle/dms/bind-unbind-unsupported/script b/acceptance/bundle/dms/bind-unbind-unsupported/script deleted file mode 100644 index 9af766ca4d3..00000000000 --- a/acceptance/bundle/dms/bind-unbind-unsupported/script +++ /dev/null @@ -1,16 +0,0 @@ -envsubst < databricks.yml.tmpl > databricks.yml - -# The gate that refuses bind while recording deployment history is on the config, so it fires -# before any deploy. Create a real workspace job so bind gets past fetching the resource and -# reaches the gate, instead of failing because the resource does not exist. -title "bind is refused while recording deployment history" -job_id=$($CLI jobs create --json '{"name": "bind-target-'$UNIQUE_NAME'"}' | jq -r '.job_id') -add_repl "$job_id" JOB_ID -trace musterr $CLI bundle deployment bind foo "$job_id" - -# unbind refuses when it opens an existing recording state, so it needs a deployment first. -title "Deploy so the bundle records state, then unbind is refused too" -trace $CLI bundle deploy -trace musterr $CLI bundle deployment unbind foo - -rm -f out.requests.txt diff --git a/acceptance/bundle/dms/bind-unbind-unsupported/test.toml b/acceptance/bundle/dms/bind-unbind-unsupported/test.toml deleted file mode 100644 index 8bc2d5e8cf8..00000000000 --- a/acceptance/bundle/dms/bind-unbind-unsupported/test.toml +++ /dev/null @@ -1,3 +0,0 @@ -# bind/unbind refusal is a client-side gate on the recording config; it needs no real -# workspace, so this stays local-only rather than inheriting the suite's cloud run. -Cloud = false diff --git a/acceptance/bundle/dms/bind-unbind-unsupported/databricks.yml.tmpl b/acceptance/bundle/dms/bind-unbind/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/dms/bind-unbind-unsupported/databricks.yml.tmpl rename to acceptance/bundle/dms/bind-unbind/databricks.yml.tmpl diff --git a/acceptance/bundle/dms/bind-unbind-unsupported/out.test.toml b/acceptance/bundle/dms/bind-unbind/out.test.toml similarity index 83% rename from acceptance/bundle/dms/bind-unbind-unsupported/out.test.toml rename to acceptance/bundle/dms/bind-unbind/out.test.toml index 9921e91a794..23c07f70dca 100644 --- a/acceptance/bundle/dms/bind-unbind-unsupported/out.test.toml +++ b/acceptance/bundle/dms/bind-unbind/out.test.toml @@ -1,3 +1,3 @@ -Cloud = false +Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] EnvMatrix.DMS = ["true"] diff --git a/acceptance/bundle/dms/bind-unbind/output.txt b/acceptance/bundle/dms/bind-unbind/output.txt new file mode 100644 index 00000000000..c3a79402e8c --- /dev/null +++ b/acceptance/bundle/dms/bind-unbind/output.txt @@ -0,0 +1,60 @@ + +=== Bind an existing job to the bundle, recorded with the deployment history service +>>> [CLI] bundle deployment bind foo [JOB_ID] --auto-approve +Successfully bound job with an id '[JOB_ID]' +Run 'bundle deploy' to deploy changes to your workspace + +=== Unbind it again, dropping it from what the service records +>>> [CLI] bundle deployment unbind foo + +=== Each recorded a version whose operation carries the bind / unbind action type +>>> print_requests.py --dms --method POST //versions --nostamp +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions", + "q": { + "version_id": "1" + }, + "body": { + "cli_version": "[CLI_VERSION]", + "version_type": "VERSION_TYPE_DEPLOY", + "operations": [ + { + "resource_key": "jobs.foo", + "action_type": "OPERATION_ACTION_TYPE_BIND" + } + ] + } +} +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions/1/complete", + "body": { + "completion_reason": "VERSION_COMPLETE_SUCCESS" + } +} +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions", + "q": { + "version_id": "2" + }, + "body": { + "cli_version": "[CLI_VERSION]", + "version_type": "VERSION_TYPE_DEPLOY", + "previous_version_id": "1", + "operations": [ + { + "resource_key": "jobs.foo", + "action_type": "OPERATION_ACTION_TYPE_UNBIND" + } + ] + } +} +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions/2/complete", + "body": { + "completion_reason": "VERSION_COMPLETE_SUCCESS" + } +} diff --git a/acceptance/bundle/dms/bind-unbind/script b/acceptance/bundle/dms/bind-unbind/script new file mode 100644 index 00000000000..1dde9788809 --- /dev/null +++ b/acceptance/bundle/dms/bind-unbind/script @@ -0,0 +1,12 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +title "Bind an existing job to the bundle, recorded with the deployment history service" +job_id=$($CLI jobs create --json '{"name": "bind-target-'$UNIQUE_NAME'"}' | jq -r '.job_id') +add_repl "$job_id" JOB_ID +trace $CLI bundle deployment bind foo "$job_id" --auto-approve + +title "Unbind it again, dropping it from what the service records" +trace $CLI bundle deployment unbind foo + +title "Each recorded a version whose operation carries the bind / unbind action type" +trace print_requests.py --dms --method POST //versions --nostamp diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go index f4fb00c6b9e..bcf165835d3 100644 --- a/bundle/direct/dstate/state.go +++ b/bundle/direct/dstate/state.go @@ -1020,6 +1020,30 @@ func (db *DeploymentState) unlockedSave() error { return nil } +// SnapshotToPlainState writes what this deployment-history state currently holds to path as an +// ordinary state file with no history marker, so the file-based bind flow can compute its plan and +// resolved state against a copy of what the service records without touching it. The state must be +// open for read. +func (db *DeploymentState) SnapshotToPlainState(path string) error { + db.AssertOpenedForRead() + db.mu.Lock() + defer db.mu.Unlock() + + snapshot := Database{ + Header: Header{ + StateVersion: currentStateVersion, + CLIVersion: build.GetInfo().Version, + Lineage: db.Data.Lineage, + }, + State: db.Data.State, + } + data, err := json.MarshalIndent(snapshot, "", " ") + if err != nil { + return err + } + return os.WriteFile(path, data, 0o600) +} + // Data to persist in the remote resources.json file. func (db *DeploymentState) dataForFile() Database { if db.isDeploymentMetadataService() { diff --git a/bundle/phases/bind.go b/bundle/phases/bind.go index 8fb5967196c..db335e9df4b 100644 --- a/bundle/phases/bind.go +++ b/bundle/phases/bind.go @@ -13,6 +13,7 @@ import ( "github.com/databricks/cli/bundle/deploy/lock" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/bundle/direct" "github.com/databricks/cli/bundle/statemgmt" "github.com/databricks/cli/libs/agent" "github.com/databricks/cli/libs/cmdio" @@ -33,11 +34,6 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions, en }() if engine.IsDirect() { - if b.ConfiguresDeploymentHistory(ctx) { - logdiag.LogError(ctx, errors.New("bind is not supported for a bundle target that records deployment history")) - return - } - // Direct engine: import into temp state, run plan, check for changes // This follows the same pattern as terraform import groupName, ok := terraform.TerraformToGroupName[opts.ResourceType] @@ -45,57 +41,32 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions, en groupName = opts.ResourceType } resourceKey := fmt.Sprintf("resources.%s.%s", groupName, opts.ResourceKey) - _, statePath := b.StateFilenameDirect(ctx) - result, err := b.DeploymentBundle.Bind(ctx, b.WorkspaceClient(ctx), &b.Config, statePath, resourceKey, opts.ResourceId) - if err != nil { - logdiag.LogError(ctx, err) - return - } - - // If there are changes and auto-approve is not set, show plan and ask for confirmation - if result.HasChanges && !opts.AutoApprove { - // Display the planned changes for the bound resource - cmdio.LogString(ctx, fmt.Sprintf("Plan: %s %s", result.Action, resourceKey)) - - // Show details of what will change - if result.Plan != nil { - if entry, ok := result.Plan.Plan[resourceKey]; ok && entry != nil && len(entry.Changes) > 0 { - cmdio.LogString(ctx, "\nChanges detected:") - for _, field := range slices.Sorted(maps.Keys(entry.Changes)) { - change := entry.Changes[field] - if change.Action != deployplan.Skip { - cmdio.LogString(ctx, fmt.Sprintf(" ~ %s: %v -> %v", field, jsonDump(ctx, change.Remote, field), jsonDump(ctx, change.New, field))) - } - } - cmdio.LogString(ctx, "") - } - } - - if !cmdio.IsPromptSupported(ctx) { - result.Cancel() - logdiag.LogError(ctx, fmt.Errorf("this bind operation requires user confirmation, but the current console does not support prompting.\nTo proceed, use --auto-approve after reviewing the plan above.%s", agent.AgentNotice())) + if b.ConfiguresDeploymentHistory(ctx) { + // A recorded deployment keeps its resources in the metadata service, so the bind is + // recorded there rather than written to the state file. + bindWithHistory(ctx, b, resourceKey, opts.ResourceId, opts.AutoApprove) + if logdiag.HasError(ctx) { return } + } else { + _, statePath := b.StateFilenameDirect(ctx) - ans, err := cmdio.AskYesOrNo(ctx, "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'.") + result, err := b.DeploymentBundle.Bind(ctx, b.WorkspaceClient(ctx), &b.Config, statePath, resourceKey, opts.ResourceId) if err != nil { - result.Cancel() logdiag.LogError(ctx, err) return } - if !ans { - result.Cancel() - logdiag.LogError(ctx, errors.New("import aborted")) + + if !confirmBindPlan(ctx, resourceKey, result, opts.AutoApprove) { return } - } - // Finalize: rename temp state to final location - err = result.Finalize() - if err != nil { - logdiag.LogError(ctx, err) - return + // Finalize: rename temp state to final location + if err := result.Finalize(); err != nil { + logdiag.LogError(ctx, err) + return + } } } else { // Terraform engine: use terraform import @@ -122,6 +93,50 @@ func jsonDump(ctx context.Context, v any, field string) string { return string(b) } +// confirmBindPlan shows the plan for the bound resource and, unless autoApprove, asks the user to +// confirm. It reports whether the bind should proceed; on decline or an unpromptable console it +// cancels result, logs the reason, and returns false so the caller returns. +func confirmBindPlan(ctx context.Context, resourceKey string, result *direct.BindResult, autoApprove bool) bool { + if !result.HasChanges || autoApprove { + return true + } + + cmdio.LogString(ctx, fmt.Sprintf("Plan: %s %s", result.Action, resourceKey)) + + if result.Plan != nil { + if entry, ok := result.Plan.Plan[resourceKey]; ok && entry != nil && len(entry.Changes) > 0 { + cmdio.LogString(ctx, "\nChanges detected:") + for _, field := range slices.Sorted(maps.Keys(entry.Changes)) { + change := entry.Changes[field] + if change.Action != deployplan.Skip { + cmdio.LogString(ctx, fmt.Sprintf(" ~ %s: %v -> %v", field, jsonDump(ctx, change.Remote, field), jsonDump(ctx, change.New, field))) + } + } + cmdio.LogString(ctx, "") + } + } + + if !cmdio.IsPromptSupported(ctx) { + result.Cancel() + logdiag.LogError(ctx, fmt.Errorf("this bind operation requires user confirmation, but the current console does not support prompting.\nTo proceed, use --auto-approve after reviewing the plan above.%s", agent.AgentNotice())) + return false + } + + ans, err := cmdio.AskYesOrNo(ctx, "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'.") + if err != nil { + result.Cancel() + logdiag.LogError(ctx, err) + return false + } + if !ans { + result.Cancel() + logdiag.LogError(ctx, errors.New("import aborted")) + return false + } + + return true +} + func Unbind(ctx context.Context, b *bundle.Bundle, bundleType, tfResourceType, resourceKey string, engine engine.EngineType) { log.Info(ctx, "Phase: unbind") @@ -140,11 +155,20 @@ func Unbind(ctx context.Context, b *bundle.Bundle, bundleType, tfResourceType, r groupName = tfResourceType } fullResourceKey := fmt.Sprintf("resources.%s.%s", groupName, resourceKey) - _, statePath := b.StateFilenameDirect(ctx) - err := b.DeploymentBundle.Unbind(ctx, statePath, fullResourceKey) - if err != nil { - logdiag.LogError(ctx, err) - return + + if b.ConfiguresDeploymentHistory(ctx) { + // A recorded deployment keeps its resources in the metadata service, so the unbind is + // recorded there rather than removed from the state file. + unbindWithHistory(ctx, b, fullResourceKey) + if logdiag.HasError(ctx) { + return + } + } else { + _, statePath := b.StateFilenameDirect(ctx) + if err := b.DeploymentBundle.Unbind(ctx, statePath, fullResourceKey); err != nil { + logdiag.LogError(ctx, err) + return + } } } else { bundle.ApplySeqContext( diff --git a/bundle/phases/bind_dms.go b/bundle/phases/bind_dms.go new file mode 100644 index 00000000000..7d269879d7b --- /dev/null +++ b/bundle/phases/bind_dms.go @@ -0,0 +1,262 @@ +package phases + +import ( + "context" + "fmt" + "os" + "slices" + "strconv" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/direct/dstate" + "github.com/databricks/cli/libs/cmdctx" + "github.com/databricks/cli/libs/dms" + "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/logdiag" + "github.com/databricks/databricks-sdk-go/service/bundledeployments" +) + +// bindWithHistory records a bind for a deployment that tracks history with the metadata service. +// The engine's bind computes the plan preview and resolved state against a throwaway copy of what +// the service holds, then the bind is recorded as an operation carrying that state, so the next +// deploy sees the resource as managed rather than new. +func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourceID string, autoApprove bool) { + wsc := b.WorkspaceClient(ctx) + + deploymentID, deployment, err := dms.FetchDeployment(ctx, wsc, b.Config.Workspace.StatePath) + if err != nil { + logdiag.LogError(ctx, err) + return + } + lastVersionID, err := deploymentVersion(deployment) + if err != nil { + logdiag.LogError(ctx, err) + return + } + + seedPath, cleanup, err := seedStateFromService(ctx, b, deploymentID, lastVersionID) + if err != nil { + logdiag.LogError(ctx, err) + return + } + defer cleanup() + + result, err := b.DeploymentBundle.Bind(ctx, wsc, &b.Config, seedPath, resourceKey, resourceID) + if err != nil { + logdiag.LogError(ctx, err) + return + } + if !confirmBindPlan(ctx, resourceKey, result, autoApprove) { + return + } + // The seed's temp state is discarded; the bind is recorded with the service instead. + defer result.Cancel() + + // Record exactly what the engine resolved for the resource - the same state, id and + // dependencies a file-based bind would persist (etags and all) - read straight back out of the + // throwaway state it wrote rather than the state cache, which the plan step overwrites. + entry, ok, err := resolvedEntry(ctx, result.TempStatePath, resourceKey) + if err != nil { + logdiag.LogError(ctx, err) + return + } + if !ok { + logdiag.LogError(ctx, fmt.Errorf("internal error: no resolved state for %q after bind", resourceKey)) + return + } + + recordBind(ctx, b, deploymentID, deployment, lastVersionID, resourceKey, entry) +} + +// recordBind creates the deployment on a first bind, then records a single bind operation carrying +// the resolved state, so the deployment lists the resource as managed. +func recordBind(ctx context.Context, b *bundle.Bundle, deploymentID string, deployment *bundledeployments.Deployment, lastVersionID int, resourceKey string, entry dstate.ResourceEntry) { + ctx = withWorkspaceClient(ctx, b) + db := &b.DeploymentBundle.StateDB + if err := db.Open(ctx, localStatePath(ctx, b), dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}); err != nil { + logdiag.LogError(ctx, err) + return + } + + // Creates the deployment on a first bind, or refreshes stale metadata. + createOrUpdateDeployment(ctx, b, deployment) + if logdiag.HasError(ctx) { + return + } + + if err := db.UpgradeToWrite(); err != nil { + logdiag.LogError(ctx, err) + return + } + + staged := []dms.StagedOperation{{ResourceKey: resourceKey, ActionType: dms.ActionBind}} + if err := startVersion(ctx, b, dms.VersionTypeDeploy, staged); err != nil { + logdiag.LogError(ctx, err) + return + } + + if err := db.SaveState(ctx, resourceKey, entry.ID, entry.State, entry.DependsOn); err != nil { + logdiag.LogError(ctx, err) + return + } + + if _, err := db.Finalize(ctx); err != nil { + logdiag.LogError(ctx, err) + return + } + if _, err := db.CompleteVersion(ctx, true); err != nil { + logdiag.LogError(ctx, err) + } +} + +// unbindWithHistory records an unbind: the resource and its sub-resources (permissions, grants) are +// dropped from what the service records, so the next deploy re-creates them. The workspace +// resources themselves are left untouched. +func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string) { + wsc := b.WorkspaceClient(ctx) + + deploymentID, deployment, err := dms.FetchDeployment(ctx, wsc, b.Config.Workspace.StatePath) + if err != nil { + logdiag.LogError(ctx, err) + return + } + if deploymentID == "" { + // Nothing is recorded, so there is nothing to unbind. + return + } + lastVersionID, err := deploymentVersion(deployment) + if err != nil { + logdiag.LogError(ctx, err) + return + } + + ctx = withWorkspaceClient(ctx, b) + db := &b.DeploymentBundle.StateDB + if err := db.Open(ctx, localStatePath(ctx, b), dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}); err != nil { + logdiag.LogError(ctx, err) + return + } + + keys := recordedKeys(db, resourceKey) + if len(keys) == 0 { + // The resource is not recorded, so unbind is a no-op, matching a file-based deployment. + if _, err := db.Finalize(ctx); err != nil { + logdiag.LogError(ctx, err) + } + return + } + + if err := db.UpgradeToWrite(); err != nil { + logdiag.LogError(ctx, err) + return + } + + staged := make([]dms.StagedOperation, 0, len(keys)) + for _, k := range keys { + staged = append(staged, dms.StagedOperation{ResourceKey: k, ActionType: dms.ActionUnbind}) + } + if err := startVersion(ctx, b, dms.VersionTypeDeploy, staged); err != nil { + logdiag.LogError(ctx, err) + return + } + + for _, k := range keys { + if err := db.DeleteState(ctx, k, false); err != nil { + logdiag.LogError(ctx, err) + return + } + log.Infof(ctx, "Unbound %s", k) + } + + if _, err := db.Finalize(ctx); err != nil { + logdiag.LogError(ctx, err) + return + } + if _, err := db.CompleteVersion(ctx, true); err != nil { + logdiag.LogError(ctx, err) + } +} + +// seedStateFromService writes a throwaway local state holding what the service currently records, +// so the engine's file-based bind can compute its plan and resolved state against it. Returns a +// path that does not exist yet when no deployment has been recorded, which the engine reads as an +// empty state. cleanup removes the seed and any temporary files the engine leaves beside it. +func seedStateFromService(ctx context.Context, b *bundle.Bundle, deploymentID string, lastVersionID int) (string, func(), error) { + seedPath := localStatePath(ctx, b) + ".bind-seed" + cleanup := func() { + for _, p := range []string{seedPath, seedPath + ".temp-bind", seedPath + ".wal"} { + _ = os.Remove(p) + } + } + + if deploymentID == "" { + return seedPath, cleanup, nil + } + + ctx = withWorkspaceClient(ctx, b) + var src dstate.DeploymentState + if err := src.Open(ctx, localStatePath(ctx, b), dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}); err != nil { + cleanup() + return "", nil, err + } + err := src.SnapshotToPlainState(seedPath) + if _, ferr := src.Finalize(ctx); ferr != nil { + log.Warnf(ctx, "failed to finalize state: %v", ferr) + } + if err != nil { + cleanup() + return "", nil, err + } + return seedPath, cleanup, nil +} + +// resolvedEntry reads back the state the engine's bind resolved for resourceKey from the throwaway +// state file it wrote, which is what a file-based bind would have persisted. +func resolvedEntry(ctx context.Context, tempStatePath, resourceKey string) (dstate.ResourceEntry, bool, error) { + var src dstate.DeploymentState + if err := src.Open(ctx, tempStatePath, dstate.WithRecovery(true), dstate.WithWrite(false), dstate.WithDeploymentHistory(false), dstate.OpenDmsArgs{}); err != nil { + return dstate.ResourceEntry{}, false, err + } + entry, ok := src.GetResourceEntry(resourceKey) + if _, err := src.Finalize(ctx); err != nil { + log.Warnf(ctx, "failed to finalize state: %v", err) + } + return entry, ok, nil +} + +// recordedKeys returns resourceKey and its sub-resource keys (permissions, grants, ...) that the +// service holds, sorted so the staged operations and their requests are deterministic. +func recordedKeys(db *dstate.DeploymentState, resourceKey string) []string { + var keys []string + for k := range db.Data.State { + if k == resourceKey || strings.HasPrefix(k, resourceKey+".") { + keys = append(keys, k) + } + } + slices.Sort(keys) + return keys +} + +func deploymentVersion(deployment *bundledeployments.Deployment) (int, error) { + if deployment == nil || deployment.LastVersionId == "" { + return 0, nil + } + v, err := strconv.Atoi(deployment.LastVersionId) + if err != nil { + return 0, fmt.Errorf("failed to parse last_version_id %q: %w", deployment.LastVersionId, err) + } + return v, nil +} + +func localStatePath(ctx context.Context, b *bundle.Bundle) string { + _, localPath := b.StateFilenameDirect(ctx) + return localPath +} + +func withWorkspaceClient(ctx context.Context, b *bundle.Bundle) context.Context { + if !cmdctx.HasWorkspaceClient(ctx) { + return cmdctx.SetWorkspaceClient(ctx, b.WorkspaceClient(ctx)) + } + return ctx +} diff --git a/cmd/bundle/utils/process.go b/cmd/bundle/utils/process.go index ea29079f580..88bf69c07ac 100644 --- a/cmd/bundle/utils/process.go +++ b/cmd/bundle/utils/process.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "path" "path/filepath" "strconv" "time" @@ -31,8 +30,6 @@ import ( "github.com/databricks/cli/libs/logdiag" "github.com/databricks/cli/libs/sync" "github.com/databricks/cli/libs/telemetry/protos" - "github.com/databricks/databricks-sdk-go" - "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/bundledeployments" "github.com/spf13/cobra" "golang.org/x/mod/semver" @@ -248,7 +245,7 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle if b.ConfiguresDeploymentHistory(ctx) { var err error - dmsDeploymentID, dmsDeployment, err = fetchDeploymentFromStatePath(ctx, b.WorkspaceClient(ctx), b.Config.Workspace.StatePath) + dmsDeploymentID, dmsDeployment, err = dms.FetchDeployment(ctx, b.WorkspaceClient(ctx), b.Config.Workspace.StatePath) if err != nil { logdiag.LogError(ctx, err) return b, stateDesc, root.ErrAlreadyPrinted @@ -484,36 +481,6 @@ func ResolveEngineSetting(ctx context.Context, b *bundle.Bundle) (engine.EngineS return engine.EngineSetting{Type: engine.Default, Source: engine.SourceDefault, IsDefault: true}, nil } -// Lookup and return the deployment object from ${workspace.state_path}/resources.deployment.json -// -// TODO: a deployment is only usable when both the node and the service's record exist, and a -// half-created one - node present, record missing - blocks the bundle here even though -// CreateDeployment already recovers from it. Move this behind a dms.ReadDeployment(ctx, statePath) -// that returns an empty id and version unless both halves are there, leaving the caller to call -// CreateDeployment to create or finalize it. -// -// TODO: ask the service for a lookup by state path, so this is one round trip rather than two - a -// workspace lookup to turn the node into an id, then a get by that id. -func fetchDeploymentFromStatePath(ctx context.Context, w *databricks.WorkspaceClient, statePath string) (string, *bundledeployments.Deployment, error) { - nodePath := path.Join(statePath, dms.DeploymentNodeName) - - obj, err := w.Workspace.GetStatusByPath(ctx, nodePath) - if errors.Is(err, apierr.ErrNotFound) || errors.Is(err, apierr.ErrResourceDoesNotExist) { - return "", nil, nil - } - if err != nil { - return "", nil, fmt.Errorf("looking up deployment at %s: %w", nodePath, err) - } - deploymentID := strconv.FormatInt(obj.ObjectId, 10) - deployment, err := w.BundleDeployments.GetDeployment(ctx, bundledeployments.GetDeploymentRequest{ - Name: dms.DeploymentName(deploymentID), - }) - if err != nil { - return "", nil, err - } - return deploymentID, deployment, nil -} - // isNewerVersion reports whether the state's recorded CLI version is strictly // newer than the running build. Both are bare versions without a leading "v". // An empty stateVersion (state not written by any CLI yet) or an unparseable diff --git a/libs/dms/client.go b/libs/dms/client.go index 605b4bbd4db..60c0865ecc2 100644 --- a/libs/dms/client.go +++ b/libs/dms/client.go @@ -2,13 +2,16 @@ package dms import ( "context" + "errors" "fmt" "net/http" + "path" "strconv" "strings" "github.com/databricks/cli/libs/auth" "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/client" "github.com/databricks/databricks-sdk-go/service/bundledeployments" ) @@ -67,6 +70,33 @@ func (c *Client) CreateDeployment(ctx context.Context, parentPath string, metada return deploymentIDFromName(created.Name) } +// FetchDeployment looks up the deployment recorded under statePath, returning its id and record. +// It returns "", nil, nil when none exists yet: the deployment node is the workspace node the +// service creates on the first recorded deploy, and its id is that node's object id. +// +// TODO: ask the service for a lookup by state path, so this is one round trip rather than two - a +// workspace lookup to turn the node into an id, then a get by that id. +func FetchDeployment(ctx context.Context, w *databricks.WorkspaceClient, statePath string) (string, *bundledeployments.Deployment, error) { + nodePath := path.Join(statePath, DeploymentNodeName) + + obj, err := w.Workspace.GetStatusByPath(ctx, nodePath) + if errors.Is(err, apierr.ErrNotFound) || errors.Is(err, apierr.ErrResourceDoesNotExist) { + return "", nil, nil + } + if err != nil { + return "", nil, fmt.Errorf("looking up deployment at %s: %w", nodePath, err) + } + + deploymentID := strconv.FormatInt(obj.ObjectId, 10) + deployment, err := w.BundleDeployments.GetDeployment(ctx, bundledeployments.GetDeploymentRequest{ + Name: DeploymentName(deploymentID), + }) + if err != nil { + return "", nil, err + } + return deploymentID, deployment, nil +} + // UpdateDeployment writes the fields mask names onto the deployment. The service ignores every // other field, so the mask is what decides the write. func (c *Client) UpdateDeployment(ctx context.Context, deploymentID string, metadata Metadata, mask string) error { @@ -143,6 +173,14 @@ type CreateVersionRequest struct { Operations []StagedOperation `json:"operations,omitempty"` } +// ActionBind and ActionUnbind are the operation action types recorded for `bundle deployment +// bind` and `unbind`. They have no deployplan action, so they are staged directly. Bind reuses +// the SDK's constant; the SDK has none for unbind yet, so it is the string the service expects. +const ( + ActionBind = bundledeployments.OperationActionTypeOperationActionTypeBind + ActionUnbind = bundledeployments.OperationActionType("OPERATION_ACTION_TYPE_UNBIND") +) + // StagedOperation is one resource the version will record an operation for. The service // creates it in OPERATION_STATUS_PENDING at sequence id 0, and the CLI fills in the outcome // with UpdateOperation as the resource is applied. diff --git a/libs/dms/client_test.go b/libs/dms/client_test.go index 867655bca4e..2904cd51020 100644 --- a/libs/dms/client_test.go +++ b/libs/dms/client_test.go @@ -16,6 +16,13 @@ func TestClientNamesEveryResourceTheSameWay(t *testing.T) { assert.Equal(t, "deployments/dep-1/versions/2", versionName("dep-1", 2)) } +func TestBindUnbindActionTypes(t *testing.T) { + // The service records bind and unbind as their own action types; the SDK has a constant for + // bind but not yet for unbind, which is the literal string the service expects. + assert.Equal(t, "OPERATION_ACTION_TYPE_BIND", string(ActionBind)) + assert.Equal(t, "OPERATION_ACTION_TYPE_UNBIND", string(ActionUnbind)) +} + func TestDeploymentIDFromName(t *testing.T) { id, err := deploymentIDFromName("deployments/abc-123") require.NoError(t, err) From 3f9074a9f5fdffd5cedec64dff4643a9d1ecddd5 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 9 Sep 2026 22:17:16 +0000 Subject: [PATCH 2/5] Address review feedback on DMS bind/unbind - Always complete the DMS version (with failure on error) after starting it, so a bind/unbind recording failure does not leak the version's lease, matching the deploy and destroy paths. - FetchDeployment now returns the parsed last version id, dropping the duplicate parse in phases and process.go, and keeps the half-created-deployment TODO that was dropped when it moved to libs/dms. - Extract openRecordedState and tighten comments. - New bundle/dms/record-op-fault test asserts the version completes with failure when the operation upload fails; bundle/dms/bind-unbind now cleans up and asserts the resource is managed after bind and re-created after unbind. Co-authored-by: Isaac --- acceptance/bundle/dms/bind-unbind/output.txt | 20 +++++ acceptance/bundle/dms/bind-unbind/script | 17 ++++- .../dms/record-op-fault/databricks.yml.tmpl | 9 +++ .../bundle/dms/record-op-fault/out.test.toml | 3 + .../bundle/dms/record-op-fault/output.txt | 39 ++++++++++ acceptance/bundle/dms/record-op-fault/script | 11 +++ .../bundle/dms/record-op-fault/test.toml | 3 + bundle/direct/dstate/state.go | 7 +- bundle/phases/bind_dms.go | 73 +++++++------------ cmd/bundle/utils/process.go | 14 +--- libs/dms/client.go | 30 ++++++-- 11 files changed, 156 insertions(+), 70 deletions(-) create mode 100644 acceptance/bundle/dms/record-op-fault/databricks.yml.tmpl create mode 100644 acceptance/bundle/dms/record-op-fault/out.test.toml create mode 100644 acceptance/bundle/dms/record-op-fault/output.txt create mode 100644 acceptance/bundle/dms/record-op-fault/script create mode 100644 acceptance/bundle/dms/record-op-fault/test.toml diff --git a/acceptance/bundle/dms/bind-unbind/output.txt b/acceptance/bundle/dms/bind-unbind/output.txt index c3a79402e8c..b36693c783f 100644 --- a/acceptance/bundle/dms/bind-unbind/output.txt +++ b/acceptance/bundle/dms/bind-unbind/output.txt @@ -4,9 +4,21 @@ Successfully bound job with an id '[JOB_ID]' Run 'bundle deploy' to deploy changes to your workspace +=== The service now records the job as managed, so it plans as a change not a create +>>> [CLI] bundle plan +update jobs.foo + +Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged + === Unbind it again, dropping it from what the service records >>> [CLI] bundle deployment unbind foo +=== No longer managed, so it plans as a fresh create +>>> [CLI] bundle plan +create jobs.foo + +Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged + === Each recorded a version whose operation carries the bind / unbind action type >>> print_requests.py --dms --method POST //versions --nostamp { @@ -58,3 +70,11 @@ Run 'bundle deploy' to deploy changes to your workspace "completion_reason": "VERSION_COMPLETE_SUCCESS" } } + +=== Test cleanup +>>> [CLI] bundle destroy --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/dms-bind-unbind-[UNIQUE_NAME]/default + +Destroy: 0 deleted + +>>> [CLI] jobs delete [JOB_ID] diff --git a/acceptance/bundle/dms/bind-unbind/script b/acceptance/bundle/dms/bind-unbind/script index 1dde9788809..1b247eaaffd 100644 --- a/acceptance/bundle/dms/bind-unbind/script +++ b/acceptance/bundle/dms/bind-unbind/script @@ -1,12 +1,27 @@ envsubst < databricks.yml.tmpl > databricks.yml -title "Bind an existing job to the bundle, recorded with the deployment history service" job_id=$($CLI jobs create --json '{"name": "bind-target-'$UNIQUE_NAME'"}' | jq -r '.job_id') add_repl "$job_id" JOB_ID + +cleanup() { + title "Test cleanup" + trace $CLI bundle destroy --auto-approve + trace $CLI jobs delete "$job_id" + rm -f out.requests.txt +} +trap cleanup EXIT + +title "Bind an existing job to the bundle, recorded with the deployment history service" trace $CLI bundle deployment bind foo "$job_id" --auto-approve +title "The service now records the job as managed, so it plans as a change not a create" +trace $CLI bundle plan + title "Unbind it again, dropping it from what the service records" trace $CLI bundle deployment unbind foo +title "No longer managed, so it plans as a fresh create" +trace $CLI bundle plan + title "Each recorded a version whose operation carries the bind / unbind action type" trace print_requests.py --dms --method POST //versions --nostamp diff --git a/acceptance/bundle/dms/record-op-fault/databricks.yml.tmpl b/acceptance/bundle/dms/record-op-fault/databricks.yml.tmpl new file mode 100644 index 00000000000..235760efa24 --- /dev/null +++ b/acceptance/bundle/dms/record-op-fault/databricks.yml.tmpl @@ -0,0 +1,9 @@ +bundle: + name: dms-record-op-fault-$UNIQUE_NAME +experimental: + deployment_history: true + +resources: + jobs: + foo: + name: foo diff --git a/acceptance/bundle/dms/record-op-fault/out.test.toml b/acceptance/bundle/dms/record-op-fault/out.test.toml new file mode 100644 index 00000000000..9921e91a794 --- /dev/null +++ b/acceptance/bundle/dms/record-op-fault/out.test.toml @@ -0,0 +1,3 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +EnvMatrix.DMS = ["true"] diff --git a/acceptance/bundle/dms/record-op-fault/output.txt b/acceptance/bundle/dms/record-op-fault/output.txt new file mode 100644 index 00000000000..2c48b275b4d --- /dev/null +++ b/acceptance/bundle/dms/record-op-fault/output.txt @@ -0,0 +1,39 @@ + +=== The service rejects the operation upload, so recording the bind fails +>>> fault.py PATCH /api/2.0/bundle/* 500 0 1 + +>>> musterr [CLI] bundle deployment bind foo [JOB_ID] --auto-approve +Error: recording operation for resources.jobs.foo: Fault injected by test. (500 INJECTED) + +Endpoint: PATCH [DATABRICKS_URL]/api/2.0/bundle/deployments/[NUMID]/versions/1/operations/jobs.foo?update_mask=state%2Cerror_message%2Cresource_id%2Cstatus +HTTP Status: 500 Internal Server Error +API error_code: INJECTED +API message: Fault injected by test. + + +=== The version is still completed - with failure - rather than left holding its lease +>>> print_requests.py --dms --method POST //versions +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions", + "q": { + "version_id": "1" + }, + "body": { + "cli_version": "[CLI_VERSION]", + "version_type": "VERSION_TYPE_DEPLOY", + "operations": [ + { + "resource_key": "jobs.foo", + "action_type": "OPERATION_ACTION_TYPE_BIND" + } + ] + } +} +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions/1/complete", + "body": { + "completion_reason": "VERSION_COMPLETE_FAILURE" + } +} diff --git a/acceptance/bundle/dms/record-op-fault/script b/acceptance/bundle/dms/record-op-fault/script new file mode 100644 index 00000000000..fdc8afc00bf --- /dev/null +++ b/acceptance/bundle/dms/record-op-fault/script @@ -0,0 +1,11 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +job_id=$($CLI jobs create --json '{"name": "target-'$UNIQUE_NAME'"}' | jq -r '.job_id') +add_repl "$job_id" JOB_ID + +title "The service rejects the operation upload, so recording the bind fails" +trace fault.py "PATCH /api/2.0/bundle/*" 500 0 1 +trace musterr $CLI bundle deployment bind foo "$job_id" --auto-approve + +title "The version is still completed - with failure - rather than left holding its lease" +trace print_requests.py --dms --method POST //versions diff --git a/acceptance/bundle/dms/record-op-fault/test.toml b/acceptance/bundle/dms/record-op-fault/test.toml new file mode 100644 index 00000000000..24f1970a8f2 --- /dev/null +++ b/acceptance/bundle/dms/record-op-fault/test.toml @@ -0,0 +1,3 @@ +# Local only: the case needs an injected failure (DMS must reject the operation upload once), and +# fault.py registers its rule on the fake, which a cloud run does not have. +Cloud = false diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go index bcf165835d3..6694f951b4f 100644 --- a/bundle/direct/dstate/state.go +++ b/bundle/direct/dstate/state.go @@ -1020,10 +1020,9 @@ func (db *DeploymentState) unlockedSave() error { return nil } -// SnapshotToPlainState writes what this deployment-history state currently holds to path as an -// ordinary state file with no history marker, so the file-based bind flow can compute its plan and -// resolved state against a copy of what the service records without touching it. The state must be -// open for read. +// SnapshotToPlainState writes what this deployment-history state holds to path as an ordinary +// state file (no history marker), so a file-based flow can run against a copy of what the service +// records. The state must be open for read. func (db *DeploymentState) SnapshotToPlainState(path string) error { db.AssertOpenedForRead() db.mu.Lock() diff --git a/bundle/phases/bind_dms.go b/bundle/phases/bind_dms.go index 7d269879d7b..3fb7e25da11 100644 --- a/bundle/phases/bind_dms.go +++ b/bundle/phases/bind_dms.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "slices" - "strconv" "strings" "github.com/databricks/cli/bundle" @@ -18,18 +17,13 @@ import ( ) // bindWithHistory records a bind for a deployment that tracks history with the metadata service. -// The engine's bind computes the plan preview and resolved state against a throwaway copy of what -// the service holds, then the bind is recorded as an operation carrying that state, so the next -// deploy sees the resource as managed rather than new. +// It reuses the engine's file-based bind against a throwaway copy of what the service holds to get +// the plan preview and resolved state, then records that state as a bind operation so the next +// deploy sees the resource as managed. func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourceID string, autoApprove bool) { wsc := b.WorkspaceClient(ctx) - deploymentID, deployment, err := dms.FetchDeployment(ctx, wsc, b.Config.Workspace.StatePath) - if err != nil { - logdiag.LogError(ctx, err) - return - } - lastVersionID, err := deploymentVersion(deployment) + deploymentID, deployment, lastVersionID, err := dms.FetchDeployment(ctx, wsc, b.Config.Workspace.StatePath) if err != nil { logdiag.LogError(ctx, err) return @@ -53,9 +47,8 @@ func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourc // The seed's temp state is discarded; the bind is recorded with the service instead. defer result.Cancel() - // Record exactly what the engine resolved for the resource - the same state, id and - // dependencies a file-based bind would persist (etags and all) - read straight back out of the - // throwaway state it wrote rather than the state cache, which the plan step overwrites. + // Read the resolved state from the throwaway state the engine wrote, not the state cache, which + // the plan step overwrites (dropping the etag for dashboards/genie_spaces). entry, ok, err := resolvedEntry(ctx, result.TempStatePath, resourceKey) if err != nil { logdiag.LogError(ctx, err) @@ -74,7 +67,7 @@ func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourc func recordBind(ctx context.Context, b *bundle.Bundle, deploymentID string, deployment *bundledeployments.Deployment, lastVersionID int, resourceKey string, entry dstate.ResourceEntry) { ctx = withWorkspaceClient(ctx, b) db := &b.DeploymentBundle.StateDB - if err := db.Open(ctx, localStatePath(ctx, b), dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}); err != nil { + if err := openRecordedState(ctx, db, localStatePath(ctx, b), deploymentID, lastVersionID); err != nil { logdiag.LogError(ctx, err) return } @@ -95,18 +88,12 @@ func recordBind(ctx context.Context, b *bundle.Bundle, deploymentID string, depl logdiag.LogError(ctx, err) return } + // The version exists now, so close it out on every path; otherwise a failure recording the + // operation leaks its lease, as deploy and destroy also guard against. + defer completeRecordedVersion(ctx, b) if err := db.SaveState(ctx, resourceKey, entry.ID, entry.State, entry.DependsOn); err != nil { logdiag.LogError(ctx, err) - return - } - - if _, err := db.Finalize(ctx); err != nil { - logdiag.LogError(ctx, err) - return - } - if _, err := db.CompleteVersion(ctx, true); err != nil { - logdiag.LogError(ctx, err) } } @@ -116,7 +103,7 @@ func recordBind(ctx context.Context, b *bundle.Bundle, deploymentID string, depl func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string) { wsc := b.WorkspaceClient(ctx) - deploymentID, deployment, err := dms.FetchDeployment(ctx, wsc, b.Config.Workspace.StatePath) + deploymentID, _, lastVersionID, err := dms.FetchDeployment(ctx, wsc, b.Config.Workspace.StatePath) if err != nil { logdiag.LogError(ctx, err) return @@ -125,15 +112,10 @@ func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string // Nothing is recorded, so there is nothing to unbind. return } - lastVersionID, err := deploymentVersion(deployment) - if err != nil { - logdiag.LogError(ctx, err) - return - } ctx = withWorkspaceClient(ctx, b) db := &b.DeploymentBundle.StateDB - if err := db.Open(ctx, localStatePath(ctx, b), dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}); err != nil { + if err := openRecordedState(ctx, db, localStatePath(ctx, b), deploymentID, lastVersionID); err != nil { logdiag.LogError(ctx, err) return } @@ -160,6 +142,8 @@ func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string logdiag.LogError(ctx, err) return } + // The version exists now, so close it out on every path (see recordBind). + defer completeRecordedVersion(ctx, b) for _, k := range keys { if err := db.DeleteState(ctx, k, false); err != nil { @@ -168,20 +152,24 @@ func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string } log.Infof(ctx, "Unbound %s", k) } +} +// completeRecordedVersion drains the buffered operations and closes the version out, completing +// with failure if anything went wrong. Deferred once a version exists so every path completes it. +func completeRecordedVersion(ctx context.Context, b *bundle.Bundle) { + db := &b.DeploymentBundle.StateDB if _, err := db.Finalize(ctx); err != nil { logdiag.LogError(ctx, err) - return } - if _, err := db.CompleteVersion(ctx, true); err != nil { + if _, err := db.CompleteVersion(ctx, !logdiag.HasError(ctx)); err != nil { logdiag.LogError(ctx, err) } } // seedStateFromService writes a throwaway local state holding what the service currently records, -// so the engine's file-based bind can compute its plan and resolved state against it. Returns a -// path that does not exist yet when no deployment has been recorded, which the engine reads as an -// empty state. cleanup removes the seed and any temporary files the engine leaves beside it. +// so the engine's file-based bind can compute its plan and resolved state against it. The returned +// path does not exist yet when no deployment has been recorded, which the engine reads as empty +// state. cleanup removes the seed and any temporary files the engine leaves beside it. func seedStateFromService(ctx context.Context, b *bundle.Bundle, deploymentID string, lastVersionID int) (string, func(), error) { seedPath := localStatePath(ctx, b) + ".bind-seed" cleanup := func() { @@ -196,7 +184,7 @@ func seedStateFromService(ctx context.Context, b *bundle.Bundle, deploymentID st ctx = withWorkspaceClient(ctx, b) var src dstate.DeploymentState - if err := src.Open(ctx, localStatePath(ctx, b), dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}); err != nil { + if err := openRecordedState(ctx, &src, localStatePath(ctx, b), deploymentID, lastVersionID); err != nil { cleanup() return "", nil, err } @@ -238,15 +226,10 @@ func recordedKeys(db *dstate.DeploymentState, resourceKey string) []string { return keys } -func deploymentVersion(deployment *bundledeployments.Deployment) (int, error) { - if deployment == nil || deployment.LastVersionId == "" { - return 0, nil - } - v, err := strconv.Atoi(deployment.LastVersionId) - if err != nil { - return 0, fmt.Errorf("failed to parse last_version_id %q: %w", deployment.LastVersionId, err) - } - return v, nil +// openRecordedState opens the deployment's recorded state for read, reading its resources from the +// metadata service. +func openRecordedState(ctx context.Context, db *dstate.DeploymentState, path, deploymentID string, lastVersionID int) error { + return db.Open(ctx, path, dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}) } func localStatePath(ctx context.Context, b *bundle.Bundle) string { diff --git a/cmd/bundle/utils/process.go b/cmd/bundle/utils/process.go index 88bf69c07ac..39a2ed24641 100644 --- a/cmd/bundle/utils/process.go +++ b/cmd/bundle/utils/process.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "path/filepath" - "strconv" "time" "github.com/databricks/cli/bundle" @@ -245,7 +244,8 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle if b.ConfiguresDeploymentHistory(ctx) { var err error - dmsDeploymentID, dmsDeployment, err = dms.FetchDeployment(ctx, b.WorkspaceClient(ctx), b.Config.Workspace.StatePath) + var lastVersionID int + dmsDeploymentID, dmsDeployment, lastVersionID, err = dms.FetchDeployment(ctx, b.WorkspaceClient(ctx), b.Config.Workspace.StatePath) if err != nil { logdiag.LogError(ctx, err) return b, stateDesc, root.ErrAlreadyPrinted @@ -255,16 +255,6 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle // the plan carries them. version_id is always known (last recorded + 1); deployment_id // does not exist until a first deploy creates it, so it is left off here and the deploy // phase stamps the created id. - // The service reports the version as a string; parse it here so everything below - // carries a number. - lastVersionID := 0 - if dmsDeployment != nil && dmsDeployment.LastVersionId != "" { - lastVersionID, err = strconv.Atoi(dmsDeployment.LastVersionId) - if err != nil { - logdiag.LogError(ctx, fmt.Errorf("failed to parse last_version_id %q: %w", dmsDeployment.LastVersionId, err)) - return b, stateDesc, root.ErrAlreadyPrinted - } - } nextVersion := lastVersionID + 1 muts := []bundle.Mutator{metadata.AnnotateDeploymentVersion(nextVersion)} if dmsDeploymentID != "" { diff --git a/libs/dms/client.go b/libs/dms/client.go index 60c0865ecc2..c56f0826f10 100644 --- a/libs/dms/client.go +++ b/libs/dms/client.go @@ -70,21 +70,27 @@ func (c *Client) CreateDeployment(ctx context.Context, parentPath string, metada return deploymentIDFromName(created.Name) } -// FetchDeployment looks up the deployment recorded under statePath, returning its id and record. -// It returns "", nil, nil when none exists yet: the deployment node is the workspace node the -// service creates on the first recorded deploy, and its id is that node's object id. +// FetchDeployment looks up the deployment recorded under statePath, returning its id, record, and +// last recorded version number. It returns "", nil, 0, nil when none exists yet: the deployment +// node is the workspace node the service creates on the first recorded deploy, and its id is that +// node's object id. +// +// TODO: a deployment is usable only when both the node and the service's record exist; a +// half-created one (node present, record missing) errors here even though CreateDeployment can +// recover from it. A dms.ReadDeployment returning empty unless both halves are present would let +// the caller create or finalize it instead. // // TODO: ask the service for a lookup by state path, so this is one round trip rather than two - a // workspace lookup to turn the node into an id, then a get by that id. -func FetchDeployment(ctx context.Context, w *databricks.WorkspaceClient, statePath string) (string, *bundledeployments.Deployment, error) { +func FetchDeployment(ctx context.Context, w *databricks.WorkspaceClient, statePath string) (string, *bundledeployments.Deployment, int, error) { nodePath := path.Join(statePath, DeploymentNodeName) obj, err := w.Workspace.GetStatusByPath(ctx, nodePath) if errors.Is(err, apierr.ErrNotFound) || errors.Is(err, apierr.ErrResourceDoesNotExist) { - return "", nil, nil + return "", nil, 0, nil } if err != nil { - return "", nil, fmt.Errorf("looking up deployment at %s: %w", nodePath, err) + return "", nil, 0, fmt.Errorf("looking up deployment at %s: %w", nodePath, err) } deploymentID := strconv.FormatInt(obj.ObjectId, 10) @@ -92,9 +98,17 @@ func FetchDeployment(ctx context.Context, w *databricks.WorkspaceClient, statePa Name: DeploymentName(deploymentID), }) if err != nil { - return "", nil, err + return "", nil, 0, err + } + + lastVersionID := 0 + if deployment.LastVersionId != "" { + lastVersionID, err = strconv.Atoi(deployment.LastVersionId) + if err != nil { + return "", nil, 0, fmt.Errorf("failed to parse last_version_id %q: %w", deployment.LastVersionId, err) + } } - return deploymentID, deployment, nil + return deploymentID, deployment, lastVersionID, nil } // UpdateDeployment writes the fields mask names onto the deployment. The service ignores every From 14886e16a7a263ccdb5200b285b0f90cd516a90c Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 10 Sep 2026 09:40:29 +0000 Subject: [PATCH 3/5] Rework DMS bind as a plan action that deploys immediately Bind is now a first-class plan action. `bundle deployment bind` plans the resource as Bind (config already matches the existing workspace resource) or BindAndUpdate (config differs) and applies it immediately, because the metadata service is the source of truth - unlike the file-based bind, which stages the change for the next deploy. Adds Bind/BindAndUpdate to deployplan and the direct engine's plan/apply, maps them to the BIND / BIND_AND_UPDATE operation types, and drops the seed-state machinery the previous approach needed. The general bind/unbind acceptance tests stay on the file-based (deferred) path, since immediate DMS bind cannot share their goldens; DMS bind/unbind is covered by the dedicated bundle/dms/bind-unbind and record-op-fault tests. Co-authored-by: Isaac --- .../deployment/bind/alert/out.test.toml | 2 +- .../deployment/bind/catalog/out.test.toml | 2 +- .../deployment/bind/cluster/out.test.toml | 2 +- .../deployment/bind/dashboard/out.test.toml | 2 +- .../bind/dashboard/recreation/test.toml | 5 - .../bind/database_instance/out.test.toml | 2 +- .../deployment/bind/experiment/out.test.toml | 2 +- .../bind/external_location/out.test.toml | 2 +- .../deployment/bind/genie_space/out.test.toml | 2 +- .../already-managed-different/out.test.toml | 2 +- .../job/already-managed-same/out.test.toml | 2 +- .../bind/job/engine-from-config/out.test.toml | 2 +- .../bind/job/generate-and-bind/out.test.toml | 2 +- .../bind/job/job-abort-bind/out.test.toml | 2 +- .../job/job-spark-python-task/out.test.toml | 2 +- .../deployment/bind/job/noop-job/test.toml | 5 - .../deployment/bind/job/python-job/test.toml | 5 - .../bind/job/stale-state/out.test.toml | 2 +- .../bind/model-serving-endpoint/out.test.toml | 2 +- .../deployment/bind/pipelines/test.toml | 5 - .../bind/postgres_database/out.test.toml | 2 +- .../bind/postgres_role/out.test.toml | 2 +- .../postgres_snapshot_schedule/out.test.toml | 2 +- .../bind/quality-monitor/out.test.toml | 2 +- .../bind/registered-model/out.test.toml | 2 +- .../deployment/bind/schema/out.test.toml | 2 +- .../bind/secret-scope/out.test.toml | 2 +- .../bind/sql_warehouse/out.test.toml | 2 +- acceptance/bundle/deployment/bind/test.toml | 4 + .../bind/vector_search_endpoint/out.test.toml | 2 +- .../bind/vector_search_index/out.test.toml | 2 +- .../deployment/bind/volume/out.test.toml | 2 +- .../unbind/engine-from-config/out.test.toml | 2 +- .../deployment/unbind/grants/out.test.toml | 2 +- .../bundle/deployment/unbind/job/test.toml | 5 - .../unbind/permissions/out.test.toml | 2 +- .../deployment/unbind/python-job/test.toml | 5 - acceptance/bundle/deployment/unbind/test.toml | 4 + acceptance/bundle/dms/bind-unbind/output.txt | 8 +- acceptance/bundle/dms/bind-unbind/script | 2 +- .../bundle/dms/record-op-fault/output.txt | 2 +- bundle/deployplan/action.go | 39 +++-- bundle/deployplan/plan.go | 5 +- bundle/direct/apply.go | 21 ++- bundle/direct/bind.go | 34 ++-- bundle/direct/bundle_plan.go | 59 +++++-- bundle/direct/pkg.go | 6 + bundle/phases/bind.go | 42 ++--- bundle/phases/bind_dms.go | 157 ++++++++---------- bundle/phases/dms.go | 4 + libs/dms/client.go | 14 +- libs/dms/client_test.go | 6 +- 52 files changed, 262 insertions(+), 235 deletions(-) delete mode 100644 acceptance/bundle/deployment/bind/dashboard/recreation/test.toml create mode 100644 acceptance/bundle/deployment/bind/test.toml create mode 100644 acceptance/bundle/deployment/unbind/test.toml diff --git a/acceptance/bundle/deployment/bind/alert/out.test.toml b/acceptance/bundle/deployment/bind/alert/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/alert/out.test.toml +++ b/acceptance/bundle/deployment/bind/alert/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/catalog/out.test.toml b/acceptance/bundle/deployment/bind/catalog/out.test.toml index ae5c7bd798f..d73c45e3119 100644 --- a/acceptance/bundle/deployment/bind/catalog/out.test.toml +++ b/acceptance/bundle/deployment/bind/catalog/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/cluster/out.test.toml b/acceptance/bundle/deployment/bind/cluster/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/cluster/out.test.toml +++ b/acceptance/bundle/deployment/bind/cluster/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/dashboard/out.test.toml b/acceptance/bundle/deployment/bind/dashboard/out.test.toml index b1bef937a27..2acec25aece 100644 --- a/acceptance/bundle/deployment/bind/dashboard/out.test.toml +++ b/acceptance/bundle/deployment/bind/dashboard/out.test.toml @@ -1,4 +1,4 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] EnvMatrix.READPLAN = ["", "1"] diff --git a/acceptance/bundle/deployment/bind/dashboard/recreation/test.toml b/acceptance/bundle/deployment/bind/dashboard/recreation/test.toml deleted file mode 100644 index a9e9f0c3488..00000000000 --- a/acceptance/bundle/deployment/bind/dashboard/recreation/test.toml +++ /dev/null @@ -1,5 +0,0 @@ -# This dumps the state file after bind, whose serial counts local state writes; under DMS the -# service owns the version number instead, so the dumped serial differs. That is an engine-level -# difference, not a bind one, and DMS bind is covered by bundle/dms/bind-unbind, so skip the DMS -# variant here. -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/database_instance/out.test.toml b/acceptance/bundle/deployment/bind/database_instance/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/database_instance/out.test.toml +++ b/acceptance/bundle/deployment/bind/database_instance/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/experiment/out.test.toml b/acceptance/bundle/deployment/bind/experiment/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/experiment/out.test.toml +++ b/acceptance/bundle/deployment/bind/experiment/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/external_location/out.test.toml b/acceptance/bundle/deployment/bind/external_location/out.test.toml index 59b56a2037c..27ec2a7fcd6 100644 --- a/acceptance/bundle/deployment/bind/external_location/out.test.toml +++ b/acceptance/bundle/deployment/bind/external_location/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/genie_space/out.test.toml b/acceptance/bundle/deployment/bind/genie_space/out.test.toml index 59b56a2037c..27ec2a7fcd6 100644 --- a/acceptance/bundle/deployment/bind/genie_space/out.test.toml +++ b/acceptance/bundle/deployment/bind/genie_space/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml b/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/already-managed-different/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml b/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/already-managed-same/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml b/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml index 59b56a2037c..27ec2a7fcd6 100644 --- a/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/engine-from-config/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml b/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/generate-and-bind/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml b/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/job-abort-bind/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml b/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/job-spark-python-task/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/noop-job/test.toml b/acceptance/bundle/deployment/bind/job/noop-job/test.toml index ffb9a92d09b..339a7a29f54 100644 --- a/acceptance/bundle/deployment/bind/job/noop-job/test.toml +++ b/acceptance/bundle/deployment/bind/job/noop-job/test.toml @@ -1,6 +1 @@ Cloud = false # test leaves deployed job - -# `jobs get` shows the job's deployment block, which under DMS also carries the -# deployment_id/version_id stamp - an engine-level difference, not a bind one. DMS bind is covered -# by bundle/dms/bind-unbind, so skip the DMS variant here. -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/python-job/test.toml b/acceptance/bundle/deployment/bind/job/python-job/test.toml index f7d685ac5a9..39101095ee6 100644 --- a/acceptance/bundle/deployment/bind/job/python-job/test.toml +++ b/acceptance/bundle/deployment/bind/job/python-job/test.toml @@ -1,7 +1,2 @@ Cloud = false # test leaves deployed job Timeout = '40s' # python can be slow in CI likely due to caching problems - -# `jobs get` shows the job's deployment block, which under DMS also carries the -# deployment_id/version_id stamp - an engine-level difference, not a bind one. DMS bind is covered -# by bundle/dms/bind-unbind, so skip the DMS variant here. -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml b/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml +++ b/acceptance/bundle/deployment/bind/job/stale-state/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml b/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml +++ b/acceptance/bundle/deployment/bind/model-serving-endpoint/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/pipelines/test.toml b/acceptance/bundle/deployment/bind/pipelines/test.toml index 3f801a08b9a..18b1a88417e 100644 --- a/acceptance/bundle/deployment/bind/pipelines/test.toml +++ b/acceptance/bundle/deployment/bind/pipelines/test.toml @@ -1,6 +1 @@ Cloud = false - -# These assert the deploy's request bodies, which under DMS also carry the pipeline's -# deployment_id/version_id stamp - an engine-level difference, not a bind one. DMS bind is covered -# by bundle/dms/bind-unbind, so skip the DMS variant here. -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/postgres_database/out.test.toml b/acceptance/bundle/deployment/bind/postgres_database/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/postgres_database/out.test.toml +++ b/acceptance/bundle/deployment/bind/postgres_database/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/postgres_role/out.test.toml b/acceptance/bundle/deployment/bind/postgres_role/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/postgres_role/out.test.toml +++ b/acceptance/bundle/deployment/bind/postgres_role/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml b/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml index 59b56a2037c..27ec2a7fcd6 100644 --- a/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml +++ b/acceptance/bundle/deployment/bind/postgres_snapshot_schedule/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml b/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml +++ b/acceptance/bundle/deployment/bind/quality-monitor/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/registered-model/out.test.toml b/acceptance/bundle/deployment/bind/registered-model/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/registered-model/out.test.toml +++ b/acceptance/bundle/deployment/bind/registered-model/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/schema/out.test.toml b/acceptance/bundle/deployment/bind/schema/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/schema/out.test.toml +++ b/acceptance/bundle/deployment/bind/schema/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/secret-scope/out.test.toml b/acceptance/bundle/deployment/bind/secret-scope/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/secret-scope/out.test.toml +++ b/acceptance/bundle/deployment/bind/secret-scope/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml b/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml index e1af1a235ad..ae800809893 100644 --- a/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml +++ b/acceptance/bundle/deployment/bind/sql_warehouse/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/test.toml b/acceptance/bundle/deployment/bind/test.toml new file mode 100644 index 00000000000..b2554375c4e --- /dev/null +++ b/acceptance/bundle/deployment/bind/test.toml @@ -0,0 +1,4 @@ +# These tests cover the file-based bind, which stages a change for the next deploy. Under the +# deployment metadata service (DMS) bind is a distinct operation - it plans and applies the change +# immediately - so it cannot share these goldens and is covered by bundle/dms/bind-unbind instead. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml b/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml index ae5c7bd798f..d73c45e3119 100644 --- a/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml +++ b/acceptance/bundle/deployment/bind/vector_search_endpoint/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml b/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml index f9f4880725d..12effa8e96d 100644 --- a/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml +++ b/acceptance/bundle/deployment/bind/vector_search_index/out.test.toml @@ -1,4 +1,4 @@ Cloud = true CloudSlow = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/bind/volume/out.test.toml b/acceptance/bundle/deployment/bind/volume/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/bind/volume/out.test.toml +++ b/acceptance/bundle/deployment/bind/volume/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml b/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml index 59b56a2037c..27ec2a7fcd6 100644 --- a/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml +++ b/acceptance/bundle/deployment/unbind/engine-from-config/out.test.toml @@ -1,3 +1,3 @@ Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/grants/out.test.toml b/acceptance/bundle/deployment/unbind/grants/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/unbind/grants/out.test.toml +++ b/acceptance/bundle/deployment/unbind/grants/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/job/test.toml b/acceptance/bundle/deployment/unbind/job/test.toml index 91e1ea914d5..5c721ad6e79 100644 --- a/acceptance/bundle/deployment/unbind/job/test.toml +++ b/acceptance/bundle/deployment/unbind/job/test.toml @@ -1,7 +1,2 @@ Cloud = false # test leaves deployed job Ignore = [".databricks"] - -# The final `jobs get` shows the job's deployment block, which under DMS also carries the -# deployment_id/version_id stamp; that is an engine-level difference, not an unbind one, and DMS -# unbind is covered by bundle/dms/bind-unbind. So skip the DMS variant here. -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/permissions/out.test.toml b/acceptance/bundle/deployment/unbind/permissions/out.test.toml index cef45fe553a..9b8ddfcd2bd 100644 --- a/acceptance/bundle/deployment/unbind/permissions/out.test.toml +++ b/acceptance/bundle/deployment/unbind/permissions/out.test.toml @@ -1,3 +1,3 @@ Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] -EnvMatrix.DMS = ["", "true"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/python-job/test.toml b/acceptance/bundle/deployment/unbind/python-job/test.toml index 5b93f3eb3cf..22a0568dce8 100644 --- a/acceptance/bundle/deployment/unbind/python-job/test.toml +++ b/acceptance/bundle/deployment/unbind/python-job/test.toml @@ -1,8 +1,3 @@ Cloud = false # test leaves deployed job Ignore = [".databricks"] Timeout = '40s' # python can be slow in CI likely due to caching problems - -# The final `jobs get` shows the job's deployment block, which under DMS also carries the -# deployment_id/version_id stamp; that is an engine-level difference, not an unbind one, and DMS -# unbind is covered by bundle/dms/bind-unbind. So skip the DMS variant here. -EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/test.toml b/acceptance/bundle/deployment/unbind/test.toml new file mode 100644 index 00000000000..af0bf04cc94 --- /dev/null +++ b/acceptance/bundle/deployment/unbind/test.toml @@ -0,0 +1,4 @@ +# These tests cover the file-based unbind. Under the deployment metadata service (DMS) unbind +# records its change with the service immediately rather than in the state file, so it cannot share +# these goldens and is covered by bundle/dms/bind-unbind instead. +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/dms/bind-unbind/output.txt b/acceptance/bundle/dms/bind-unbind/output.txt index b36693c783f..22cc35c925e 100644 --- a/acceptance/bundle/dms/bind-unbind/output.txt +++ b/acceptance/bundle/dms/bind-unbind/output.txt @@ -4,11 +4,9 @@ Successfully bound job with an id '[JOB_ID]' Run 'bundle deploy' to deploy changes to your workspace -=== The service now records the job as managed, so it plans as a change not a create +=== The bind applied the config and recorded the resource, so a re-plan is a no-op >>> [CLI] bundle plan -update jobs.foo - -Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged === Unbind it again, dropping it from what the service records >>> [CLI] bundle deployment unbind foo @@ -33,7 +31,7 @@ Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged "operations": [ { "resource_key": "jobs.foo", - "action_type": "OPERATION_ACTION_TYPE_BIND" + "action_type": "OPERATION_ACTION_TYPE_BIND_AND_UPDATE" } ] } diff --git a/acceptance/bundle/dms/bind-unbind/script b/acceptance/bundle/dms/bind-unbind/script index 1b247eaaffd..f59d273f403 100644 --- a/acceptance/bundle/dms/bind-unbind/script +++ b/acceptance/bundle/dms/bind-unbind/script @@ -14,7 +14,7 @@ trap cleanup EXIT title "Bind an existing job to the bundle, recorded with the deployment history service" trace $CLI bundle deployment bind foo "$job_id" --auto-approve -title "The service now records the job as managed, so it plans as a change not a create" +title "The bind applied the config and recorded the resource, so a re-plan is a no-op" trace $CLI bundle plan title "Unbind it again, dropping it from what the service records" diff --git a/acceptance/bundle/dms/record-op-fault/output.txt b/acceptance/bundle/dms/record-op-fault/output.txt index 2c48b275b4d..d65e155ac1f 100644 --- a/acceptance/bundle/dms/record-op-fault/output.txt +++ b/acceptance/bundle/dms/record-op-fault/output.txt @@ -25,7 +25,7 @@ API message: Fault injected by test. "operations": [ { "resource_key": "jobs.foo", - "action_type": "OPERATION_ACTION_TYPE_BIND" + "action_type": "OPERATION_ACTION_TYPE_BIND_AND_UPDATE" } ] } diff --git a/bundle/deployplan/action.go b/bundle/deployplan/action.go index e855dbb2197..b1ede32887d 100644 --- a/bundle/deployplan/action.go +++ b/bundle/deployplan/action.go @@ -31,25 +31,32 @@ type ActionType string // If case of several options, action with highest severity wins. // Note, Create/Delete are handled explicitly and never compared. const ( - Undefined ActionType = "" - Skip ActionType = "skip" - Resize ActionType = "resize" - Update ActionType = "update" - UpdateWithID ActionType = "update_id" - Create ActionType = "create" - Recreate ActionType = "recreate" - Delete ActionType = "delete" + Undefined ActionType = "" + Skip ActionType = "skip" + Resize ActionType = "resize" + Update ActionType = "update" + // Bind adopts an existing workspace resource whose config already matches it: record it in + // state without touching the resource. BindAndUpdate adopts it and applies the config in one + // step. Both are set only for the resource named by `bundle deployment bind`. + Bind ActionType = "bind" + BindAndUpdate ActionType = "bind_and_update" + UpdateWithID ActionType = "update_id" + Create ActionType = "create" + Recreate ActionType = "recreate" + Delete ActionType = "delete" ) var actionOrder = map[ActionType]int{ - Undefined: 0, - Skip: 1, - Resize: 2, - Update: 3, - UpdateWithID: 4, - Create: 5, - Recreate: 6, - Delete: 7, + Undefined: 0, + Skip: 1, + Bind: 2, + Resize: 3, + Update: 4, + BindAndUpdate: 5, + UpdateWithID: 6, + Create: 7, + Recreate: 8, + Delete: 9, } func (a ActionType) KeepsID() bool { diff --git a/bundle/deployplan/plan.go b/bundle/deployplan/plan.go index 67236f74766..267e96070b1 100644 --- a/bundle/deployplan/plan.go +++ b/bundle/deployplan/plan.go @@ -54,7 +54,7 @@ func (p *Plan) CountActions() ActionCounts { switch entry.Action { case Create: c.Create++ - case Update, UpdateWithID, Resize: + case Update, UpdateWithID, Resize, BindAndUpdate: c.Change++ case Delete: c.Delete++ @@ -62,7 +62,8 @@ func (p *Plan) CountActions() ActionCounts { // A recreate counts as both a delete and a create. c.Delete++ c.Create++ - case Skip, Undefined: + case Skip, Undefined, Bind: + // Bind adopts an existing resource without changing it. c.Unchanged++ } } diff --git a/bundle/direct/apply.go b/bundle/direct/apply.go index 21822cd0fb3..fe9c2c53934 100644 --- a/bundle/direct/apply.go +++ b/bundle/direct/apply.go @@ -35,8 +35,18 @@ func (d *DeploymentUnit) Destroy(ctx context.Context, db *dstate.DeploymentState func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState, newState any, actionType deployplan.ActionType, planEntry *deployplan.PlanEntry) error { ctx = log.WithPrefix(ctx, "deploying "+d.ResourceKey) ctx = d.withResourceKey(ctx) - if actionType == deployplan.Create { + + // Bind adopts an existing resource, so its id comes from the plan entry rather than state. + switch actionType { + case deployplan.Create: return d.Create(ctx, db, newState) + case deployplan.Bind: + return d.Bind(ctx, db, planEntry.ID, newState, planEntry) + case deployplan.BindAndUpdate: + // Adopt and apply the config in one step; the update is the same as any other. + return d.Update(ctx, db, planEntry.ID, newState, planEntry) + default: + // The remaining actions act on a resource already in state; handled below. } oldID := db.GetResourceID(d.ResourceKey) @@ -58,6 +68,15 @@ func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState, } } +// Bind adopts an existing workspace resource: it records the id and config as state without any +// API write, since the config already matches the resource. The remote etag is copied in so +// etag-based drift detection (dashboards, genie_spaces) still works on the next plan. +func (d *DeploymentUnit) Bind(ctx context.Context, db *dstate.DeploymentState, id string, newState any, planEntry *deployplan.PlanEntry) error { + copyRemoteEtag(d.ResourceKey, planEntry.RemoteState, newState) + log.Infof(ctx, "Bound %s id=%#v", d.ResourceKey, id) + return d.saveState(ctx, db, id, newState, d.DependsOn) +} + // Create creates the resource and records its state. func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState, newState any) error { var newID string diff --git a/bundle/direct/bind.go b/bundle/direct/bind.go index 115673db731..92f9dcc10ff 100644 --- a/bundle/direct/bind.go +++ b/bundle/direct/bind.go @@ -49,6 +49,25 @@ type BindResult struct { StatePath string } +// copyRemoteEtag copies the remote etag into newState for resources that use etag-based drift +// detection (dashboards, genie_spaces). The etag comes from remote, not the user; without it the +// next plan reports a bogus update. A no-op for other resources or when there is no remote state. +func copyRemoteEtag(resourceKey string, remoteState, newState any) { + if remoteState == nil { + return + } + if !strings.Contains(resourceKey, ".dashboards.") && !strings.Contains(resourceKey, ".genie_spaces.") { + return + } + etag, err := structaccess.Get(remoteState, structpath.NewStringKey(nil, "etag")) + if err != nil || etag == nil { + return + } + if etagStr, ok := etag.(string); ok && etagStr != "" { + _ = structaccess.Set(newState, structpath.NewStringKey(nil, "etag"), etagStr) + } +} + // Bind adds an existing workspace resource to a temporary state and calculates // if there will be any changes when deploying. // @@ -135,18 +154,11 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac dependsOn = entry.DependsOn } - // Copy etag from remote state for resources that use etag-based drift - // detection (dashboards and genie spaces). The etag is not provided by the - // user; it comes from remote. If we don't store it in state, we won't - // detect remote drift correctly and the next plan shows a bogus update. - if (strings.Contains(resourceKey, ".dashboards.") || strings.Contains(resourceKey, ".genie_spaces.")) && entry != nil && entry.RemoteState != nil { - etag, err := structaccess.Get(entry.RemoteState, structpath.NewStringKey(nil, "etag")) - if err == nil && etag != nil { - if etagStr, ok := etag.(string); ok && etagStr != "" { - _ = structaccess.Set(sv.Value, structpath.NewStringKey(nil, "etag"), etagStr) - } - } + var remoteState any + if entry != nil { + remoteState = entry.RemoteState } + copyRemoteEtag(resourceKey, remoteState, sv.Value) err = b.StateDB.Open(ctx, tmpStatePath, dstate.WithRecovery(true), dstate.WithWrite(true), dstate.WithDeploymentHistory(false), dstate.OpenDmsArgs{}) if err != nil { diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 48d76abe97f..3e9a9efddcf 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -258,32 +258,47 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks } dbentry, hasEntry := b.StateDB.GetResourceEntry(resourceKey) + + // adopt is `bundle deployment bind`: take over the existing workspace resource b.BindID + // rather than creating a new one. It has no prior state, so the id and baseline below come + // from the bind request and config instead of the state file. + adopt := b.BindKey == resourceKey + // Tolerate empty-ID entries from older partial-recreate failures // (apply.Recreate now deletes state on the way through, but pre-fix // state files may still carry a malformed entry). Treat as missing // and let the resource be re-created on this plan. - if !hasEntry || dbentry.ID == "" { + if !adopt && (!hasEntry || dbentry.ID == "") { entry.Action = deployplan.Create return true } - savedState, err := parseState(adapter.StateType(), dbentry.State) - if err != nil { - logdiag.LogError(ctx, fmt.Errorf("%s: interpreting state: %w", errorPrefix, err)) + sv, ok := b.StateCache.Load(resourceKey) + if !ok { + logdiag.LogError(ctx, fmt.Errorf("%s: internal error: no state cache entry found for %q", errorPrefix, resourceKey)) return false } + resourceID := dbentry.ID + var savedState any + if adopt { + resourceID = b.BindID + // No prior state, so diff config against itself: only remote drift shows below. + savedState = sv.Value + } else { + savedState, err = parseState(adapter.StateType(), dbentry.State) + if err != nil { + logdiag.LogError(ctx, fmt.Errorf("%s: interpreting state: %w", errorPrefix, err)) + return false + } + } + // Note, currently we're diffing static structs, not dynamic value. // This means for fields that contain references like ${resources.group.foo.id} we do one of the following: // for strings: comparing unresolved string like "${resoures.group.foo.id}" with actual object id. As long as IDs do not have ${...} format we're good. // for integers: compare 0 with actual object ID. As long as real object IDs are never 0 we're good. // Once we add non-id fields or add per-field details to "bundle plan", we must read dynamic data and deal with references as first class citizen. // This means distinguishing between 0 that are actually object ids and 0 that are there because typed struct integer cannot contain ${...} string. - sv, ok := b.StateCache.Load(resourceKey) - if !ok { - logdiag.LogError(ctx, fmt.Errorf("%s: internal error: no state cache entry found for %q", errorPrefix, resourceKey)) - return false - } localDiff, err := structdiff.GetStructDiff(savedState, sv.Value, adapter.KeyedSlices()) if err != nil { logdiag.LogError(ctx, fmt.Errorf("%s: diffing local state: %w", errorPrefix, err)) @@ -291,13 +306,13 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks } remoteState, err := retryOnTransient(ctx, func() (any, error) { - return adapter.DoRead(ctx, dbentry.ID) + return adapter.DoRead(ctx, resourceID) }) if err != nil { if apierr.IsMissing(err) { remoteState = nil } else { - logdiag.LogError(ctx, fmt.Errorf("%s: reading id=%q: %w", errorPrefix, dbentry.ID, err)) + logdiag.LogError(ctx, fmt.Errorf("%s: reading id=%q: %w", errorPrefix, resourceID, err)) return false } } @@ -313,7 +328,7 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks if remoteState != nil { remoteStateComparable, err = adapter.RemapState(remoteState) if err != nil { - logdiag.LogError(ctx, fmt.Errorf("%s: interpreting remote state id=%q: %w", errorPrefix, dbentry.ID, err)) + logdiag.LogError(ctx, fmt.Errorf("%s: interpreting remote state id=%q: %w", errorPrefix, resourceID, err)) return false } @@ -336,11 +351,25 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks return false } - if remoteState == nil { + switch { + case remoteState == nil && adopt: + logdiag.LogError(ctx, fmt.Errorf("%s: cannot bind to id=%q: resource not found", errorPrefix, resourceID)) + return false + case remoteState == nil: // Even if local action is "recreate" which is higher than "create", we should still pick "create" here // because we know remote does not exist. action = deployplan.Create - } else { + case adopt: + // Adopting an existing resource: no change needed is a plain bind, a change is a + // bind-and-update that applies the config in the same step. + if getMaxAction(entry.Changes) == deployplan.Skip { + action = deployplan.Bind + } else { + action = deployplan.BindAndUpdate + } + // The id is not in state yet, so carry it to apply on the plan entry. + entry.ID = resourceID + default: action = getMaxAction(entry.Changes) } @@ -349,7 +378,7 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks b.RemoteStateCache.Store(resourceKey, remoteState) // Validate that resources without DoUpdate don't have update actions - if action == deployplan.Update && !adapter.HasDoUpdate() { + if (action == deployplan.Update || action == deployplan.BindAndUpdate) && !adapter.HasDoUpdate() { logdiag.LogError(ctx, fmt.Errorf("%s: resource does not support update action but plan produced update", errorPrefix)) return false } diff --git a/bundle/direct/pkg.go b/bundle/direct/pkg.go index ea7ae1e51b6..98bffde2494 100644 --- a/bundle/direct/pkg.go +++ b/bundle/direct/pkg.go @@ -50,6 +50,12 @@ type DeploymentBundle struct { Plan *deployplan.Plan RemoteStateCache sync.Map StateCache structvar.Cache + + // BindKey and BindID, when BindKey is set, make CalculatePlan adopt the existing workspace + // resource BindID as resource BindKey - planned as Bind (config already matches) or + // BindAndUpdate (config differs) instead of Create. Set only by `bundle deployment bind`. + BindKey string + BindID string } // SetRemoteState updates the remote state with type validation and marks as fresh. diff --git a/bundle/phases/bind.go b/bundle/phases/bind.go index db335e9df4b..a0e5d1c07be 100644 --- a/bundle/phases/bind.go +++ b/bundle/phases/bind.go @@ -13,7 +13,6 @@ import ( "github.com/databricks/cli/bundle/deploy/lock" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/deployplan" - "github.com/databricks/cli/bundle/direct" "github.com/databricks/cli/bundle/statemgmt" "github.com/databricks/cli/libs/agent" "github.com/databricks/cli/libs/cmdio" @@ -58,7 +57,8 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions, en return } - if !confirmBindPlan(ctx, resourceKey, result, opts.AutoApprove) { + if !confirmBindPlan(ctx, resourceKey, result.Plan, opts.AutoApprove) { + result.Cancel() return } @@ -93,43 +93,43 @@ func jsonDump(ctx context.Context, v any, field string) string { return string(b) } -// confirmBindPlan shows the plan for the bound resource and, unless autoApprove, asks the user to -// confirm. It reports whether the bind should proceed; on decline or an unpromptable console it -// cancels result, logs the reason, and returns false so the caller returns. -func confirmBindPlan(ctx context.Context, resourceKey string, result *direct.BindResult, autoApprove bool) bool { - if !result.HasChanges || autoApprove { +// confirmBindPlan shows the bound resource's planned action and, unless autoApprove, asks the user +// to confirm. It reports whether the bind should proceed; on decline or an unpromptable console it +// logs the reason and returns false. A plain bind or skip changes nothing, so it proceeds without a +// prompt. The caller owns any cleanup on a false return. +func confirmBindPlan(ctx context.Context, resourceKey string, plan *deployplan.Plan, autoApprove bool) bool { + var entry *deployplan.PlanEntry + if plan != nil { + entry = plan.Plan[resourceKey] + } + changesWorkspace := entry != nil && entry.Action != deployplan.Skip && entry.Action != deployplan.Bind && entry.Action != deployplan.Undefined + if !changesWorkspace || autoApprove { return true } - cmdio.LogString(ctx, fmt.Sprintf("Plan: %s %s", result.Action, resourceKey)) - - if result.Plan != nil { - if entry, ok := result.Plan.Plan[resourceKey]; ok && entry != nil && len(entry.Changes) > 0 { - cmdio.LogString(ctx, "\nChanges detected:") - for _, field := range slices.Sorted(maps.Keys(entry.Changes)) { - change := entry.Changes[field] - if change.Action != deployplan.Skip { - cmdio.LogString(ctx, fmt.Sprintf(" ~ %s: %v -> %v", field, jsonDump(ctx, change.Remote, field), jsonDump(ctx, change.New, field))) - } + cmdio.LogString(ctx, fmt.Sprintf("Plan: %s %s", entry.Action, resourceKey)) + if len(entry.Changes) > 0 { + cmdio.LogString(ctx, "\nChanges detected:") + for _, field := range slices.Sorted(maps.Keys(entry.Changes)) { + change := entry.Changes[field] + if change.Action != deployplan.Skip { + cmdio.LogString(ctx, fmt.Sprintf(" ~ %s: %v -> %v", field, jsonDump(ctx, change.Remote, field), jsonDump(ctx, change.New, field))) } - cmdio.LogString(ctx, "") } + cmdio.LogString(ctx, "") } if !cmdio.IsPromptSupported(ctx) { - result.Cancel() logdiag.LogError(ctx, fmt.Errorf("this bind operation requires user confirmation, but the current console does not support prompting.\nTo proceed, use --auto-approve after reviewing the plan above.%s", agent.AgentNotice())) return false } ans, err := cmdio.AskYesOrNo(ctx, "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'.") if err != nil { - result.Cancel() logdiag.LogError(ctx, err) return false } if !ans { - result.Cancel() logdiag.LogError(ctx, errors.New("import aborted")) return false } diff --git a/bundle/phases/bind_dms.go b/bundle/phases/bind_dms.go index 3fb7e25da11..0a93955b2d0 100644 --- a/bundle/phases/bind_dms.go +++ b/bundle/phases/bind_dms.go @@ -2,24 +2,25 @@ package phases import ( "context" - "fmt" - "os" "slices" "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/deploy/metadata" + "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/bundle/direct" "github.com/databricks/cli/bundle/direct/dstate" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/dms" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/logdiag" - "github.com/databricks/databricks-sdk-go/service/bundledeployments" ) -// bindWithHistory records a bind for a deployment that tracks history with the metadata service. -// It reuses the engine's file-based bind against a throwaway copy of what the service holds to get -// the plan preview and resolved state, then records that state as a bind operation so the next -// deploy sees the resource as managed. +// bindWithHistory adopts an existing workspace resource for a deployment that tracks history with +// the metadata service. Unlike the file-based bind, which stages the change for the next deploy, +// DMS is the source of truth, so the bind is planned and applied here and now: the resource is +// recorded as a Bind (config already matches) or BindAndUpdate (config differs) operation in its +// own version. func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourceID string, autoApprove bool) { wsc := b.WorkspaceClient(ctx) @@ -29,71 +30,86 @@ func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourc return } - seedPath, cleanup, err := seedStateFromService(ctx, b, deploymentID, lastVersionID) - if err != nil { + ctx = withWorkspaceClient(ctx, b) + db := &b.DeploymentBundle.StateDB + if err := openRecordedState(ctx, db, localStatePath(ctx, b), deploymentID, lastVersionID); err != nil { logdiag.LogError(ctx, err) return } - defer cleanup() - result, err := b.DeploymentBundle.Bind(ctx, wsc, &b.Config, seedPath, resourceKey, resourceID) - if err != nil { - logdiag.LogError(ctx, err) + if existingID := db.GetResourceID(resourceKey); existingID != "" { + finalizeState(ctx, db) + logdiag.LogError(ctx, direct.ErrResourceAlreadyBound{ResourceKey: resourceKey, ExistingID: existingID, NewID: resourceID}) return } - if !confirmBindPlan(ctx, resourceKey, result, autoApprove) { + + // Stamp the deployment metadata into config so the adopted resource records the same state a + // deploy would, and a later plan sees no drift. Mirrors the DMS setup in + // cmd/bundle/utils.ProcessBundleRet; deployment_id is unknown until a first bind creates it, so + // it is stamped into the plan afterwards (StampDeploymentIdForFirstVersion). + firstBind := deploymentID == "" + muts := []bundle.Mutator{metadata.AnnotateDeploymentVersion(lastVersionID + 1)} + if !firstBind { + muts = append(muts, metadata.AnnotateDeployment(deploymentID)) + } + bundle.ApplySeqContext(ctx, b, muts...) + if logdiag.HasError(ctx) { return } - // The seed's temp state is discarded; the bind is recorded with the service instead. - defer result.Cancel() - // Read the resolved state from the throwaway state the engine wrote, not the state cache, which - // the plan step overwrites (dropping the etag for dashboards/genie_spaces). - entry, ok, err := resolvedEntry(ctx, result.TempStatePath, resourceKey) + // Plan the resource as an adoption of the existing id, then keep only that resource so the bind + // leaves the rest of the deployment untouched. + b.DeploymentBundle.BindKey = resourceKey + b.DeploymentBundle.BindID = resourceID + plan, err := b.DeploymentBundle.CalculatePlan(ctx, wsc, &b.Config) if err != nil { logdiag.LogError(ctx, err) return } - if !ok { - logdiag.LogError(ctx, fmt.Errorf("internal error: no resolved state for %q after bind", resourceKey)) + scopeToResource(plan, resourceKey) + + if !confirmBindPlan(ctx, resourceKey, plan, autoApprove) { + finalizeState(ctx, db) return } - recordBind(ctx, b, deploymentID, deployment, lastVersionID, resourceKey, entry) -} - -// recordBind creates the deployment on a first bind, then records a single bind operation carrying -// the resolved state, so the deployment lists the resource as managed. -func recordBind(ctx context.Context, b *bundle.Bundle, deploymentID string, deployment *bundledeployments.Deployment, lastVersionID int, resourceKey string, entry dstate.ResourceEntry) { - ctx = withWorkspaceClient(ctx, b) - db := &b.DeploymentBundle.StateDB - if err := openRecordedState(ctx, db, localStatePath(ctx, b), deploymentID, lastVersionID); err != nil { + // Commit now: claim a version, apply the adoption, and complete it. + if err := db.UpgradeToWrite(); err != nil { logdiag.LogError(ctx, err) return } - - // Creates the deployment on a first bind, or refreshes stale metadata. createOrUpdateDeployment(ctx, b, deployment) if logdiag.HasError(ctx) { return } - - if err := db.UpgradeToWrite(); err != nil { + if firstBind { + if err := b.DeploymentBundle.StampDeploymentIdForFirstVersion(db.DeploymentID); err != nil { + logdiag.LogError(ctx, err) + return + } + } + staged, err := stagedOperations(plan) + if err != nil { logdiag.LogError(ctx, err) return } - - staged := []dms.StagedOperation{{ResourceKey: resourceKey, ActionType: dms.ActionBind}} if err := startVersion(ctx, b, dms.VersionTypeDeploy, staged); err != nil { logdiag.LogError(ctx, err) return } - // The version exists now, so close it out on every path; otherwise a failure recording the - // operation leaks its lease, as deploy and destroy also guard against. + // The version exists now, so complete it on every path (see completeRecordedVersion). defer completeRecordedVersion(ctx, b) - if err := db.SaveState(ctx, resourceKey, entry.ID, entry.State, entry.DependsOn); err != nil { - logdiag.LogError(ctx, err) + b.DeploymentBundle.Apply(ctx, wsc, plan) +} + +// scopeToResource sets every resource other than resourceKey to Skip, so an apply touches only that +// resource. The others stay in the plan so references from the adopted resource still resolve. +func scopeToResource(plan *deployplan.Plan, resourceKey string) { + for key, entry := range plan.Plan { + if key != resourceKey { + entry.Action = deployplan.Skip + } } } @@ -123,9 +139,7 @@ func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string keys := recordedKeys(db, resourceKey) if len(keys) == 0 { // The resource is not recorded, so unbind is a no-op, matching a file-based deployment. - if _, err := db.Finalize(ctx); err != nil { - logdiag.LogError(ctx, err) - } + finalizeState(ctx, db) return } @@ -142,7 +156,7 @@ func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string logdiag.LogError(ctx, err) return } - // The version exists now, so close it out on every path (see recordBind). + // The version exists now, so complete it on every path (see recordBind's completeRecordedVersion). defer completeRecordedVersion(ctx, b) for _, k := range keys { @@ -166,53 +180,6 @@ func completeRecordedVersion(ctx context.Context, b *bundle.Bundle) { } } -// seedStateFromService writes a throwaway local state holding what the service currently records, -// so the engine's file-based bind can compute its plan and resolved state against it. The returned -// path does not exist yet when no deployment has been recorded, which the engine reads as empty -// state. cleanup removes the seed and any temporary files the engine leaves beside it. -func seedStateFromService(ctx context.Context, b *bundle.Bundle, deploymentID string, lastVersionID int) (string, func(), error) { - seedPath := localStatePath(ctx, b) + ".bind-seed" - cleanup := func() { - for _, p := range []string{seedPath, seedPath + ".temp-bind", seedPath + ".wal"} { - _ = os.Remove(p) - } - } - - if deploymentID == "" { - return seedPath, cleanup, nil - } - - ctx = withWorkspaceClient(ctx, b) - var src dstate.DeploymentState - if err := openRecordedState(ctx, &src, localStatePath(ctx, b), deploymentID, lastVersionID); err != nil { - cleanup() - return "", nil, err - } - err := src.SnapshotToPlainState(seedPath) - if _, ferr := src.Finalize(ctx); ferr != nil { - log.Warnf(ctx, "failed to finalize state: %v", ferr) - } - if err != nil { - cleanup() - return "", nil, err - } - return seedPath, cleanup, nil -} - -// resolvedEntry reads back the state the engine's bind resolved for resourceKey from the throwaway -// state file it wrote, which is what a file-based bind would have persisted. -func resolvedEntry(ctx context.Context, tempStatePath, resourceKey string) (dstate.ResourceEntry, bool, error) { - var src dstate.DeploymentState - if err := src.Open(ctx, tempStatePath, dstate.WithRecovery(true), dstate.WithWrite(false), dstate.WithDeploymentHistory(false), dstate.OpenDmsArgs{}); err != nil { - return dstate.ResourceEntry{}, false, err - } - entry, ok := src.GetResourceEntry(resourceKey) - if _, err := src.Finalize(ctx); err != nil { - log.Warnf(ctx, "failed to finalize state: %v", err) - } - return entry, ok, nil -} - // recordedKeys returns resourceKey and its sub-resource keys (permissions, grants, ...) that the // service holds, sorted so the staged operations and their requests are deterministic. func recordedKeys(db *dstate.DeploymentState, resourceKey string) []string { @@ -232,6 +199,14 @@ func openRecordedState(ctx context.Context, db *dstate.DeploymentState, path, de return db.Open(ctx, path, dstate.WithRecovery(false), dstate.WithWrite(false), dstate.WithDeploymentHistory(true), dstate.OpenDmsArgs{DeploymentID: deploymentID, LastVersionID: lastVersionID}) } +// finalizeState drains and closes the state without recording a version, for the paths that open it +// but do not commit (a no-op or a declined bind). +func finalizeState(ctx context.Context, db *dstate.DeploymentState) { + if _, err := db.Finalize(ctx); err != nil { + logdiag.LogError(ctx, err) + } +} + func localStatePath(ctx context.Context, b *bundle.Bundle) string { _, localPath := b.StateFilenameDirect(ctx) return localPath diff --git a/bundle/phases/dms.go b/bundle/phases/dms.go index 9ceea29509d..b448c945ff4 100644 --- a/bundle/phases/dms.go +++ b/bundle/phases/dms.go @@ -48,6 +48,10 @@ func actionToSDK(a deployplan.ActionType) (bundledeployments.OperationActionType switch a { case deployplan.Create: return bundledeployments.OperationActionTypeOperationActionTypeCreate, nil + case deployplan.Bind: + return bundledeployments.OperationActionTypeOperationActionTypeBind, nil + case deployplan.BindAndUpdate: + return bundledeployments.OperationActionTypeOperationActionTypeBindAndUpdate, nil case deployplan.Update: return bundledeployments.OperationActionTypeOperationActionTypeUpdate, nil case deployplan.UpdateWithID: diff --git a/libs/dms/client.go b/libs/dms/client.go index c56f0826f10..1d6baf85149 100644 --- a/libs/dms/client.go +++ b/libs/dms/client.go @@ -187,13 +187,13 @@ type CreateVersionRequest struct { Operations []StagedOperation `json:"operations,omitempty"` } -// ActionBind and ActionUnbind are the operation action types recorded for `bundle deployment -// bind` and `unbind`. They have no deployplan action, so they are staged directly. Bind reuses -// the SDK's constant; the SDK has none for unbind yet, so it is the string the service expects. -const ( - ActionBind = bundledeployments.OperationActionTypeOperationActionTypeBind - ActionUnbind = bundledeployments.OperationActionType("OPERATION_ACTION_TYPE_UNBIND") -) +// ActionUnbind is the operation action type recorded for `bundle deployment unbind`. Bind is a +// plan action (Bind/BindAndUpdate) mapped in phases.actionToSDK; unbind has no plan action, so it +// is staged directly. +// +// TODO(DMS): the SDK/proto has no unbind action type yet (only BIND and BIND_AND_UPDATE), so this +// is the string the service is expected to accept. Replace with the SDK constant once it exists. +const ActionUnbind = bundledeployments.OperationActionType("OPERATION_ACTION_TYPE_UNBIND") // StagedOperation is one resource the version will record an operation for. The service // creates it in OPERATION_STATUS_PENDING at sequence id 0, and the CLI fills in the outcome diff --git a/libs/dms/client_test.go b/libs/dms/client_test.go index 2904cd51020..dbb4a1580f4 100644 --- a/libs/dms/client_test.go +++ b/libs/dms/client_test.go @@ -16,10 +16,8 @@ func TestClientNamesEveryResourceTheSameWay(t *testing.T) { assert.Equal(t, "deployments/dep-1/versions/2", versionName("dep-1", 2)) } -func TestBindUnbindActionTypes(t *testing.T) { - // The service records bind and unbind as their own action types; the SDK has a constant for - // bind but not yet for unbind, which is the literal string the service expects. - assert.Equal(t, "OPERATION_ACTION_TYPE_BIND", string(ActionBind)) +func TestUnbindActionType(t *testing.T) { + // The SDK has no unbind action type yet, so this pins the string the service expects. assert.Equal(t, "OPERATION_ACTION_TYPE_UNBIND", string(ActionUnbind)) } From b08b320148cecd400fa615bafb3c25adcdb966cb Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 10 Sep 2026 09:42:05 +0000 Subject: [PATCH 4/5] Remove now-unused SnapshotToPlainState The plan-based bind no longer builds a seed state, so drop the helper it needed. Co-authored-by: Isaac --- bundle/direct/dstate/state.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go index 6694f951b4f..f4fb00c6b9e 100644 --- a/bundle/direct/dstate/state.go +++ b/bundle/direct/dstate/state.go @@ -1020,29 +1020,6 @@ func (db *DeploymentState) unlockedSave() error { return nil } -// SnapshotToPlainState writes what this deployment-history state holds to path as an ordinary -// state file (no history marker), so a file-based flow can run against a copy of what the service -// records. The state must be open for read. -func (db *DeploymentState) SnapshotToPlainState(path string) error { - db.AssertOpenedForRead() - db.mu.Lock() - defer db.mu.Unlock() - - snapshot := Database{ - Header: Header{ - StateVersion: currentStateVersion, - CLIVersion: build.GetInfo().Version, - Lineage: db.Data.Lineage, - }, - State: db.Data.State, - } - data, err := json.MarshalIndent(snapshot, "", " ") - if err != nil { - return err - } - return os.WriteFile(path, data, 0o600) -} - // Data to persist in the remote resources.json file. func (db *DeploymentState) dataForFile() Database { if db.isDeploymentMetadataService() { From 633c9b58c0723259bc6b1fbfe7cbe907b41de559 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 10 Sep 2026 11:20:59 +0000 Subject: [PATCH 5/5] Rework DMS bind review feedback; descope DMS unbind Descope unbind under the deployment metadata service: no unbind operation action type exists yet, so it reverts to erroring rather than staging an unverified enum. Fix reference resolution so binding a resource that has permissions resolves the adopted id from the plan entry instead of state. Add end-to-end coverage for a plain bind (schema, unchanged) and a bind-and-update (job, reconciled) plus FilterToSelected multi-resource exclusion, and share the deployment create-and-stamp helper between the deploy and bind phases. Co-authored-by: Isaac --- acceptance/bundle/deployment/bind/test.toml | 2 +- acceptance/bundle/deployment/unbind/test.toml | 4 +- .../dms/bind-unbind/databricks.yml.tmpl | 9 -- acceptance/bundle/dms/bind-unbind/output.txt | 78 ----------- acceptance/bundle/dms/bind-unbind/script | 27 ---- .../bundle/dms/bind/databricks.yml.tmpl | 18 +++ .../dms/{bind-unbind => bind}/out.test.toml | 0 acceptance/bundle/dms/bind/output.txt | 110 ++++++++++++++++ acceptance/bundle/dms/bind/script | 29 +++++ bundle/deployplan/plan.go | 2 + bundle/direct/bind.go | 23 +--- bundle/direct/bundle_plan.go | 20 ++- bundle/phases/bind.go | 34 +++-- bundle/phases/bind_dms.go | 121 +++--------------- bundle/phases/deploy.go | 9 +- bundle/phases/dms.go | 18 +++ bundle/phases/dms_test.go | 14 +- libs/dms/client.go | 8 -- libs/dms/client_test.go | 5 - 19 files changed, 243 insertions(+), 288 deletions(-) delete mode 100644 acceptance/bundle/dms/bind-unbind/databricks.yml.tmpl delete mode 100644 acceptance/bundle/dms/bind-unbind/output.txt delete mode 100644 acceptance/bundle/dms/bind-unbind/script create mode 100644 acceptance/bundle/dms/bind/databricks.yml.tmpl rename acceptance/bundle/dms/{bind-unbind => bind}/out.test.toml (100%) create mode 100644 acceptance/bundle/dms/bind/output.txt create mode 100644 acceptance/bundle/dms/bind/script diff --git a/acceptance/bundle/deployment/bind/test.toml b/acceptance/bundle/deployment/bind/test.toml index b2554375c4e..117379e3907 100644 --- a/acceptance/bundle/deployment/bind/test.toml +++ b/acceptance/bundle/deployment/bind/test.toml @@ -1,4 +1,4 @@ # These tests cover the file-based bind, which stages a change for the next deploy. Under the # deployment metadata service (DMS) bind is a distinct operation - it plans and applies the change -# immediately - so it cannot share these goldens and is covered by bundle/dms/bind-unbind instead. +# immediately - so it cannot share these goldens and is covered by bundle/dms/bind instead. EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deployment/unbind/test.toml b/acceptance/bundle/deployment/unbind/test.toml index af0bf04cc94..9f5f5d49a05 100644 --- a/acceptance/bundle/deployment/unbind/test.toml +++ b/acceptance/bundle/deployment/unbind/test.toml @@ -1,4 +1,2 @@ -# These tests cover the file-based unbind. Under the deployment metadata service (DMS) unbind -# records its change with the service immediately rather than in the state file, so it cannot share -# these goldens and is covered by bundle/dms/bind-unbind instead. +# Unbind operations are not yet supported by the Deployment Metadata Service (DMS) EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/dms/bind-unbind/databricks.yml.tmpl b/acceptance/bundle/dms/bind-unbind/databricks.yml.tmpl deleted file mode 100644 index e60f4605610..00000000000 --- a/acceptance/bundle/dms/bind-unbind/databricks.yml.tmpl +++ /dev/null @@ -1,9 +0,0 @@ -bundle: - name: dms-bind-unbind-$UNIQUE_NAME -experimental: - deployment_history: true - -resources: - jobs: - foo: - name: foo diff --git a/acceptance/bundle/dms/bind-unbind/output.txt b/acceptance/bundle/dms/bind-unbind/output.txt deleted file mode 100644 index 22cc35c925e..00000000000 --- a/acceptance/bundle/dms/bind-unbind/output.txt +++ /dev/null @@ -1,78 +0,0 @@ - -=== Bind an existing job to the bundle, recorded with the deployment history service ->>> [CLI] bundle deployment bind foo [JOB_ID] --auto-approve -Successfully bound job with an id '[JOB_ID]' -Run 'bundle deploy' to deploy changes to your workspace - -=== The bind applied the config and recorded the resource, so a re-plan is a no-op ->>> [CLI] bundle plan -Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged - -=== Unbind it again, dropping it from what the service records ->>> [CLI] bundle deployment unbind foo - -=== No longer managed, so it plans as a fresh create ->>> [CLI] bundle plan -create jobs.foo - -Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged - -=== Each recorded a version whose operation carries the bind / unbind action type ->>> print_requests.py --dms --method POST //versions --nostamp -{ - "method": "POST", - "path": "/api/2.0/bundle/deployments/[NUMID]/versions", - "q": { - "version_id": "1" - }, - "body": { - "cli_version": "[CLI_VERSION]", - "version_type": "VERSION_TYPE_DEPLOY", - "operations": [ - { - "resource_key": "jobs.foo", - "action_type": "OPERATION_ACTION_TYPE_BIND_AND_UPDATE" - } - ] - } -} -{ - "method": "POST", - "path": "/api/2.0/bundle/deployments/[NUMID]/versions/1/complete", - "body": { - "completion_reason": "VERSION_COMPLETE_SUCCESS" - } -} -{ - "method": "POST", - "path": "/api/2.0/bundle/deployments/[NUMID]/versions", - "q": { - "version_id": "2" - }, - "body": { - "cli_version": "[CLI_VERSION]", - "version_type": "VERSION_TYPE_DEPLOY", - "previous_version_id": "1", - "operations": [ - { - "resource_key": "jobs.foo", - "action_type": "OPERATION_ACTION_TYPE_UNBIND" - } - ] - } -} -{ - "method": "POST", - "path": "/api/2.0/bundle/deployments/[NUMID]/versions/2/complete", - "body": { - "completion_reason": "VERSION_COMPLETE_SUCCESS" - } -} - -=== Test cleanup ->>> [CLI] bundle destroy --auto-approve -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/dms-bind-unbind-[UNIQUE_NAME]/default - -Destroy: 0 deleted - ->>> [CLI] jobs delete [JOB_ID] diff --git a/acceptance/bundle/dms/bind-unbind/script b/acceptance/bundle/dms/bind-unbind/script deleted file mode 100644 index f59d273f403..00000000000 --- a/acceptance/bundle/dms/bind-unbind/script +++ /dev/null @@ -1,27 +0,0 @@ -envsubst < databricks.yml.tmpl > databricks.yml - -job_id=$($CLI jobs create --json '{"name": "bind-target-'$UNIQUE_NAME'"}' | jq -r '.job_id') -add_repl "$job_id" JOB_ID - -cleanup() { - title "Test cleanup" - trace $CLI bundle destroy --auto-approve - trace $CLI jobs delete "$job_id" - rm -f out.requests.txt -} -trap cleanup EXIT - -title "Bind an existing job to the bundle, recorded with the deployment history service" -trace $CLI bundle deployment bind foo "$job_id" --auto-approve - -title "The bind applied the config and recorded the resource, so a re-plan is a no-op" -trace $CLI bundle plan - -title "Unbind it again, dropping it from what the service records" -trace $CLI bundle deployment unbind foo - -title "No longer managed, so it plans as a fresh create" -trace $CLI bundle plan - -title "Each recorded a version whose operation carries the bind / unbind action type" -trace print_requests.py --dms --method POST //versions --nostamp diff --git a/acceptance/bundle/dms/bind/databricks.yml.tmpl b/acceptance/bundle/dms/bind/databricks.yml.tmpl new file mode 100644 index 00000000000..e5b40a1ed15 --- /dev/null +++ b/acceptance/bundle/dms/bind/databricks.yml.tmpl @@ -0,0 +1,18 @@ +bundle: + name: dms-bind-$UNIQUE_NAME +experimental: + deployment_history: true + +resources: + schemas: + sc: + name: dms_bind_$UNIQUE_NAME + catalog_name: main + jobs: + foo: + name: foo + permissions: + - group_name: users + level: CAN_MANAGE + bar: + name: bar diff --git a/acceptance/bundle/dms/bind-unbind/out.test.toml b/acceptance/bundle/dms/bind/out.test.toml similarity index 100% rename from acceptance/bundle/dms/bind-unbind/out.test.toml rename to acceptance/bundle/dms/bind/out.test.toml diff --git a/acceptance/bundle/dms/bind/output.txt b/acceptance/bundle/dms/bind/output.txt new file mode 100644 index 00000000000..1da486db75a --- /dev/null +++ b/acceptance/bundle/dms/bind/output.txt @@ -0,0 +1,110 @@ + +>>> [CLI] schemas create dms_bind_[UNIQUE_NAME] main +{ + "browse_only": false, + "catalog_name": "main", + "catalog_type": "MANAGED_CATALOG", + "created_at": [UNIX_TIME_MILLIS][0], + "created_by": "[USERNAME]", + "effective_predictive_optimization_flag": { + "inherited_from_name": "[METASTORE_NAME]", + "inherited_from_type": "METASTORE", + "value": "ENABLE" + }, + "enable_predictive_optimization": "INHERIT", + "full_name": "main.dms_bind_[UNIQUE_NAME]", + "metastore_id": "[UUID]", + "name": "dms_bind_[UNIQUE_NAME]", + "owner": "[USERNAME]", + "schema_id": "[UUID]", + "updated_at": [UNIX_TIME_MILLIS][0], + "updated_by": "[USERNAME]" +} + +=== Bind sc, the first bind: it creates the deployment. The config already matches, so this is a plain bind that records the resource without touching it +>>> [CLI] bundle deployment bind sc main.dms_bind_[UNIQUE_NAME] --auto-approve +Successfully bound schema with an id 'main.dms_bind_[UNIQUE_NAME]' +Run 'bundle deploy' to deploy changes to your workspace + +=== Version 1 records only sc, with the plain bind action type; the jobs are left untouched +>>> print_requests.py --dms --method POST //versions --nostamp +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions", + "q": { + "version_id": "1" + }, + "body": { + "cli_version": "[CLI_VERSION]", + "version_type": "VERSION_TYPE_DEPLOY", + "operations": [ + { + "resource_key": "schemas.sc", + "action_type": "OPERATION_ACTION_TYPE_BIND" + } + ] + } +} +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions/1/complete", + "body": { + "completion_reason": "VERSION_COMPLETE_SUCCESS" + } +} + +=== Bind foo into the existing deployment: its config differs, so this is a bind-and-update that applies the config now +>>> [CLI] bundle deployment bind foo [FOO_ID] --auto-approve +Successfully bound job with an id '[FOO_ID]' +Run 'bundle deploy' to deploy changes to your workspace + +=== Version 2 records only foo and its permissions (bar is untouched) and chains to version 1 +>>> print_requests.py --dms --method POST //versions --nostamp +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions", + "q": { + "version_id": "2" + }, + "body": { + "cli_version": "[CLI_VERSION]", + "version_type": "VERSION_TYPE_DEPLOY", + "previous_version_id": "1", + "operations": [ + { + "resource_key": "jobs.foo", + "action_type": "OPERATION_ACTION_TYPE_BIND_AND_UPDATE" + }, + { + "resource_key": "jobs.foo.permissions", + "action_type": "OPERATION_ACTION_TYPE_CREATE" + } + ] + } +} +{ + "method": "POST", + "path": "/api/2.0/bundle/deployments/[NUMID]/versions/2/complete", + "body": { + "completion_reason": "VERSION_COMPLETE_SUCCESS" + } +} + +=== sc and foo are now managed; bar is still an unmanaged create +>>> [CLI] bundle plan +create jobs.bar + +Plan: 1 to add, 0 to change, 0 to delete, 3 unchanged + +=== Test cleanup: the bound resources are now bundle-managed, so destroy removes them +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.jobs.foo + delete resources.schemas.sc + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.sc + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/dms-bind-[UNIQUE_NAME]/default + +Destroy: 2 deleted diff --git a/acceptance/bundle/dms/bind/script b/acceptance/bundle/dms/bind/script new file mode 100644 index 00000000000..6858036c7d1 --- /dev/null +++ b/acceptance/bundle/dms/bind/script @@ -0,0 +1,29 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +# sc's config matches this existing schema exactly, so binding it changes nothing (a plain bind). +trace $CLI schemas create "dms_bind_$UNIQUE_NAME" main +# foo's config ("foo") differs from this existing job, so binding it applies an update. +foo_id=$($CLI jobs create --json '{"name": "bind-target-'$UNIQUE_NAME'"}' | jq -r '.job_id') +add_repl "$foo_id" FOO_ID + +cleanup() { + title "Test cleanup: the bound resources are now bundle-managed, so destroy removes them" + trace $CLI bundle destroy --auto-approve + rm -f out.requests.txt +} +trap cleanup EXIT + +title "Bind sc, the first bind: it creates the deployment. The config already matches, so this is a plain bind that records the resource without touching it" +trace $CLI bundle deployment bind sc "main.dms_bind_$UNIQUE_NAME" --auto-approve + +title "Version 1 records only sc, with the plain bind action type; the jobs are left untouched" +trace print_requests.py --dms --method POST //versions --nostamp + +title "Bind foo into the existing deployment: its config differs, so this is a bind-and-update that applies the config now" +trace $CLI bundle deployment bind foo "$foo_id" --auto-approve + +title "Version 2 records only foo and its permissions (bar is untouched) and chains to version 1" +trace print_requests.py --dms --method POST //versions --nostamp + +title "sc and foo are now managed; bar is still an unmanaged create" +trace $CLI bundle plan diff --git a/bundle/deployplan/plan.go b/bundle/deployplan/plan.go index 267e96070b1..23d5e7b75b8 100644 --- a/bundle/deployplan/plan.go +++ b/bundle/deployplan/plan.go @@ -117,6 +117,8 @@ func LoadPlanFromFile(path string) (*Plan, error) { } type PlanEntry struct { + // ID carries the workspace id for a Bind/BindAndUpdate action, whose resource is not yet in + // state; every other action reads the id from state instead, so it is empty for them. ID string `json:"id,omitempty"` DependsOn []DependsOnEntry `json:"depends_on,omitempty"` Action ActionType `json:"action,omitempty"` diff --git a/bundle/direct/bind.go b/bundle/direct/bind.go index 92f9dcc10ff..d2e86005ce7 100644 --- a/bundle/direct/bind.go +++ b/bundle/direct/bind.go @@ -35,13 +35,9 @@ func (e ErrResourceAlreadyBound) Error() string { e.ResourceKey, e.ExistingID) } -// BindResult contains the result of a bind operation including any detected changes. +// BindResult contains the result of a bind operation. type BindResult struct { - // HasChanges is true if deploying after bind would make changes to the resource - HasChanges bool - // Action is the planned action for the bound resource (e.g., "skip", "update", "recreate") - Action deployplan.ActionType - // Plan contains the full deployment plan for the bound resource + // Plan contains the full deployment plan; confirmBindPlan reads the bound resource's action. Plan *deployplan.Plan // TempStatePath is the path to the temporary state file TempStatePath string @@ -194,22 +190,11 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac return nil, err } - // Check if the bound resource has changes - result := &BindResult{ - HasChanges: false, - Action: deployplan.Skip, + return &BindResult{ Plan: plan, TempStatePath: tmpStatePath, StatePath: statePath, - } - - entry = plan.Plan[resourceKey] - if entry != nil { - result.Action = entry.Action - result.HasChanges = result.Action != deployplan.Skip && result.Action != deployplan.Undefined - } - - return result, nil + }, nil } // Finalize completes the bind operation by renaming the temp state to the final location. diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 3e9a9efddcf..56228358d80 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -360,12 +360,19 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks // because we know remote does not exist. action = deployplan.Create case adopt: - // Adopting an existing resource: no change needed is a plain bind, a change is a - // bind-and-update that applies the config in the same step. - if getMaxAction(entry.Changes) == deployplan.Skip { + // Adopting an existing resource: no change is a plain bind, an in-place update is a + // bind-and-update that applies the config in the same step. A heavier change + // (recreate/resize) cannot be applied by adopting, so reject it rather than silently + // downgrading to an update and skipping the destructive-change confirmation a deploy + // would show. + switch maxAction := getMaxAction(entry.Changes); maxAction { + case deployplan.Skip: action = deployplan.Bind - } else { + case deployplan.Update: action = deployplan.BindAndUpdate + default: + logdiag.LogError(ctx, fmt.Errorf("%s: cannot bind id=%q: the config differs from the resource in a field that requires %s, which bind does not apply; align the config with the existing resource first", errorPrefix, resourceID, maxAction)) + return false } // The id is not in state yet, so carry it to apply on the plan entry. entry.ID = resourceID @@ -837,6 +844,11 @@ func (b *DeploymentBundle) LookupReferencePreDeploy(ctx context.Context, path *s if fieldPathS == "id" { if targetAction.KeepsID() { id := b.StateDB.GetResourceID(targetResourceKey) + if id == "" { + // A resource being adopted (bundle deployment bind) is not in state yet; its id is + // carried on the plan entry instead, so a sub-resource can resolve it here. + id = targetEntry.ID + } if id == "" { return nil, errors.New("internal error: no db entry") } diff --git a/bundle/phases/bind.go b/bundle/phases/bind.go index a0e5d1c07be..21d817a4507 100644 --- a/bundle/phases/bind.go +++ b/bundle/phases/bind.go @@ -57,7 +57,7 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions, en return } - if !confirmBindPlan(ctx, resourceKey, result.Plan, opts.AutoApprove) { + if !confirmBindPlan(ctx, resourceKey, result.Plan, opts.AutoApprove, false) { result.Cancel() return } @@ -96,8 +96,10 @@ func jsonDump(ctx context.Context, v any, field string) string { // confirmBindPlan shows the bound resource's planned action and, unless autoApprove, asks the user // to confirm. It reports whether the bind should proceed; on decline or an unpromptable console it // logs the reason and returns false. A plain bind or skip changes nothing, so it proceeds without a -// prompt. The caller owns any cleanup on a false return. -func confirmBindPlan(ctx context.Context, resourceKey string, plan *deployplan.Plan, autoApprove bool) bool { +// prompt. immediate is true when the caller applies the change now (the DMS path) rather than +// deferring it to the next deploy, so the prompt describes the right timing. The caller owns any +// cleanup on a false return. +func confirmBindPlan(ctx context.Context, resourceKey string, plan *deployplan.Plan, autoApprove, immediate bool) bool { var entry *deployplan.PlanEntry if plan != nil { entry = plan.Plan[resourceKey] @@ -124,7 +126,11 @@ func confirmBindPlan(ctx context.Context, resourceKey string, plan *deployplan.P return false } - ans, err := cmdio.AskYesOrNo(ctx, "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'.") + prompt := "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'." + if immediate { + prompt = "Confirm bind? The change will be applied to the workspace now." + } + ans, err := cmdio.AskYesOrNo(ctx, prompt) if err != nil { logdiag.LogError(ctx, err) return false @@ -155,20 +161,12 @@ func Unbind(ctx context.Context, b *bundle.Bundle, bundleType, tfResourceType, r groupName = tfResourceType } fullResourceKey := fmt.Sprintf("resources.%s.%s", groupName, resourceKey) - - if b.ConfiguresDeploymentHistory(ctx) { - // A recorded deployment keeps its resources in the metadata service, so the unbind is - // recorded there rather than removed from the state file. - unbindWithHistory(ctx, b, fullResourceKey) - if logdiag.HasError(ctx) { - return - } - } else { - _, statePath := b.StateFilenameDirect(ctx) - if err := b.DeploymentBundle.Unbind(ctx, statePath, fullResourceKey); err != nil { - logdiag.LogError(ctx, err) - return - } + // Unbind under the deployment metadata service is not supported yet (no unbind operation + // action type exists); DeploymentBundle.Unbind errors for a recorded deployment. + _, statePath := b.StateFilenameDirect(ctx) + if err := b.DeploymentBundle.Unbind(ctx, statePath, fullResourceKey); err != nil { + logdiag.LogError(ctx, err) + return } } else { bundle.ApplySeqContext( diff --git a/bundle/phases/bind_dms.go b/bundle/phases/bind_dms.go index 0a93955b2d0..40fd05057ab 100644 --- a/bundle/phases/bind_dms.go +++ b/bundle/phases/bind_dms.go @@ -2,25 +2,21 @@ package phases import ( "context" - "slices" "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/metadata" - "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/direct" "github.com/databricks/cli/bundle/direct/dstate" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/dms" - "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/logdiag" ) -// bindWithHistory adopts an existing workspace resource for a deployment that tracks history with -// the metadata service. Unlike the file-based bind, which stages the change for the next deploy, -// DMS is the source of truth, so the bind is planned and applied here and now: the resource is -// recorded as a Bind (config already matches) or BindAndUpdate (config differs) operation in its -// own version. +// bindWithHistory adopts an existing workspace resource for a deployment that records history with +// the metadata service. The service is the source of truth, so the bind is planned and applied now +// (as a Bind or BindAndUpdate operation in its own version), unlike the file-based bind which +// defers the change to the next deploy. func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourceID string, autoApprove bool) { wsc := b.WorkspaceClient(ctx) @@ -43,10 +39,8 @@ func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourc return } - // Stamp the deployment metadata into config so the adopted resource records the same state a - // deploy would, and a later plan sees no drift. Mirrors the DMS setup in - // cmd/bundle/utils.ProcessBundleRet; deployment_id is unknown until a first bind creates it, so - // it is stamped into the plan afterwards (StampDeploymentIdForFirstVersion). + // Stamp deployment metadata into config (as a deploy does) so the recorded state matches and a + // later plan sees no drift. A first bind's deployment_id is stamped after it is created below. firstBind := deploymentID == "" muts := []bundle.Mutator{metadata.AnnotateDeploymentVersion(lastVersionID + 1)} if !firstBind { @@ -54,21 +48,23 @@ func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourc } bundle.ApplySeqContext(ctx, b, muts...) if logdiag.HasError(ctx) { + finalizeState(ctx, db) return } - // Plan the resource as an adoption of the existing id, then keep only that resource so the bind - // leaves the rest of the deployment untouched. + // Plan the resource as an adoption of the existing id, then narrow the plan to it (and its own + // grants/permissions) so the bind leaves the rest of the deployment untouched. b.DeploymentBundle.BindKey = resourceKey b.DeploymentBundle.BindID = resourceID plan, err := b.DeploymentBundle.CalculatePlan(ctx, wsc, &b.Config) if err != nil { + finalizeState(ctx, db) logdiag.LogError(ctx, err) return } - scopeToResource(plan, resourceKey) + plan.FilterToSelected([]string{strings.TrimPrefix(resourceKey, "resources.")}) - if !confirmBindPlan(ctx, resourceKey, plan, autoApprove) { + if !confirmBindPlan(ctx, resourceKey, plan, autoApprove, true) { finalizeState(ctx, db) return } @@ -78,16 +74,13 @@ func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourc logdiag.LogError(ctx, err) return } - createOrUpdateDeployment(ctx, b, deployment) - if logdiag.HasError(ctx) { + // From here the state is open for write and may hold a version; drain and complete it (with + // failure on error) on every path, so nothing is left open or a version left dangling. + defer completeRecordedVersion(ctx, b) + + if !createDeploymentAndStamp(ctx, b, deployment, firstBind) { return } - if firstBind { - if err := b.DeploymentBundle.StampDeploymentIdForFirstVersion(db.DeploymentID); err != nil { - logdiag.LogError(ctx, err) - return - } - } staged, err := stagedOperations(plan) if err != nil { logdiag.LogError(ctx, err) @@ -97,77 +90,10 @@ func bindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey, resourc logdiag.LogError(ctx, err) return } - // The version exists now, so complete it on every path (see completeRecordedVersion). - defer completeRecordedVersion(ctx, b) b.DeploymentBundle.Apply(ctx, wsc, plan) } -// scopeToResource sets every resource other than resourceKey to Skip, so an apply touches only that -// resource. The others stay in the plan so references from the adopted resource still resolve. -func scopeToResource(plan *deployplan.Plan, resourceKey string) { - for key, entry := range plan.Plan { - if key != resourceKey { - entry.Action = deployplan.Skip - } - } -} - -// unbindWithHistory records an unbind: the resource and its sub-resources (permissions, grants) are -// dropped from what the service records, so the next deploy re-creates them. The workspace -// resources themselves are left untouched. -func unbindWithHistory(ctx context.Context, b *bundle.Bundle, resourceKey string) { - wsc := b.WorkspaceClient(ctx) - - deploymentID, _, lastVersionID, err := dms.FetchDeployment(ctx, wsc, b.Config.Workspace.StatePath) - if err != nil { - logdiag.LogError(ctx, err) - return - } - if deploymentID == "" { - // Nothing is recorded, so there is nothing to unbind. - return - } - - ctx = withWorkspaceClient(ctx, b) - db := &b.DeploymentBundle.StateDB - if err := openRecordedState(ctx, db, localStatePath(ctx, b), deploymentID, lastVersionID); err != nil { - logdiag.LogError(ctx, err) - return - } - - keys := recordedKeys(db, resourceKey) - if len(keys) == 0 { - // The resource is not recorded, so unbind is a no-op, matching a file-based deployment. - finalizeState(ctx, db) - return - } - - if err := db.UpgradeToWrite(); err != nil { - logdiag.LogError(ctx, err) - return - } - - staged := make([]dms.StagedOperation, 0, len(keys)) - for _, k := range keys { - staged = append(staged, dms.StagedOperation{ResourceKey: k, ActionType: dms.ActionUnbind}) - } - if err := startVersion(ctx, b, dms.VersionTypeDeploy, staged); err != nil { - logdiag.LogError(ctx, err) - return - } - // The version exists now, so complete it on every path (see recordBind's completeRecordedVersion). - defer completeRecordedVersion(ctx, b) - - for _, k := range keys { - if err := db.DeleteState(ctx, k, false); err != nil { - logdiag.LogError(ctx, err) - return - } - log.Infof(ctx, "Unbound %s", k) - } -} - // completeRecordedVersion drains the buffered operations and closes the version out, completing // with failure if anything went wrong. Deferred once a version exists so every path completes it. func completeRecordedVersion(ctx context.Context, b *bundle.Bundle) { @@ -180,19 +106,6 @@ func completeRecordedVersion(ctx context.Context, b *bundle.Bundle) { } } -// recordedKeys returns resourceKey and its sub-resource keys (permissions, grants, ...) that the -// service holds, sorted so the staged operations and their requests are deterministic. -func recordedKeys(db *dstate.DeploymentState, resourceKey string) []string { - var keys []string - for k := range db.Data.State { - if k == resourceKey || strings.HasPrefix(k, resourceKey+".") { - keys = append(keys, k) - } - } - slices.Sort(keys) - return keys -} - // openRecordedState opens the deployment's recorded state for read, reading its resources from the // metadata service. func openRecordedState(ctx context.Context, db *dstate.DeploymentState, path, deploymentID string, lastVersionID int) error { diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 425c4874283..0d8c57db3be 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -321,16 +321,9 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand // IsDirect first: the state must be open to read its features, and only the direct engine opens it. if stateEngine.IsDirect() && b.DeploymentBundle.StateDB.IsDeploymentMetadataService() { firstDeploy := b.DeploymentBundle.StateDB.DeploymentID == "" - createOrUpdateDeployment(ctx, b, dmsDeployment) - if logdiag.HasError(ctx) { + if !createDeploymentAndStamp(ctx, b, dmsDeployment, firstDeploy) { return } - if firstDeploy { - if err := b.DeploymentBundle.StampDeploymentIdForFirstVersion(b.DeploymentBundle.StateDB.DeploymentID); err != nil { - logdiag.LogError(ctx, err) - return - } - } // Only create a version when the plan has at least one operation to record, so it moves // with serial. Done after the prompt, so a declined deploy never claims a number. diff --git a/bundle/phases/dms.go b/bundle/phases/dms.go index b448c945ff4..7f162c2864c 100644 --- a/bundle/phases/dms.go +++ b/bundle/phases/dms.go @@ -97,6 +97,24 @@ func createOrUpdateDeployment(ctx context.Context, b *bundle.Bundle, current *bu }) } +// createDeploymentAndStamp runs createOrUpdateDeployment, then on a first deploy stamps the created +// id into the plan the apply reads (the id did not exist at plan time). It reports whether it +// succeeded; on failure it has already logged. firstDeploy is captured before the create, which +// assigns the id. Shared by the deploy and bind phases. +func createDeploymentAndStamp(ctx context.Context, b *bundle.Bundle, current *bundledeployments.Deployment, firstDeploy bool) bool { + createOrUpdateDeployment(ctx, b, current) + if logdiag.HasError(ctx) { + return false + } + if firstDeploy { + if err := b.DeploymentBundle.StampDeploymentIdForFirstVersion(b.DeploymentBundle.StateDB.DeploymentID); err != nil { + logdiag.LogError(ctx, err) + return false + } + } + return true +} + // startVersion claims the version the run settled on and opens the buffer that records // each state write under it. Called after approval, so a declined deploy never claims a number. // A no-op when the bundle does not record deployment history. diff --git a/bundle/phases/dms_test.go b/bundle/phases/dms_test.go index f6854999db9..0528db54dce 100644 --- a/bundle/phases/dms_test.go +++ b/bundle/phases/dms_test.go @@ -14,10 +14,12 @@ func TestStagedOperationsCoversEveryTouchedResource(t *testing.T) { // The version fixes its operation set, so anything the apply will write has to appear // here. Keys go out in the service's form, without the CLI's "resources." prefix. plan := &deployplan.Plan{Plan: map[string]*deployplan.PlanEntry{ - "resources.jobs.foo": {Action: deployplan.Create}, - "resources.pipelines.bar": {Action: deployplan.Recreate}, - "resources.schemas.baz": {Action: deployplan.Delete}, - "resources.clusters.small": {Action: deployplan.Resize}, + "resources.jobs.foo": {Action: deployplan.Create}, + "resources.pipelines.bar": {Action: deployplan.Recreate}, + "resources.schemas.baz": {Action: deployplan.Delete}, + "resources.clusters.small": {Action: deployplan.Resize}, + "resources.jobs.adopted": {Action: deployplan.Bind}, + "resources.jobs.adopted_upd": {Action: deployplan.BindAndUpdate}, }} staged, err := stagedOperations(plan) @@ -28,6 +30,8 @@ func TestStagedOperationsCoversEveryTouchedResource(t *testing.T) { {ResourceKey: "resources.pipelines.bar", ActionType: bundledeployments.OperationActionTypeOperationActionTypeRecreate}, {ResourceKey: "resources.schemas.baz", ActionType: bundledeployments.OperationActionTypeOperationActionTypeDelete}, {ResourceKey: "resources.clusters.small", ActionType: bundledeployments.OperationActionTypeOperationActionTypeResize}, + {ResourceKey: "resources.jobs.adopted", ActionType: bundledeployments.OperationActionTypeOperationActionTypeBind}, + {ResourceKey: "resources.jobs.adopted_upd", ActionType: bundledeployments.OperationActionTypeOperationActionTypeBindAndUpdate}, }, staged) } @@ -60,6 +64,8 @@ func TestActionToSDK(t *testing.T) { want bundledeployments.OperationActionType }{ {deployplan.Create, bundledeployments.OperationActionTypeOperationActionTypeCreate}, + {deployplan.Bind, bundledeployments.OperationActionTypeOperationActionTypeBind}, + {deployplan.BindAndUpdate, bundledeployments.OperationActionTypeOperationActionTypeBindAndUpdate}, {deployplan.Update, bundledeployments.OperationActionTypeOperationActionTypeUpdate}, {deployplan.UpdateWithID, bundledeployments.OperationActionTypeOperationActionTypeUpdateWithId}, {deployplan.Recreate, bundledeployments.OperationActionTypeOperationActionTypeRecreate}, diff --git a/libs/dms/client.go b/libs/dms/client.go index 1d6baf85149..57e033fb9b4 100644 --- a/libs/dms/client.go +++ b/libs/dms/client.go @@ -187,14 +187,6 @@ type CreateVersionRequest struct { Operations []StagedOperation `json:"operations,omitempty"` } -// ActionUnbind is the operation action type recorded for `bundle deployment unbind`. Bind is a -// plan action (Bind/BindAndUpdate) mapped in phases.actionToSDK; unbind has no plan action, so it -// is staged directly. -// -// TODO(DMS): the SDK/proto has no unbind action type yet (only BIND and BIND_AND_UPDATE), so this -// is the string the service is expected to accept. Replace with the SDK constant once it exists. -const ActionUnbind = bundledeployments.OperationActionType("OPERATION_ACTION_TYPE_UNBIND") - // StagedOperation is one resource the version will record an operation for. The service // creates it in OPERATION_STATUS_PENDING at sequence id 0, and the CLI fills in the outcome // with UpdateOperation as the resource is applied. diff --git a/libs/dms/client_test.go b/libs/dms/client_test.go index dbb4a1580f4..867655bca4e 100644 --- a/libs/dms/client_test.go +++ b/libs/dms/client_test.go @@ -16,11 +16,6 @@ func TestClientNamesEveryResourceTheSameWay(t *testing.T) { assert.Equal(t, "deployments/dep-1/versions/2", versionName("dep-1", 2)) } -func TestUnbindActionType(t *testing.T) { - // The SDK has no unbind action type yet, so this pins the string the service expects. - assert.Equal(t, "OPERATION_ACTION_TYPE_UNBIND", string(ActionUnbind)) -} - func TestDeploymentIDFromName(t *testing.T) { id, err := deploymentIDFromName("deployments/abc-123") require.NoError(t, err)