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

azurerm_virtual_machine: add support for Ultra SSD's to the managed_disk_type property #3860

Merged
merged 21 commits into from
Sep 1, 2019

Conversation

venkey1000
Copy link
Contributor

@venkey1000 venkey1000 commented Jul 17, 2019

added ultra ssd support for azure
steps:

  1. added additionalCapabilitites with ultra_ssd_enabled field
  2. added storage_data_disk with managed_disk_type as UltraSSD_LRS

PS:
Azure guys has given our subscription capability to attach ultra ssd disk, that is how we abled to test the feature

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @venkey1000

Thanks for this PR :)

Taking a look through this mostly LGTM - if we can add documentation for this field this otherwise LGTM 👍

Thanks!

azurerm/resource_arm_virtual_machine.go Show resolved Hide resolved
azurerm/resource_arm_virtual_machine.go Outdated Show resolved Hide resolved
@tombuildsstuff tombuildsstuff added this to the v1.32.0 milestone Jul 17, 2019
@tombuildsstuff tombuildsstuff changed the title added ultra ssd support r/virtual_machine: support for Ultra SSD's Jul 17, 2019
@ghost ghost added the documentation label Jul 18, 2019
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @venkey1000

Thanks for pushing those changes - if we can add in the final link (and the tests pass) then this otherwise LGTM 👍

website/docs/r/virtual_machine.html.markdown Show resolved Hide resolved
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Hi @venkey1000,

Thank you for the additional docs, could we add this to a new additional_capabilities test to ensure its set/readback/imported correctly?

@tombuildsstuff tombuildsstuff modified the milestones: v1.32.0, v1.33.0 Jul 19, 2019
@venkey1000
Copy link
Contributor Author

venkey1000 commented Jul 20, 2019

Hi @venkey1000,

Thank you for the additional docs, could we add this to a new additional_capabilities test to ensure its set/readback/imported correctly?

Hi @katbyte
Sorry for the late reply.
I have added test cases for additional_capabilities in resource_arm_virtual_machine_unmanaged_disks_test.go.

@ghost ghost removed the waiting-response label Jul 20, 2019
@ghost ghost added size/L and removed size/S labels Jul 20, 2019
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @venkey1000

Thanks for pushing those changes - taking a look through I spotted another couple of comments (and one question about the schema design) that I think need to be resolved before we can merge this; but this is otherwise looking good 👍

Thanks!

azurerm/resource_arm_virtual_machine.go Outdated Show resolved Hide resolved
azurerm/resource_arm_virtual_machine.go Outdated Show resolved Hide resolved
azurerm/resource_arm_virtual_machine.go Show resolved Hide resolved
venkey1000 and others added 7 commits July 22, 2019 19:39
Co-Authored-By: Tom Harvey <tombuildsstuff@users.noreply.github.com>
Co-Authored-By: Tom Harvey <tombuildsstuff@users.noreply.github.com>
Co-Authored-By: Tom Harvey <tombuildsstuff@users.noreply.github.com>
@venkey1000
Copy link
Contributor Author

@venkey1000,

running the tests i am getting:

