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

Add computed data to VDC storage profile data source #782

Merged
merged 13 commits into from
Feb 22, 2022

Conversation

vbauzys
Copy link
Contributor

@vbauzys vbauzys commented Feb 7, 2022

ref: #692, #605
dep: vmware/go-vcloud-director#435

Added additional information returned from API to be written to state.

Additional fields:

  • limit
  • used_storage
  • default
  • enabled
  • iops_allocated
  • units
  • iops_settings

IOPS settings details:

  • iops_limiting_enabled
  • maximum_disk_iops
  • default_disk_iops
  • disk_iops_per_gb_max
  • iops_limit

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys vbauzys self-assigned this Feb 7, 2022
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys vbauzys marked this pull request as ready for review February 8, 2022 08:18
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
.changes/v3.6.0/782-improvements.md Outdated Show resolved Hide resolved
vcd/datasource_vcd_storage_profile.go Outdated Show resolved Hide resolved
@@ -653,6 +653,9 @@ func genericResourceVmCreate(d *schema.ResourceData, meta interface{}, vmType ty
vmComputePolicy = nil
}
vmTemplate := vmTemplatefromVappTemplate(d.Get("vm_name_in_template").(string), vappTemplate.VAppTemplate)
if vmTemplate == nil {
return fmt.Errorf("[VM creation] vm template isn't found. Please check vApp templat %s : %s", vmName, err)
Copy link
Collaborator

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] vm template isn't found. Please check vApp templat %s : %s", vmName, err)
return fmt.Errorf("[VM creation] VM template isn't found. Please check vApp template %s : %s", vmName, 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.

fixed

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
vcd/datasource_vcd_storage_profile.go Outdated Show resolved Hide resolved
vcd/datasource_vcd_storage_profile.go Outdated Show resolved Hide resolved
vcd/datasource_vcd_storage_profile.go Outdated Show resolved Hide resolved
website/docs/d/storage_profile.html.markdown Outdated Show resolved Hide resolved
website/docs/d/storage_profile.html.markdown Outdated Show resolved Hide resolved
website/docs/d/storage_profile.html.markdown Outdated Show resolved Hide resolved
* `limit` - Maximum number of storage bytes (scaled by Units) allocated for this profile. A value of 0 is understood to mean `maximum possible`
* `used_storage` - Storage used, in Megabytes, by the storage profile
* `default` - True if this is default storage profile for this vDC. The default storage profile is used when an object that can specify a storage profile is created with no storage profile specified
* `enabled` - True if this storage profile is enabled for use in the vDC"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we use Org and VDC

Suggested change
* `enabled` - True if this storage profile is enabled for use in the vDC"
* `enabled` - True if this storage profile is enabled for use in the VDC"

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


* `limit` - Maximum number of storage bytes (scaled by Units) allocated for this profile. A value of 0 is understood to mean `maximum possible`
* `used_storage` - Storage used, in Megabytes, by the storage profile
* `default` - True if this is default storage profile for this vDC. The default storage profile is used when an object that can specify a storage profile is created with no storage profile specified
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `default` - True if this is default storage profile for this vDC. The default storage profile is used when an object that can specify a storage profile is created with no storage profile specified
* `default` - True if this is default storage profile for this VDC. The default storage profile is used when an object that can specify a storage profile is created with no storage profile specified

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

(Supported in provider *v3.6+*)

* `limit` - Maximum number of storage bytes (scaled by Units) allocated for this profile. A value of 0 is understood to mean `maximum possible`
* `used_storage` - Storage used, in Megabytes, by the storage profile
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is just me, but it sounds strange - could this be simpler.

Suggested change
* `used_storage` - Storage used, in Megabytes, by the storage profile
* `used_storage` - Storage used, by the storage profile (in Megabytes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

This data source exports only `id` field.
(Supported in provider *v3.6+*)

* `limit` - Maximum number of storage bytes (scaled by Units) allocated for this profile. A value of 0 is understood to mean `maximum possible`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `limit` - Maximum number of storage bytes (scaled by Units) allocated for this profile. A value of 0 is understood to mean `maximum possible`
* `limit` - Maximum number of storage bytes (scaled by 'units' field) allocated for this profile. A value of 0 is understood to mean `maximum possible`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

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

A few more inconsistencies in docs, but the code itself worked in my manual testing

(Supported in provider *v3.6+*)

* `limit` - Maximum number of storage bytes (scaled by 'units' field) allocated for this profile. `0` means `maximum possible`
* `used_storage` - Storage used, by the storage profile (in Megabytes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

* `used_storage` - Storage used by the storage profile (in Megabytes)

* `default` - True if this is default storage profile for this VDC. The default storage profile is used when an object that can specify a storage profile is created with no storage profile specified
* `enabled` - True if this storage profile is enabled for use in the VDC
* `iops_allocated` - Total IOPS currently allocated to this storage profile
* `units` - Scale used to define Limit
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `units` - Scale used to define Limit
* `units` - Scale used to define limit

* `maximum_disk_iops` - The maximum IOPS value that this storage profile is permitted to deliver. Value of 0 means this max setting is disabled and there is no max disk IOPS restriction
* `default_disk_iops` - Value of 0 for disk IOPS means that no IOPS would be reserved or provisioned for that virtual disk
* `disk_iops_per_gb_max` - The maximum disk IOPs per GB value that this storage profile is permitted to deliver. A value of 0 means there is no per GB IOPS restriction
* `iops_limit` - Maximum number of IOPs that can be allocated for this profile. `0` means `maximum possible`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `iops_limit` - Maximum number of IOPs that can be allocated for this profile. `0` means `maximum possible`
* `iops_limit` - Maximum number of IOPS that can be allocated for this profile. `0` means `maximum possible`

* `iops_limiting_enabled` - True if this storage profile is IOPS-based placement enabled
* `maximum_disk_iops` - The maximum IOPS value that this storage profile is permitted to deliver. Value of 0 means this max setting is disabled and there is no max disk IOPS restriction
* `default_disk_iops` - Value of 0 for disk IOPS means that no IOPS would be reserved or provisioned for that virtual disk
* `disk_iops_per_gb_max` - The maximum disk IOPs per GB value that this storage profile is permitted to deliver. A value of 0 means there is no per GB IOPS restriction
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `disk_iops_per_gb_max` - The maximum disk IOPs per GB value that this storage profile is permitted to deliver. A value of 0 means there is no per GB IOPS restriction
* `disk_iops_per_gb_max` - The maximum disk IOPS per GB value that this storage profile is permitted to deliver. A value of 0 means there is no per GB IOPS restriction

"iops_limit": &schema.Schema{
Type: schema.TypeInt,
Computed: true,
Description: "Maximum number of IOPs that can be allocated for this profile. `0` means `maximum possible`",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Description: "Maximum number of IOPs that can be allocated for this profile. `0` means `maximum possible`",
Description: "Maximum number of IOPS that can be allocated for this profile. `0` means `maximum possible`",

"units": &schema.Schema{
Type: schema.TypeString,
Computed: true,
Description: "Scale used to define Limit",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Description: "Scale used to define Limit",
Description: "Scale used to define limit",

"used_storage": &schema.Schema{
Type: schema.TypeInt,
Computed: true,
Description: "Storage used, in Megabytes, by the storage profile",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Description: "Storage used, in Megabytes, by the storage profile",
Description: "Storage used, by the storage profile (in Megabytes)",

Copy link
Contributor

@mikeletux mikeletux left a comment

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
@vbauzys vbauzys merged commit b26b39d into vmware:main Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants