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 Managed Disks #12455

Merged
merged 43 commits into from
Apr 6, 2017
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
a6d8b45
mcardosos/azure-for-go-sdk/arm/compute@v8.1.0-beta
brandontosch Mar 1, 2017
9f6a7dc
compute managed disk support
brandontosch Mar 1, 2017
a3a2c38
make fmt
brandontosch Mar 1, 2017
d1bca91
updated other arm/compute references to mcardosos
brandontosch Mar 1, 2017
6379153
provider/azurerm: Bumped AzureRM/compute SDK to v8.1.0-beta
brandontosch Mar 2, 2017
909976e
fixed nil check in resourceArmVirtualMachineStorageOsDiskHash
brandontosch Mar 2, 2017
00df220
fixed nil check in resourceArmVirtualMachineStorageOsDiskHash
brandontosch Mar 2, 2017
5c0b750
acceptance tests for managed disks
brandontosch Mar 2, 2017
e761374
fixed postConfig check to verify the corrent vm size
brandontosch Mar 2, 2017
b7df7d4
changed DS1 to DS1_v2
brandontosch Mar 2, 2017
e3dbe47
Merge branch 'master' into brandontosch/GH-11874
brandontosch Mar 2, 2017
0a07c14
[WIP] arm/disk implementation
brandontosch Mar 3, 2017
2df44e0
[WIP] arm/disk implementation
brandontosch Mar 3, 2017
fc3efec
fmt
brandontosch Mar 3, 2017
ca6bc88
renamed arm_disk to arm_managed_disk
brandontosch Mar 3, 2017
9158b95
managed disk acceptance tests
brandontosch Mar 4, 2017
a084aa0
implemented copy for managed disks
brandontosch Mar 4, 2017
ac8eae9
implemented attaching existing managed disks on vm creation
brandontosch Mar 4, 2017
5d915dd
flattened managed disk structure in virtual machine
brandontosch Mar 6, 2017
193c257
Merge branch 'master' into brandontosch/GH-11874
brandontosch Mar 6, 2017
8fa69da
re-added arm/disk after merge
brandontosch Mar 6, 2017
c256549
removed TF_ACC setter
brandontosch Mar 6, 2017
a17da26
added nil check for DiskSizeGB
brandontosch Mar 6, 2017
eb2bef1
updated/added documentation for managed disks
brandontosch Mar 6, 2017
fa56000
Merge branch 'master' into brandontosch/GH-11874
brandontosch Mar 8, 2017
972bc08
Removed context timeout due "request cancelled" to acc test failures …
brandontosch Mar 10, 2017
a2716f7
Lowered data disk size to decrease provisioning time and cost
brandontosch Mar 10, 2017
91db3e1
Corrected variable reference
edevil Mar 10, 2017
34c0dd7
fixed typo in vm_size
brandontosch Mar 10, 2017
58325b9
Merge pull request #1 from edevil/patch-1
brandontosch Mar 10, 2017
dbb4daa
disk_size_gb is required
brandontosch Mar 14, 2017
8f2d2ed
refactored flatten methods to also set values
brandontosch Mar 14, 2017
e94c1f7
Added link to official azure documentation
brandontosch Mar 14, 2017
4294754
Corrected error message
brandontosch Mar 14, 2017
7703d6c
Added supported for ConflictsWith for fields within lists/sets
brandontosch Mar 18, 2017
03b8f05
Implemented ConflictsWith for vhd_uri and managed_disk fields
brandontosch Mar 18, 2017
808a9f0
removed commented import
brandontosch Mar 23, 2017
a327648
Reverted ConflictsWith fix (moved to separate PR: GH-13019)
brandontosch Mar 23, 2017
0168829
Merge branch 'master' into brandontosch/GH-11874
brandontosch Mar 23, 2017
2ac79c3
trigger ci
brandontosch Mar 23, 2017
eb6f36f
Re-added custom conflict validation for managed disks
brandontosch Mar 27, 2017
fedb170
added disk_size_gb to config for TestAccAzureRMManagedDisk_import
brandontosch Mar 28, 2017
380f55b
Merge branch 'master' into brandontosch/GH-11874
brandontosch Mar 29, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions builtin/providers/azurerm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/Azure/azure-sdk-for-go/arm/compute"
"github.com/Azure/azure-sdk-for-go/arm/containerregistry"
"github.com/Azure/azure-sdk-for-go/arm/containerservice"
"github.com/Azure/azure-sdk-for-go/arm/disk"
"github.com/Azure/azure-sdk-for-go/arm/eventhub"
"github.com/Azure/azure-sdk-for-go/arm/keyvault"
"github.com/Azure/azure-sdk-for-go/arm/network"
Expand Down Expand Up @@ -47,6 +48,8 @@ type ArmClient struct {
vmImageClient compute.VirtualMachineImagesClient
vmClient compute.VirtualMachinesClient

diskClient disk.DisksClient

appGatewayClient network.ApplicationGatewaysClient
ifaceClient network.InterfacesClient
loadBalancerClient network.LoadBalancersClient
Expand Down Expand Up @@ -245,6 +248,12 @@ func (c *Config) getArmClient() (*ArmClient, error) {
csc.Sender = autorest.CreateSender(withRequestLogging())
client.containerServicesClient = csc

dkc := disk.NewDisksClientWithBaseURI(endpoint, c.SubscriptionID)
setUserAgent(&dkc.Client)
dkc.Authorizer = spt
dkc.Sender = autorest.CreateSender(withRequestLogging())
client.diskClient = dkc

ehc := eventhub.NewEventHubsClientWithBaseURI(endpoint, c.SubscriptionID)
setUserAgent(&ehc.Client)
ehc.Authorizer = spt
Expand Down
30 changes: 30 additions & 0 deletions builtin/providers/azurerm/import_arm_managed_disk_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package azurerm

import (
"fmt"
"testing"

"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
)

func TestAccAzureRMManagedDisk_importEmpty(t *testing.T) {
ri := acctest.RandInt()
config := fmt.Sprintf(testAccAzureRMManagedDisk_empty, ri, ri)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMManagedDiskDestroy,
Steps: []resource.TestStep{
{
Config: config,
},
{
ResourceName: "azurerm_managed_disk.test",
ImportState: true,
ImportStateVerify: true,
},
},
})
}
32 changes: 30 additions & 2 deletions builtin/providers/azurerm/import_arm_virtual_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,39 @@ func TestAccAzureRMVirtualMachine_importBasic(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMVirtualMachineDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: config,
},

