[Fix] Fix drift in `databricks_cluster` when `is_single_node` is set to `true` #4416

ashenm · 2025-01-17T16:20:43Z

Changes

Tests

make test run locally
relevant change in docs/ folder
covered with integration tests in internal/acceptance
using Go SDK
using TF Plugin Framework

In addition to the unit tests, deployed provider locally with following resource definition and verified diff

resource "databricks_cluster" "single_node" {
  spark_version      = "15.4.x-scala2.12"
  single_user_name   = "[email protected]"
  runtime_engine     = "STANDARD"
  node_type_id       = "r5.large"
  kind               = "CLASSIC_PREVIEW"
  is_single_node     = true
  data_security_mode = "SINGLE_USER"
  cluster_name       = upper("demo-single-node")

  custom_tags = {
    "owner" = "ashenm"
  }
  autotermination_minutes = 10
}

ashenm · 2025-01-17T16:22:24Z

@alexott please take a stab at it when u get a slot

alexott · 2025-01-17T16:29:40Z

Please add tests for it - we have example of test for CustomizeDiff in resource_sql_table

alexott · 2025-01-20T09:59:35Z

@mgyucht given the work on porting clusters to the plugin framework - do we need this PR?

mgyucht · 2025-01-20T14:36:14Z

That work may take a bit longer than we thought to finish, so I'm OK merging incremental improvements like this.

mgyucht

Implementation seems OK. Several comments can be addressed.

Please include any testing you did in the PR description.

Can you add a unit test to verify that the diff is as expected? Create a new test, and use a qa.ResourceFixture where you supply the HCL, InstanceState, and ExpectedDiff fields, in addition to the Resource field. Take a look at catalog/resource_sql_table_test.go TestResourceSqlTable_Diff_ExistingResource for an example of how to do this. Add your test to clusters/resource_cluster_test.go.

mgyucht · 2025-01-20T14:37:26Z

clusters/resource_cluster.go

