direct: don't treat an explicit scalar zero as an empty no-op change

.nextchanges/bundles/gcp-attributes-local-ssd-count-zero-update.md

@@ -0,0 +1 @@
+* Direct engine no longer silently drops an update that sets an explicit zero on a scalar field left unset by a previous deploy (e.g. `gcp_attributes.local_ssd_count`), when the remote API doesn't echo a value back for it.

acceptance/bundle/resources/clusters/deploy/local_ssd_count_update/databricks.yml

@@ -0,0 +1,13 @@
+bundle:
+  name: test-deploy-cluster-local-ssd-count-update
+
+resources:
+  clusters:
+    test_cluster:
+      cluster_name: test-cluster-1
+      spark_version: 13.3.x-scala2.12
+      node_type_id: i3.xlarge
+      num_workers: 1
+      gcp_attributes:
+        availability: PREEMPTIBLE_GCP
+        zone_id: auto

acceptance/bundle/resources/clusters/deploy/local_ssd_count_update/output.txt

@@ -0,0 +1,46 @@
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-deploy-cluster-local-ssd-count-update/default/files...
+Created clusters.test_cluster
+Files: 3 uploaded, 0 deleted
+Resources: 1 created, 0 changed, 0 deleted, 0 unchanged
+
+=== Adding local_ssd_count: 0 to an existing cluster is detected as an update
+
+>>> [CLI] bundle plan
+update clusters.test_cluster
+
+Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-deploy-cluster-local-ssd-count-update/default/files...
+Updated clusters.test_cluster
+Files: 1 uploaded, 0 deleted
+Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
+
+>>> print_requests.py //clusters/edit
+{
+  "method": "POST",
+  "path": "/api/2.1/clusters/edit",
+  "body": {
+    "autotermination_minutes": 60,
+    "cluster_id": "[UUID]",
+    "cluster_name": "test-cluster-1",
+    "gcp_attributes": {
+      "availability": "PREEMPTIBLE_GCP",
+      "local_ssd_count": 0,
+      "zone_id": "auto"
+    },
+    "node_type_id": "[NODE_TYPE_ID]",
+    "num_workers": 1,
+    "spark_version": "13.3.x-scala2.12"
+  }
+}
+
+>>> [CLI] bundle destroy --auto-approve
+The following resources will be deleted:
+  delete resources.clusters.test_cluster
+
+All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-deploy-cluster-local-ssd-count-update/default
+
+Destroy: 1 deleted

acceptance/bundle/resources/clusters/deploy/local_ssd_count_update/script

@@ -0,0 +1,14 @@
+cleanup() {
+    trace $CLI bundle destroy --auto-approve
+    rm -f "$OUT_REQUESTS"
+}
+trap cleanup EXIT
+
+trace $CLI bundle deploy
+
+title "Adding local_ssd_count: 0 to an existing cluster is detected as an update\n"
+update_file.py databricks.yml "zone_id: auto" $'zone_id: auto\n        local_ssd_count: 0'
+trace $CLI bundle plan
+trace $CLI bundle deploy
+
+trace print_requests.py //clusters/edit

acceptance/bundle/resources/clusters/deploy/local_ssd_count_update/test.toml

@@ -0,0 +1,14 @@
+Cloud = false
+RecordRequests = true
+
+# Terraform never sends local_ssd_count at all (databricks/terraform-provider-databricks#4089),
+# so the update-detection fix under test is direct-engine only.
+EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]
+
+Ignore = [
+    "databricks.yml",
+]
+
+[[Repls]]
+Old = "[0-9]{4}-[0-9]{6}-[0-9a-z]{8}"
+New = "[CLUSTER-ID]"

bundle/direct/bundle_plan.go

@@ -484,7 +484,7 @@
 		if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && !isFieldMissingInRemote(adapter, path) {
 			ch.Action = deployplan.Skip
 			ch.Reason = deployplan.ReasonRemoteAlreadySet
-		} else if allEmpty(ch.Old, ch.New, ch.Remote) {
+		} else if allEmptyChange(ch) {
 			ch.Action = deployplan.Skip
 			ch.Reason = deployplan.ReasonEmpty
 		} else if reason, ok := shouldSkip(cfg, path, ch); ok {
@@ -832,6 +832,28 @@
 	return true
 }
 
