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

Vm disk feature #412

Merged
merged 117 commits into from
Jan 13, 2020
Merged

Vm disk feature #412

merged 117 commits into from
Jan 13, 2020

Conversation

vbauzys
Copy link
Contributor

@vbauzys vbauzys commented Dec 2, 2019

Ref: vmware/go-vcloud-director#250
Allows to create, update, delete and import VM internal disks.
Also override template disk details before first boot up.

  • new resource vcd_vm_internal_disk
  • vcd_vapp_vm has new structure override_template_disk
  • vcd_vapp_vm has new attribute structure internal_disk

vbauzys added 30 commits July 15, 2019 15:39
Signed-off-by: Vaidotas Bauzys <[email protected]>
Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd
Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd
Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
@@ -191,6 +216,21 @@ example for usage details. **Deprecates**: `network_name`, `ip`, `vapp_network_n

* `ip_allocation_mode=NONE` - **`ip`** field can be omitted or set to an empty string "". Empty string may be useful when doing HCL variable interpolation.

<a id="override-template-disk"></a>
## Override template disk
Allows to update internal disk in template before first VM boot. Disk are matched by `bus_type`, `bus_number` and `unit_number`
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Disk is" or "disks are" ....

Suggested change
Allows to update internal disk in template before first VM boot. Disk are matched by `bus_type`, `bus_number` and `unit_number`
Allows to update internal disk in template before first VM boot. Disk is matched by `bus_type`, `bus_number` and `unit_number`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Copy link
Contributor

@dataclouder dataclouder left a comment

Choose a reason for hiding this comment

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

Some more changes are needed

}
vapp, err := vdc.GetVAppByName(d.Get("vapp_name").(string), false)
if err != nil {
return nil, nil, fmt.Errorf("[Error] failed to get vApp: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, nil, fmt.Errorf("[Error] failed to get vApp: %s", err)
return nil, nil, fmt.Errorf("[getVm] failed to get vApp: %s", err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
vm, err := vapp.GetVMByName(d.Get("vm_name").(string), false)
if err != nil {
return nil, nil, fmt.Errorf("[Error] failed to get VM: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, nil, fmt.Errorf("[Error] failed to get VM: %s", err)
return nil, nil, fmt.Errorf("[getVm] failed to get VM: %s", err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if storageProfileName, ok := d.GetOk("storage_profile"); ok {
storageProfile, err := vdc.FindStorageProfileReference(storageProfileName.(string))
if err != nil {
return fmt.Errorf("[vm creation] error retrieving storage profile %s : %s", storageProfileName, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("[vm creation] error retrieving storage profile %s : %s", storageProfileName, err)
return fmt.Errorf("[internal disk creation] error retrieving storage profile %s : %s", storageProfileName, err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


err = vm.DeleteInternalDisk(d.Id())
if err != nil {
return fmt.Errorf("[Error] failed to delete internal disk: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("[Error] failed to delete internal disk: %s", err)
return fmt.Errorf("[resourceVmInternalDiskDelete] failed to delete internal disk: %s", err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -535,6 +643,7 @@ func expandDisksProperties(v interface{}) ([]diskParams, error) {
func getVmIndependentDisks(vm govcd.VM) []string {

var disks []string
// We use VirtualHardwareSection due in time of implementation we didn't have access to VmSpecSection which we used for internal disks.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// We use VirtualHardwareSection due in time of implementation we didn't have access to VmSpecSection which we used for internal disks.
// We use VirtualHardwareSection because in time of implementation we didn't have access to VmSpecSection which we used for internal disks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return err
}

existingInternalDisks := vm.VM.VmSpecSection.DiskSection.DiskSettings
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a check for VmSpecSection and fail gracefully if it is nil.
If this section is not initialized, we'll get a panic error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return fmt.Errorf(errorRetrievingOrgAndVdc, err)
}

diskSettings := vm.VM.VmSpecSection.DiskSection.DiskSettings
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check if VmSpecSection is not nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

_ = d.Set("size_in_mb", diskSettings.SizeMb)
_ = d.Set("bus_number", diskSettings.BusNumber)
_ = d.Set("unit_number", diskSettings.UnitNumber)
_ = d.Set("thin_provisioned", *diskSettings.ThinProvisioned)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check that diskSettings.ThinProvisioned is not nil before using it in assignment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


writer := tabwriter.NewWriter(getTerraformStdout(), 0, 8, 1, '\t', tabwriter.AlignRight)

fmt.Fprintln(writer, "No\tID\tBusType\tBusNumber\tUnitNumber\tSize\tStoragePofile\tIops\tThinProvisioned")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fmt.Fprintln(writer, "No\tID\tBusType\tBusNumber\tUnitNumber\tSize\tStoragePofile\tIops\tThinProvisioned")
fmt.Fprintln(writer, "No\tID\tBusType\tBusNumber\tUnitNumber\tSize\tStorageProfile\tIops\tThinProvisioned")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

LGTM now, thank you!

Signed-off-by: Vaidotas Bauzys <[email protected]>
Copy link
Contributor

@dataclouder dataclouder left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants