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

Jump to bottom

Use disk API to load managed disk info when VM is stopped. #1100

Merged
merged 6 commits into from
Apr 23, 2018
Merged

Use disk API to load managed disk info when VM is stopped. #1100

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
30d7d0a
Use disk API to load managed disk info when VM is stopped.
Apr 10, 2018
913171e
Abstract the storage revise procedures according to the comments.
Apr 13, 2018
ed82631
Add more status codes to check for deallocation.
Apr 13, 2018
d7fe03d
Update managed disk information when flattening.
Apr 19, 2018
b9d9347
Add test case for disk info of a deallocated virtual machine
Apr 20, 2018
9a43ac3
Fixing comments identified in the PR:
tombuildsstuff Apr 23, 2018
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
85 changes: 78 additions & 7 deletions azurerm/resource_arm_virtual_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,6 @@ func resourceArmVirtualMachineRead(d *schema.ResourceData, meta interface{}) err
name := id.Path["virtualMachines"]

resp, err := vmClient.Get(ctx, resGroup, name, "")

if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
d.SetId("")
Expand All @@ -641,6 +640,21 @@ func resourceArmVirtualMachineRead(d *schema.ResourceData, meta interface{}) err
return fmt.Errorf("Error making Read request on Azure Virtual Machine %s: %+v", name, err)
}

instance, err := vmClient.InstanceView(ctx, resGroup, name)
if err != nil {
return fmt.Errorf("Error making InstanceView request on Azure Virtual Machine %s: %+v", name, err)
}

stopped := false
if instance.Statuses != nil {
for _, status := range *instance.Statuses {
if status.Code != nil && *status.Code == "PowerState/deallocated" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Azure's API's have a tendency to return things in different casings (particularly for older resources which haven't been modified recently), as such can we check this in a case insensitive manner?

Copy link
Contributor

@metacpp metacpp Apr 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Please do experimentation or check with service team: != running or == deallocated.

Copy link
Author

Choose a reason for hiding this comment

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

Tested for "PowerState/deallocating", "PowerState/deallocated" and "PowerState/starting". Other statuses didn't appear, but I would still add them for safety.

stopped = true
break
}
}
}

d.Set("name", resp.Name)
d.Set("resource_group_name", resGroup)
d.Set("zones", resp.Zones)
Expand Down Expand Up @@ -668,12 +682,20 @@ func resourceArmVirtualMachineRead(d *schema.ResourceData, meta interface{}) err
}
}

