azurerm_windows_virtual_machine_scale_set | azurerm_linux_virtual_machine_scale_set | azurerm_orchestrated_virtual_machine_scale_set - expose the action attribute in the automatic_instance_repair code block
#26227
Conversation
There was a problem hiding this comment.
Thanks @WodansSon! In addition to the one minor comment left in-line, there is also a test failure:
------- Stdout: -------
=== RUN TestAccLinuxVirtualMachineScaleSet_otherAutomaticRepairsPolicy
=== PAUSE TestAccLinuxVirtualMachineScaleSet_otherAutomaticRepairsPolicy
=== CONT TestAccLinuxVirtualMachineScaleSet_otherAutomaticRepairsPolicy
testcase.go:113: Step 1/8 error: After applying this test step, the plan was not empty.
stdout:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# azurerm_linux_virtual_machine_scale_set.test will be updated in-place
~ resource "azurerm_linux_virtual_machine_scale_set" "test" {
id = "/subscriptions/*******/resourceGroups/acctestRG-240605072813414110/providers/Microsoft.Compute/virtualMachineScaleSets/acctestvmss-240605072813414110"
name = "acctestvmss-240605072813414110"
# (25 unchanged attributes hidden)
~ automatic_instance_repair {
~ action = "Replace" -> "Restart"
# (2 unchanged attributes hidden)
}
~ network_interface {
+ dns_servers = []
name = "example"
# (3 unchanged attributes hidden)
~ ip_configuration {
+ application_gateway_backend_address_pool_ids = []
+ application_security_group_ids = []
name = "internal"
# (5 unchanged attributes hidden)
}
}
# (4 unchanged blocks hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccLinuxVirtualMachineScaleSet_otherAutomaticRepairsPolicy (457.25s)
FAIL
| ValidateFunc: validation.StringInSlice([]string{ | ||
| string(virtualmachinescalesets.RepairActionReimage), | ||
| string(virtualmachinescalesets.RepairActionRestart), | ||
| string(virtualmachinescalesets.RepairActionReplace), | ||
| }, false), |
There was a problem hiding this comment.
We can replace these with
| ValidateFunc: validation.StringInSlice([]string{ | |
| string(virtualmachinescalesets.RepairActionReimage), | |
| string(virtualmachinescalesets.RepairActionRestart), | |
| string(virtualmachinescalesets.RepairActionReplace), | |
| }, false), | |
| ValidateFunc: validation.StringInSlice(virtualmachinescalesets.PossibleValuesForRepairAction(), false), |
…r-azurerm into e_vmss_repair_action
|
The test failures are due to other v4.0 issues not related to this PR: 
|
There was a problem hiding this comment.
Thanks @WodansSon. I have a question about the behaviour and there are some minor things to fix up as well. Once those are resolved and the relevant tests pass then this should be good to go.
internal/services/compute/linux_virtual_machine_scale_set_resource_disk_data_test.go
Outdated
Show resolved
Hide resolved
internal/services/compute/orchestrated_virtual_machine_scale_set_resource_test.go
Show resolved
Hide resolved
|
All of the failures are not related to this PR and are service related or test misconfigurations in v4.0: 
|
There was a problem hiding this comment.
Thanks @WodansSon! I was hoping this would be good to go now, but there is a scenario which we have not tested for and raises some questions which would be good to have answered before we proceed with the current behaviour:
How would a user remove automatic_instance_repair if they wanted to removed health_probe_id?
I messed around with this in an attempt to understand what that update path would look like but removing automatic_instance_repair with health_probe_id unset in the config results in an API error even though automatic_instance_repair isn't in the payload which raises questions as to how the Service Team want the interplay between these two properties to work.
PATCH request payload:
{
"properties": {
"upgradePolicy": {
"mode": "Manual"
},
"virtualMachineProfile": {
"networkProfile": {
"networkInterfaceConfigurations": [
{
"name": "example",
"properties": {
"dnsSettings": {
"dnsServers": []
},
"enableAcceleratedNetworking": false,
"enableIPForwarding": false,
"ipConfigurations": [
{
"name": "internal",
"properties": {
"applicationGatewayBackendAddressPools": [],
"applicationSecurityGroups": [],
"loadBalancerBackendAddressPools": [
{
"id": "***"
}
],
"loadBalancerInboundNatPools": [
{
"id": "***"
}
],
"primary": true,
"privateIPAddressVersion": "IPv4",
"subnet": {
"id": "***"
}
}
}
],
"primary": true
}
}
]
},
"storageProfile": {
"imageReference": {
"offer": "UbuntuServer",
"publisher": "Canonical",
"sku": "16.04-LTS",
"version": "latest"
}
}
}
}
}
Response:
{
"error": {
"code": "BadRequest",
"message": "Automatic repairs not supported for this Virtual Machine Scale Set because a health probe or health extension was not provided."
}
}
Could you raise this and get some clarification?
| * `grace_period` - (Optional) Amount of time (in minutes, between 30 and 90) for which automatic repairs will be delayed. The grace period starts right after the VM is found unhealthy. The time duration should be specified in ISO 8601 format. Defaults to `PT30M`. | ||
| * `grace_period` - (Optional) Amount of time for which automatic repairs will be delayed. The grace period starts right after the VM is found unhealthy. Possible values are between `10` and `90` minutes. The time duration should be specified in `ISO 8601` format (e.g. `PT10M` to `PT90M`). | ||
|
|
||
| -> **Note:** Once the `grace_period` field has been set it cannot be removed. |
There was a problem hiding this comment.
Nit-picking here but strictly speaking this isn't correct. Since the property has Computed set users can remove this from their config without hitting any API limitations or experiencing a diff so I think this should be removed, or perhaps rephrased?
There was a problem hiding this comment.
Not sure if a commit is missing here but these have not been changed?
There was a problem hiding this comment.
I believe I misunderstood what you were saying, now that I have read it a few more times, what you want is for the note to be removed since if you remove it from the config is will just pull it from state?
|
|
||
| * `action` - (Optional) The repair action that will be used for repairing unhealthy virtual machines in the scale set. Possible values include `Replace`, `Restart`, `Reimage`. | ||
|
|
||
| -> **Note:** Once the `action` field has been set it cannot be removed. |
IIRC, once the Viewing and updating the service state of automatic instance repairs policy |
There was a problem hiding this comment.
I see, it's just a bit jarring to receive the following error:
"message": "Automatic repairs not supported for this Virtual Machine Scale Set because a health probe or health extension was not provided."
When neither automatic repairs and health probe ID are sent in the payload. If the Service Team are open to feedback here then a more contextual error message would go a long way and have saved us some back and forth on this, e.g.
Automatic repairs have been enabled for this Virtual Machine Scale Set previously and cannot be removed
In any case, the comment I left on the docs doesn't seem to have been addressed or there's a commit missing? Once that's sorted and been applied to all three docs then this should be good to go!
| @@ -632,7 +632,7 @@ func resourceManagedDiskUpdate(d *pluginsdk.ResourceData, meta interface{}) erro | |||
| disk, err := client.Get(ctx, *id) | |||
| if err != nil { | |||
| if response.WasNotFound(disk.HttpResponse) { | |||
| return fmt.Errorf("Managed Disk %q (Resource Group %q) was not found", name, resourceGroup) | |||
| return fmt.Errorf("managed disk %q (Resource Group %q) was not found", name, resourceGroup) | |||
There was a problem hiding this comment.
No need to fix this here and now but just so you're aware this is a legacy pattern. If you see more like these, they should be updated to use the ID to be consistent and up to date with the rest of the provider.
| return fmt.Errorf("managed disk %q (Resource Group %q) was not found", name, resourceGroup) | |
| return fmt.Errorf("%s was not found", id) |
| * `grace_period` - (Optional) Amount of time (in minutes, between 30 and 90) for which automatic repairs will be delayed. The grace period starts right after the VM is found unhealthy. The time duration should be specified in ISO 8601 format. Defaults to `PT30M`. | ||
| * `grace_period` - (Optional) Amount of time for which automatic repairs will be delayed. The grace period starts right after the VM is found unhealthy. Possible values are between `10` and `90` minutes. The time duration should be specified in `ISO 8601` format (e.g. `PT10M` to `PT90M`). | ||
|
|
||
| -> **Note:** Once the `grace_period` field has been set it cannot be removed. |
There was a problem hiding this comment.
Not sure if a commit is missing here but these have not been changed?
|
|
When automatic_instance_repair is used in linux vmss module, even though when we set enable=false, it is taking it as true, In main.tf: variable: In Terraform Plan: Terraform apply error: FYI - The same setup is working with provider version : v3.113.0 |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
Exposes the
actionattribute in theautomatic_instance_repaircode block for theazurerm_windows_virtual_machine_scale_set,azurerm_linux_virtual_machine_scale_set, andazurerm_orchestrated_virtual_machine_scale_setresources which has now GA'ed.PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_linux_virtual_machine_scale_set- expose theactionattribute in theautomatic_instance_repaircode blockazurerm_orchestrated_virtual_machine_scale_set- expose theactionattribute in theautomatic_instance_repaircode blockazurerm_windows_virtual_machine_scale_set- expose theactionattribute in theautomatic_instance_repaircode blockThis is a (please select all that apply):
Related Issue(s)
Supersedes PR #22330
Note
If this PR changes meaningfully during the course of review please update the title and description as required.