New Resource: `azurerm_hybrid_compute_machine_extension` #21027

liuwuliuyun · 2023-03-20T03:23:43Z

New Resource: azurerm_hybrid_compute_machine_extension
Swagger: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/hybridcompute/resource-manager/Microsoft.HybridCompute/stable/2022-11-10/HybridCompute.json

Testing evidence will be provided after TC run completes.

…xtensions in different order

# Conflicts: # internal/services/hybridcompute/client/client.go # vendor/github.com/hashicorp/go-azure-sdk/resource-manager/hybridcompute/2022-11-10/machineextensions/id_extension.go # vendor/github.com/hashicorp/go-azure-sdk/resource-manager/hybridcompute/2022-11-10/machineextensions/id_machine.go # vendor/github.com/hashicorp/go-azure-sdk/resource-manager/hybridcompute/2022-11-10/machineextensions/predicates.go # vendor/modules.txt

catriona-m

Thanks for this @liuwuliuyun - I've left a couple of minor comments here for you to consider.

Also, given that this is known as "Azure Arc" in the portal, we think it might make more sense to rename this along those same lines - azurerm_arc_machine_extension perhaps? This would also involve renaming the corresponding datasource too. Thanks!

internal/services/hybridcompute/hybrid_compute_machine_extension_resource_test.go

internal/services/hybridcompute/hybrid_compute_machine_extension_resource.go

…on_resource_test.go Co-authored-by: catriona-m <[email protected]>

liuwuliuyun · 2023-03-31T07:11:42Z

Done the renaming. Here is the testing evidence.

stephybun · 2023-04-05T06:15:56Z

internal/services/hybridcompute/arc_machine_data_source.go