resource.TestStep{
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"delete_data_disks_on_termination",
"delete_os_disk_on_termination",
},
},
},
})
}

func TestAccAzureRMVirtualMachine_importBasic_managedDisk(t *testing.T) {
resourceName := "azurerm_virtual_machine.test"

ri := acctest.RandInt()
config := fmt.Sprintf(testAccAzureRMVirtualMachine_basicLinuxMachine_managedDisk_explicit, ri, ri, ri, ri, ri, ri, ri)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMVirtualMachineDestroy,
Steps: []resource.TestStep{
{
Config: config,
},

{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
Expand Down
2 changes: 2 additions & 0 deletions builtin/providers/azurerm/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ func Provider() terraform.ResourceProvider {
"azurerm_lb_probe": resourceArmLoadBalancerProbe(),
"azurerm_lb_rule": resourceArmLoadBalancerRule(),

"azurerm_managed_disk": resourceArmManagedDisk(),

"azurerm_key_vault": resourceArmKeyVault(),
"azurerm_local_network_gateway": resourceArmLocalNetworkGateway(),
"azurerm_network_interface": resourceArmNetworkInterface(),
Expand Down
256 changes: 256 additions & 0 deletions builtin/providers/azurerm/resource_arm_managed_disk.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
package azurerm

import (
"fmt"
"github.com/Azure/azure-sdk-for-go/arm/disk"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
"log"
"net/http"
"strings"
)

func resourceArmManagedDisk() *schema.Resource {
return &schema.Resource{
Create: resourceArmManagedDiskCreate,
Read: resourceArmManagedDiskRead,
Update: resourceArmManagedDiskCreate,
Delete: resourceArmManagedDiskDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},

"location": locationSchema(),

"resource_group_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},

"storage_account_type": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
string(disk.PremiumLRS),
string(disk.StandardLRS),
}, true),
},

"create_option": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
string(disk.Import),
string(disk.Empty),
string(disk.Copy),
}, true),
},

"source_uri": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},

"source_resource_id": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},

"os_type": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
string(disk.Windows),
string(disk.Linux),
}, true),
},

"disk_size_gb": {
Type: schema.TypeInt,
Optional: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be required?

Copy link
Contributor

Choose a reason for hiding this comment

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

that should be required

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 disk_size_gb to required

ValidateFunc: validateDiskSizeGB,
},

"tags": tagsSchema(),
},
}
}

func validateDiskSizeGB(v interface{}, k string) (ws []string, errors []error) {
value := v.(int)
if value < 1 || value > 1023 {
Copy link
Contributor

Choose a reason for hiding this comment

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

From looking at the docs it seems the valid disk dizes are:

  • premium: 128, 512, 1024
  • standard: 32, 64, 128, 512, 1024
    Shouldn't this be checked here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had actually just moved that function over from the vm resource (was pre-existing, just figured it made more sense to be based out of the new disk resource).
Also, according to some of the Microsoft folks I've been working with, the max size will be increasing to 4TB at some point hopefully soon.. Not sure when exactly, but I'll try and keep an eye out for when that happens and update this accordingly.

errors = append(errors, fmt.Errorf(
"The `disk_size_gb` can only be between 1 and 1023"))
}
return
}

func resourceArmManagedDiskCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*ArmClient)
diskClient := client.diskClient

