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.

Sign up for GitHub

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

provider/azurerm: Fix VHD deletion when VM and Storage account are in separate resource groups #9631

Merged
merged 7 commits into from
Oct 27, 2016
Merged

provider/azurerm: Fix VHD deletion when VM and Storage account are in separate resource groups #9631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9f0c267
Improve VHD deletion test when storage account is in different resour…
Oct 26, 2016
9177a1d
Improve messaging when storage account isn't found.
Oct 26, 2016
bafdf10
Add client for finding resources when you don't know it's resource gr…
Oct 26, 2016
40e4b86
Add function to find Storage Account resource group name.
Oct 26, 2016
82929bd
Use the storage account resource group, not the virtual machine's res…
Oct 26, 2016
70d8b5e
Add description of storage account ID for clarity.
Oct 26, 2016
a068da0
Use common function for ID parsing of storage account.
Oct 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions builtin/providers/azurerm/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type ArmClient struct {
providers resources.ProvidersClient
resourceGroupClient resources.GroupsClient
tagsClient resources.TagsClient
resourceFindClient resources.Client

jobsClient scheduler.JobsClient
jobsCollectionsClient scheduler.JobCollectionsClient
Expand Down Expand Up @@ -314,6 +315,12 @@ func (c *Config) getArmClient() (*ArmClient, error) {
tc.Sender = autorest.CreateSender(withRequestLogging())
client.tagsClient = tc

rf := resources.NewClient(c.SubscriptionID)
setUserAgent(&rf.Client)
rf.Authorizer = spt
rf.Sender = autorest.CreateSender(withRequestLogging())
client.resourceFindClient = rf

jc := scheduler.NewJobsClient(c.SubscriptionID)
setUserAgent(&jc.Client)
jc.Authorizer = spt
Expand Down
39 changes: 34 additions & 5 deletions builtin/providers/azurerm/resource_arm_virtual_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ func resourceArmVirtualMachineDelete(d *schema.ResourceData, meta interface{}) e
return fmt.Errorf("Error expanding OS Disk: %s", err)
}

if err = resourceArmVirtualMachineDeleteVhd(*osDisk.Vhd.URI, resGroup, meta); err != nil {
if err = resourceArmVirtualMachineDeleteVhd(*osDisk.Vhd.URI, meta); err != nil {
return fmt.Errorf("Error deleting OS Disk VHD: %s", err)
}
}
Expand All @@ -664,7 +664,7 @@ func resourceArmVirtualMachineDelete(d *schema.ResourceData, meta interface{}) e
}

for _, disk := range disks {
if err = resourceArmVirtualMachineDeleteVhd(*disk.Vhd.URI, resGroup, meta); err != nil {
if err = resourceArmVirtualMachineDeleteVhd(*disk.Vhd.URI, meta); err != nil {
return fmt.Errorf("Error deleting Data Disk VHD: %s", err)
}
}
Expand All @@ -673,7 +673,7 @@ func resourceArmVirtualMachineDelete(d *schema.ResourceData, meta interface{}) e
return nil
}

