Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mounts now handle deleted cluster correctly #87

Merged
merged 13 commits into from
Jun 11, 2020
18 changes: 13 additions & 5 deletions databricks/resource_databricks_azure_adls_gen2_mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package databricks

import (
"fmt"
"log"
"strings"

"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"strings"
)

func resourceAzureAdlsGen2Mount() *schema.Resource {
Expand Down Expand Up @@ -129,10 +131,6 @@ func resourceAzureAdlsGen2Create(d *schema.ResourceData, m interface{}) error {
func resourceAzureAdlsGen2Read(d *schema.ResourceData, m interface{}) error {
client := m.(*service.DBApiClient)
clusterID := d.Get("cluster_id").(string)
err := changeClusterIntoRunningState(clusterID, client)
if err != nil {
return err
}
containerName := d.Get("container_name").(string)
storageAccountName := d.Get("storage_account_name").(string)
directory := d.Get("directory").(string)
Expand All @@ -143,6 +141,16 @@ func resourceAzureAdlsGen2Read(d *schema.ResourceData, m interface{}) error {
clientSecretKey := d.Get("client_secret_key").(string)
initializeFileSystem := d.Get("initialize_file_system").(bool)

err := changeClusterIntoRunningState(clusterID, client)
if err != nil {
if isClusterMissing(err.Error(), clusterID) {
log.Printf("Unable to refresh mount '%s' as cluster '%s' is missing", mountName, clusterID)
d.SetId("")
return nil
}
return err
}

adlsGen2Mount := NewAzureADLSGen2Mount(containerName, storageAccountName, directory, mountName, clientID, tenantID,
clientSecretScope, clientSecretKey, initializeFileSystem)

Expand Down
52 changes: 50 additions & 2 deletions databricks/resource_databricks_azure_adls_gen2_mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ import (
"regexp"
"testing"

"github.com/databrickslabs/databricks-terraform/client/model"
"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/terraform"
"github.com/stretchr/testify/assert"
)

func TestAccAzureAdlsGen2Mount_correctly_mounts(t *testing.T) {
terraformToApply := testAccAzureAdlsGen2Mount_correctly_mounts()

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Expand All @@ -22,6 +27,29 @@ func TestAccAzureAdlsGen2Mount_correctly_mounts(t *testing.T) {
})
}

func TestAccAzureAdlsGen2Mount_cluster_deleted_correctly_mounts(t *testing.T) {
terraformToApply := testAccAzureAdlsGen2Mount_correctly_mounts()
var cluster model.ClusterInfo

resource.Test(t, resource.TestCase{
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: terraformToApply,
Check: testClusterResourceExists("databricks_cluster.cluster", &cluster, t),
},
{
PreConfig: func() {
client := testAccProvider.Meta().(*service.DBApiClient)
err := client.Clusters().Delete(cluster.ClusterID)
assert.NoError(t, err, err)
},
Config: terraformToApply,
},
},
})
}

func TestAccAzureAdlsGen2Mount_capture_error(t *testing.T) {
terraformToApply := testAccAzureAdlsGen2Mount_capture_error()

Expand All @@ -39,7 +67,7 @@ func TestAccAzureAdlsGen2Mount_capture_error(t *testing.T) {
}

func testAccAzureAdlsGen2Mount_correctly_mounts() string {
clientID := os.Getenv("ARM_CLIENT_ID")
clientID := os.Getenv("DATABRICKS_AZURE_CLIENT_ID")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reason for changing this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmmmmm, i think this was when i was struggling to get the tests running as the .env wasn't applying and clientID was being set to null causing tests to fail.

I since figured out the issue there so can probably roll this back. The actual values of ARM_CLIENT_ID and DATABRICKS_AZURE_CLIENT_ID are the same though.

Will revert this change :)

clientSecret := os.Getenv("ARM_CLIENT_SECRET")
tenantID := os.Getenv("ARM_TENANT_ID")
subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID")
Expand Down Expand Up @@ -103,7 +131,7 @@ func testAccAzureAdlsGen2Mount_correctly_mounts() string {
}

func testAccAzureAdlsGen2Mount_capture_error() string {
clientID := os.Getenv("ARM_CLIENT_ID")
clientID := os.Getenv("DATABRICKS_AZURE_CLIENT_ID")
clientSecret := os.Getenv("ARM_CLIENT_SECRET")
tenantID := os.Getenv("ARM_TENANT_ID")
subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID")
Expand Down Expand Up @@ -165,3 +193,23 @@ func testAccAzureAdlsGen2Mount_capture_error() string {
`, clientID, clientSecret, tenantID, subscriptionID, workspaceName, resourceGroupName, managedResourceGroupName, location, gen2AdalName)
return definition
}

// testClusterResourceExists queries the API and retrieves the matching Cluster.
func testClusterResourceExists(n string, cluster *model.ClusterInfo, t *testing.T) resource.TestCheckFunc {
return func(s *terraform.State) error {
// find the corresponding state object
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("Not found: %s", n)
}

conn := testAccProvider.Meta().(*service.DBApiClient)
resp, err := conn.Clusters().Get(rs.Primary.ID)
if err != nil {
return err
}

*cluster = resp
return nil
}
}
18 changes: 13 additions & 5 deletions databricks/resource_databricks_azure_blob_mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package databricks

import (
"fmt"
"log"
"strings"

"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"strings"
)

func resourceAzureBlobMount() *schema.Resource {
Expand Down Expand Up @@ -121,10 +123,6 @@ func resourceAzureBlobMountCreate(d *schema.ResourceData, m interface{}) error {
func resourceAzureBlobMountRead(d *schema.ResourceData, m interface{}) error {
client := m.(*service.DBApiClient)
clusterID := d.Get("cluster_id").(string)
err := changeClusterIntoRunningState(clusterID, client)
if err != nil {
return err
}
containerName := d.Get("container_name").(string)
storageAccountName := d.Get("storage_account_name").(string)
directory := d.Get("directory").(string)
Expand All @@ -133,6 +131,16 @@ func resourceAzureBlobMountRead(d *schema.ResourceData, m interface{}) error {
tokenSecretScope := d.Get("token_secret_scope").(string)
tokenSecretKey := d.Get("token_secret_key").(string)

err := changeClusterIntoRunningState(clusterID, client)
if err != nil {
if isClusterMissing(err.Error(), clusterID) {
log.Printf("Unable to refresh mount '%s' as cluster '%s' is missing", mountName, clusterID)
d.SetId("")
return nil
}
return err
}

blobMount := NewAzureBlobMount(containerName, storageAccountName, directory, mountName, authType,
tokenSecretScope, tokenSecretKey)

Expand Down
30 changes: 30 additions & 0 deletions databricks/resource_databricks_azure_blob_mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,36 @@ func TestAccAzureBlobMount_correctly_mounts(t *testing.T) {
})
}

func TestAccAzureBlobMount_cluster_deleted_correctly_mounts(t *testing.T) {
terraformToApply := testAccAzureBlobMount_correctly_mounts()
var clusterInfo model.ClusterInfo
var azureBlobMount AzureBlobMount

resource.Test(t, resource.TestCase{
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: terraformToApply,
Check: resource.ComposeTestCheckFunc(
testAccAzureBlobMount_cluster_exists("databricks_cluster.cluster", &clusterInfo),
testAccAzureBlobMount_mount_exists("databricks_azure_blob_mount.mount", &azureBlobMount, &clusterInfo),
),
},
{
PreConfig: func() {
client := testAccProvider.Meta().(*service.DBApiClient)
err := client.Clusters().Delete(clusterInfo.ClusterID)
assert.NoError(t, err, err)
},
Config: terraformToApply,
Check: resource.ComposeTestCheckFunc(
testAccAzureBlobMount_mount_exists("databricks_azure_blob_mount.mount", &azureBlobMount, &clusterInfo),
),
},
},
})
}

func testAccAzureBlobMount_cluster_exists(n string, clusterInfo *model.ClusterInfo) resource.TestCheckFunc {
return func(s *terraform.State) error {
// find the corresponding state object
Expand Down
13 changes: 4 additions & 9 deletions databricks/resource_databricks_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package databricks

import (
"errors"
"fmt"
"github.com/databrickslabs/databricks-terraform/client/model"
"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"log"
"strings"
"time"

"github.com/databrickslabs/databricks-terraform/client/model"
"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func resourceCluster() *schema.Resource {
Expand Down Expand Up @@ -1453,8 +1453,3 @@ func getMapFromOneItemSet(input interface{}) map[string]interface{} {
}
return nil
}

func isClusterMissing(errorMsg, resourceID string) bool {
return strings.Contains(errorMsg, "INVALID_PARAMETER_VALUE") &&
strings.Contains(errorMsg, fmt.Sprintf("Cluster %s does not exist", resourceID))
}
10 changes: 8 additions & 2 deletions databricks/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package databricks

import (
"fmt"
"github.com/databrickslabs/databricks-terraform/client/model"
"github.com/databrickslabs/databricks-terraform/client/service"
"strings"
"time"

"github.com/databrickslabs/databricks-terraform/client/model"
"github.com/databrickslabs/databricks-terraform/client/service"
)

func changeClusterIntoRunningState(clusterID string, client *service.DBApiClient) error {
Expand Down Expand Up @@ -68,3 +69,8 @@ func changeClusterIntoRunningState(clusterID string, client *service.DBApiClient
return fmt.Errorf("cluster is in a non recoverable state: %s", currentState)

}

func isClusterMissing(errorMsg, resourceID string) bool {
return strings.Contains(errorMsg, "INVALID_PARAMETER_VALUE") &&
strings.Contains(errorMsg, fmt.Sprintf("Cluster %s does not exist", resourceID))
}
31 changes: 31 additions & 0 deletions databricks/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package databricks

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestIsClusterMissingTrueWhenClusterIdSpecifiedPresent(t *testing.T) {
errorMessage := "{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Cluster 123 does not exist\"}"

result := isClusterMissing(errorMessage, "123")

assert.True(t, result)
}

func TestIsClusterMissingFalseWhenClusterIdSpecifiedNotPresent(t *testing.T) {
errorMessage := "{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Cluster 123 does not exist\"}"

result := isClusterMissing(errorMessage, "xyz")

assert.False(t, result)
}

func TestIsClusterMissingFalseWhenErrorNotInCorrectFormat(t *testing.T) {
errorMessage := "{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Something random went bang xyz\"}"

result := isClusterMissing(errorMessage, "xyz")

assert.False(t, result)
}