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

provider/azurerm: Add support for managed availability sets. #12532

Merged
merged 2 commits into from
Mar 8, 2017

Conversation

rrudduck
Copy link
Contributor

@rrudduck rrudduck commented Mar 8, 2017

This PR enables specifying an availability set as being managed. An availability set must be managed in order to have VMs with managed disks as members.

Copy link
Contributor

@pmcatominey pmcatominey left a comment

Choose a reason for hiding this comment

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

Hey @rrudduck

Just need clarification on the SKU changes and an update to the docs.

@@ -93,6 +102,13 @@ func resourceArmAvailabilitySetCreate(d *schema.ResourceData, meta interface{})
Tags: expandTags(tags),
}

if managed == true {
n := "Aligned"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify this behaviour? The normal approach is to expose the SKU options on on the resource schema as special behaviour can put us in tricky situations with future API updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the documentation here: https://docs.microsoft.com/en-us/rest/api/compute/availabilitysets/availabilitysets-create it's not a full sku object. It's just sku.name that is used.

sku.name Specifies whether the availability set is managed or not. Possible values are: Aligned or Classic. An Aligned availability set is managed, Classic is not.

I thought the easier change was to specify managed or not as a boolean since that is how it's represented in the portal. Additionally in the sdk (https://github.com/Azure/azure-sdk-for-go/blob/master/arm/compute/models.go#L415) there is a property for "managed" but it does not appear in the REST docs and does not appear to be used when I tested it.

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 should also not that there did not appear to be a constant for that value defined in the SDK either.

https://github.com/Azure/azure-sdk-for-go/search?q=Aligned&type=Code&utf8=%E2%9C%93

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification!

resource_group_name = "${azurerm_resource_group.test.name}"
platform_update_domain_count = 10
platform_fault_domain_count = 1
managed = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: formatting

@stack72
Copy link
Contributor

stack72 commented Mar 8, 2017

Tests pass @pmcatominey :)

% make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMAvailabilitySet_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/08 20:06:49 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMAvailabilitySet_ -timeout 120m
=== RUN   TestAccAzureRMAvailabilitySet_importBasic
--- PASS: TestAccAzureRMAvailabilitySet_importBasic (70.50s)
=== RUN   TestAccAzureRMAvailabilitySet_basic
--- PASS: TestAccAzureRMAvailabilitySet_basic (68.69s)
=== RUN   TestAccAzureRMAvailabilitySet_disappears
--- PASS: TestAccAzureRMAvailabilitySet_disappears (67.36s)
=== RUN   TestAccAzureRMAvailabilitySet_withTags
--- PASS: TestAccAzureRMAvailabilitySet_withTags (63.83s)
=== RUN   TestAccAzureRMAvailabilitySet_withDomainCounts
--- PASS: TestAccAzureRMAvailabilitySet_withDomainCounts (66.06s)
=== RUN   TestAccAzureRMAvailabilitySet_managed
--- PASS: TestAccAzureRMAvailabilitySet_managed (68.72s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	405.220s

@rrudduck
Copy link
Contributor Author

rrudduck commented Mar 8, 2017

Formatting should be resolved.

@stack72 stack72 merged commit bf4d6d5 into hashicorp:master Mar 8, 2017
stack72 pushed a commit that referenced this pull request Mar 8, 2017
* Add support for managed availability sets.

* Formatting.
yanndegat pushed a commit to yanndegat/terraform that referenced this pull request Mar 13, 2017
…rp#12532)

* Add support for managed availability sets.

* Formatting.
@ghost
Copy link

ghost commented Apr 15, 2020

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.

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

Successfully merging this pull request may close these issues.

3 participants