Azure Managed Disks

[ThatRendle]

I couldn't find an issue about support for the the new Managed Disks for Azure VMs. I'm guessing it'll require a change to https://github.com/hashicorp/terraform/blob/master/builtin/providers/azurerm/resource_arm_virtual_machine.go ?

[stack72]

@markrendle do you know if the SDK supports this yet? I can't find anything in it

[ThatRendle]

Looks like it's in the 8.0.0-beta release: Azure/azure-sdk-for-go#538 (comment)

[stack72]

@markrendle nps - we have some breaking changes in that SDK upgrade to deal with first then we can start to look at this new feature

[brandontosch]

@stack72 Hi Paul, is there an open issue for the SDK upgrade that we can track as well?

[stack72]

Hi @brandontosch

There is indeed #11866

Once we get the breaking changes taken care of, we can merge it

P.

[rrudduck]

I took a look at adding this in now that the SDK has been updated and it appears there is still issues with the SDK. Unless I'm missing something the Compute provider in the SDK wasn't updated to version 2016-04-30-preview which means the managed disk support isn't there.

[mzupan]

@rrudduck i was able to create a managed disk with some work I did but didn't get too far with it but did you add the section to import manage disk support to vendor/vendor.json ?

Path should be like

github.com/Azure/azure-sdk-for-go/arm/disk

[rrudduck]

I didn't try creating the disk itself, I was trying to use the implicit create support. Reading through the docs it appeared that by omitting some of the storage profile details a managed disk would be automatically created when the VM was created. I could be wrong though - that's just my impression after looking at the ARM templates.

[rrudduck]

I've confirmed that by manually modifying the go sdk in vendor to use the correct api version for compute, updating the client and terraform validations, this manifest works:

resource "azurerm_resource_group" "test" {
    name     = "acctestrg%s"
    location = "southcentralus"
}

resource "azurerm_virtual_network" "test" {
    name                = "acctvn%s"
    address_space       = ["10.0.0.0/16"]
    location            = "southcentralus"
    resource_group_name = "${azurerm_resource_group.test.name}"
}

resource "azurerm_subnet" "test" {
    name                 = "acctsub%s"
    resource_group_name  = "${azurerm_resource_group.test.name}"
    virtual_network_name = "${azurerm_virtual_network.test.name}"
    address_prefix       = "10.0.2.0/24"
}

resource "azurerm_network_interface" "test" {
    name                = "acctni%s"
    location            = "southcentralus"
    resource_group_name = "${azurerm_resource_group.test.name}"

    ip_configuration {
    	name                          = "testconfiguration1"
    	subnet_id                     = "${azurerm_subnet.test.id}"
    	private_ip_address_allocation = "dynamic"
    }
}

resource "azurerm_virtual_machine" "test" {
    name                  = "acctvm%s"
    location              = "southcentralus"
    resource_group_name   = "${azurerm_resource_group.test.name}"
    network_interface_ids = ["${azurerm_network_interface.test.id}"]
    vm_size               = "Standard_A0"

    storage_image_reference {
        publisher = "Canonical"
        offer     = "UbuntuServer"
        sku       = "14.04.2-LTS"
        version   = "latest"
    }

    storage_os_disk {
        name          = "acctvmosdisk"
        create_option = "FromImage"
    }

    os_profile {
        computer_name  = "hostname%s"
        admin_username = "testadmin"
        admin_password = "Password1234!"
    }

    os_profile_linux_config {
	    disable_password_authentication = false
    }
}

Which results in this:

So we definitely need an sdk update. I've opened an issue on that side to request it.

EDIT: Looks like the PR Azure/azure-sdk-for-go/pull/561 has the update for the compute provider in it.

[brandontosch]

Looks like the azure-sdk-for-go PR from @mcardosos (Azure/azure-sdk-for-go#561) was merged so I took a crack at implementing the explicit support for managed disks in compute.

Would love feedback: brandontosch/GH-11874

resource "azurerm_virtual_machine" "test" {
    ...

    storage_os_disk {
       name          = "testosd"
       caching       = "ReadWrite"
       create_option = "FromImage"
       managed_disk_type = "Standard_LRS"
    }
   
    storage_data_disk {
       name          = "testdd"
       create_option = "empty"
       disk_size_gb  = "1023"
       lun                  = 0
       managed_disk_type = "Premium_LRS"
    }
    
    ...
}

Thanks!

**Updated structure to reflect code changes

[davidobrien1985]

@brandontosch sorry to be so frank to just ask... not sure if you're a packer user... but if you are, would you mind checking this out? hashicorp/packer#4511
My golang is really not up to the point where I can take a stab at this...
Cheers!

[rrudduck]

@brandontosch Those changes look pretty good. Once additional change you could make is supporting the explicit attach scenario. There is an id property in ManagedDiskProperties.

https://github.com/Azure/azure-sdk-for-go/blob/master/arm/compute/models.go#L616

[brandontosch]

@davidobrien1985 Haven't had much exporsure to packer, but then again I didn't have any experience with Terraform a few weeks ago either.. I'd love to take a look when I get some time, just not quite sure when that would be at the moment. Had to jump on this because my team is currently in the middle of a migration to azure and we needed the managed disk support asap.

@rrudduck Thanks for the feedback! I'll take a look at that today. I'm assuming this can be used when creating managed disks via arm/disk rather than via a compute resource?

[rrudduck]

@brandontosch Yep. You can see an example in the python docs here: https://azure-sdk-for-python.readthedocs.io/en/latest/sample_azure-mgmt-manageddisks.html#virtual-machine-with-managed-disks

[brandontosch]

@rrudduck awesome, thanks for the example!

[brandontosch]

Created a PR with what I've come up with for this: #12455

[djsly]

really looking for this! PR looks almost done, do we know if this will be back ported to 0.8.X or only 0.9.1 ?
Thanks!

[stack72]

Hi @djsly - this will be 0.9.2 only - there will no longer be any 0.8.x releases

[djsly]

@stack72 thanks, looking forward to #11874 to be merged! :)

[tombuildsstuff]

@markrendle @rrudduck @mzupan @davidobrien1985 @djsly FYI I've just merged @brandontosch's PR (#12455) which adds support for Managed Disks - which will be in the next release :)

@brandontosch thanks once again for this great contribution :)

[glenjamin]

Sorry to resurrect a bit, but I've just used this feature to create a fresh VM with an automatically created managed disk (via create_option = "FromImage"), and then deleted the VM to rebuild - but the managed disk was still around and caused the rebuild to fail.

Have I missed something, or should I open a new issue / get this re-opened to cover this?

[brandontosch]

Hi @glenjamin
You'll need to use the delete_os_disk_on_termination and/or delete_data_disks_on_termination flags for the disks to be deleted along with the VM (they both default to false). https://www.terraform.io/docs/providers/azurerm/r/virtual_machine.html#delete_os_disk_on_termination

[glenjamin]

Awesome, thanks! I missed this because I was only looking at the storage_os_disk section

[ghost]

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.