log.Printf("[INFO] preparing arguments for Azure ARM Managed Disk creation.")

name := d.Get("name").(string)
location := d.Get("location").(string)
resGroup := d.Get("resource_group_name").(string)
tags := d.Get("tags").(map[string]interface{})
expandedTags := expandTags(tags)

createDisk := disk.Model{
Name: &name,
Location: &location,
Tags: expandedTags,
}

storageAccountType := d.Get("storage_account_type").(string)
osType := d.Get("os_type").(string)

createDisk.Properties = &disk.Properties{
AccountType: disk.StorageAccountTypes(storageAccountType),
OsType: disk.OperatingSystemTypes(osType),
}

if v := d.Get("disk_size_gb"); v != 0 {
diskSize := int32(v.(int))
createDisk.Properties.DiskSizeGB = &diskSize
}
createOption := d.Get("create_option").(string)

creationData := &disk.CreationData{
CreateOption: disk.CreateOption(createOption),
}

if strings.EqualFold(createOption, string(disk.Import)) {
if sourceUri := d.Get("source_uri").(string); sourceUri != "" {
creationData.SourceURI = &sourceUri
} else {
return fmt.Errorf("[ERROR] source_uri must be specified when create_option is `%s`", disk.Import)
}
} else if strings.EqualFold(createOption, string(disk.Copy)) {
if sourceResourceId := d.Get("source_resource_id").(string); sourceResourceId != "" {
creationData.SourceResourceID = &sourceResourceId
} else {
return fmt.Errorf("[ERROR] source_resource_id must be specified when create_option is `%s`", disk.Copy)
}
}

createDisk.CreationData = creationData

_, diskErr := diskClient.CreateOrUpdate(resGroup, name, createDisk, make(chan struct{}))
if diskErr != nil {
return diskErr
}

read, err := diskClient.Get(resGroup, name)
if err != nil {
return err
}
if read.ID == nil {
return fmt.Errorf("[ERROR] Cannot read Managed Disk %s (resource group %s) ID", name, resGroup)
}

d.SetId(*read.ID)

return resourceArmManagedDiskRead(d, meta)
}

func resourceArmManagedDiskRead(d *schema.ResourceData, meta interface{}) error {
diskClient := meta.(*ArmClient).diskClient

id, err := parseAzureResourceID(d.Id())
if err != nil {
return err
}
resGroup := id.ResourceGroup
name := id.Path["disks"]

resp, err := diskClient.Get(resGroup, name)
if err != nil {
if resp.StatusCode == http.StatusNotFound {
d.SetId("")
return nil
}
return fmt.Errorf("[ERROR] Error making Read request on Azure Managed Disk %s (resource group %s): %s", name, resGroup, err)
}

d.Set("name", resp.Name)
d.Set("resource_group_name", resGroup)
d.Set("location", resp.Location)

if resp.Properties != nil {
m := flattenAzureRmManagedDiskProperties(resp.Properties)
d.Set("storage_account_type", m["storage_account_type"])
d.Set("disk_size_gb", m["disk_size_gb"])
if m["os_type"] != nil {
d.Set("os_type", m["os_type"])
}
Copy link
Contributor

Choose a reason for hiding this comment

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

it'd be good to make the flatten method also set the values in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored to set values

}

if resp.CreationData != nil {
m := flattenAzureRmManagedDiskCreationData(resp.CreationData)
d.Set("create_option", m["create_option"])
if m["source_uri"] != nil {
d.Set("source_uri", m["source_uri"])
}
if m["source_resource_id"] != nil {
d.Set("source_resource_id", m["source_resource_id"])
}
Copy link
Contributor

Choose a reason for hiding this comment

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

it'd be good to make the flatten method also set the values in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored to set values

}

flattenAndSetTags(d, resp.Tags)

return nil
}

func resourceArmManagedDiskDelete(d *schema.ResourceData, meta interface{}) error {
diskClient := meta.(*ArmClient).diskClient

id, err := parseAzureResourceID(d.Id())
if err != nil {
return err
}
resGroup := id.ResourceGroup
name := id.Path["disks"]

if _, err = diskClient.Delete(resGroup, name, make(chan struct{})); err != nil {
return err
}

return nil
}

func flattenAzureRmManagedDiskProperties(properties *disk.Properties) map[string]interface{} {
result := make(map[string]interface{})
result["storage_account_type"] = string(properties.AccountType)
if properties.DiskSizeGB != nil {
result["disk_size_gb"] = *properties.DiskSizeGB
}
if properties.OsType != "" {
result["os_type"] = string(properties.OsType)
}

return result
}

func flattenAzureRmManagedDiskCreationData(creationData *disk.CreationData) map[string]interface{} {
result := make(map[string]interface{})
result["create_option"] = string(creationData.CreateOption)
if creationData.SourceURI != nil {
result["source_uri"] = *creationData.SourceURI
}

return result
}
Loading