-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
VMSS
- Support updating zones
#27288
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ms-zhenhua, thanks for this! Just updating the wording on the docs and wondering if we need to do the third test step for this change
* `zones` - (Optional) Specifies a list of Availability Zones in which this Linux Virtual Machine Scale Set should be located. Changing this forces a new Linux Virtual Machine Scale Set to be created. | ||
* `zones` - (Optional) Specifies a list of Availability Zones in which this Linux Virtual Machine Scale Set should be located. | ||
|
||
-> **Note:** Removing existing zones from existing Virtual Machine Scale Set is currently not supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> **Note:** Removing existing zones from existing Virtual Machine Scale Set is currently not supported. | |
-> **Note:** Updating `zones` to remove an existing zone forces a new Virtual Machine Scale Set to be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Thanks.
* `zones` - (Optional) Specifies a list of Availability Zones across which the Virtual Machine Scale Set will create instances. Changing this forces a new Virtual Machine Scale Set to be created. | ||
* `zones` - (Optional) Specifies a list of Availability Zones across which the Virtual Machine Scale Set will create instances. | ||
|
||
-> **Note:** Removing existing zones from existing Virtual Machine Scale Set is currently not supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> **Note:** Removing existing zones from existing Virtual Machine Scale Set is currently not supported. | |
-> **Note:** Updating `zones` to remove an existing zone forces a new Virtual Machine Scale Set to be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Thanks.
* `zones` - (Optional) Specifies a list of Availability Zones in which this Windows Virtual Machine Scale Set should be located. Changing this forces a new Windows Virtual Machine Scale Set to be created. | ||
* `zones` - (Optional) Specifies a list of Availability Zones in which this Windows Virtual Machine Scale Set should be located. | ||
|
||
-> **Note:** Removing existing zones from existing Virtual Machine Scale Set is currently not supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> **Note:** Removing existing zones from existing Virtual Machine Scale Set is currently not supported. | |
-> **Note:** Updating `zones` to remove an existing zone forces a new Virtual Machine Scale Set to be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Thanks.
}, | ||
data.ImportStep("admin_password"), | ||
{ | ||
Config: r.scalingZonesSingle(data), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that we can add new zones but can't remove existing zones, so will this step recreate the resource? If so, we shouldn't have this step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct. It will recreate the resource. I have removed this step. Thanks.
), | ||
}, | ||
{ | ||
Config: r.singleZone(data), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that we can add new zones but can't remove existing zones, so will this step recreate the resource? If so, we shouldn't have this step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct. It will recreate the resource. I have removed this step. Thanks.
}, | ||
data.ImportStep("admin_password"), | ||
{ | ||
Config: r.scalingZonesSingle(data), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that we can add new zones but can't remove existing zones, so will this step recreate the resource? If so, we shouldn't have this step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct. It will recreate the resource. I have removed this step. Thanks.
Hi @mbfrahry, Thank you for your review. I have updated the code. Please kindly take another review. |
👍🏾 |
@mbfrahry thanks for the review! Please let us know if there are any issues remaining! Would love to make the deployment train this week |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🍒
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
Support updating
zones
field inazurerm_linux_virtual_machine_scale_set
,azurerm_orchestrated_virtual_machine_scale_set
andazurerm_windows_virtual_machine_scale_set
.PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
--- PASS: TestAccOrchestratedVirtualMachineScaleSet_zoneUpdate (342.49s)
--- PASS: TestAccWindowsVirtualMachineScaleSet_scalingZonesUpdate (940.20s)
--- PASS: TestAccLinuxVirtualMachineScaleSet_scalingZonesUpdate (1305.01s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/compute 1340.901s
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
VMSS
- Support updatingzones
This is a (please select all that apply):
Related Issue(s)
Fixes #0000
Note
If this PR changes meaningfully during the course of review please update the title and description as required.