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

azurerm_windows_virtual_machine_scale_set /azurerm_linux_virtual_machine_scale_set /azurerm_orchestrated_virtual_machine_scale_set/azurerm_linux_virtual_machine/azurerm_windows_virtual_machine - Add support for missing fields #17571

Merged
merged 37 commits into from
Sep 2, 2022
Merged

azurerm_windows_virtual_machine_scale_set /azurerm_linux_virtual_machine_scale_set /azurerm_orchestrated_virtual_machine_scale_set/azurerm_linux_virtual_machine/azurerm_windows_virtual_machine - Add support for missing fields #17571

merged 37 commits into from
Sep 2, 2022

Conversation

WodansSon
Copy link
Collaborator

@WodansSon WodansSon commented Jul 9, 2022 •
edited
Loading

Adding coverage for the below missing fields which are currently exposed in the new version of the API:

Orchestrated VMSS:

  • capacity_reservation_group_id
  • single_placement_group
  • extension_operations_enabled

extension:

  • protected_settings_from_key_vault
  • suppress_failures_enabled

additional_capabilities:

  • ultra_ssd_enabled

automatic_instance_repair:

  • repair_action

network_interface:

  • fpga_enabled

public_ip_address:

  • version
  • sku_name

linux_configuration:

  • patch_assessment_mode

windows_configuration:

  • patch_assessment_mode

source_image_id:

  • add support for Community Gallery Images
  • add support for Shared Gallery Images

Linux VMSS:

  • host_group_id
  • extension_operations_enabled

data_disk:

  • name

scale_in [NEW CODE BLOCK]:

  • rule
  • force_deletion_enabled

additional_capabilities:

  • hibernation_enabled

hardware_profile [NEW CODE BLOCK]:

  • virtual_cpus_available
  • virtual_cpus_per_core

rolling_upgrade_policy:

  • cross_zone_upgrade_enabled
  • prioritize_unhealthy_instances_enabled

automatic_instance_repair:

  • repair_action

spot_restore [NEW CODE BLOCK]:

  • enabled
  • timeout

network_interface:

  • delete_action

  • fpga_enabled

    public_ip_address:

    • delete_action
    • version

source_image_id:

  • add support for Community Gallery Images
  • add support for Shared Gallery Images

gallery_applications [NEW CODE BLOCK]:

  • configuration_reference_blob_uri
  • order
  • package_referenceId
  • tag

Deprecated:

  • scale_in_policy -> new scale_in code block

Windows VMSS:

  • host_group_id
  • extension_operations_enabled

data_disk:

  • name

scale_in [NEW CODE BLOCK]:

  • rule
  • force_deletion_enabled

additional_capabilities:

  • hibernation_enabled

hardware_profile [NEW CODE BLOCK]:

  • virtual_cpus_available
  • virtual_cpus_per_core

rolling_upgrade_policy:

  • cross_zone_upgrade_enabled
  • prioritize_unhealthy_instances_enabled

automatic_instance_repair:

  • repair_action

spot_restore [NEW CODE BLOCK]:

  • enabled
  • timeout

network_interface:

  • delete_action

  • fpga_enabled

    public_ip_address:

    • delete_action
    • version

source_image_id:

  • add support for Community Gallery Images
  • add support for Shared Gallery Images

gallery_applications [NEW CODE BLOCK]:

  • configuration_reference_blob_uri
  • order
  • package_referenceId
  • tag

Deprecated:

  • scale_in_policy -> new scale_in code block

Linux VM:

source_image_id:

  • add support for Community Gallery Images
  • add support for Shared Gallery Images

Windows VM:

source_image_id:

  • add support for Community Gallery Images
  • add support for Shared Gallery Images

