`azurerm_managed_disk` - add support for the UltraSSD `disk_iops_read_write` & `disk_mbps_read_write` properties #4102

WodansSon · 2019-08-15T23:16:56Z

[fixes: #3765 ]

tombuildsstuff

otherwise LGTM 👍

tombuildsstuff · 2019-08-19T15:49:42Z

azurerm/data_source_managed_disk.go

@@ -82,6 +92,12 @@ func dataSourceArmManagedDiskRead(d *schema.ResourceData, meta interface{}) erro
 		if diskSize := props.DiskSizeGB; diskSize != nil {
 			d.Set("disk_size_gb", *diskSize)
 		}
+		if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {
+			d.Set("disk_iops_read_write", *diskIOPS)
+		}


this can just be:

Suggested change

}

d.Set("disk_iops_read_write", props.DiskIOPSReadWrite)

tombuildsstuff · 2019-08-19T15:49:58Z

azurerm/data_source_managed_disk.go

+		}
+		if diskMBps := props.DiskMBpsReadWrite; diskMBps != nil {
+			d.Set("disk_mbps_read_write", *diskMBps)
+		}


this can just be:

d.Set("disk_mbps_read_write", props.DiskMBpsReadWrite)

tombuildsstuff · 2019-08-19T15:50:44Z

azurerm/resource_arm_managed_disk.go

+		}
+		if diskMBps := props.DiskMBpsReadWrite; diskMBps != nil {
+			d.Set("disk_mbps_read_write", *diskMBps)
+		}


(same here)

katbyte

Thanks for the PR @jeffreyCline,

In addition to my inline comments could we get a test with the ultraSSD type and the new properties?

website/docs/d/managed_disk.html.markdown

katbyte · 2019-08-27T03:55:29Z

website/docs/d/managed_disk.html.markdown

