Skip to content

Commit 196b05b

Browse files
edwardfeng-dbedwardfeng-db
authored
Fix resource_cluster bug with ebs volume fields (#3613)
* update * update * update
1 parent 5f5472f commit 196b05b

File tree

4 files changed

+166
-36
lines changed

4 files changed

+166
-36
lines changed

‎clusters/resource_cluster.go‎

Lines changed: 109 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"log"
7+
"slices"
78
"strings"
89
"time"
910

@@ -23,8 +24,8 @@ var clusterSchema = resourceClusterSchema()
2324
var clusterSchemaVersion = 4
2425

2526
const (
26-
numWorkerErr = "NumWorkers could be 0 only for SingleNode clusters. See https://docs.databricks.com/clusters/single-node.html for more details"
27-
unsupportedExceptCreateOrEditErr = "unsupported type %T, must be one of %scompute.CreateCluster, %scompute.ClusterSpec or %scompute.EditCluster. Please report this issue to the GitHub repo"
27+
numWorkerErr = "NumWorkers could be 0 only for SingleNode clusters. See https://docs.databricks.com/clusters/single-node.html for more details"
28+
unsupportedExceptCreateEditClusterSpecErr = "unsupported type %T, must be one of %scompute.CreateCluster, %scompute.ClusterSpec or %scompute.EditCluster. Please report this issue to the GitHub repo"
2829
)
2930

3031
func ResourceCluster() common.Resource {
@@ -135,7 +136,7 @@ func Validate(cluster any) error {
135136
master = c.SparkConf["spark.master"]
136137
resourceClass = c.CustomTags["ResourceClass"]
137138
default:
138-
return fmt.Errorf(unsupportedExceptCreateOrEditErr, cluster, "", "", "")
139+
return fmt.Errorf(unsupportedExceptCreateEditClusterSpecErr, cluster, "", "", "")
139140
}
140141
if profile == "singleNode" && strings.HasPrefix(master, "local") && resourceClass == "SingleNode" {
141142
return nil
@@ -232,7 +233,7 @@ func ModifyRequestOnInstancePool(cluster any) error {
232233
c.DriverNodeTypeId = ""
233234
return nil
234235
default:
235-
return fmt.Errorf(unsupportedExceptCreateOrEditErr, cluster, "*", "*", "*")
236+
return fmt.Errorf(unsupportedExceptCreateEditClusterSpecErr, cluster, "*", "*", "*")
236237
}
237238
}
238239

@@ -260,7 +261,7 @@ func FixInstancePoolChangeIfAny(d *schema.ResourceData, cluster any) error {
260261
}
261262
return nil
262263
default:
263-
return fmt.Errorf(unsupportedExceptCreateOrEditErr, cluster, "*", "*", "*")
264+
return fmt.Errorf(unsupportedExceptCreateEditClusterSpecErr, cluster, "*", "*", "*")
264265
}
265266
}
266267

@@ -439,6 +440,105 @@ func setPinnedStatus(ctx context.Context, d *schema.ResourceData, clusterAPI com
439440
return d.Set("is_pinned", pinnedEvent == compute.EventTypePinned)
440441
}
441442

443+
func RemoveUnnecessaryFieldsFromForceSendFields(cluster any) error {
444+
switch clusterSpec := cluster.(type) {
445+
case *compute.ClusterSpec:
446+
if clusterSpec.AwsAttributes != nil {
447+
newAwsAttributesForceSendFields := []string{}
448+
// These fields should never be 0.
449+
unnecessaryFieldNamesForAwsAttributes := []string{
450+
"SpotBidPricePercent",
451+
"EbsVolumeCount",
452+
"EbsVolumeIops",
453+
"EbsVolumeSize",
454+
"EbsVolumeThroughput",
455+
}
456+
for _, field := range clusterSpec.AwsAttributes.ForceSendFields {
457+
if !slices.Contains(unnecessaryFieldNamesForAwsAttributes, field) {
458+
newAwsAttributesForceSendFields = append(newAwsAttributesForceSendFields, field)
459+
}
460+
}
461+
clusterSpec.AwsAttributes.ForceSendFields = newAwsAttributesForceSendFields
462+
}
463+
if clusterSpec.GcpAttributes != nil {
464+
newGcpAttributesForceSendFields := []string{}
465+
// Should never be 0.
466+
unnecessaryFieldNamesForGcpAttributes := []string{
467+
"BootDiskSize",
468+
}
469+
for _, field := range clusterSpec.GcpAttributes.ForceSendFields {
470+
if !slices.Contains(unnecessaryFieldNamesForGcpAttributes, field) {
471+
newGcpAttributesForceSendFields = append(newGcpAttributesForceSendFields, field)
472+
}
473+
}
474+
clusterSpec.GcpAttributes.ForceSendFields = newGcpAttributesForceSendFields
475+
}
476+
if clusterSpec.AzureAttributes != nil {
477+
newAzureAttributesForceSendFields := []string{}
478+
// Should never be 0.
479+
unnecessaryFieldNamesForAzureAttributes := []string{
480+
"FirstOnDemand",
481+
"SpotBidMaxPrice",
482+
}
483+
for _, field := range clusterSpec.AzureAttributes.ForceSendFields {
484+
if !slices.Contains(unnecessaryFieldNamesForAzureAttributes, field) {
485+
newAzureAttributesForceSendFields = append(newAzureAttributesForceSendFields, field)
486+
}
487+
}
488+
clusterSpec.AzureAttributes.ForceSendFields = newAzureAttributesForceSendFields
489+
}
490+
return nil
491+
case *compute.EditCluster:
492+
if clusterSpec.AwsAttributes != nil {
493+
newAwsAttributesForceSendFields := []string{}
494+
// These fields should never be 0.
495+
unnecessaryFieldNamesForAwsAttributes := []string{
496+
"SpotBidPricePercent",
497+
"EbsVolumeCount",
498+
"EbsVolumeIops",
499+
"EbsVolumeSize",
500+
"EbsVolumeThroughput",
501+
}
502+
for _, field := range clusterSpec.AwsAttributes.ForceSendFields {
503+
if !slices.Contains(unnecessaryFieldNamesForAwsAttributes, field) {
504+
newAwsAttributesForceSendFields = append(newAwsAttributesForceSendFields, field)
505+
}
506+
}
507+
clusterSpec.AwsAttributes.ForceSendFields = newAwsAttributesForceSendFields
508+
}
509+
if clusterSpec.GcpAttributes != nil {
510+
newGcpAttributesForceSendFields := []string{}
511+
// Should never be 0.
512+
unnecessaryFieldNamesForGcpAttributes := []string{
513+
"BootDiskSize",
514+
}
515+
for _, field := range clusterSpec.GcpAttributes.ForceSendFields {
516+
if !slices.Contains(unnecessaryFieldNamesForGcpAttributes, field) {
517+
newGcpAttributesForceSendFields = append(newGcpAttributesForceSendFields, field)
518+
}
519+
}
520+
clusterSpec.GcpAttributes.ForceSendFields = newGcpAttributesForceSendFields
521+
}
522+
if clusterSpec.AzureAttributes != nil {
523+
newAzureAttributesForceSendFields := []string{}
524+
// Should never be 0.
525+
unnecessaryFieldNamesForAzureAttributes := []string{
526+
"FirstOnDemand",
527+
"SpotBidMaxPrice",
528+
}
529+
for _, field := range clusterSpec.AzureAttributes.ForceSendFields {
530+
if !slices.Contains(unnecessaryFieldNamesForAzureAttributes, field) {
531+
newAzureAttributesForceSendFields = append(newAzureAttributesForceSendFields, field)
532+
}
533+
}
534+
clusterSpec.AzureAttributes.ForceSendFields = newAzureAttributesForceSendFields
535+
}
536+
return nil
537+
default:
538+
return fmt.Errorf(unsupportedExceptCreateEditClusterSpecErr, cluster, "*", "*", "*")
539+
}
540+
}
541+
442542
func resourceClusterRead(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
443543
w, err := c.WorkspaceClient()
444544
if err != nil {
@@ -572,6 +672,10 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, c *commo
572672
Autoscale: cluster.Autoscale,
573673
})
574674
} else {
675+
err = RemoveUnnecessaryFieldsFromForceSendFields(&cluster)
676+
if err != nil {
677+
return err
678+
}
575679
_, err = clusters.Edit(ctx, cluster)
576680
}
577681
if err != nil {

‎internal/acceptance/cluster_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,30 @@ func TestAccClusterResource_CreateSingleNodeCluster(t *testing.T) {
7777
Template: singleNodeClusterTemplate("20"),
7878
})
7979
}
80+
81+
func awsClusterTemplate(availability string) string {
82+
return fmt.Sprintf(`
83+
data "databricks_spark_version" "latest" {
84+
}
85+
resource "databricks_cluster" "this" {
86+
cluster_name = "aws-cluster-{var.RANDOM}"
87+
spark_version = data.databricks_spark_version.latest.id
88+
num_workers = 1
89+
autotermination_minutes = 10
90+
aws_attributes {
91+
availability = "%s"
92+
}
93+
node_type_id = "i3.xlarge"
94+
}
95+
`, availability)
96+
}
97+
98+
func TestAwsClusterResource_CreateAndUpdateAwsAttributes(t *testing.T) {
99+
if isAws(t) {
100+
workspaceLevel(t, step{
101+
Template: awsClusterTemplate("SPOT"),
102+
}, step{
103+
Template: awsClusterTemplate("SPOT_WITH_FALLBACK"),
104+
})
105+
}
106+
}

‎jobs/jobs_api_go_sdk.go‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"slices"
87
"sort"
98
"time"
109

@@ -157,47 +156,44 @@ func (c controlRunStateLifecycleManagerGoSdk) OnUpdate(ctx context.Context) erro
157156
return StopActiveRun(jobID, c.d.Timeout(schema.TimeoutUpdate), w, ctx)
158157
}
159158

160-
// Removing unnecesary fields out of ClusterSpec's ForceSendFields because the current terraform plugin sdk
161-
// has a limitation that it will always set unspecified values to the zero value.
162-
func removeUnnecessaryFieldsFromForceSendFields(clusterSpec *compute.ClusterSpec) error {
163-
if clusterSpec.AwsAttributes != nil {
164-
newAwsAttributesForceSendFields := []string{}
165-
// These fields should never be 0.
166-
unnecessaryFieldNamesForAwsAttributes := []string{
167-
"SpotBidPricePercent",
168-
"EbsVolumeCount",
169-
"EbsVolumeIops",
170-
"EbsVolumeSize",
171-
"EbsVolumeThroughput",
172-
}
173-
for _, field := range clusterSpec.AwsAttributes.ForceSendFields {
174-
if !slices.Contains(unnecessaryFieldNamesForAwsAttributes, field) {
175-
newAwsAttributesForceSendFields = append(newAwsAttributesForceSendFields, field)
176-
}
177-
}
178-
clusterSpec.AwsAttributes.ForceSendFields = newAwsAttributesForceSendFields
159+
func updateJobClusterSpec(clusterSpec *compute.ClusterSpec, d *schema.ResourceData) error {
160+
err := clusters.ModifyRequestOnInstancePool(clusterSpec)
161+
if err != nil {
162+
return err
163+
}
164+
err = clusters.FixInstancePoolChangeIfAny(d, clusterSpec)
165+
if err != nil {
166+
return err
167+
}
168+
err = clusters.RemoveUnnecessaryFieldsFromForceSendFields(clusterSpec)
169+
if err != nil {
170+
return err
179171
}
180172
return nil
181173
}
182174

183-
func updateJobClusterSpec(clusterSpec *compute.ClusterSpec, d *schema.ResourceData) {
184-
clusters.ModifyRequestOnInstancePool(clusterSpec)
185-
clusters.FixInstancePoolChangeIfAny(d, clusterSpec)
186-
removeUnnecessaryFieldsFromForceSendFields(clusterSpec)
187-
}
188-
189-
func prepareJobSettingsForUpdateGoSdk(d *schema.ResourceData, js *JobSettingsResource) {
175+
func prepareJobSettingsForUpdateGoSdk(d *schema.ResourceData, js *JobSettingsResource) error {
190176
if js.NewCluster != nil {
191-
updateJobClusterSpec(js.NewCluster, d)
177+
err := updateJobClusterSpec(js.NewCluster, d)
178+
if err != nil {
179+
return err
180+
}
192181
}
193182
for _, task := range js.Tasks {
194183
if task.NewCluster != nil {
195-
updateJobClusterSpec(task.NewCluster, d)
184+
err := updateJobClusterSpec(task.NewCluster, d)
185+
if err != nil {
186+
return err
187+
}
196188
}
197189
}
198190
for i := range js.JobClusters {
199-
updateJobClusterSpec(&js.JobClusters[i].NewCluster, d)
191+
err := updateJobClusterSpec(&js.JobClusters[i].NewCluster, d)
192+
if err != nil {
193+
return err
194+
}
200195
}
196+
return nil
201197
}
202198

203199
func Create(createJob jobs.CreateJob, w *databricks.WorkspaceClient, ctx context.Context) (int64, error) {

‎jobs/resource_job.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,10 @@ func ResourceJob() common.Resource {
11511151
common.DataToStructPointer(d, jobsGoSdkSchema, &jsr)
11521152
if jsr.isMultiTask() {
11531153
// Api 2.1
1154-
prepareJobSettingsForUpdateGoSdk(d, &jsr)
1154+
err := prepareJobSettingsForUpdateGoSdk(d, &jsr)
1155+
if err != nil {
1156+
return err
1157+
}
11551158
jobID, err := parseJobId(d.Id())
11561159
if err != nil {
11571160
return err

0 commit comments

Comments
 (0)