Skip to content

Commit

Permalink
Fix handling of empty blocks for clusters/jobs/instance pools
Browse files Browse the repository at this point in the history
We don't need to specify empty blocks for aws_attributes/azure_attributes/gcp_attributes
to prevent edit/re-creation of the clusters/jobs/instance pools.

For instance pools even specification of empty blocks didn't work because we were need to
specify availability & other attributes as well to prevent recreation of objects
  • Loading branch information
alexott authored and nfx committed Apr 26, 2021
1 parent 9e2b9c0 commit 22cdf2f
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 21 deletions.
2 changes: 1 addition & 1 deletion common/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package common
import "context"

var (
version = "0.3.3"
version = "0.3.4"
// ResourceName is resource name without databricks_ prefix
ResourceName contextKey = 1
// Provider is the current instance of provider
Expand Down
4 changes: 2 additions & 2 deletions compute/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,15 +378,15 @@ type Command struct {
// InstancePoolAwsAttributes contains aws attributes for AWS Databricks deployments for instance pools
type InstancePoolAwsAttributes struct {
Availability Availability `json:"availability,omitempty"`
ZoneID string `json:"zone_id"`
ZoneID string `json:"zone_id,omitempty" tf:"computed"`
SpotBidPricePercent int32 `json:"spot_bid_price_percent,omitempty"`
}

// InstancePoolAzureAttributes contains aws attributes for Azure Databricks deployments for instance pools
// https://docs.microsoft.com/en-us/azure/databricks/dev-tools/api/latest/instance-pools#clusterinstancepoolazureattributes
type InstancePoolAzureAttributes struct {
Availability Availability `json:"availability,omitempty"`
SpotBidMaxPrice float64 `json:"spot_bid_max_price,omitempty" tf:"computed"`
SpotBidMaxPrice float64 `json:"spot_bid_max_price,omitempty"`
}

// InstancePoolDiskType contains disk type information for each of the different cloud service providers
Expand Down
4 changes: 4 additions & 0 deletions compute/resource_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ func resourceClusterSchema() map[string]*schema.Schema {
s["aws_attributes"].ConflictsWith = []string{"azure_attributes", "gcp_attributes"}
s["azure_attributes"].ConflictsWith = []string{"aws_attributes", "gcp_attributes"}
s["gcp_attributes"].ConflictsWith = []string{"aws_attributes", "azure_attributes"}
s["aws_attributes"].DiffSuppressFunc = makeEmptyBlockSuppressFunc("aws_attributes.#")
s["azure_attributes"].DiffSuppressFunc = makeEmptyBlockSuppressFunc("azure_attributes.#")
s["gcp_attributes"].DiffSuppressFunc = makeEmptyBlockSuppressFunc("gcp_attributes.#")

s["is_pinned"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Expand Down
25 changes: 25 additions & 0 deletions compute/resource_instance_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package compute

import (
"context"
"log"

"github.com/databrickslabs/terraform-provider-databricks/common"

Expand Down Expand Up @@ -53,6 +54,17 @@ func (a InstancePoolsAPI) Delete(instancePoolID string) error {
}, nil)
}

func makeEmptyBlockSuppressFunc(name string) func(k, old, new string, d *schema.ResourceData) bool {
return func(k, old, new string, d *schema.ResourceData) bool {
log.Printf("[DEBUG] k='%v', old='%v', new='%v'", k, old, new)
if k == name && old == "1" && new == "0" {
log.Printf("[DEBUG] Disable removal of empty block")
return true
}
return false
}
}

// ResourceInstancePool ...
func ResourceInstancePool() *schema.Resource {
s := common.StructToSchema(InstancePool{}, func(s map[string]*schema.Schema) map[string]*schema.Schema {
Expand All @@ -63,18 +75,31 @@ func ResourceInstancePool() *schema.Resource {
s["enable_elastic_disk"].Default = true
s["aws_attributes"].ConflictsWith = []string{"azure_attributes"}
s["azure_attributes"].ConflictsWith = []string{"aws_attributes"}
s["aws_attributes"].DiffSuppressFunc = makeEmptyBlockSuppressFunc("aws_attributes.#")
s["azure_attributes"].DiffSuppressFunc = makeEmptyBlockSuppressFunc("azure_attributes.#")
// TODO: check if it's really force new...
if v, err := common.SchemaPath(s, "aws_attributes", "availability"); err == nil {
v.ForceNew = true
v.Default = AwsAvailabilitySpot
v.ValidateFunc = validation.StringInSlice([]string{
AwsAvailabilityOnDemand,
AwsAvailabilitySpot,
}, false)
}
if v, err := common.SchemaPath(s, "aws_attributes", "zone_id"); err == nil {
v.ForceNew = true
}
if v, err := common.SchemaPath(s, "aws_attributes", "spot_bid_price_percent"); err == nil {
v.ForceNew = true
v.Default = 100
}
if v, err := common.SchemaPath(s, "azure_attributes", "availability"); err == nil {
v.ForceNew = true
v.Default = AzureAvailabilityOnDemand
v.ValidateFunc = validation.StringInSlice([]string{
AzureAvailabilitySpot,
AzureAvailabilityOnDemand,
}, false)
}
if v, err := common.SchemaPath(s, "azure_attributes", "spot_bid_max_price"); err == nil {
v.ForceNew = true
Expand Down
17 changes: 11 additions & 6 deletions compute/resource_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,18 @@ var jobSchema = common.StructToSchema(JobSettings{},
return false
}
}
s["email_notifications"].DiffSuppressFunc = func(k, old, new string, d *schema.ResourceData) bool {
log.Printf("[INFO] k='%v', old='%v', new='%v'", k, old, new)
if old == "1" && new == "0" {
return true
}
return false

if v, err := common.SchemaPath(s, "new_cluster", "aws_attributes"); err == nil {
v.DiffSuppressFunc = makeEmptyBlockSuppressFunc("new_cluster.0.aws_attributes.#")
}
if v, err := common.SchemaPath(s, "new_cluster", "azure_attributes"); err == nil {
v.DiffSuppressFunc = makeEmptyBlockSuppressFunc("new_cluster.0.azure_attributes.#")
}
if v, err := common.SchemaPath(s, "new_cluster", "gcp_attributes"); err == nil {
v.DiffSuppressFunc = makeEmptyBlockSuppressFunc("new_cluster.0.gcp_attributes.#")
}

s["email_notifications"].DiffSuppressFunc = makeEmptyBlockSuppressFunc("email_notifications.#")

s["name"].Description = "An optional name for the job. The default value is Untitled."
s["library"].Description = "An optional list of libraries to be installed on " +
Expand Down
6 changes: 0 additions & 6 deletions docs/resources/cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,6 @@ init_scripts {

`aws_attributes` optional configuration block contains attributes related to [clusters running on Amazon Web Services](https://docs.databricks.com/clusters/configure.html#aws-configurations).

-> **Note** *(AWS only)* Please specify empty configuration block (`aws_attributes {}`), even if you're not setting any custom values. This will prevent any resource update issues.

Here is the example of shared autoscaling cluster with some of AWS options set:

```hcl
Expand Down Expand Up @@ -279,8 +277,6 @@ The following options are available:

`azure_attributes` optional configuration block contains attributes related to [clusters running on Azure](https://docs.microsoft.com/en-us/azure/databricks/dev-tools/api/latest/clusters#--azureattributes).

-> **Note** *(Azure only)* Please specify empty configuration block (`azure_attributes {}`), even if you're not setting any custom values. This will prevent any resource update issues.

Here is the example of shared autoscaling cluster with some of AWS options set:

```hcl
Expand Down Expand Up @@ -311,8 +307,6 @@ The following options are [available](https://docs.microsoft.com/en-us/azure/dat

`gcp_attributes` optional configuration block contains attributes related to [clusters running on GCP](https://docs.gcp.databricks.com/dev-tools/api/latest/clusters.html#clustergcpattributes).

-> **Note** *(GCP only)* Please specify empty configuration block (`gcp_attributes {}`), even if you're not setting any custom values. This will prevent any resource update issues.

The following options are available:

* `use_preemptible_executors` - (Optional, bool) if we should use preemptible executors ([GCP documentation](https://cloud.google.com/compute/docs/instances/preemptible))
Expand Down
6 changes: 2 additions & 4 deletions docs/resources/instance_pool.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,16 @@ The following arguments are required:

### aws_attributes Configuration Block

-> **Note** *(AWS only)* Please specify empty configuration block (`aws_attributes {}`), even if you're not setting any custom values. This will prevent any resource update issues.
The following options are [available](https://docs.databricks.com/dev-tools/api/latest/instance-pools.html#clusterinstancepoolawsattributes):

* `spot_bid_price_percent` - (Optional) (Integer) The max price for AWS spot instances, as a percentage of the corresponding instance type’s on-demand price. For example, if this field is set to 50, and the instance pool needs a new i3.xlarge spot instance, then the max price is half of the price of on-demand i3.xlarge instances. Similarly, if this field is set to 200, the max price is twice the price of on-demand i3.xlarge instances. If not specified, the *default value is 100*. When spot instances are requested for this instance pool, only spot instances whose max price percentage matches this field are considered. *For safety, this field cannot be greater than 10000.*
* `availability` - (Optional) (String) Availability type used for all instances in the pool. Only `ON_DEMAND` and `SPOT` are supported.
* `zone_id` - (Required) (String) Identifier for the availability zone/datacenter in which the instance pool resides. This string is of a form like `"us-west-2a"`. The provided availability zone must be in the same region as the Databricks deployment. For example, `"us-west-2a"` is not a valid zone ID if the Databricks deployment resides in the `"us-east-1"` region. This is an optional field. If not specified, a default zone is used. You can find the list of available zones as well as the default value by using the [List Zones API](https://docs.databricks.com/dev-tools/api/latest/clusters.html#clusterclusterservicelistavailablezones).
* `zone_id` - (Optional) (String) Identifier for the availability zone/datacenter in which the instance pool resides. This string is of a form like `"us-west-2a"`. The provided availability zone must be in the same region as the Databricks deployment. For example, `"us-west-2a"` is not a valid zone ID if the Databricks deployment resides in the `"us-east-1"` region. This is an optional field. If not specified, a default zone is used. You can find the list of available zones as well as the default value by using the [List Zones API](https://docs.databricks.com/dev-tools/api/latest/clusters.html#clusterclusterservicelistavailablezones).

## azure_attributes Configuration Block

`azure_attributes` optional configuration block contains attributes related to [instance pools on Azure](https://docs.microsoft.com/en-us/azure/databricks/dev-tools/api/latest/instance-pools#--instancepoolazureattributes).

-> **Note** *(Azure only)* Please specify empty configuration block (`azure_attributes {}`), even if you're not setting any custom values. This will prevent any resource update issues.

The following options are [available](https://docs.microsoft.com/en-us/azure/databricks/dev-tools/api/latest/clusters#--azureattributes):

* `availability` - (Optional) Availability type used for all subsequent nodes past the `first_on_demand` ones. Valid values are `SPOT_AZURE` and `ON_DEMAND_AZURE`.
Expand Down
2 changes: 0 additions & 2 deletions docs/resources/job.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ resource "databricks_job" "this" {
notebook_task {
notebook_path = databricks_notebook.this.path
}
email_notifications {}
}
output "notebook_url" {
Expand Down

0 comments on commit 22cdf2f

Please sign in to comment.