func resourceArmVirtualMachineDeleteVhd(uri, resGroup string, meta interface{}) error {
func resourceArmVirtualMachineDeleteVhd(uri string, meta interface{}) error {
vhdURL, err := url.Parse(uri)
if err != nil {
return fmt.Errorf("Cannot parse Disk VHD URI: %s", err)
Expand All @@ -685,13 +685,18 @@ func resourceArmVirtualMachineDeleteVhd(uri, resGroup string, meta interface{})
containerName := path[0]
blobName := path[1]

blobClient, saExists, err := meta.(*ArmClient).getBlobStorageClientForStorageAccount(resGroup, storageAccountName)
storageAccountResourceGroupName, err := findStorageAccountResourceGroup(meta, storageAccountName)
if err != nil {
return fmt.Errorf("Error finding resource group for storage account %s: %s", storageAccountName, err)
}

blobClient, saExists, err := meta.(*ArmClient).getBlobStorageClientForStorageAccount(storageAccountResourceGroupName, storageAccountName)
if err != nil {
return fmt.Errorf("Error creating blob store client for VHD deletion: %s", err)
}

if !saExists {
log.Printf("[INFO] Storage Account %q doesn't exist so the VHD blob won't exist", storageAccountName)
log.Printf("[INFO] Storage Account %q in resource group %q doesn't exist so the VHD blob won't exist", storageAccountName, storageAccountResourceGroupName)
return nil
}

Expand Down Expand Up @@ -1276,3 +1281,27 @@ func expandAzureRmVirtualMachineOsDisk(d *schema.ResourceData) (*compute.OSDisk,

return osDisk, nil
}

func findStorageAccountResourceGroup(meta interface{}, storageAccountName string) (string, error) {
client := meta.(*ArmClient).resourceFindClient
filter := fmt.Sprintf("name eq '%s' and resourceType eq 'Microsoft.Storage/storageAccounts'", storageAccountName)
expand := ""
var pager *int32

rf, err := client.List(filter, expand, pager)
if err != nil {
return "", fmt.Errorf("Error making resource request for query %s: %s", filter, err)
}

results := *rf.Value
if len(results) != 1 {
return "", fmt.Errorf("Wrong number of results making resource request for query %s: %s", filter, len(results))
}

id, err := parseAzureResourceID(*results[0].ID)
if err != nil {
return "", err
}

return id.ResourceGroup, nil
}
71 changes: 66 additions & 5 deletions builtin/providers/azurerm/resource_arm_virtual_machine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ func TestAccAzureRMVirtualMachine_deleteVHDOptOut(t *testing.T) {
func TestAccAzureRMVirtualMachine_deleteVHDOptIn(t *testing.T) {
var vm compute.VirtualMachine
ri := acctest.RandInt()
preConfig := fmt.Sprintf(testAccAzureRMVirtualMachine_basicLinuxMachineDestroyDisks, ri, ri, ri, ri, ri, ri, ri)
postConfig := fmt.Sprintf(testAccAzureRMVirtualMachine_basicLinuxMachineDeleteVM, ri, ri, ri, ri, ri)
preConfig := fmt.Sprintf(testAccAzureRMVirtualMachine_basicLinuxMachineDestroyDisksBefore, ri, ri, ri, ri, ri, ri, ri, ri)
postConfig := fmt.Sprintf(testAccAzureRMVirtualMachine_basicLinuxMachineDestroyDisksAfter, ri, ri, ri, ri, ri, ri)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Expand Down Expand Up @@ -633,12 +633,17 @@ resource "azurerm_virtual_machine" "test" {
}
`

var testAccAzureRMVirtualMachine_basicLinuxMachineDestroyDisks = `
var testAccAzureRMVirtualMachine_basicLinuxMachineDestroyDisksBefore = `
resource "azurerm_resource_group" "test" {
name = "acctestRG-%d"
location = "West US"
}

resource "azurerm_resource_group" "test-sa" {
name = "acctestRG-sa-%d"
location = "West US"
}

resource "azurerm_virtual_network" "test" {
name = "acctvn-%d"
address_space = ["10.0.0.0/16"]
Expand Down Expand Up @@ -667,7 +672,7 @@ resource "azurerm_network_interface" "test" {

resource "azurerm_storage_account" "test" {
name = "accsa%d"
resource_group_name = "${azurerm_resource_group.test.name}"
resource_group_name = "${azurerm_resource_group.test-sa.name}"
location = "westus"
account_type = "Standard_LRS"

Expand All @@ -678,7 +683,7 @@ resource "azurerm_storage_account" "test" {

resource "azurerm_storage_container" "test" {
name = "vhds"
resource_group_name = "${azurerm_resource_group.test.name}"
resource_group_name = "${azurerm_resource_group.test-sa.name}"
storage_account_name = "${azurerm_storage_account.test.name}"
container_access_type = "private"
}
Expand Down Expand Up @@ -733,6 +738,62 @@ resource "azurerm_virtual_machine" "test" {
}
`

var testAccAzureRMVirtualMachine_basicLinuxMachineDestroyDisksAfter = `
resource "azurerm_resource_group" "test" {
name = "acctestRG-%d"
location = "West US"
}

resource "azurerm_resource_group" "test-sa" {
name = "acctestRG-sa-%d"
location = "West US"
}

resource "azurerm_virtual_network" "test" {
name = "acctvn-%d"
address_space = ["10.0.0.0/16"]
location = "West US"
resource_group_name = "${azurerm_resource_group.test.name}"
}

resource "azurerm_subnet" "test" {
name = "acctsub-%d"
resource_group_name = "${azurerm_resource_group.test.name}"
virtual_network_name = "${azurerm_virtual_network.test.name}"
address_prefix = "10.0.2.0/24"
}

resource "azurerm_network_interface" "test" {
name = "acctni-%d"
location = "West US"
resource_group_name = "${azurerm_resource_group.test.name}"

ip_configuration {
name = "testconfiguration1"
subnet_id = "${azurerm_subnet.test.id}"
private_ip_address_allocation = "dynamic"
}
}

resource "azurerm_storage_account" "test" {
name = "accsa%d"
resource_group_name = "${azurerm_resource_group.test-sa.name}"
location = "westus"
account_type = "Standard_LRS"

tags {
environment = "staging"
}
}

resource "azurerm_storage_container" "test" {
name = "vhds"
resource_group_name = "${azurerm_resource_group.test-sa.name}"
storage_account_name = "${azurerm_storage_account.test.name}"
container_access_type = "private"
}
`

var testAccAzureRMVirtualMachine_basicLinuxMachineDeleteVM = `
resource "azurerm_resource_group" "test" {
name = "acctestRG-%d"
Expand Down