@@ -110,5 +110,8 @@ resource "azurerm_virtual_machine" "test" {
 * `source_resource_id` - ID of an existing managed disk that the current resource was created from.
 * `os_type` - The operating system for managed disk. Valid values are `Linux` or `Windows`
 * `disk_size_gb` - The size of the managed disk in gigabytes.
+* `disk_iops_read_write` - The number of IOPS allowed for this disk; only settable for UltraSSD disks. One operation can transfer between 4k and 256k bytes.
+* `disk_mbps_read_write` - The bandwidth allowed for this disk; only settable for UltraSSD disks. MBps means millions of bytes per second.
+~> **Note**: A `storage_account_type` of type `UltraSSD_LRS` and the attributes `disk_iops_read_write` and `disk_mbps_read_write` are currently in private preview and are not available to subscriptions that have not opted-in to the `Azure ultra disks` private preview. For more information see the `Azure ultra disks` [product documentation](https://docs.microsoft.com/en-us/azure/virtual-machines/windows/disks-enable-ultra-ssd).


I don't think we need this note here as its just the datasource and exists in the resource docs. WDYT?

Sure... I'm fine with that.

Co-Authored-By: kt <kt@katbyte.me>

WodansSon · 2019-08-29T00:18:31Z

Thanks for the PR @jeffreyCline,
In addition to my inline comments could we get a test with the ultraSSD type and the new properties?

I didn't add the test as this is a private preview and you need to set up your subscription to use this in one of the three regions it is currently in. I believe that it is supposed to GA on Oct. 1st, so I would be happy to add one then.

katbyte

@jeffreyCline, Looks like you missed some of toms comments. Even thou it is in preview i'd like to see the tests for these properties. I believe our subscription is now whitelisted so we should be able to run them.

katbyte · 2019-08-30T22:55:21Z

azurerm/data_source_managed_disk.go

-			d.Set("disk_size_gb", *diskSize)
+			d.Set("disk_size_gb", props.DiskSizeGB)
+		}
+		if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {


We don't need the check here as d.Set() will do it for us:

Suggested change

if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {

d.Set("disk_iops_read_write", props.DiskIOPSReadWrite)

katbyte · 2019-08-30T22:55:30Z

azurerm/data_source_managed_disk.go

+		if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {
+			d.Set("disk_iops_read_write", props.DiskIOPSReadWrite)
+		}
+		if diskMBps := props.DiskMBpsReadWrite; diskMBps != nil {


katbyte · 2019-08-30T22:55:57Z

azurerm/resource_arm_managed_disk.go

@@ -268,6 +298,12 @@ func resourceArmManagedDiskRead(d *schema.ResourceData, meta interface{}) error
 		if osType := props.OsType; osType != "" {
 			d.Set("os_type", string(osType))
 		}
+		if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {


We don't need the check here as d.Set will do it for us:

Suggested change

if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {

d.Set("disk_iops_read_write", props.DiskIOPSReadWrite)

katbyte · 2019-08-30T22:56:04Z

azurerm/resource_arm_managed_disk.go

+		if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {
+			d.Set("disk_iops_read_write", props.DiskIOPSReadWrite)
+		}
+		if diskMBps := props.DiskMBpsReadWrite; diskMBps != nil {


website/docs/d/managed_disk.html.markdown

katbyte · 2019-08-30T22:57:28Z

website/docs/r/managed_disk.html.markdown

+
+* `disk_mbps_read_write` - (Optional) The bandwidth allowed for this disk; only settable for UltraSSD disks. MBps means millions of bytes per second.
+
+-> **Note**: A `storage_account_type` of type `UltraSSD_LRS` and the arguments `disk_iops_read_write` and `disk_mbps_read_write` are currently in private preview and are not available to subscriptions that have not requested onboarding to `Azure Ultra Disk Storage` private preview. `Azure Ultra Disk Storage` is only available in `East US 2`, `North Europe`, and `Southeast Asia` regions. For more information see the `Azure Ultra Disk Storage` [product documentation](https://docs.microsoft.com/en-us/azure/virtual-machines/windows/disks-enable-ultra-ssd), [product blog](https://azure.microsoft.com/en-us/blog/announcing-the-general-availability-of-azure-ultra-disk-storage/) and [FAQ](https://docs.microsoft.com/en-us/azure/virtual-machines/windows/faq-for-disks#ultra-disks).


Given this applies to the "type" could we move it up to under the type??

kk... moved. :)

website/docs/r/managed_disk.html.markdown

Co-Authored-By: kt <kt@katbyte.me>

WodansSon · 2019-09-05T22:13:03Z

@kt

Even thou it is in preview i'd like to see the tests for these properties. I believe our subscription is now whitelisted so we should be able to run them.

I have added the tests to both data source and resource.

katbyte

Thanks for the changes @jeffreyCline! LGTM 👍

ghost · 2019-09-18T12:13:17Z

This has been released in version 1.34.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.34.0"
}
# ... other configuration ...

ghost · 2019-10-07T14:50:37Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

expose ultrassd

e8f417d

WodansSon added enhancement documentation labels Aug 15, 2019

WodansSon added this to the v1.33.0 milestone Aug 15, 2019

ghost added the size/S label Aug 15, 2019

WodansSon added 2 commits August 16, 2019 14:44

Updated docs for clarity

bfc2101

added note to documentation

cbf529b

ghost added size/M and removed size/S labels Aug 16, 2019

WodansSon added the preview label Aug 17, 2019

katbyte changed the title ~~[WIP]Enhancement: Add support for UltraSSD to Managed Disk~~ azurerm_managed_disk add support for the UltraSSD disk_iops_read_write & disk_mbps_read_write properties Aug 18, 2019

katbyte changed the title ~~azurerm_managed_disk add support for the UltraSSD disk_iops_read_write & disk_mbps_read_write properties~~ azurerm_managed_disk - add support for the UltraSSD disk_iops_read_write & disk_mbps_read_write properties Aug 18, 2019

tombuildsstuff approved these changes Aug 19, 2019

View reviewed changes

tombuildsstuff added the waiting-response label Aug 19, 2019

tombuildsstuff assigned WodansSon Aug 19, 2019

WodansSon added 3 commits August 19, 2019 17:33

Merge branch 'master' into nr_ultrassd

0f27177

changes per PR review

5605ab8

Missed one change

87f811f

WodansSon removed the waiting-response label Aug 20, 2019

Updated doc to show availability

d0619b7

katbyte modified the milestones: v1.33.0, v1.34.0 Aug 21, 2019

katbyte requested changes Aug 27, 2019

View reviewed changes

katbyte added the waiting-response label Aug 27, 2019

WodansSon and others added 2 commits August 28, 2019 16:52

Update website/docs/d/managed_disk.html.markdown

39b4b21

Co-Authored-By: kt <kt@katbyte.me>

Update website/docs/d/managed_disk.html.markdown

272d711

Co-Authored-By: kt <kt@katbyte.me>

Update per PR comments

de6e530

WodansSon removed the waiting-response label Aug 29, 2019

katbyte requested changes Aug 30, 2019

View reviewed changes

katbyte added the waiting-response label Aug 30, 2019

WodansSon and others added 7 commits September 5, 2019 12:40

Merge branch 'master' into nr_ultrassd

bbe584a

Update website/docs/r/managed_disk.html.markdown

9c3d019

Co-Authored-By: kt <kt@katbyte.me>

Update website/docs/d/managed_disk.html.markdown

620569d

Co-Authored-By: kt <kt@katbyte.me>

Updates per PR request

3c77ab3

Merge Conflict

7223ed1

Fix lint errors

17ba65d

Update website/docs/d/managed_disk.html.markdown

d50d2f0

Co-Authored-By: kt <kt@katbyte.me>

Added UltraSSD test cases

80d4bc4

ghost added size/L and removed size/M labels Sep 5, 2019

WodansSon added size/M and removed size/L waiting-response labels Sep 5, 2019

gofmt

1684dfc

ghost added size/L and removed size/M labels Sep 5, 2019

katbyte added 2 commits September 5, 2019 19:01

Update data_source_managed_disk_test.go

204c249

Update resource_arm_managed_disk_test.go

501747b

katbyte approved these changes Sep 6, 2019

View reviewed changes

katbyte merged commit 3ef7d49 into master Sep 6, 2019

katbyte deleted the nr_ultrassd branch September 6, 2019 19:14

katbyte added a commit that referenced this pull request Sep 6, 2019

Update CHANGELOG.md to include #4102

28ed4cc

ghost locked and limited conversation to collaborators Oct 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_managed_disk` - add support for the UltraSSD `disk_iops_read_write` & `disk_mbps_read_write` properties #4102

`azurerm_managed_disk` - add support for the UltraSSD `disk_iops_read_write` & `disk_mbps_read_write` properties #4102

WodansSon commented Aug 15, 2019 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Aug 19, 2019

WodansSon Aug 20, 2019

tombuildsstuff Aug 19, 2019

WodansSon Aug 20, 2019

tombuildsstuff Aug 19, 2019

WodansSon Aug 20, 2019

katbyte left a comment

katbyte Aug 27, 2019

WodansSon Aug 29, 2019

WodansSon commented Aug 29, 2019

katbyte left a comment

katbyte Aug 30, 2019

WodansSon Sep 5, 2019

katbyte Aug 30, 2019

WodansSon Sep 5, 2019

katbyte Aug 30, 2019

WodansSon Sep 5, 2019

katbyte Aug 30, 2019

WodansSon Sep 5, 2019

katbyte Aug 30, 2019

WodansSon Sep 5, 2019

WodansSon commented Sep 5, 2019

katbyte left a comment

ghost commented Sep 18, 2019

ghost commented Oct 7, 2019

	if diskIOPS := props.DiskIOPSReadWrite; diskIOPS != nil {
	d.Set("disk_iops_read_write", props.DiskIOPSReadWrite)


		* `disk_mbps_read_write` - (Optional) The bandwidth allowed for this disk; only settable for UltraSSD disks. MBps means millions of bytes per second.

		-> Note: A `storage_account_type` of type `UltraSSD_LRS` and the arguments `disk_iops_read_write` and `disk_mbps_read_write` are currently in private preview and are not available to subscriptions that have not requested onboarding to `Azure Ultra Disk Storage` private preview. `Azure Ultra Disk Storage` is only available in `East US 2`, `North Europe`, and `Southeast Asia` regions. For more information see the `Azure Ultra Disk Storage` [product documentation](https://docs.microsoft.com/en-us/azure/virtual-machines/windows/disks-enable-ultra-ssd), [product blog](https://azure.microsoft.com/en-us/blog/announcing-the-general-availability-of-azure-ultra-disk-storage/) and [FAQ](https://docs.microsoft.com/en-us/azure/virtual-machines/windows/faq-for-disks#ultra-disks).

azurerm_managed_disk - add support for the UltraSSD disk_iops_read_write & disk_mbps_read_write properties #4102

azurerm_managed_disk - add support for the UltraSSD disk_iops_read_write & disk_mbps_read_write properties #4102

Conversation

WodansSon commented Aug 15, 2019 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WodansSon commented Aug 29, 2019

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WodansSon commented Sep 5, 2019

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Sep 18, 2019

ghost commented Oct 7, 2019

`azurerm_managed_disk` - add support for the UltraSSD `disk_iops_read_write` & `disk_mbps_read_write` properties #4102

`azurerm_managed_disk` - add support for the UltraSSD `disk_iops_read_write` & `disk_mbps_read_write` properties #4102

WodansSon commented Aug 15, 2019 •

edited

Loading