Skip to content

Commit

Permalink
Merge pull request #4205 from terraform-providers/f/storage-account-k…
Browse files Browse the repository at this point in the history
…ey-caching

storage: caching the Resource Group Name / Account Key
  • Loading branch information
tombuildsstuff authored Sep 2, 2019
2 parents d10eb9c + 7b00638 commit 27e7206
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 30 deletions.
46 changes: 46 additions & 0 deletions azurerm/internal/services/storage/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,41 @@ package storage
import (
"context"
"fmt"
"log"
"strings"
"sync"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
)

var (
accountKeysCache = map[string]string{}
resourceGroupNamesCache = map[string]string{}
writeLock = sync.RWMutex{}
)

func (client Client) ClearFromCache(resourceGroup, accountName string) {
writeLock.Lock()

log.Printf("[DEBUG] Removing Account %q (Resource Group %q) from the cache", accountName, resourceGroup)
accountCacheKey := fmt.Sprintf("%s-%s", resourceGroup, accountName)
delete(accountKeysCache, accountCacheKey)

resourceGroupsCacheKey := accountName
delete(resourceGroupNamesCache, resourceGroupsCacheKey)

log.Printf("[DEBUG] Removed Account %q (Resource Group %q) from the cache", accountName, resourceGroup)
writeLock.Unlock()
}

func (client Client) FindResourceGroup(ctx context.Context, accountName string) (*string, error) {
cacheKey := accountName
if v, ok := resourceGroupNamesCache[cacheKey]; ok {
return &v, nil
}

log.Printf("[DEBUG] Cache Miss - looking up the resource group for storage account %q..", accountName)
writeLock.Lock()
accounts, err := client.AccountsClient.List(ctx)
if err != nil {
return nil, fmt.Errorf("Error listing Storage Accounts (to find Resource Group for %q): %s", accountName, err)
Expand All @@ -35,10 +64,23 @@ func (client Client) FindResourceGroup(ctx context.Context, accountName string)
}
}

if resourceGroup != nil {
resourceGroupNamesCache[cacheKey] = *resourceGroup
}

writeLock.Unlock()

return resourceGroup, nil
}

func (client Client) findAccountKey(ctx context.Context, resourceGroup, accountName string) (*string, error) {
cacheKey := fmt.Sprintf("%s-%s", resourceGroup, accountName)
if v, ok := accountKeysCache[cacheKey]; ok {
return &v, nil
}

writeLock.Lock()
log.Printf("[DEBUG] Cache Miss - looking up the account key for storage account %q..", accountName)
props, err := client.AccountsClient.ListKeys(ctx, resourceGroup, accountName)
if err != nil {
return nil, fmt.Errorf("Error Listing Keys for Storage Account %q (Resource Group %q): %+v", accountName, resourceGroup, err)
Expand All @@ -50,5 +92,9 @@ func (client Client) findAccountKey(ctx context.Context, resourceGroup, accountN

keys := *props.Keys
firstKey := keys[0].Value

accountKeysCache[cacheKey] = *firstKey
writeLock.Unlock()

return firstKey, nil
}
6 changes: 5 additions & 1 deletion azurerm/resource_arm_storage_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,8 @@ func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) err

func resourceArmStorageAccountDelete(d *schema.ResourceData, meta interface{}) error {
ctx := meta.(*ArmClient).StopContext
client := meta.(*ArmClient).storage.AccountsClient
storageClient := meta.(*ArmClient).storage
client := storageClient.AccountsClient

id, err := azure.ParseAzureResourceID(d.Id())
if err != nil {
Expand Down Expand Up @@ -1150,6 +1151,9 @@ func resourceArmStorageAccountDelete(d *schema.ResourceData, meta interface{}) e
}
}

// remove this from the cache
storageClient.ClearFromCache(resourceGroup, name)

return nil
}

Expand Down
8 changes: 3 additions & 5 deletions azurerm/resource_arm_storage_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,10 @@ func testCheckAzureRMStorageContainerDestroy(s *terraform.State) error {

props, err := client.GetProperties(ctx, accountName, containerName)
if err != nil {
if utils.ResponseWasNotFound(props.Response) {
return nil
}

return fmt.Errorf("Error retrieving Container %q in Storage Account %q: %s", containerName, accountName, err)
return nil
}

return fmt.Errorf("Container still exists: %+v", props)
}

return nil
Expand Down
10 changes: 3 additions & 7 deletions azurerm/resource_arm_storage_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,16 +213,12 @@ func testCheckAzureRMStorageQueueDestroy(s *terraform.State) error {
return fmt.Errorf("Error building Queues Client: %s", err)
}

metaData, err := queueClient.GetMetaData(ctx, accountName, name)
props, err := queueClient.GetMetaData(ctx, accountName, name)
if err != nil {
if utils.ResponseWasNotFound(metaData.Response) {
return nil
}

return fmt.Errorf("Unexpected error getting MetaData for Queue %q: %s", name, err)
return nil
}

return fmt.Errorf("Bad: Storage Queue %q (storage account: %q) still exists", name, accountName)
return fmt.Errorf("Queue still exists: %+v", props)
}

return nil
Expand Down
6 changes: 2 additions & 4 deletions azurerm/resource_arm_storage_share_directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,10 @@ func testCheckAzureRMStorageShareDirectoryDestroy(s *terraform.State) error {

resp, err := client.Get(ctx, accountName, shareName, name)
if err != nil {
return fmt.Errorf("Bad: Get on FileShareDirectoriesClient: %+v", err)
return nil
}

if resp.StatusCode != http.StatusNotFound {
return fmt.Errorf("File Share still exists:\n%#v", resp)
}
return fmt.Errorf("File Share still exists:\n%#v", resp)
}

return nil
Expand Down
9 changes: 2 additions & 7 deletions azurerm/resource_arm_storage_share_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/hashicorp/terraform/terraform"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

func TestAccAzureRMStorageShare_basic(t *testing.T) {
Expand Down Expand Up @@ -289,14 +288,10 @@ func testCheckAzureRMStorageShareDestroy(s *terraform.State) error {

props, err := client.GetProperties(ctx, accountName, shareName)
if err != nil {
if utils.ResponseWasNotFound(props.Response) {
return nil
}

return fmt.Errorf("Error retrieving Share %q: %s", shareName, accountName)
return nil
}

return fmt.Errorf("Bad: Share %q (storage account: %q) still exists", shareName, accountName)
return fmt.Errorf("Share still exists: %+v", props)
}

return nil
Expand Down
8 changes: 2 additions & 6 deletions azurerm/resource_arm_storage_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,14 +240,10 @@ func testCheckAzureRMStorageTableDestroy(s *terraform.State) error {

props, err := client.Exists(ctx, accountName, tableName)
if err != nil {
if utils.ResponseWasNotFound(props) {
return nil
}

return fmt.Errorf("Error retrieving Table %q: %s", tableName, accountName)
return nil
}

return fmt.Errorf("Bad: Table %q (storage account: %q) still exists", tableName, accountName)
return fmt.Errorf("Table still exists: %+v", props)
}

return nil
Expand Down

0 comments on commit 27e7206

Please sign in to comment.