From 54fd5b03627019dbf409a82629403b2b923a0ec0 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Mon, 14 May 2018 16:55:46 -0700 Subject: [PATCH] Removing the `create_option` field --- azurerm/disks.go | 40 ++++++++++ azurerm/resource_arm_managed_disk.go | 2 +- azurerm/resource_arm_virtual_machine.go | 26 ++----- ...rm_virtual_machine_data_disk_attachment.go | 75 ++++++++++++++++--- ...rtual_machine_data_disk_attachment_test.go | 5 -- 5 files changed, 111 insertions(+), 37 deletions(-) create mode 100644 azurerm/disks.go diff --git a/azurerm/disks.go b/azurerm/disks.go new file mode 100644 index 000000000000..6d975221fd2f --- /dev/null +++ b/azurerm/disks.go @@ -0,0 +1,40 @@ +package azurerm + +import ( + "fmt" + "net/url" + "strings" +) + +type UnmanagedDiskMetadata struct { + StorageContainerName string + StorageAccountName string + BlobName string + ResourceGroupName string +} + +func storageAccountNameForUnmanagedDisk(uri string, meta interface{}) (*UnmanagedDiskMetadata, error) { + vhdURL, err := url.Parse(uri) + if err != nil { + return nil, fmt.Errorf("Cannot parse Disk VHD URI: %s", err) + } + + // VHD URI is in the form: https://storageAccountName.blob.core.windows.net/containerName/blobName + storageAccountName := strings.Split(vhdURL.Host, ".")[0] + path := strings.Split(strings.TrimPrefix(vhdURL.Path, "/"), "/") + containerName := path[0] + blobName := path[1] + + resourceGroupName, err := findStorageAccountResourceGroup(meta, storageAccountName) + if err != nil { + return nil, fmt.Errorf("Error finding resource group for storage account %s: %+v", storageAccountName, err) + } + + metadata := UnmanagedDiskMetadata{ + BlobName: blobName, + StorageContainerName: containerName, + StorageAccountName: storageAccountName, + ResourceGroupName: resourceGroupName, + } + return &metadata, nil +} diff --git a/azurerm/resource_arm_managed_disk.go b/azurerm/resource_arm_managed_disk.go index c3bf268ba9ce..055f2543b69e 100644 --- a/azurerm/resource_arm_managed_disk.go +++ b/azurerm/resource_arm_managed_disk.go @@ -137,7 +137,7 @@ func resourceArmManagedDiskCreate(d *schema.ResourceData, meta interface{}) erro OsType: compute.OperatingSystemTypes(osType), }, Sku: &compute.DiskSku{ - Name: (skuName), + Name: skuName, }, Tags: expandedTags, Zones: zones, diff --git a/azurerm/resource_arm_virtual_machine.go b/azurerm/resource_arm_virtual_machine.go index cd578db3fbf6..19e38df7aec2 100644 --- a/azurerm/resource_arm_virtual_machine.go +++ b/azurerm/resource_arm_virtual_machine.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "log" - "net/url" "strings" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2017-12-01/compute" @@ -824,38 +823,27 @@ func resourceArmVirtualMachineDelete(d *schema.ResourceData, meta interface{}) e } func resourceArmVirtualMachineDeleteVhd(uri string, meta interface{}) error { - vhdURL, err := url.Parse(uri) + metadata, err := storageAccountNameForUnmanagedDisk(uri, meta) if err != nil { - return fmt.Errorf("Cannot parse Disk VHD URI: %s", err) - } - - // VHD URI is in the form: https://storageAccountName.blob.core.windows.net/containerName/blobName - storageAccountName := strings.Split(vhdURL.Host, ".")[0] - path := strings.Split(strings.TrimPrefix(vhdURL.Path, "/"), "/") - containerName := path[0] - blobName := path[1] - - storageAccountResourceGroupName, err := findStorageAccountResourceGroup(meta, storageAccountName) - if err != nil { - return fmt.Errorf("Error finding resource group for storage account %s: %+v", storageAccountName, err) + return err } armClient := meta.(*ArmClient) ctx := armClient.StopContext - blobClient, saExists, err := armClient.getBlobStorageClientForStorageAccount(ctx, storageAccountResourceGroupName, storageAccountName) + blobClient, saExists, err := armClient.getBlobStorageClientForStorageAccount(ctx, metadata.ResourceGroupName, metadata.StorageAccountName) if err != nil { return fmt.Errorf("Error creating blob store client for VHD deletion: %+v", err) } if !saExists { - log.Printf("[INFO] Storage Account %q in resource group %q doesn't exist so the VHD blob won't exist", storageAccountName, storageAccountResourceGroupName) + log.Printf("[INFO] Storage Account %q in resource group %q doesn't exist so the VHD blob won't exist", metadata.StorageAccountName, metadata.ResourceGroupName) return nil } - log.Printf("[INFO] Deleting VHD blob %s", blobName) - container := blobClient.GetContainerReference(containerName) - blob := container.GetBlobReference(blobName) + log.Printf("[INFO] Deleting VHD blob %s", metadata.BlobName) + container := blobClient.GetContainerReference(metadata.StorageContainerName) + blob := container.GetBlobReference(metadata.BlobName) options := &storage.DeleteBlobOptions{} err = blob.Delete(options) if err != nil { diff --git a/azurerm/resource_arm_virtual_machine_data_disk_attachment.go b/azurerm/resource_arm_virtual_machine_data_disk_attachment.go index 94c666bad50d..ce5de59cc2e7 100644 --- a/azurerm/resource_arm_virtual_machine_data_disk_attachment.go +++ b/azurerm/resource_arm_virtual_machine_data_disk_attachment.go @@ -6,6 +6,8 @@ import ( "log" "strings" + "context" + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2017-12-01/compute" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" @@ -35,13 +37,6 @@ func resourceArmVirtualMachineDataDiskAttachment() *schema.Resource { ForceNew: true, }, - // TODO: can we remove this option, in favour of looking up if the disk exists? - "create_option": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: ignoreCaseDiffSuppressFunc, - }, - "lun": { Type: schema.TypeInt, Required: true, @@ -115,13 +110,11 @@ func resourceArmVirtualMachineDataDiskAttachmentCreateUpdate(d *schema.ResourceD } name := d.Get("name").(string) - createOption := d.Get("create_option").(string) lun := int32(d.Get("lun").(int)) expandedDisk := compute.DataDisk{ - Name: utils.String(name), - Lun: utils.Int32(lun), - CreateOption: compute.DiskCreateOptionTypes(createOption), + Name: utils.String(name), + Lun: utils.Int32(lun), } if v, ok := d.GetOk("vhd_uri"); ok { expandedDisk.Vhd = &compute.VirtualHardDisk{ @@ -146,6 +139,14 @@ func resourceArmVirtualMachineDataDiskAttachmentCreateUpdate(d *schema.ResourceD expandedDisk.DiskSizeGB = utils.Int32(int32(v.(int))) } + createOption, err := determineAzureDataDiskCreateOption(ctx, meta, expandedDisk) + if err != nil { + return fmt.Errorf("Error determining Create Option for Data Disk %q: %+v", name, err) + } + if createOption != nil { + expandedDisk.CreateOption = compute.DiskCreateOptionTypes(*createOption) + } + disks := *virtualMachine.StorageProfile.DataDisks if !d.IsNewResource() { // find the existing disk, remove it from the array @@ -228,7 +229,6 @@ func resourceArmVirtualMachineDataDiskAttachmentRead(d *schema.ResourceData, met d.Set("managed_disk_type", string(managedDisk.StorageAccountType)) } - d.Set("create_option", string(disk.CreateOption)) d.Set("caching", string(disk.Caching)) if diskSizeGb := disk.DiskSizeGB; diskSizeGb != nil { d.Set("disk_size_gb", int(*diskSizeGb)) @@ -286,3 +286,54 @@ func resourceArmVirtualMachineDataDiskAttachmentDelete(d *schema.ResourceData, m return nil } + +func determineAzureDataDiskCreateOption(ctx context.Context, meta interface{}, disk compute.DataDisk) (*string, error) { + attach := string(compute.DiskCreateOptionTypesAttach) + + if disk.ManagedDisk != nil && disk.ManagedDisk.ID != nil { + diskId := *disk.ManagedDisk.ID + id, err := parseAzureResourceID(diskId) + if err != nil { + return nil, fmt.Errorf("Error parsing Managed Disk Resource ID %q: %+v", diskId, err) + } + + client := meta.(*ArmClient).diskClient + resourceGroup := id.ResourceGroup + name := id.Path["disks"] + + _, err = client.Get(ctx, resourceGroup, name) + if err != nil { + return nil, fmt.Errorf("Error loading Data Disk %q (Resource Group %q): %+v", name, resourceGroup, err) + } + + return &attach, nil + } + + if disk.Vhd != nil && disk.Vhd.URI != nil { + diskId := *disk.Vhd.URI + metadata, err := storageAccountNameForUnmanagedDisk(diskId, meta) + if err != nil { + return nil, fmt.Errorf("Error locating Storage Account for Unmanaged Disk %q: %+v", diskId, err) + } + + client := meta.(*ArmClient) + storageClient, _, err := client.getBlobStorageClientForStorageAccount(ctx, metadata.ResourceGroupName, metadata.StorageAccountName) + if err != nil { + return nil, fmt.Errorf("Error getting Blob Storage Client for Account %q (Resource Group %q): %+v", metadata.StorageAccountName, metadata.ResourceGroupName, err) + } + + container := storageClient.GetContainerReference(metadata.StorageContainerName) + blob := container.GetBlobReference(metadata.BlobName) + exists, err := blob.Exists() + if err != nil { + return nil, fmt.Errorf("Error locating Blob for Unmanaged Disk %q: %+v", diskId, err) + } + + if exists { + return &attach, nil + } + } + + empty := string(compute.DiskCreateOptionTypesEmpty) + return &empty, nil +} diff --git a/azurerm/resource_arm_virtual_machine_data_disk_attachment_test.go b/azurerm/resource_arm_virtual_machine_data_disk_attachment_test.go index dafc1a1bc675..7e34c5c18e80 100644 --- a/azurerm/resource_arm_virtual_machine_data_disk_attachment_test.go +++ b/azurerm/resource_arm_virtual_machine_data_disk_attachment_test.go @@ -301,7 +301,6 @@ resource "azurerm_virtual_machine_data_disk_attachment" "test" { name = "disk1-%d" virtual_machine_id = "${azurerm_virtual_machine.test.id}" vhd_uri = "${azurerm_storage_account.test.primary_blob_endpoint}${azurerm_storage_container.test.name}/mydatadisk1.vhd" - create_option = "Empty" disk_size_gb = 10 lun = 1 } @@ -377,7 +376,6 @@ resource "azurerm_virtual_machine" "test" { resource "azurerm_virtual_machine_data_disk_attachment" "test" { name = "disk1-%d" virtual_machine_id = "${azurerm_virtual_machine.test.id}" - create_option = "Empty" managed_disk_type = "Standard_LRS" disk_size_gb = 10 lun = 1 @@ -464,7 +462,6 @@ resource "azurerm_virtual_machine_data_disk_attachment" "test" { name = "${azurerm_managed_disk.test.name}" virtual_machine_id = "${azurerm_virtual_machine.test.id}" managed_disk_id = "${azurerm_managed_disk.test.id}" - create_option = "Attach" managed_disk_type = "Standard_LRS" lun = 1 } @@ -540,7 +537,6 @@ resource "azurerm_virtual_machine" "test" { resource "azurerm_virtual_machine_data_disk_attachment" "first" { name = "disk1-%d" virtual_machine_id = "${azurerm_virtual_machine.test.id}" - create_option = "Empty" managed_disk_type = "Standard_LRS" disk_size_gb = 10 lun = %d @@ -549,7 +545,6 @@ resource "azurerm_virtual_machine_data_disk_attachment" "first" { resource "azurerm_virtual_machine_data_disk_attachment" "second" { name = "disk2-%d" virtual_machine_id = "${azurerm_virtual_machine.test.id}" - create_option = "Empty" managed_disk_type = "Standard_LRS" disk_size_gb = 20 lun = %d