-func (r HybridComputeMachineDataSource) ResourceType() string {
-	return "azurerm_hybrid_compute_machine"
+func (r ArcMachineDataSource) ResourceType() string {
+	return "azurerm_arc_machine"


This is a breaking change, we'd need to add a new data source with the correct name and deprecate this one.

Should I just change this name back? Since this RP is named hybridcompute, why dont we just call it hybrid_compute_machine and hybrid_compute_machine_extension. Which I believe make more sense than "arc_machine" for any new users. Plus, it will not cause a breaking change.

stephybun · 2023-04-05T06:19:49Z

internal/services/hybridcompute/arc_machine_extension_resource.go

+								"code": {
+									Type:     pluginsdk.TypeString,
+									Computed: true,
+								},
+
+								"display_status": {
+									Type:     pluginsdk.TypeString,
+									Computed: true,
+								},
+
+								"level": {
+									Type:     pluginsdk.TypeString,
+									Computed: true,
+								},
+
+								"message": {
+									Type:     pluginsdk.TypeString,
+									Computed: true,
+								},
+
+								"time": {
+									Type:     pluginsdk.TypeString,
+									Computed: true,
+								},


Is it possible to have multiple statuses? These properties can probably be flattened and moved up a level

stephybun · 2023-04-05T06:20:33Z

internal/services/hybridcompute/arc_machine_extension_resource.go

+					"type": {
+						Type:     pluginsdk.TypeString,
+						Computed: true,
+					},


Same comment regarding the field type we do not usually expose these fields.

stephybun · 2023-04-05T06:22:01Z

internal/services/hybridcompute/arc_machine_extension_resource.go

+			model := resp.Model
+			if model == nil {
+				return fmt.Errorf("retrieving %s: model was nil", id)
+			}


We don't need to return an error if Model is nil (same applies to Properties) and for readability we assign it to a variable

Suggested change

model := resp.Model

if model == nil {

return fmt.Errorf("retrieving %s: model was nil", id)

}

if model := resp.Model; model != nil {

...

if props := model.Properties; props != nil {

...

stephybun · 2023-04-05T06:25:03Z

internal/services/hybridcompute/arc_machine_extension_resource_test.go

+func (r HybridComputeMachineExtensionResource) complete(data acceptance.TestData, template string) string {
+	return fmt.Sprintf(`
+			%s
+
+resource "azurerm_arc_machine_extension" "test" {
+  name                               = "acctest-hcme-%d"
+  arc_machine_id                     = data.azurerm_arc_machine.test.id
+  location                           = "%s"
+  automatic_upgrade_enabled          = false
+  publisher                          = "Microsoft.Azure.Monitor"
+  type                               = "AzureMonitorLinuxAgent"
+  type_handler_version               = "1.24"
+}
+`, template, data.RandomInteger, data.Locations.Primary)
+}


It looks like this config is missing a fair few properties for it to be complete?

Let me add settings and protected settings example to complete config.

stephybun · 2023-04-05T06:27:00Z

internal/services/hybridcompute/arc_machine_extension_resource_test.go

+func (r HybridComputeMachineExtensionResource) template(data acceptance.TestData) string {
+	d := HybridComputeMachineDataSource{}
+	clientSecret := os.Getenv("ARM_CLIENT_SECRET")
+	randomUUID, _ := uuid.GenerateUUID()
+	password := generateRandomPassword(10)
+	return d.basic(data, clientSecret, randomUUID, password)
+}


For consistency the template should exist within the resource's tests and be referenced by the data source tests, not the other way around.

Ok, Let me change this.

liuwuliuyun · 2023-04-13T01:55:18Z

@stephybun @catriona-m what do you guys think of the naming of resources in the resource provider? Currently we have two options.

keeping every resource starts with azurerm_hybrid_compute_machine*. This solution has no breaking change for users and it aligns with the RP name. If we choose this one, I will revert the name back in this RP with all the suggested change.
change to azurerm_arc_machine*. This solution makes breaking change to current datasource so we have to keep both until 4.0. If we choose this one, I may need to change the datasource first and add this extension with another PR.

stephybun · 2023-04-13T09:10:55Z

@liuwuliuyun - the commercial product name which is user facing and carries more weight than the resource provider's name is Azure Arc (for future reference the words hybrid and connected are synonymous with arc in Azure). For consistency with other Arc specific resources within the provider i.e. azurerm_arc_kubernetes_cluster these need to be named/renamed azurerm_arc_*.

It's better to fix these now to set the correct precedent.

liuwuliuyun · 2023-04-17T00:59:24Z

Ok, let me start working on that.

liuwuliuyun · 2023-04-23T02:21:52Z

Close this PR due to pending completion name change on the exsiting resources. Will open a new one.

github-actions · 2024-06-02T02:10:17Z

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.

liuwuliuyun added 14 commits February 22, 2023 14:46

add new resource azurerm_hybrid_compute_machine_extension

b9ccff1

modify new resource azurerm_hybrid_compute_machine_extension

bf481f1

modify new resource azurerm_hybrid_compute_machine_extension

05bb278

modify new resource azurerm_hybrid_compute_machine_extension

fe4f05e

modify new resource azurerm_hybrid_compute_machine_extension

0f78979

modify new resource azurerm_hybrid_compute_machine_extension

82e3241

modify new resource azurerm_hybrid_compute_machine_extension

b6efc48

using languageExtensionDiffSuppress to suppress diff caused by same e…

6db6415

…xtensions in different order

update code for requireImport Test

f2b904a

remove support for a write-only property

8e1f04f

change to fix accTest

bde44d1

change to fix accTest

c07b337

add doc

7e76815

github-actions bot added documentation service/hybrid-compute size/XL labels Mar 20, 2023

update on lint problem

3020048

catriona-m requested changes Mar 28, 2023

View reviewed changes

github-actions bot added the waiting-response label Mar 28, 2023

Update internal/services/hybridcompute/hybrid_compute_machine_extensi…

3598ca6

…on_resource_test.go Co-authored-by: catriona-m <[email protected]>

github-actions bot removed the waiting-response label Mar 31, 2023

liuwuliuyun added 4 commits March 31, 2023 17:22

change name and property update

ad6c896

change name and property update

79de659

change hybrid_compute_machine_id to arc_machine_id

52e6848

update document name for arc service

21cd275

stephybun requested changes Apr 5, 2023

View reviewed changes

stephybun added the new-resource label Apr 5, 2023

github-actions bot added the waiting-response label Apr 5, 2023

github-actions bot removed the waiting-response label Apr 11, 2023

katbyte added the waiting-response label Apr 21, 2023

liuwuliuyun closed this Apr 23, 2023

github-actions bot removed the waiting-response label Apr 23, 2023

liuwuliuyun mentioned this pull request May 16, 2023

New Datasource: azurerm_arc_machine #21796

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2024

liuwuliuyun deleted the hc-ext branch September 19, 2024 06:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_hybrid_compute_machine_extension` #21027

New Resource: `azurerm_hybrid_compute_machine_extension` #21027

liuwuliuyun commented Mar 20, 2023 •

edited

Loading

catriona-m left a comment

liuwuliuyun commented Mar 31, 2023 •

edited

Loading

stephybun Apr 5, 2023

liuwuliuyun Apr 11, 2023 •

edited

Loading

stephybun Apr 5, 2023

stephybun Apr 5, 2023

stephybun Apr 5, 2023

stephybun Apr 5, 2023

liuwuliuyun Apr 11, 2023

stephybun Apr 5, 2023

liuwuliuyun Apr 11, 2023

liuwuliuyun commented Apr 13, 2023 •

edited

Loading

stephybun commented Apr 13, 2023

liuwuliuyun commented Apr 17, 2023

liuwuliuyun commented Apr 23, 2023 •

edited

Loading

github-actions bot commented Jun 2, 2024

New Resource: azurerm_hybrid_compute_machine_extension #21027

New Resource: azurerm_hybrid_compute_machine_extension #21027

Conversation

liuwuliuyun commented Mar 20, 2023 • edited Loading

catriona-m left a comment

Choose a reason for hiding this comment

liuwuliuyun commented Mar 31, 2023 • edited Loading

Choose a reason for hiding this comment

liuwuliuyun Apr 11, 2023 • 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

Choose a reason for hiding this comment

liuwuliuyun commented Apr 13, 2023 • edited Loading

stephybun commented Apr 13, 2023

liuwuliuyun commented Apr 17, 2023

liuwuliuyun commented Apr 23, 2023 • edited Loading

github-actions bot commented Jun 2, 2024

New Resource: `azurerm_hybrid_compute_machine_extension` #21027

New Resource: `azurerm_hybrid_compute_machine_extension` #21027

liuwuliuyun commented Mar 20, 2023 •

edited

Loading

liuwuliuyun commented Mar 31, 2023 •

edited

Loading

liuwuliuyun Apr 11, 2023 •

edited

Loading

liuwuliuyun commented Apr 13, 2023 •

edited

Loading

liuwuliuyun commented Apr 23, 2023 •

edited

Loading