+// allEmptyChange reports whether a change is a no-op. ch.Old and ch.Remote are checked with the
+// permissive allEmpty (a zero-ish backend echo for a field nobody asked to change is still empty),
+// but ch.New is checked more strictly: a concrete scalar zero there already survived the
+// ForceSendFields-aware nil substitution in structdiff, so it is a real, explicitly force-sent
+// value (e.g. gcp_attributes.local_ssd_count: 0), not an unset field, and must not be skipped.
+func allEmptyChange(ch *deployplan.ChangeDesc) bool {
+	if !allEmpty(ch.Old, ch.Remote) {
+		return false
+	}
+	if ch.New == nil {
+		return true
+	}
+	rv := reflect.ValueOf(ch.New)
+	switch rv.Kind() {
+	case reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
+		reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr,
+		reflect.Float32, reflect.Float64:
+		return false
+	}
+	return isEmpty(rv)
+}
+
 func isEmpty(rv reflect.Value) bool {
 	// certain fields can change between "" and null when processed by backend.
 	// in some cases, e.g. model_serving_endpoints.descriptions those fields are also marked as recreate, so we ignore such cases

bundle/direct/bundle_plan_test.go

@@ -664,3 +664,59 @@ func TestLadderBackendDefaultBeforeRemoteAddition(t *testing.T) {
 	assert.Equal(t, deployplan.Skip, changes["enable_elastic_disk"].Action)
 	assert.Equal(t, deployplan.ReasonBackendDefault, changes["enable_elastic_disk"].Reason)
 }
+
+// TestScalarZeroIsNotEmpty guards against allEmpty/isEmpty treating an explicitly
+// force-sent scalar zero the same as an unset field: the field was never set before
+// (Old=nil), config now force-sends an explicit 0 (New=0), and the backend doesn't
+// echo a value back on GET (Remote=nil, e.g. gcp_attributes.local_ssd_count's
+// "default N local SSDs" behavior isn't reflected in the cluster spec response).
+func TestScalarZeroIsNotEmpty(t *testing.T) {
+	adapter, err := dresources.NewAdapter(dresources.SupportedResources["clusters"], "clusters", nil)
+	require.NoError(t, err)
+
+	changes := deployplan.Changes{
+		"gcp_attributes.local_ssd_count": &deployplan.ChangeDesc{Old: nil, New: 0, Remote: nil},
+	}
+	require.NoError(t, addPerFieldActions(t.Context(), adapter, changes, nil, nil))
+	assert.Equal(t, deployplan.Update, changes["gcp_attributes.local_ssd_count"].Action)
+}
+
+// TestUnsetScalarWithBackendEchoIsStillEmpty guards the other direction of the
+// TestScalarZeroIsNotEmpty fix: a field the config never sets (Old=New=nil) whose
+// remote echo happens to be a scalar zero/false (no backend_defaults entry needed)
+// must stay a no-op, not turn into a spurious update just because Remote is scalar.
+func TestUnsetScalarWithBackendEchoIsStillEmpty(t *testing.T) {
+	adapters, err := dresources.InitAll(nil)
+	require.NoError(t, err)
+	adapter, ok := adapters["jobs"]
+	require.True(t, ok)
+
+	changes := deployplan.Changes{
+		"timeout_seconds": &deployplan.ChangeDesc{Old: nil, New: nil, Remote: 0},
+	}
+	require.NoError(t, addPerFieldActions(t.Context(), adapter, changes, nil, nil))
+	assert.Equal(t, deployplan.Skip, changes["timeout_seconds"].Action)
+	assert.Equal(t, deployplan.ReasonEmpty, changes["timeout_seconds"].Reason)
+}
+
+// TestRemovingManagedOverrideMatchingRemoteIsEmpty pins intentional behavior for a
+// "managed" field (gcp_attributes: ignore_remote_changes) when the config drops an
+// override that matches what's already on the backend: Old=0 (last explicit value,
+// still in state), New=nil (removed from config), Remote=0 (backend echoes the same
+// value). This is a no-op, not a bug: the tool doesn't try to actively reset a
+// managed cloud attribute just because the user stopped specifying it, the same way
+// it never actively sets one on create either.
+func TestRemovingManagedOverrideMatchingRemoteIsEmpty(t *testing.T) {
+	adapters, err := dresources.InitAll(nil)
+	require.NoError(t, err)
+	adapter, ok := adapters["jobs"]
+	require.True(t, ok)
+
+	changes := deployplan.Changes{
+		"tasks[task_key='t'].new_cluster.gcp_attributes.local_ssd_count": &deployplan.ChangeDesc{Old: 0, New: nil, Remote: 0},
+	}
+	require.NoError(t, addPerFieldActions(t.Context(), adapter, changes, nil, nil))
+	ch := changes["tasks[task_key='t'].new_cluster.gcp_attributes.local_ssd_count"]
+	assert.Equal(t, deployplan.Skip, ch.Action)
+	assert.Equal(t, deployplan.ReasonEmpty, ch.Reason)
+}