------- Stdout: -------
=== RUN   TestAccAzureRMVirtualMachine_additionalCapabilities
=== PAUSE TestAccAzureRMVirtualMachine_additionalCapabilities
=== CONT  TestAccAzureRMVirtualMachine_additionalCapabilities
--- FAIL: TestAccAzureRMVirtualMachine_additionalCapabilities (115.11s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: compute.VirtualMachinesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="InvalidParameter" Message="UltraSSD can be used only with Virtual Machine in an Availability Zone." Target="additionalCapabilities.ultraSSDEnabled"
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test832387319/main.tf line 52:
          (source code not available)
        
        
FAIL

so you might need to adjust your test configuration

Hi @katbyte
I have updated test case for this feature, please have a look at it.

@ghost ghost removed the waiting-response label Aug 3, 2019
@katbyte
Copy link
Collaborator

katbyte commented Aug 4, 2019

@normseq & @venkey1000,

I am now getting quota errors in west europe:

------- Stdout: -------
=== RUN   TestAccAzureRMVirtualMachine_additionalCapabilities
=== PAUSE TestAccAzureRMVirtualMachine_additionalCapabilities
=== CONT  TestAccAzureRMVirtualMachine_additionalCapabilities
--- FAIL: TestAccAzureRMVirtualMachine_additionalCapabilities (151.65s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: Code="OperationNotAllowed" Message="Operation results in exceeding quota limits of UltraSSDDiskSizeInGB. Maximum allowed: 0, Current in use: 0, Additional requested: 68719476736. Please read more about quota increase at http://aka.ms/corequotaincrease."
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test276565280/main.tf line 33:
          (source code not available)
        
        
FAIL

@katbyte
Copy link
Collaborator

katbyte commented Aug 4, 2019

And switching to eastus2 i get:

------- Stdout: -------
=== RUN   TestAccAzureRMVirtualMachine_additionalCapabilities
=== PAUSE TestAccAzureRMVirtualMachine_additionalCapabilities
=== CONT  TestAccAzureRMVirtualMachine_additionalCapabilities
--- FAIL: TestAccAzureRMVirtualMachine_additionalCapabilities (88.90s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: compute.VirtualMachinesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="BadRequest" Message="Storage Account type 'UltraSSD_LRS' is not supported in availability zone '3'."
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test059273537/main.tf line 33:
          (source code not available)
        
        
FAIL

@venkey1000
Copy link
Contributor Author

And switching to eastus2 i get:

------- Stdout: -------
=== RUN   TestAccAzureRMVirtualMachine_additionalCapabilities
=== PAUSE TestAccAzureRMVirtualMachine_additionalCapabilities
=== CONT  TestAccAzureRMVirtualMachine_additionalCapabilities
--- FAIL: TestAccAzureRMVirtualMachine_additionalCapabilities (88.90s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: compute.VirtualMachinesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="BadRequest" Message="Storage Account type 'UltraSSD_LRS' is not supported in availability zone '3'."
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test059273537/main.tf line 33:
          (source code not available)
        
        
FAIL

@katbyte can you share the resource(vm and subscription id) details you are trying to launch so that Azure team can debug the issue?

@ghost ghost removed the waiting-response label Aug 5, 2019
@normseq
Copy link

normseq commented Aug 5, 2019

@katbyte Even in EastUS, UltraSSD will not be available in all availability zones.
For your subscription, can you please run the following?
CLI: az vm list-skus --resource-type disks --query "[?name=='UltraSSD_LRS'].locationInfo"

This will give you info on the AZ to use while creating an UltraSSD based VM specifically for your subscription.
Please try creating an UltraSSD enabled VM directly in the Azure Portal/CLI before trying with the provider so that we can confirm if its a whitelisting issue or provider code issue.
For more info, please check
https://docs.microsoft.com/en-us/azure/virtual-machines/windows/disks-enable-ultra-ssd

@katbyte
Copy link
Collaborator

katbyte commented Aug 8, 2019

@normseq, @venkey1000

If that is the case we are going to have to update the test to use availability zones and the new compute SKUs resource once its merged.

@katbyte
Copy link
Collaborator

katbyte commented Aug 18, 2019

@normseq & @venkey1000, has something changed with this moving to GA? When i attempt to run the test now i am getting:

 Error: Code="OperationNotAllowed" Message="Operation results in exceeding quota limits of UltraSSDDiskSizeInGB. Maximum allowed: 0, Current in use: 0, Additional requested: 68719476736. Please read more about quota increase at http://aka.ms/corequotaincrease."     

and i don't see anywhere in the portal to request an increase

@katbyte katbyte modified the milestones: Blocked, v1.34.0 Aug 18, 2019
@katbyte katbyte changed the title r/virtual_machine: support for Ultra SSD's azurerm_virtual_machine: add support for Ultra SSD's to the managed_disk_type property Aug 18, 2019
@normseq
Copy link

normseq commented Aug 20, 2019

@katbyte Though Ultra Disk Storage is now GA, one still needs to fill up a form (separate from the usual portal quota request) to get access to Ultra Disks. Are you trying to create a disk in your subscription which was already whitelisted?
https://azure.microsoft.com/en-us/blog/announcing-the-general-availability-of-azure-ultra-disk-storage/
https://aka.ms/UltraDiskSignup

@ghost ghost removed the waiting-response label Aug 20, 2019
@normseq
Copy link

normseq commented Aug 27, 2019

@katbyte Were you able to provision an Ultra Disk directly using CLI in your subscription? If that goes through, the provider should go through.

@venkey1000
Copy link
Contributor Author

Hi @katbyte i am unable to figure out the issue in website test, can you please point me to the issue why it is failing if possible.

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

@venkey1000,

Hope you don't mind but i pushed the required changes so i could merge this. LGTM now 👍

@katbyte katbyte merged commit 14dc093 into hashicorp:master Sep 1, 2019
katbyte added a commit that referenced this pull request Sep 1, 2019
@venkey1000
Copy link
Contributor Author

@venkey1000,

Hope you don't mind but i pushed the required changes so i could merge this. LGTM now 👍

@katbyte.. Thank you so much :)

@ghost
Copy link

ghost commented Sep 18, 2019

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
Copy link

ghost commented Oct 1, 2019

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!

@ghost ghost locked and limited conversation to collaborators Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants