New Resource: `azurerm_system_center_virtual_machine_manager_virtual_machine_instance` #27622

neil-yechenwei · 2024-10-11T07:18:40Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

This PR is to support System Center Virtual Machine Manager Virtual Machine Instance. It is a virtual machine instance management tool primarily used for managing virtual resources in private and hybrid cloud environments.

Note: SCVMM is part of Microsoft System Center and is specifically designed to centrally manage data center virtualization infrastructure.

API reference: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/scvmm/resource-manager/Microsoft.ScVmm/stable/2023-10-07/scvmm.json

Overview: https://learn.microsoft.com/en-us/azure/azure-arc/system-center-virtual-machine-manager/overview

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevent documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/basic (764.98s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/requiresImport (840.68s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/complete (825.70s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/update (934.30s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/inventoryItemId (330.69s)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

New Resource: azurerm_system_center_virtual_machine_manager_virtual_machine_instance

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

…achine_instance

…machinemanagervirtualmachineinstance

neil-yechenwei · 2024-10-15T09:38:49Z

@ms-zhenhua , thanks for the comments. Please take another look.

ms-zhenhua

Hi @neil-yechenwei ,

Thank you for your updates. LGTM~

sidsinghms · 2024-10-18T08:46:38Z

Hi @rcskosir
With this PR we are adding the VirtualMachineInstance resource.
We had earlier added other resources, for our RP Azure Arc enabled SCVMM.

Requesting review of this PR so that we are able to complete our support for our RP on Terraform.
Several customers are lined up, waiting for Terraform support for quite some time now. We have a good opportunity to drive adoption from some of the S500 customers to our service. One of our major customers, NVIDIA, is currently blocked without Terraform support.

catriona-m

Thanks for this @neil-yechenwei - I left a couple of comments for you to consider but once those are addressed we can take another look. Thanks!

catriona-m · 2024-12-04T12:58:48Z

...chinemanager/system_center_virtual_machine_manager_virtual_machine_instance_resource_test.go

+func TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential(t *testing.T) {
+	// NOTE: this is a combined test rather than separate split out tests because only one System Center Virtual Machine Manager Server can be onboarded at a time on a given Custom Location
+
+	if os.Getenv("ARM_TEST_CUSTOM_LOCATION_ID") == "" || os.Getenv("ARM_TEST_FQDN") == "" || os.Getenv("ARM_TEST_USERNAME") == "" || os.Getenv("ARM_TEST_PASSWORD") == "" || os.Getenv("ARM_TEST_VIRTUAL_NETWORK_INVENTORY_NAME") == "" {


Is it necessary to require an ARM_TEST_CUSTOM_LOCATION_ID env var here? It looks like we have a resource that could be used for this here.

Also, would it be possible to omit ARM_TEST_USERNAME and ARM_TEST_PASSWORD and just specify these in the test config?

The test custom location was provided by the service team because provisioning this resource requires additional configurations for SCVMM VM Instance. So we have to use the custom location provided by service team.

ARM_TEST_USERNAME is provided by the service team. It must specify the real username, and the username currently in use is associated with the resources/configurations in the lab. So we have to use ARM_TEST_USERNAME.

catriona-m · 2024-12-04T13:15:26Z

...chinemanager/system_center_virtual_machine_manager_virtual_machine_instance_resource_test.go

+provider "azurerm" {
+  features {}
+}
+
+data "azurerm_system_center_virtual_machine_manager_inventory_items" "test" {
+  inventory_type                                  = "Cloud"
+  system_center_virtual_machine_manager_server_id = azurerm_system_center_virtual_machine_manager_server.test.id
+}
+
+resource "azurerm_system_center_virtual_machine_manager_cloud" "test" {
+  name                                                           = "acctest-scvmmc-%d"
+  location                                                       = azurerm_resource_group.test.location
+  resource_group_name                                            = azurerm_resource_group.test.name
+  custom_location_id                                             = azurerm_system_center_virtual_machine_manager_server.test.custom_location_id
+  system_center_virtual_machine_manager_server_inventory_item_id = data.azurerm_system_center_virtual_machine_manager_inventory_items.test.inventory_items[0].id
+}
+
+data "azurerm_system_center_virtual_machine_manager_inventory_items" "test2" {
+  inventory_type                                  = "VirtualMachineTemplate"
+  system_center_virtual_machine_manager_server_id = azurerm_system_center_virtual_machine_manager_server.test.id
+}
+
+resource "azurerm_system_center_virtual_machine_manager_virtual_machine_template" "test" {
+  name                                                           = "acctest-scvmmvmt-%d"
+  location                                                       = azurerm_resource_group.test.location
+  resource_group_name                                            = azurerm_resource_group.test.name
+  custom_location_id                                             = azurerm_system_center_virtual_machine_manager_server.test.custom_location_id
+  system_center_virtual_machine_manager_server_inventory_item_id = data.azurerm_system_center_virtual_machine_manager_inventory_items.test2.inventory_items[0].id
+}
+
+data "azurerm_system_center_virtual_machine_manager_inventory_items" "test3" {
+  inventory_type                                  = "VirtualNetwork"
+  system_center_virtual_machine_manager_server_id = azurerm_system_center_virtual_machine_manager_server.test.id
+}
+
+resource "azurerm_system_center_virtual_machine_manager_virtual_network" "test" {
+  name                                                           = "acctest-scvmmvnet-%d"
+  location                                                       = azurerm_resource_group.test.location
+  resource_group_name                                            = azurerm_resource_group.test.name
+  custom_location_id                                             = azurerm_system_center_virtual_machine_manager_server.test.custom_location_id
+  system_center_virtual_machine_manager_server_inventory_item_id = [for item in data.azurerm_system_center_virtual_machine_manager_inventory_items.test3.inventory_items : item.id if item.name == "%s"][0]
+}
+
+resource "azurerm_system_center_virtual_machine_manager_availability_set" "test" {
+  name                                            = "acctest-scvmmas-%d"
+  resource_group_name                             = azurerm_resource_group.test.name
+  location                                        = azurerm_resource_group.test.location
+  custom_location_id                              = azurerm_system_center_virtual_machine_manager_server.test.custom_location_id
+  system_center_virtual_machine_manager_server_id = azurerm_system_center_virtual_machine_manager_server.test.id
+}


could some of this be moved into a template?

I have moved the common parts in the test cases to the template.

catriona-m · 2024-12-04T13:42:38Z

...ualmachinemanager/system_center_virtual_machine_manager_virtual_machine_instance_resource.go

+				parameters.Properties.AvailabilitySets = availabilitySets
+			}
+
+			needToRestart := metadata.ResourceData.HasChange("hardware") || metadata.ResourceData.HasChange("network_interface") || metadata.ResourceData.HasChange("storage_disk")


should we note in the docs that changing these will cause a restart?

catriona-m · 2024-12-04T13:44:13Z

...ualmachinemanager/system_center_virtual_machine_manager_virtual_machine_instance_resource.go

+	result := virtualmachineinstances.HardwareProfile{}
+
+	// As TF always sets bool value to false when it isn't set, so it has to use d.GetRawConfig() to determine whether it is set in the tf config
+	if v := d.GetRawConfig().AsValueMap()["hardware"].AsValueSlice()[0].AsValueMap()["limit_cpu_for_migration_enabled"]; !v.IsNull() {


is there a default value for this that would make sense for this property so we could avoid this?

The default value of limit_cpu_for_migration_enabled would be changed based on the different configurations. So we can't set a default value for it. The service team also suggested that we should not set a default value.

catriona-m · 2024-12-04T15:01:52Z

...ger/validate/system_center_virtual_machine_manager_virtual_machine_instance_computer_name.go

+	}
+
+	if !regexp.MustCompile("^[a-zA-Z0-9]{1,}$").MatchString(v) {
+		errors = append(errors, fmt.Errorf("%q may contain alphanumeric characters", k))


Suggested change

errors = append(errors, fmt.Errorf("%q may contain alphanumeric characters", k))

errors = append(errors, fmt.Errorf("%q must only contain alphanumeric characters", k))

…machinemanagervirtualmachineinstance

neil-yechenwei · 2024-12-05T08:31:38Z

@catriona-m , thanks for the comments. I updated PR. Please take another look. Below is the latest test result that I just now triggered.

--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/basic (850.75s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/requiresImport (943.84s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/complete (1119.72s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/update (2363.51s)
--- PASS: TestAccSystemCenterVirtualMachineManagerVirtualMachineInstanceSequential/scvmmVirtualMachineInstance/inventoryItemId (878.34s)

…machinemanagervirtualmachineinstance

neil-yechenwei added 30 commits December 5, 2023 12:12

New Resource: azurerm_system_center_virtual_machine_manager_virtual_m…

e3ac09d

…achine_instance

update code

ada2088

update code

8e5a5bf

update code

5b46fee

update code

997cf29

update code

6f7d2ea

merge latest change

abe39d6

merge latest change

7b084ed

rename property

2fb19a0

Merge remote-tracking branch 'upstream/main' into systemcentervirtual…

7c6fa7e

…machinemanagervirtualmachineinstance

add md file and tc

bf3371f

update schema

1d36a47

remove requiredwith

71bfa14

update shcema

da46bb1

update schema

53c70b2

mark os.computer_name to required

7483211

update schema

5104437

expand os

65b0065

add validation for storage disk name

906c477

add validation

a3499b1

add tc

f9d483b

add restart

5e6dada

remove default value

ad411da

update validation for vnetid

2f31577

remove readonly property createDiffdisk

550d3e2

add stop

57963ec

update tc

54a190b

update tc to add avs

157aa81

update tc to test vnetid

718d648

update md

141f00f

github-actions bot added the waiting-response label Oct 14, 2024

rcskosir added the new-resource label Oct 14, 2024

github-actions bot removed the waiting-response label Oct 15, 2024

neil-yechenwei force-pushed the systemcentervirtualmachinemanagervirtualmachineinstance branch from 24f425f to 9d7a3b3 Compare October 15, 2024 03:46

add storagedisk in tc

0ea07d0

neil-yechenwei requested a review from ms-zhenhua October 15, 2024 09:38

ms-zhenhua approved these changes Oct 16, 2024

View reviewed changes

neil-yechenwei marked this pull request as ready for review October 16, 2024 02:41

neil-yechenwei added 2 commits October 16, 2024 14:11

update tc

4e69b8b

update tc

13a9bc8

katbyte requested review from katbyte and a team as code owners November 14, 2024 00:08

catriona-m self-assigned this Dec 2, 2024

catriona-m requested changes Dec 4, 2024

View reviewed changes

github-actions bot added the waiting-response label Dec 4, 2024

update property check and add restart notification

6890ea1

github-actions bot added waiting-response and removed waiting-response labels Dec 5, 2024

Merge remote-tracking branch 'upstream/main' into systemcentervirtual…

0299afd

…machinemanagervirtualmachineinstance

github-actions bot added waiting-response and removed waiting-response labels Dec 5, 2024

format go

76280ad

github-actions bot added waiting-response and removed waiting-response labels Dec 5, 2024

neil-yechenwei requested a review from catriona-m December 5, 2024 08:31

github-actions bot removed the waiting-response label Dec 5, 2024

Merge remote-tracking branch 'upstream/main' into systemcentervirtual…

e592613

…machinemanagervirtualmachineinstance

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_system_center_virtual_machine_manager_virtual_machine_instance` #27622

New Resource: `azurerm_system_center_virtual_machine_manager_virtual_machine_instance` #27622

neil-yechenwei commented Oct 11, 2024 •

edited

Loading

neil-yechenwei commented Oct 15, 2024

ms-zhenhua left a comment

sidsinghms commented Oct 18, 2024

catriona-m left a comment

catriona-m Dec 4, 2024

neil-yechenwei Dec 5, 2024 •

edited

Loading

catriona-m Dec 4, 2024

neil-yechenwei Dec 5, 2024 •

edited

Loading

catriona-m Dec 4, 2024

neil-yechenwei Dec 5, 2024

catriona-m Dec 4, 2024

neil-yechenwei Dec 5, 2024

catriona-m Dec 4, 2024

neil-yechenwei Dec 5, 2024

neil-yechenwei commented Dec 5, 2024 •

edited

Loading

	errors = append(errors, fmt.Errorf("%q may contain alphanumeric characters", k))
	errors = append(errors, fmt.Errorf("%q must only contain alphanumeric characters", k))

New Resource: azurerm_system_center_virtual_machine_manager_virtual_machine_instance #27622

Are you sure you want to change the base?

New Resource: azurerm_system_center_virtual_machine_manager_virtual_machine_instance #27622

Conversation

neil-yechenwei commented Oct 11, 2024 • edited Loading

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

neil-yechenwei commented Oct 15, 2024

ms-zhenhua left a comment

Choose a reason for hiding this comment

sidsinghms commented Oct 18, 2024

catriona-m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei commented Dec 5, 2024 • edited Loading

New Resource: `azurerm_system_center_virtual_machine_manager_virtual_machine_instance` #27622

New Resource: `azurerm_system_center_virtual_machine_manager_virtual_machine_instance` #27622

neil-yechenwei commented Oct 11, 2024 •

edited

Loading

neil-yechenwei Dec 5, 2024 •

edited

Loading

neil-yechenwei Dec 5, 2024 •

edited

Loading

neil-yechenwei commented Dec 5, 2024 •

edited

Loading