Update azurerm_linux|windows_virtual_machine - support platform_fault_domain for VMSS Flex support

[ArcturusZhang]

No description provided.

[jackofallops]

Hi @ArcturusZhang - Thanks for this PR.

I have concerns around the behaviour of this property. The validation suggests that -1 is not a valid value, but this is the default and the docs suggest the user can specify it (which they will not currently be able to). Can you please check which is correct and I'll continue review?

Thanks

[ArcturusZhang]

Hi @ArcturusZhang - Thanks for this PR.

I have concerns around the behaviour of this property. The validation suggests that -1 is not a valid value, but this is the default and the docs suggest the user can specify it (which they will not currently be able to). Can you please check which is correct and I'll continue review?

Thanks

Oh, sorry I missed that! I will change the validation to AtLeast(-1) to allow the user to explicit set -1 to it.

The logic in this attribute is like this:
We could set a platform_fault_domain_count in the associated azurerm_orchestrated_virtual_machine_scale_set resource to define the count of the platform fault domains. For instance, it is assigned by 2. Then all the VMs added to that azurerm_orchestrated_virtual_machine_scale_set could assign a platform_fault_domain like an index, from 0 to 1 in my example. In the REST API, you could leave this property to null to let the API choose a platform_fault_domain for you.

To support the null behaviour, I make the default of this attribute to -1 because 0 has specific meaning other than absence.

Also when we are not setting the platform_fault_domain (or setting it to -1 in terraform), the API will not return the automatically-chosen value in the GET response (the platform fault domain will still be null when you send null). We have another API the instanceView to check the actual platform_fault_domain value.

[ArcturusZhang]

Hi @jackofallops I just updated the validation function, please take a look, thanks

[tombuildsstuff]

A few comments inline but this otherwise LGTM 👍

[tombuildsstuff]

Merging the suggestions so we can get this into this release

dismissing since changes have been pushed

[tombuildsstuff]

Tests look good 👍

[ghost]

This has been released in version 2.51.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 = "~> 2.51.0"
}
# ... other configuration ...

[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 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 👉 [email protected]. Thanks!