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

azurerm_machine_learning_compute_cluster - Add support for update identity #26404

Merged
merged 9 commits into from
Jul 8, 2024
Merged

azurerm_machine_learning_compute_cluster - Add support for update identity #26404

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
35c8972
remove forewNew from identity
xuzhang3 Jun 20, 2024
5b7a531
add update
xuzhang3 Jun 20, 2024
bcb4900
add tiomeout
xuzhang3 Jun 20, 2024
f2089e2
optimize the update
xuzhang3 Jun 21, 2024
3629a51
remove mlw model in update
xuzhang3 Jun 26, 2024
1bf2541
optimize update
xuzhang3 Jun 27, 2024
3512d78
update test case
xuzhang3 Jun 27, 2024
515c029
format test case
xuzhang3 Jun 28, 2024
2a85ab0
format test case
xuzhang3 Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 67 additions & 1 deletion internal/services/machinelearning/machine_learning_compute_cluster_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func resourceComputeCluster() *pluginsdk.Resource {
return &pluginsdk.Resource{
Create: resourceComputeClusterCreate,
Read: resourceComputeClusterRead,
Update: resourceComputeClusterUpdate,
Delete: resourceComputeClusterDelete,

Importer: pluginsdk.ImporterValidatingResourceId(func(id string) error {
Expand All @@ -37,6 +38,7 @@ func resourceComputeCluster() *pluginsdk.Resource {
Timeouts: &pluginsdk.ResourceTimeout{
Create: pluginsdk.DefaultTimeout(30 * time.Minute),
Read: pluginsdk.DefaultTimeout(5 * time.Minute),
Update: pluginsdk.DefaultTimeout(30 * time.Minute),
Delete: pluginsdk.DefaultTimeout(30 * time.Minute),
},

Expand Down Expand Up @@ -69,7 +71,7 @@ func resourceComputeCluster() *pluginsdk.Resource {
ValidateFunc: validation.StringInSlice([]string{string(machinelearningcomputes.VMPriorityDedicated), string(machinelearningcomputes.VMPriorityLowPriority)}, false),
},

"identity": commonschema.SystemAssignedUserAssignedIdentityOptionalForceNew(),
"identity": commonschema.SystemAssignedUserAssignedIdentityOptional(),

"scale_settings": {
Type: pluginsdk.TypeList,
Expand Down Expand Up @@ -355,6 +357,70 @@ func resourceComputeClusterRead(d *pluginsdk.ResourceData, meta interface{}) err
return tags.FlattenAndSet(d, computeResource.Model.Tags)
}

func resourceComputeClusterUpdate(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).MachineLearning.MachineLearningComputes
ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := machinelearningcomputes.ParseComputeID(d.Id())
if err != nil {
return err
}

compute, err := client.ComputeGet(ctx, *id)
if err != nil {
return fmt.Errorf("retrieving %s: %+v", *id, err)
}

computeModel := compute.Model
if computeModel == nil {
return fmt.Errorf("retrieving %s: `model` was nil", *id)
}

identity, err := expandIdentity(d.Get("identity").([]interface{}))
if err != nil {
return fmt.Errorf("expanding `identity`: %+v", err)
}

vmPriority := machinelearningcomputes.VMPriority(d.Get("vm_priority").(string))
computeClusterAmlComputeProperties := machinelearningcomputes.AmlComputeProperties{
VMSize: utils.String(d.Get("vm_size").(string)),
VMPriority: &vmPriority,
ScaleSettings: expandScaleSettings(d.Get("scale_settings").([]interface{})),
UserAccountCredentials: expandUserAccountCredentials(d.Get("ssh").([]interface{})),
EnableNodePublicIP: pointer.To(d.Get("node_public_ip_enabled").(bool)),
}

computeClusterAmlComputeProperties.RemoteLoginPortPublicAccess = pointer.To(machinelearningcomputes.RemoteLoginPortPublicAccessDisabled)
if d.Get("ssh_public_access_enabled").(bool) {
computeClusterAmlComputeProperties.RemoteLoginPortPublicAccess = pointer.To(machinelearningcomputes.RemoteLoginPortPublicAccessEnabled)
}

if subnetId, ok := d.GetOk("subnet_resource_id"); ok && subnetId.(string) != "" {
computeClusterAmlComputeProperties.Subnet = &machinelearningcomputes.ResourceId{Id: subnetId.(string)}
}

computeClusterProperties := machinelearningcomputes.AmlCompute{
Properties: &computeClusterAmlComputeProperties,
ComputeLocation: utils.String(d.Get("location").(string)),
Description: utils.String(d.Get("description").(string)),
DisableLocalAuth: utils.Bool(!d.Get("local_auth_enabled").(bool)),
}

computeClusterParameters := machinelearningcomputes.ComputeResource{
Properties: computeClusterProperties,
Identity: identity,
Location: computeModel.Location,
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}

if err := client.ComputeCreateOrUpdateThenPoll(ctx, *id, computeClusterParameters); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}
Copy link
Member

Choose a reason for hiding this comment

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

@xuzhang3 my original comment hasn't been resolved here.

Like we do in many other updates, we get the existing resource, and patch in the changes to the existing resource's model and use that as the payload for the CreateOrUpdate call. This is explained in our guide for adding new resources in the Contributor Docs.

This whole block should be simplified to

Suggested change
compute, err := client.ComputeGet(ctx, *id)
if err != nil {
return fmt.Errorf("retrieving %s: %+v", *id, err)
}
computeModel := compute.Model
if computeModel == nil {
return fmt.Errorf("retrieving %s: `model` was nil", *id)
}
identity, err := expandIdentity(d.Get("identity").([]interface{}))
if err != nil {
return fmt.Errorf("expanding `identity`: %+v", err)
}
vmPriority := machinelearningcomputes.VMPriority(d.Get("vm_priority").(string))
computeClusterAmlComputeProperties := machinelearningcomputes.AmlComputeProperties{
VMSize: utils.String(d.Get("vm_size").(string)),
VMPriority: &vmPriority,
ScaleSettings: expandScaleSettings(d.Get("scale_settings").([]interface{})),
UserAccountCredentials: expandUserAccountCredentials(d.Get("ssh").([]interface{})),
EnableNodePublicIP: pointer.To(d.Get("node_public_ip_enabled").(bool)),
}
computeClusterAmlComputeProperties.RemoteLoginPortPublicAccess = pointer.To(machinelearningcomputes.RemoteLoginPortPublicAccessDisabled)
if d.Get("ssh_public_access_enabled").(bool) {
computeClusterAmlComputeProperties.RemoteLoginPortPublicAccess = pointer.To(machinelearningcomputes.RemoteLoginPortPublicAccessEnabled)
}
if subnetId, ok := d.GetOk("subnet_resource_id"); ok && subnetId.(string) != "" {
computeClusterAmlComputeProperties.Subnet = &machinelearningcomputes.ResourceId{Id: subnetId.(string)}
}
computeClusterProperties := machinelearningcomputes.AmlCompute{
Properties: &computeClusterAmlComputeProperties,
ComputeLocation: utils.String(d.Get("location").(string)),
Description: utils.String(d.Get("description").(string)),
DisableLocalAuth: utils.Bool(!d.Get("local_auth_enabled").(bool)),
}
computeClusterParameters := machinelearningcomputes.ComputeResource{
Properties: computeClusterProperties,
Identity: identity,
Location: computeModel.Location,
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}
if err := client.ComputeCreateOrUpdateThenPoll(ctx, *id, computeClusterParameters); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}
existing, err := client.ComputeGet(ctx, *id)
if err != nil {
return fmt.Errorf("retrieving %s: %+v", *id, err)
}
payload := existing.Model
if payload == nil {
return fmt.Errorf("retrieving %s: `model` was nil", *id)
}
identity, err := expandIdentity(d.Get("identity").([]interface{}))
if err != nil {
return fmt.Errorf("expanding `identity`: %+v", err)
}
payload.Identity = identity
if err := client.ComputeCreateOrUpdateThenPoll(ctx, *id, *payload); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xuzhang3 my original comment hasn't been resolved here.

Like we do in many other updates, we get the existing resource, and patch in the changes to the existing resource's model and use that as the payload for the CreateOrUpdate call. This is explained in our guide for adding new resources in the Contributor Docs.

This whole block should be simplified to

update as requested and all the tests passed.


return resourceComputeClusterRead(d, meta)
}

func resourceComputeClusterDelete(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).MachineLearning.MachineLearningComputes
ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/machine_learning_compute_cluster.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ The following arguments are supported:

* `description` - (Optional) The description of the Machine Learning compute. Changing this forces a new Machine Learning Compute Cluster to be created.

* `identity` - (Optional) An `identity` block as defined below. Changing this forces a new Machine Learning Compute Cluster to be created.
* `identity` - (Optional) An `identity` block as defined below.

* `local_auth_enabled` - (Optional) Whether local authentication methods is enabled. Defaults to `true`. Changing this forces a new Machine Learning Compute Cluster to be created.

Expand Down
Loading