(fixes #14820)

@WodansSon WodansSon added this to the Future milestone Jul 9, 2022
This was referenced Jul 29, 2022
Closed
Closed
@WodansSon WodansSon changed the title [WIP]azurerm_windows_virtual_machine_scale_set /azurerm_linux_virtual_machine_scale_set /azurerm_orchestrated_virtual_machine_scale_set - Add support for missing fields [WIP]azurerm_windows_virtual_machine_scale_set /azurerm_linux_virtual_machine_scale_set /azurerm_orchestrated_virtual_machine_scale_set/azurerm_linux_virtual_machine/azurerm_windows_virtual_machine - Add support for missing fields Aug 1, 2022
@WodansSon WodansSon dismissed katbyte’s stale review August 12, 2022 06:37

Comments have been addressed

@WodansSon
Copy link
Collaborator Author

NOTE: This PR is now code complete and we are now just adding test coverage for all of the new fields.

@WodansSon WodansSon changed the title [WIP]azurerm_windows_virtual_machine_scale_set /azurerm_linux_virtual_machine_scale_set /azurerm_orchestrated_virtual_machine_scale_set/azurerm_linux_virtual_machine/azurerm_windows_virtual_machine - Add support for missing fields azurerm_windows_virtual_machine_scale_set /azurerm_linux_virtual_machine_scale_set /azurerm_orchestrated_virtual_machine_scale_set/azurerm_linux_virtual_machine/azurerm_windows_virtual_machine - Add support for missing fields Aug 12, 2022
myc2h6o and others added 2 commits August 25, 2022 14:43
@myc2h6o
include #18099 for tests
41c67e1 
* Update resources

* Add tests

* Fix hardware_profile

* Fix nil data disk name

* Fix nil sku

* Fix legacy orchestrated vmss failing tests

* Fix test

* Fix EnableCrossZoneUpgrade

* Fix sku_name default
@WodansSon
Merge branch 'main' of https://github.com/hashicorp/terraform-provide…
5f3e6ad 
…r-azurerm into e-vmss-updates
@myc2h6o myc2h6o mentioned this pull request Aug 27, 2022
Merged
myc2h6o and others added 5 commits August 31, 2022 16:17
@myc2h6o
adding #18143
e9e60fb 
* Remove `fpga_enabled`

* Remove `protected_settings_from_key_vault`

* Remove `hardware_profile`

* Remove `hibernation_enabled`

* Fix flattenVirtualMachineScaleSetGalleryApplications

* Fix `gallery_applications` tests

* Fix TestAccOrchestratedVirtualMachineScaleSet_otherUltraSsd

* Remove orchestrated `single_placement_group`

* Fix TestAccOrchestratedVirtualMachineScaleSet_publicIPSkuName

* Fix setting `extension_operations_enabled`

* Remove error HealthExtension in orchestrated vmss tests

* fmt

* Fix `host_group_id` test

* Fix `rolling_upgrade_policy` tests

* Make `spot_restore` `Computed` and add default test

* Fix ipv6 tests

* Remove `automatic_instance_repair.repair_action`

* Fix `host_group_id`

* Fix legacy orchestrated vmss extension

* Fix `extension_operations_enabled` when `provision_vm_agent` is disbled

* mark public ip `version` as `ForceNew` due to no payload in update body

* Fix `TestAccWindowsVirtualMachineScaleSet_networkIPv6`

* Add missing `spot_restore` get/set in windows

* Add HealthExtension back with updated instance count

* Fix ultra_ssd test

* Fix orchestrated linuxConfig `patch_assessment_mode` update

* Fix disk caching test

* Fix extension disable test

* patch_assessment_mode update test removal

* Fix `TestAccOrchestratedVirtualMachineScaleSet_disksOSDiskCaching`

* Fix `TestAccWindowsVirtualMachineScaleSet_networkIPv6`

* Add `single_placement_group` back without test

* Add `single_placement_group` test back with skipping message

* Skipping ipv6 test before api version 2022-03-01
@WodansSon
update spg to allow null and update tests
09e379f
@WodansSon
Update docs for SPG behavior
34c3fcf
@WodansSon
Remove Orchestrated PreCheck per KT
a4c23a0
@WodansSon
remove extraneous nil check in read function
b03784e
katbyte
katbyte approved these changes Sep 1, 2022
View reviewed changes
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks jeffery, aside from a few minor comments this looks good

website/docs/r/linux_virtual_machine_scale_set.html.markdown
@@ -90,8 +90,6 @@ resource "azurerm_linux_virtual_machine_scale_set" "example" {

## Argument Reference

The following arguments are supported:
Copy link
Collaborator

Choose a reason for hiding this comment

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

we shouldn't be removing this?

website/docs/r/linux_virtual_machine_scale_set.html.markdown Outdated

---

* `additional_capabilities` - (Optional) A `additional_capabilities` block as defined below.

* `admin_password` - (Optional) The Password which should be used for the local-administrator on this Virtual Machine. Changing this forces a new resource to be created.

-> **NOTE:** When an `admin_password` is specified `disable_password_authentication` must be set to `false`.
-> **NOTE:** When a `admin_password` is specified `disable_password_authentication` must be set to `false`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

an is the correct article to use here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think so, if the first letter makes a vowel-type sound, you use "an"; if the first letter would make a consonant-type sound, you use "a." However, there are some exceptions to these rules.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

website/docs/r/linux_virtual_machine_scale_set.html.markdown
@@ -150,7 +148,7 @@ The following arguments are supported:

-> In general we'd recommend using SSH Keys for authentication rather than Passwords - but there's tradeoff's to each - please [see this thread for more information](https://security.stackexchange.com/questions/69407/why-is-using-an-ssh-key-more-secure-than-using-passwords).

-> **NOTE:** When an `admin_password` is specified `disable_password_authentication` must be set to `false`.
-> **NOTE:** When a `admin_password` is specified `disable_password_authentication` must be set to `false`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

website/docs/r/linux_virtual_machine_scale_set.html.markdown Outdated
@@ -318,7 +328,7 @@ A `diff_disk_settings` block supports the following:

---

An `extension` block supports the following:
A `extension` block supports the following:
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here?

Suggested change
A `extension` block supports the following:
An `extension` block supports the following:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

website/docs/r/linux_virtual_machine_scale_set.html.markdown
Comment on lines -539 to +593
* `identity` - An `identity` block as defined below.
* `identity` - A `identity` block as defined below.

* `unique_id` - The Unique ID for this Linux Virtual Machine Scale Set.

---

An `identity` block exports the following:
A `identity` block exports the following:
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we please fix all these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

website/docs/r/linux_virtual_machine_scale_set.html.markdown Outdated
* `identity` - (Optional) An `identity` block as defined below.
* `host_group_id` - (Optional) Specifies the ID of the dedicated host group that the virtual machine scale set resides in. Changing this forces a new resource to be created.

* `identity` - (Optional) A `identity` block as defined below.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `identity` - (Optional) A `identity` block as defined below.
* `identity` - (Optional) An `identity` block as defined below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

website/docs/r/orchestrated_virtual_machine_scale_set.html.markdown Outdated

* `public_ip_address` - (Optional) A `public_ip_address` block as defined below.

* `subnet_id` - (Optional) The ID of the Subnet which this IP Configuration should be connected to.

~> **NOTE:** `subnet_id` is required if version is set to `IPv4`.
-> **NOTE:** `subnet_id` is required if version is set to `IPv4`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-> **NOTE:** `subnet_id` is required if version is set to `IPv4`.
-> **NOTE:** `subnet_id` is required if version is set to `IPv4`.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

internal/services/compute/orchestrated_virtual_machine_scale_set_extensions_resource_test.go
Comment on lines +158 to +159
# Orchestrated VMSS allocation will timeout at service side due to extension, set instances to 0 to avoid the timeout
instances = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sounds like a bug as it timesout not errors 0- can we open an issue on the rest specs and link it here. Then add a note to the docs and maybe add a check in code to throw an error if the user doesn't correctly do this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This appears to be a known issue and has been documented as being the extensions issue.

@WodansSon WodansSon modified the milestones: Future, v3.21.0 Sep 2, 2022
@WodansSon WodansSon merged commit fa35df5 into main Sep 2, 2022
@WodansSon WodansSon deleted the e-vmss-updates branch September 2, 2022 00:33
WodansSon added a commit that referenced this pull request Sep 2, 2022
@WodansSon
updated to include #17571
95d228b
WodansSon added a commit that referenced this pull request Sep 2, 2022
@WodansSon
update to include properties for #17571
efacf63
@github-actions
Copy link

github-actions bot commented Sep 2, 2022

This functionality has been released in v3.21.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@orgads
Copy link
Contributor

orgads commented Sep 2, 2022

Any change of implementing also #6117? It's the 4th most voted open issue.

@howden howden mentioned this pull request Sep 7, 2022
Merged
@grethinam
Copy link

grethinam commented Sep 16, 2022 •
edited
Loading

LinuxVM(azurerm_linux_virtual_machine) and WindowsVM(azurerm_windows_virtual_machine) also need assessment_mode

Microsoft.Compute change log

linux_configuration:

patch_assessment_mode

windows_configuration:

patch_assessment_mode

@WodansSon

@grethinam grethinam mentioned this pull request Sep 16, 2022
Closed
1 task
@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@myc2h6o myc2h6o myc2h6o left review comments

@katbyte katbyte katbyte approved these changes

@tombuildsstuff tombuildsstuff tombuildsstuff approved these changes

Assignees
No one assigned
Labels
documentation enhancement service/virtual-machine service/vmss Virtual Machine Scale Sets size/XL
Projects
None yet
Milestone
v3.21.0
6 participants
@WodansSon @orgads @grethinam @tombuildsstuff @katbyte @myc2h6o