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.

Sign up for GitHub

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

Jump to bottom

VM/VMSS: support for StandardSSD_LRS managed disk type #1901

Merged
merged 4 commits into from
Sep 12, 2018
Merged

VM/VMSS: support for StandardSSD_LRS managed disk type #1901

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b5daf3
added support for StorageAccountTypesStandardSSDLRS
ira-bryndzei Sep 9, 2018
2a958f2
delete failed test block
ira-bryndzei Sep 12, 2018
f247314
Documenting the new `StandardSSD_LRS` type
tombuildsstuff Sep 12, 2018
bb4792f
Adding tests for VM/VMSS for the new StandardSSD_LRS type
tombuildsstuff Sep 12, 2018
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
6 changes: 4 additions & 2 deletions azurerm/resource_arm_managed_disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func resourceArmManagedDisk() *schema.Resource {
ValidateFunc: validation.StringInSlice([]string{
string(compute.StorageAccountTypesStandardLRS),
string(compute.StorageAccountTypesPremiumLRS),
string(compute.StorageAccountTypesStandardSSDLRS),
}, true),
DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
},
Expand Down Expand Up @@ -124,11 +125,12 @@ func resourceArmManagedDiskCreate(d *schema.ResourceData, meta interface{}) erro
zones := expandZones(d.Get("zones").([]interface{}))

var skuName compute.StorageAccountTypes
// TODO: support for the StandardSSD
if strings.EqualFold(storageAccountType, string(compute.StorageAccountTypesPremiumLRS)) {
skuName = compute.StorageAccountTypesPremiumLRS
} else {
} else if strings.EqualFold(storageAccountType, string(compute.StorageAccountTypesStandardLRS)) {
skuName = compute.StorageAccountTypesStandardLRS
} else if strings.EqualFold(storageAccountType, string(compute.StorageAccountTypesStandardSSDLRS)) {
skuName = compute.StorageAccountTypesStandardSSDLRS
}

createDisk := compute.Disk{
Expand Down
15 changes: 13 additions & 2 deletions azurerm/resource_arm_managed_disk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@ func TestAccAzureRMManagedDisk_update(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMManagedDiskDestroy,
Steps: []resource.TestStep{
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this test step is in error, it causes a failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@katbyte thanks for your reply. I'll take a look at it.

Copy link
Contributor Author

@ira-bryndzei ira-bryndzei Sep 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@katbyte Is it possible to run acceptance test authenticating using the Azure CLІ with user account as I have no ability to create service principal?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ira-bryndzei,

No, currently you need a SP to run them.

Copy link
Contributor

Choose a reason for hiding this comment

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

to give some additional information here, this test is failing because the configuration in the preConfig is being used for multiple entries (with different storage account types) - whereas the configuration actually creates a disk of the storage account type compute.StorageAccountTypesStandardLRS. It should be possible to add another configuration which includes the different storage account type to allow this to pass

Copy link
Contributor

Choose a reason for hiding this comment

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

@ira-bryndzei it's probably easiest to revert the additional test block in this file (but leave the updated constant/enum name) for the moment (we can then push a new test for this new disk type in a follow up commit) - what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tombuildsstuff that's work for me. I'll revert that block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tombuildsstuff I deleted that block

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thanks - I've pushed a couple of commits (one to add the new values to the docs, which I'd missed before; and another to add the tests as discussed above) - I hope you don't mind!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great! thanks

Config: preConfig,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMManagedDiskExists(resourceName, &d, true),
resource.TestCheckResourceAttr(resourceName, "tags.%", "3"),
resource.TestCheckResourceAttr(resourceName, "tags.environment", "acctest"),
resource.TestCheckResourceAttr(resourceName, "tags.cost-center", "ops"),
resource.TestCheckResourceAttr(resourceName, "disk_size_gb", "1"),
resource.TestCheckResourceAttr(resourceName, "storage_account_type", string(compute.StorageAccountTypesStandardSSDLRS)),
),
},
{
Config: preConfig,
Check: resource.ComposeTestCheckFunc(
Expand All @@ -139,7 +150,7 @@ func TestAccAzureRMManagedDisk_update(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "tags.environment", "acctest"),
resource.TestCheckResourceAttr(resourceName, "tags.cost-center", "ops"),
resource.TestCheckResourceAttr(resourceName, "disk_size_gb", "1"),
resource.TestCheckResourceAttr(resourceName, "storage_account_type", string(compute.StandardLRS)),
resource.TestCheckResourceAttr(resourceName, "storage_account_type", string(compute.StorageAccountTypesStandardLRS)),
),
},
{
Expand All @@ -149,7 +160,7 @@ func TestAccAzureRMManagedDisk_update(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.environment", "acctest"),
resource.TestCheckResourceAttr(resourceName, "disk_size_gb", "2"),
resource.TestCheckResourceAttr(resourceName, "storage_account_type", string(compute.PremiumLRS)),
resource.TestCheckResourceAttr(resourceName, "storage_account_type", string(compute.StorageAccountTypesPremiumLRS)),
),
},
},
Expand Down
10 changes: 6 additions & 4 deletions azurerm/resource_arm_virtual_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,9 @@ func resourceArmVirtualMachine() *schema.Resource {
Computed: true,
ConflictsWith: []string{"storage_os_disk.0.vhd_uri"},
ValidateFunc: validation.StringInSlice([]string{
string(compute.PremiumLRS),
string(compute.StandardLRS),
string(compute.StorageAccountTypesPremiumLRS),
string(compute.StorageAccountTypesStandardLRS),
string(compute.StorageAccountTypesStandardSSDLRS),
}, true),
},

Expand Down Expand Up @@ -285,8 +286,9 @@ func resourceArmVirtualMachine() *schema.Resource {
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice([]string{
string(compute.PremiumLRS),
string(compute.StandardLRS),
string(compute.StorageAccountTypesPremiumLRS),
string(compute.StorageAccountTypesStandardLRS),
string(compute.StorageAccountTypesStandardSSDLRS),
}, true),
},

Expand Down
10 changes: 6 additions & 4 deletions azurerm/resource_arm_virtual_machine_scale_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,9 @@ func resourceArmVirtualMachineScaleSet() *schema.Resource {
Computed: true,
ConflictsWith: []string{"storage_profile_os_disk.vhd_containers"},
ValidateFunc: validation.StringInSlice([]string{
string(compute.PremiumLRS),
string(compute.StandardLRS),
string(compute.StorageAccountTypesPremiumLRS),
string(compute.StorageAccountTypesStandardLRS),
string(compute.StorageAccountTypesStandardSSDLRS),
}, true),
},

Expand Down Expand Up @@ -537,8 +538,9 @@ func resourceArmVirtualMachineScaleSet() *schema.Resource {
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice([]string{
string(compute.PremiumLRS),
string(compute.StandardLRS),
string(compute.StorageAccountTypesPremiumLRS),
string(compute.StorageAccountTypesStandardLRS),
string(compute.StorageAccountTypesStandardSSDLRS),
}, true),
},
},
Expand Down