if err := d.Set("storage_os_disk", flattenAzureRmVirtualMachineOsDisk(resp.VirtualMachineProperties.StorageProfile.OsDisk)); err != nil {
osDisks, err := flattenAzureRmVirtualMachineOsDisk(resp.VirtualMachineProperties.StorageProfile.OsDisk, stopped, meta)
if err != nil {
return fmt.Errorf("[DEBUG] Error setting Virtual Machine Storage OS Disk error: %#v", err)
}
if err = d.Set("storage_os_disk", osDisks); err != nil {
return fmt.Errorf("[DEBUG] Error setting Virtual Machine Storage OS Disk error: %#v", err)
}

if resp.VirtualMachineProperties.StorageProfile.DataDisks != nil {
if err := d.Set("storage_data_disk", flattenAzureRmVirtualMachineDataDisk(resp.VirtualMachineProperties.StorageProfile.DataDisks)); err != nil {
dataDisks, err := flattenAzureRmVirtualMachineDataDisk(resp.VirtualMachineProperties.StorageProfile.DataDisks, stopped, meta)
if err != nil {
return fmt.Errorf("[DEBUG] Error setting Virtual Machine Storage Data Disks error: %#v", err)
}
if err = d.Set("storage_data_disk", dataDisks); err != nil {
return fmt.Errorf("[DEBUG] Error setting Virtual Machine Storage Data Disks error: %#v", err)
}
}
Expand Down Expand Up @@ -957,7 +979,27 @@ func flattenAzureRmVirtualMachineOsProfileSecrets(secrets *[]compute.VaultSecret
return result
}

func flattenAzureRmVirtualMachineDataDisk(disks *[]compute.DataDisk) interface{} {
func getStoppedVMManagedDiskInfo(idStr string, meta interface{}) (*compute.Disk, error) {
client := meta.(*ArmClient).diskClient
ctx := meta.(*ArmClient).StopContext

id, err := parseAzureResourceID(idStr)
if err != nil {
return nil, err
}

resGroup := id.ResourceGroup
name := id.Path["disks"]

resp, err := client.Get(ctx, resGroup, name)
if err != nil {
return nil, fmt.Errorf("[ERROR] Error making Read request on stopped VM Azure Managed Disk %s (resource group %s): %s", name, resGroup, err)
}

return &resp, nil
}

func flattenAzureRmVirtualMachineDataDisk(disks *[]compute.DataDisk, stopped bool, meta interface{}) (interface{}, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is taking more responsibility, we may make it simpler and only do flatten work.

result := make([]interface{}, len(*disks))
for i, disk := range *disks {
l := make(map[string]interface{})
Expand All @@ -976,9 +1018,23 @@ func flattenAzureRmVirtualMachineDataDisk(disks *[]compute.DataDisk) interface{}
}
l["lun"] = *disk.Lun

if stopped && disk.ManagedDisk != nil && disk.ManagedDisk.ID != nil {
diskInfo, err := getStoppedVMManagedDiskInfo(*disk.ManagedDisk.ID, meta)
Copy link
Contributor

Choose a reason for hiding this comment

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

thinking about it - would it be worth pulling this regardless of the VM state if managed disks are used?

if err != nil {
return nil, err
}
if diskInfo.DiskProperties != nil {
if diskSize := diskInfo.DiskProperties.DiskSizeGB; diskSize != nil {
l["disk_size_gb"] = *diskSize
}
}
if diskInfo.Sku != nil {
l["managed_disk_type"] = string(diskInfo.Sku.Name)
}
}
result[i] = l
}
return result
return result, nil
}

func flattenAzureRmVirtualMachineOsProfile(input *compute.OSProfile) []interface{} {
Expand Down Expand Up @@ -1064,7 +1120,7 @@ func flattenAzureRmVirtualMachineOsProfileLinuxConfiguration(config *compute.Lin
return []interface{}{result}
}

func flattenAzureRmVirtualMachineOsDisk(disk *compute.OSDisk) []interface{} {
func flattenAzureRmVirtualMachineOsDisk(disk *compute.OSDisk, stopped bool, meta interface{}) ([]interface{}, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same suggestion as above.

result := make(map[string]interface{})
result["name"] = *disk.Name
if disk.Vhd != nil {
Expand All @@ -1083,7 +1139,22 @@ func flattenAzureRmVirtualMachineOsDisk(disk *compute.OSDisk) []interface{} {
}
result["os_type"] = string(disk.OsType)

return []interface{}{result}
if stopped && disk.ManagedDisk != nil && disk.ManagedDisk.ID != nil {
diskInfo, err := getStoppedVMManagedDiskInfo(*disk.ManagedDisk.ID, meta)
Copy link
Contributor

Choose a reason for hiding this comment

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

(as above) I think we could probably retrieve this from this API all the time, rather than just when it's stopped?

if err != nil {
return nil, err
}
if diskInfo.DiskProperties != nil {
if diskSize := diskInfo.DiskProperties.DiskSizeGB; diskSize != nil {
result["disk_size_gb"] = *diskSize
}
}
if diskInfo.Sku != nil {
result["managed_disk_type"] = string(diskInfo.Sku.Name)
}
}

return []interface{}{result}, nil
}

func expandAzureRmVirtualMachinePlan(d *schema.ResourceData) (*compute.Plan, error) {
Expand Down