diff --git a/acceptance/bundle/deployment/bind/experiment/out.get.direct.json b/acceptance/bundle/deployment/bind/experiment/out.get.direct.json deleted file mode 100644 index 9ddcb0824ea..00000000000 --- a/acceptance/bundle/deployment/bind/experiment/out.get.direct.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", - "lifecycle_stage": "active" -} diff --git a/acceptance/bundle/deployment/bind/experiment/out.get.terraform.json b/acceptance/bundle/deployment/bind/experiment/out.get.terraform.json deleted file mode 100644 index 38934fd1cef..00000000000 --- a/acceptance/bundle/deployment/bind/experiment/out.get.terraform.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", - "lifecycle_stage": "active" -} diff --git a/acceptance/bundle/deployment/bind/experiment/out.plan1.direct.json b/acceptance/bundle/deployment/bind/experiment/out.plan1.direct.json new file mode 100644 index 00000000000..b40e51873e5 --- /dev/null +++ b/acceptance/bundle/deployment/bind/experiment/out.plan1.direct.json @@ -0,0 +1,14 @@ +{ + "plan": { + "resources.experiments.experiment1": { + "action": "skip", + "name_change": { + "action": "skip", + "reason": "alias", + "old": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", + "new": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", + "remote": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]" + } + } + } +} diff --git a/acceptance/bundle/deployment/bind/experiment/out.plan1.terraform.json b/acceptance/bundle/deployment/bind/experiment/out.plan1.terraform.json new file mode 100644 index 00000000000..884cb9c919a --- /dev/null +++ b/acceptance/bundle/deployment/bind/experiment/out.plan1.terraform.json @@ -0,0 +1,8 @@ +{ + "plan": { + "resources.experiments.experiment1": { + "action": "skip", + "name_change": null + } + } +} diff --git a/acceptance/bundle/deployment/bind/experiment/out.plan2.direct.json b/acceptance/bundle/deployment/bind/experiment/out.plan2.direct.json new file mode 100644 index 00000000000..b78c5847ffd --- /dev/null +++ b/acceptance/bundle/deployment/bind/experiment/out.plan2.direct.json @@ -0,0 +1,14 @@ +{ + "plan": { + "resources.experiments.experiment1": { + "action": "skip", + "name_change": { + "action": "skip", + "reason": "remote_already_set", + "old": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", + "new": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", + "remote": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]" + } + } + } +} diff --git a/acceptance/bundle/deployment/bind/experiment/out.plan2.terraform.json b/acceptance/bundle/deployment/bind/experiment/out.plan2.terraform.json new file mode 100644 index 00000000000..884cb9c919a --- /dev/null +++ b/acceptance/bundle/deployment/bind/experiment/out.plan2.terraform.json @@ -0,0 +1,8 @@ +{ + "plan": { + "resources.experiments.experiment1": { + "action": "skip", + "name_change": null + } + } +} diff --git a/acceptance/bundle/deployment/bind/experiment/out.test.toml b/acceptance/bundle/deployment/bind/experiment/out.test.toml index d560f1de043..01ed6822af8 100644 --- a/acceptance/bundle/deployment/bind/experiment/out.test.toml +++ b/acceptance/bundle/deployment/bind/experiment/out.test.toml @@ -1,5 +1,5 @@ Local = true -Cloud = false +Cloud = true [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/deployment/bind/experiment/output.txt b/acceptance/bundle/deployment/bind/experiment/output.txt index 9636e52ae38..1efb0ddc4dd 100644 --- a/acceptance/bundle/deployment/bind/experiment/output.txt +++ b/acceptance/bundle/deployment/bind/experiment/output.txt @@ -11,6 +11,10 @@ Updating deployment state... Deployment complete! >>> [CLI] experiments get-experiment [NUMID] +{ + "name": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", + "lifecycle_stage": "active" +} >>> [CLI] bundle deployment unbind experiment1 Updating deployment state... @@ -22,6 +26,10 @@ Deleting files... Destroy complete! >>> [CLI] experiments get-experiment [NUMID] +{ + "name": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", + "lifecycle_stage": "active" +} === Test cleanup: === Delete the pre-defined experiment: 0 diff --git a/acceptance/bundle/deployment/bind/experiment/script b/acceptance/bundle/deployment/bind/experiment/script index be2bea428b1..70cf1933811 100644 --- a/acceptance/bundle/deployment/bind/experiment/script +++ b/acceptance/bundle/deployment/bind/experiment/script @@ -17,12 +17,22 @@ trace $CLI bundle deployment bind experiment1 ${EXPERIMENT_ID} --auto-approve trace $CLI bundle deploy --force-lock --auto-approve -trace $CLI experiments get-experiment ${EXPERIMENT_ID} | jq '{name: .experiment.name, lifecycle_stage: .experiment.lifecycle_stage}' > out.get.$DATABRICKS_BUNDLE_ENGINE.json +# Plan should show no changes with /Workspace/Users/... path in config. +# Use --output json to verify the change details including skip reason. +# Write per-engine output since direct includes change details that terraform doesn't. +# Filter to only the name change (tags/artifact_location may differ between local and cloud). +$CLI bundle plan --output json | jq '{plan: .plan | map_values({action, name_change: (.changes.name // null)})}' > out.plan1.$DATABRICKS_BUNDLE_ENGINE.json + +trace $CLI experiments get-experiment ${EXPERIMENT_ID} | jq '{name: .experiment.name, lifecycle_stage: .experiment.lifecycle_stage}' + +# Now change the config to use /Users/... path (without /Workspace prefix). +# Plan should still show no changes because the diff is suppressed. +EXPERIMENT_NAME="//Users/${CURRENT_USER_NAME}/test-experiment$UNIQUE_NAME" +envsubst < databricks.yml.tmpl > databricks.yml +$CLI bundle plan --output json | jq '{plan: .plan | map_values({action, name_change: (.changes.name // null)})}' > out.plan2.$DATABRICKS_BUNDLE_ENGINE.json trace $CLI bundle deployment unbind experiment1 trace $CLI bundle destroy --auto-approve -trace $CLI experiments get-experiment ${EXPERIMENT_ID} | jq '{name: .experiment.name, lifecycle_stage: .experiment.lifecycle_stage}' > out.get2.$DATABRICKS_BUNDLE_ENGINE.json -diff out.get.$DATABRICKS_BUNDLE_ENGINE.json out.get2.$DATABRICKS_BUNDLE_ENGINE.json -rm out.get2.$DATABRICKS_BUNDLE_ENGINE.json +trace $CLI experiments get-experiment ${EXPERIMENT_ID} | jq '{name: .experiment.name, lifecycle_stage: .experiment.lifecycle_stage}' diff --git a/acceptance/bundle/deployment/bind/experiment/test.toml b/acceptance/bundle/deployment/bind/experiment/test.toml index 3ec944e783d..0e8c8a38404 100644 --- a/acceptance/bundle/deployment/bind/experiment/test.toml +++ b/acceptance/bundle/deployment/bind/experiment/test.toml @@ -1,10 +1,2 @@ -Badness = "Difference in GET request between direct and terraform; In direct, the prefix is /Workspace/Users, in TF it is /Users" Local = true - -# Fails on Cloud with: -#=== CONT TestAccept/bundle/deployment/bind/experiment/DATABRICKS_BUNDLE_ENGINE=direct -# - "name": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", -# + "name": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]", -# https://github.com/databricks/cli/issues/4285 - -Cloud = false +Cloud = true diff --git a/bundle/direct/dresources/experiment.go b/bundle/direct/dresources/experiment.go index 2225fd7e8c2..b2fc45af645 100644 --- a/bundle/direct/dresources/experiment.go +++ b/bundle/direct/dresources/experiment.go @@ -2,8 +2,11 @@ package dresources import ( "context" + "strings" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/ml" @@ -70,3 +73,47 @@ func (r *ResourceExperiment) DoDelete(ctx context.Context, id string) error { ExperimentId: id, }) } + +// OverrideChangeDesc suppresses drift for the experiment name field when the +// only difference is the /Workspace prefix. Stripping the prefix is necessary +// to avoid a persistent diff because the backend strips the /Workspace prefix, +// so remote returns "/Users/..." while the config has "/Workspace/Users/...". +// +// This matches the Terraform provider's experimentNameSuppressDiff behavior. +// https://github.com/databricks/terraform-provider-databricks/blob/8945a7b2328659b1fc976d04e32457305860131f/mlflow/resource_mlflow_experiment.go#L13 +func (*ResourceExperiment) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *ml.Experiment) error { + if change.Action == deployplan.Skip { + return nil + } + + if path.String() != "name" { + return nil + } + + newStr, newOk := change.New.(string) + remoteStr, remoteOk := change.Remote.(string) + if !newOk || !remoteOk { + return nil + } + + // Normalize by stripping the /Workspace/ prefix (keeping the trailing slash + // to avoid false matches like "/WorkspaceExtra/..."). + normalizedNew := stripWorkspacePrefix(newStr) + normalizedRemote := stripWorkspacePrefix(remoteStr) + if normalizedNew == normalizedRemote { + change.Action = deployplan.Skip + change.Reason = deployplan.ReasonAlias + } + + return nil +} + +// stripWorkspacePrefix removes the "/Workspace" portion from paths like +// "/Workspace/Users/..." while preserving the leading slash. Uses "/Workspace/" +// with trailing slash to avoid false matches on paths like "/WorkspaceExtra/...". +func stripWorkspacePrefix(s string) string { + if strings.HasPrefix(s, "/Workspace/") { + return s[len("/Workspace"):] + } + return s +}