+		new, okNew := n.(map[string]interface{})
+
+		if !okNew || !okOld {
+			return fmt.Errorf("internal type casting error")


Can you include the actual type of o/n here with %T?

clusters/resource_cluster.go

mgyucht · 2025-01-20T14:58:19Z

clusters/resource_cluster.go

+		Delete: resourceClusterDelete,
+		Schema: clusterSchema,
+		CustomizeDiff: func(ctx context.Context, d *schema.ResourceDiff) error {
+			if isSingleNode, _ := d.GetOk("is_single_node"); isSingleNode.(bool) {


Potentially avoid interface conversion panic.

Suggested change

if isSingleNode, _ := d.GetOk("is_single_node"); isSingleNode.(bool) {

if isSingleNode, ok := d.GetOk("is_single_node"); ok && isSingleNode.(bool) {

Nice catch! Fixed 🙌

ashenm · 2025-01-21T08:06:38Z

clusters/resource_cluster_test.go

+			expectedDiff["default_tags.%"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}
+			expectedDiff["driver_instance_pool_id"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}
+			expectedDiff["driver_node_type_id"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}
+			expectedDiff["enable_elastic_disk"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}
+			expectedDiff["enable_local_disk_encryption"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}
+			expectedDiff["state"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}
+			expectedDiff["url"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}
+
+			instanceState := testCase.instanceState
+			instanceState["cluster_id"] = "abc"
+			instanceState["autotermination_minutes"] = "60"
+			instanceState["cluster_name"] = "Single Node Cluster"
+			instanceState["data_security_mode"] = "SINGLE_USER"
+			instanceState["spark_version"] = "15.4.x-scala2.12"
+			instanceState["single_user_name"] = "testuser"
+			instanceState["runtime_engine"] = "STANDARD"
+			instanceState["num_workers"] = "0"
+			instanceState["node_type_id"] = "Standard_F4s"
+			instanceState["kind"] = "CLASSIC_PREVIEW"
+			instanceState["is_single_node"] = "true"


Not a fan of this; if there's a way to simplify this please lemme know

ashenm · 2025-01-28T06:19:01Z

@alexott @mgyucht whenever y'all have a slot please take a stab

…to `true`

alexott · 2025-08-05T09:11:18Z

There is still a drift in spark_conf

github-actions · 2025-08-18T11:15:27Z

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

PR number: 4416
Commit SHA: 5e148173002b1734a528d05f8da889ee7ac23049

Checks will be approved automatically on success.

Copilot

Pull Request Overview

This PR fixes a drift issue in the databricks_cluster resource when is_single_node is set to true, addressing issue #4360. The Databricks API automatically sets certain properties for single-node clusters, causing Terraform to detect unwanted drift.

Adds a CustomizeDiff function to prevent drift by preserving auto-configured properties
Marks spark_conf and custom_tags as computed and optional in the schema
Includes comprehensive unit tests to verify the drift prevention behavior

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
clusters/resource_cluster.go	Implements CustomizeDiff logic and schema changes to prevent drift for single-node clusters
clusters/resource_cluster_test.go	Adds unit tests to verify CustomizeDiff behavior with various configurations
NEXT_CHANGELOG.md	Documents the bug fix in the changelog

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

Copilot · 2025-08-18T11:24:40Z

clusters/resource_cluster.go

+		new, okNew := n.(map[string]interface{})
+
+		if !okNew || !okOld {
+			return fmt.Errorf("internal type casting error n: %T, o: %T", n, o)


The error message 'internal type casting error' is not very helpful for debugging. Consider providing more context about what was expected and what was received, such as 'failed to cast custom_tags/spark_conf values to map[string]interface{}: expected map but got %T for new value and %T for old value'.

Suggested change

return fmt.Errorf("internal type casting error n: %T, o: %T", n, o)

return fmt.Errorf("failed to cast %s values to map[string]interface{}: expected map but got %T for new value and %T for old value", key, n, o)

Copilot · 2025-08-18T11:24:41Z

clusters/resource_cluster.go

+			return fmt.Errorf("internal type casting error n: %T, o: %T", n, o)
+		}
+
+		log.Printf("[DEBUG] values for key %s, old: %v, new: %v", key, old, new)


Logging the full contents of custom_tags and spark_conf may expose sensitive information. Consider logging only the keys being processed or redacting sensitive values.

Suggested change

log.Printf("[DEBUG] values for key %s, old: %v, new: %v", key, old, new)

log.Printf("[DEBUG] values for key %s, old keys: %v, new keys: %v", key, getMapKeys(old), getMapKeys(new))

Copilot · 2025-08-18T11:24:41Z

clusters/resource_cluster_test.go

+				expectedDiff = make(map[string]*terraform.ResourceAttrDiff)
+			}
+
+			expectedDiff["default_tags.%"] = &terraform.ResourceAttrDiff{Old: "", New: "", NewComputed: true, NewRemoved: false, RequiresNew: false, Sensitive: false}


[nitpick] The repeated terraform.ResourceAttrDiff structs with identical patterns (lines 1704-1710) could be extracted into a helper function to reduce duplication and improve maintainability.

ashenm requested review from a team as code owners January 17, 2025 16:20

ashenm requested review from mgyucht and removed request for a team January 17, 2025 16:20

ashenm temporarily deployed to test-trigger-is January 17, 2025 16:30 — with GitHub Actions Inactive

mgyucht reviewed Jan 20, 2025

View reviewed changes

ashenm force-pushed the issue-4360 branch from 506d8ee to d639747 Compare January 21, 2025 07:59

ashenm commented Jan 21, 2025

View reviewed changes

ashenm force-pushed the issue-4360 branch 3 times, most recently from 3cde764 to e397efd Compare January 28, 2025 06:03

ashenm force-pushed the issue-4360 branch 3 times, most recently from 68e13be to 77067f2 Compare January 30, 2025 15:33

alexott requested a review from Copilot March 20, 2025 16:04

This comment was marked as outdated.

Sign in to view

ashenm force-pushed the issue-4360 branch from 77067f2 to a78cd0d Compare April 7, 2025 17:07

alexott temporarily deployed to test-trigger-is June 9, 2025 18:05 — with GitHub Actions Inactive

alexott temporarily deployed to test-trigger-is June 9, 2025 18:06 — with GitHub Actions Inactive

ashenm requested a review from mgyucht June 13, 2025 09:29

alexott temporarily deployed to test-trigger-is June 27, 2025 13:22 — with GitHub Actions Inactive

alexott requested a review from Copilot June 27, 2025 13:27

This comment was marked as outdated.

Sign in to view

[Fix] Fix drift in databricks_cluster when is_single_node is set …

e2eb098

…to `true`

ashenm force-pushed the issue-4360 branch from d451606 to e2eb098 Compare July 25, 2025 13:06

Merge branch 'main' into issue-4360

ca2f251

alexott temporarily deployed to test-trigger-is August 5, 2025 08:23 — with GitHub Actions Inactive

Merge branch 'main' into issue-4360

5e14817

alexott temporarily deployed to test-trigger-is August 18, 2025 11:15 — with GitHub Actions Inactive

alexott requested a review from Copilot August 18, 2025 11:23

Copilot AI reviewed Aug 18, 2025

View reviewed changes

	if isSingleNode, _ := d.GetOk("is_single_node"); isSingleNode.(bool) {
	if isSingleNode, ok := d.GetOk("is_single_node"); ok && isSingleNode.(bool) {

	return fmt.Errorf("internal type casting error n: %T, o: %T", n, o)
	return fmt.Errorf("failed to cast %s values to map[string]interface{}: expected map but got %T for new value and %T for old value", key, n, o)

	log.Printf("[DEBUG] values for key %s, old: %v, new: %v", key, old, new)
	log.Printf("[DEBUG] values for key %s, old keys: %v, new keys: %v", key, getMapKeys(old), getMapKeys(new))

[Fix] Fix drift in databricks_cluster when is_single_node is set to true #4416

Are you sure you want to change the base?

[Fix] Fix drift in databricks_cluster when is_single_node is set to true #4416

Uh oh!

Conversation

ashenm commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Uh oh!

ashenm commented Jan 17, 2025

Uh oh!

alexott commented Jan 17, 2025

Uh oh!

alexott commented Jan 20, 2025

Uh oh!

mgyucht commented Jan 20, 2025

Uh oh!

mgyucht left a comment

Choose a reason for hiding this comment

Uh oh!

mgyucht Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mgyucht Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

ashenm Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

ashenm Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

ashenm commented Jan 28, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

alexott commented Aug 5, 2025

Uh oh!

github-actions bot commented Aug 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[Fix] Fix drift in `databricks_cluster` when `is_single_node` is set to `true` #4416

[Fix] Fix drift in `databricks_cluster` when `is_single_node` is set to `true` #4416

ashenm commented Jan 17, 2025 •

edited

Loading