`azurerm_machine_learning_compute_cluster`/`azurerm_machine_learning_compute_instance` - Fix location to point to parent resource for computes #25643

nikhil-pandey · 2024-04-17T00:15:21Z

ARM does not require proxy resource to have location field and should not be using it. But due to a bug in ARM, (Bug no 25419114) Location field is used for certain operations, for this instance, during creation of compute with identities which causes the compute to succeed but the identities to fail. The compute is usable but the identity is not.

Location is completely ignored by the underlying resource provider. computeLocation determines which location the underlying compute will be created in, so this change has no change in the users intended behavior and is not a breaking change.

WodansSon

Hi @nikhil-pandey, gave this PR a look and it mostly LGTM, I did leave a few minor comments. If you can get those taken care of I give the PR another look. Thanks. 🚀

internal/services/machinelearning/machine_learning_compute_instance_resource.go

Issues have been addressed

WodansSon · 2024-04-19T03:05:41Z

katbyte

LGTM w/ a couple comments that can be addressed at a later time 😶‍🌫️

katbyte · 2024-04-19T04:12:22Z

internal/services/machinelearning/machine_learning_compute_cluster_resource.go

+	// NOTE: The 'AmlCompute' 'ComputeLocation' field should always point
+	// to configuration files 'location' field...


we should add more detail here about why this is and link to a rest API specs issue detailing what cluster.location and cluser.compute.location actually mean, that they are backwards? and the api is both confusing and is not consistent with the rest of azure

katbyte · 2024-04-19T04:12:29Z

internal/services/machinelearning/machine_learning_compute_cluster_resource.go

+	// NOTE: The 'ComputeResource' 'Location' field should always point
+	// to the workspace's 'location'...


katbyte · 2024-04-19T04:14:51Z

internal/services/machinelearning/machine_learning_compute_instance_resource.go

@@ -198,36 +212,63 @@ func resourceComputeInstanceCreate(d *pluginsdk.ResourceData, meta interface{})
 		return fmt.Errorf("`subnet_resource_id` must be set if `node_public_ip_enabled` is set to `false`")
 	}

-	computeInstance := &machinelearningcomputes.ComputeInstance{
+	// NOTE: The 'ComputeResource' struct contains the information


and then also link the issue here

website/docs/r/machine_learning_compute_instance.html.markdown

katbyte

LGTM w/ a couple comments that can be addressed at a later time 😶‍🌫️

<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.99.0" to "3.100.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.100.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.100.0
ENHANCEMENTS:

* dependencies: updating `hashicorp/go-azure-sdk` to `v0.20240417.1084633` ([#25659](https://github.com/hashicorp/terraform-provider-azurerm/issues/25659))
* `compute` - update Virtual Machine and Virtual Machine Scale Set resources and data sources to use `hashicorp/go-azure-sdk` ([#25533](https://github.com/hashicorp/terraform-provider-azurerm/issues/25533))
* `machine_learning` - Add new `machine_learning` block that supports `purge_soft_deleted_workspace_on_destroy` ([#25624](https://github.com/hashicorp/terraform-provider-azurerm/issues/25624))
* `loganalytics` - update cluster resource to use `hashicorp/go-azure-sdk` ([#23373](https://github.com/hashicorp/terraform-provider-azurerm/issues/23373))
* Data Source: `azurerm_management_group` - now exports the `tenant_scoped_id` attribute ([#25555](https://github.com/hashicorp/terraform-provider-azurerm/issues/25555))
* `azurerm_container_app` - the `ingress.ip_security_restriction.ip_address_range` property will now accept an IP address as valid input ([#25609](https://github.com/hashicorp/terraform-provider-azurerm/issues/25609))
* `azurerm_container_group` - the `identity` block can now be updated ([#25543](https://github.com/hashicorp/terraform-provider-azurerm/issues/25543))
* `azurerm_express_route_connection` - support for the `private_link_fast_path_enabled` property ([#25596](https://github.com/hashicorp/terraform-provider-azurerm/issues/25596))
* `azurerm_hdinsight_hadoop_cluster` - support for the `private_link_configuration` block ([#25629](https://github.com/hashicorp/terraform-provider-azurerm/issues/25629))
* `azurerm_hdinsight_hbase_cluster` - support for the `private_link_configuration` block ([#25629](https://github.com/hashicorp/terraform-provider-azurerm/issues/25629))
* `azurerm_hdinsight_interactive_query_cluster` - support for the `private_link_configuration` block ([#25629](https://github.com/hashicorp/terraform-provider-azurerm/issues/25629))
* `azurerm_hdinsight_kafka_cluster` - support for the `private_link_configuration` block ([#25629](https://github.com/hashicorp/terraform-provider-azurerm/issues/25629))
* `azurerm_hdinsight_spark_cluster` - support for the `private_link_configuration` block ([#25629](https://github.com/hashicorp/terraform-provider-azurerm/issues/25629))
* `azurerm_management_group` - now exports the `tenant_scoped_id` attribute ([#25555](https://github.com/hashicorp/terraform-provider-azurerm/issues/25555))
* `azurerm_monitor_activity_log_alert` - support for the `location` property ([#25389](https://github.com/hashicorp/terraform-provider-azurerm/issues/25389))
* `azurerm_mysql_flexible_server` - update validating regex for `sku_name` ([#25642](https://github.com/hashicorp/terraform-provider-azurerm/issues/25642))
* `azurerm_postgresql_flexible_server` - support for the `GeoRestore` `create_mode` ([#25664](https://github.com/hashicorp/terraform-provider-azurerm/issues/25664))
* `azurerm_virtual_network_gateway_connection` - support for the `private_link_fast_path_enabled` property ([#25650](https://github.com/hashicorp/terraform-provider-azurerm/issues/25650))
* `azurerm_windows_web_app` - support for the `handler_mapping` block ([#25631](https://github.com/hashicorp/terraform-provider-azurerm/issues/25631))
* `azurerm_windows_web_app_slot` - support for the `handler_mapping` block ([#25631](https://github.com/hashicorp/terraform-provider-azurerm/issues/25631))

BUG FIXES:

* storage: prevent a bug causing the second storage account key to be used for authentication instead of the first ([#25652](https://github.com/hashicorp/terraform-provider-azurerm/issues/25652))
* `azurerm_active_directory_domain_service` - prevent an issue where `filtered_sync_enabled` was not being updated ([#25594](https://github.com/hashicorp/terraform-provider-azurerm/issues/25594))
* `azurerm_application_insights` - add a state migration to fix the resource ID casing of Application Insights resources ([#25628](https://github.com/hashicorp/terraform-provider-azurerm/issues/25628))
* `azurerm_function_app_hybrid_connection` - can now use relay resources created in a different resource group ([#25541](https://github.com/hashicorp/terraform-provider-azurerm/issues/25541))
* `azurerm_kubernetes_cluster_node_pool` - prevent plan diff when the `windows_profile.outbound_nat_enabled` property is unset ([#25644](https://github.com/hashicorp/terraform-provider-azurerm/issues/25644))
* `azurerm_machine_learning_compute_cluster` - fix location to point to parent resource for computes ([#25643](https://github.com/hashicorp/terraform-provider-azurerm/issues/25643))
* `azurerm_machine_learning_compute_instance` - fix location to point to parent resource for computes ([#25643](https://github.com/hashicorp/terraform-provider-azurerm/issues/25643))
* `azurerm_storage_account` - check replication type when evaluating support level for shares and queues for V1 storage accounts ([#25581](https://github.com/hashicorp/terraform-provider-azurerm/issues/25581))
* `azurerm_storage_account` - added a sanity check for `dns_endpoint_type` and `blob_properties.restore_policy` ([#25450](https://github.com/hashicorp/terraform-provider-azurerm/issues/25450))
* `azurerm_web_app_hybrid_connection` - can now use relay resources created in a different resource group ([#25541](https://github.com/hashicorp/terraform-provider-azurerm/issues/25541))
* `azurerm_windows_web_app` - prevent removal of `site_config.application_stack.node_version` when `app_settings` are updated ([#25488](https://github.com/hashicorp/terraform-provider-azurerm/issues/25488))
* `azurerm_windows_web_app_slot` - prevent removal of `site_config.application_stack.node_version` when `app_settings` are updated ([#25489](https://github.com/hashicorp/terraform-provider-azurerm/issues/25489))

DEPRECATIONS:

* `logz` - the Logz resources are deprecated and will be removed in v4.0 of the AzureRM Provider since the API no longer allows new instances to be created ([#25405](https://github.com/hashicorp/terraform-provider-azurerm/issues/25405))
* `azurerm_machine_learning_compute_instance` - marked the `location` field as deprecated in v4.0 of the provider ([#25643](https://github.com/hashicorp/terraform-provider-azurerm/issues/25643))
* `azurerm_kubernetes_cluster` - the following properties have been deprecated since the API no longer supports cluster creation with legacy Azure Entra integration: `client_app_id`, `server_app_id`, `server_app_secret` and `managed` ([#25200](https://github.com/hashicorp/terraform-provider-azurerm/issues/25200))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/117/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>

github-actions · 2024-05-20T02:08:23Z

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.

nikhil-pandey added 3 commits October 6, 2023 17:53

Fix

dc0ef0d

Fix

bca9f0e

Merge remote-tracking branch 'upstream/main' into fix-compute-location

42d7479

github-actions bot added service/machine-learning size/XS waiting-response labels Apr 17, 2024

nikhil-pandey changed the title ~~Fix compute location~~ Fix location to point to parent resource for computes Apr 17, 2024

Update machine_learning_compute_instance_resource.go

4275abf

github-actions bot removed the waiting-response label Apr 17, 2024

WodansSon changed the title ~~Fix location to point to parent resource for computes~~ azurerm_machine_learning_compute_cluster/azurerm_machine_learning_compute_instance - Fix location to point to parent resource for computes Apr 17, 2024

WodansSon previously requested changes Apr 17, 2024

View reviewed changes

internal/services/machinelearning/machine_learning_compute_instance_resource.go Outdated Show resolved Hide resolved

internal/services/machinelearning/machine_learning_compute_instance_resource.go Outdated Show resolved Hide resolved

github-actions bot added the waiting-response label Apr 17, 2024

WodansSon reviewed Apr 17, 2024

View reviewed changes

internal/services/machinelearning/machine_learning_compute_instance_resource.go Outdated Show resolved Hide resolved

Update machine_learning_compute_instance_resource.go

5667333

github-actions bot removed the waiting-response label Apr 17, 2024

Update machine_learning_compute_cluster_resource.go

d6268ac

github-actions bot added the waiting-response label Apr 17, 2024

Add validation...

cdeb497

github-actions bot added size/M waiting-response and removed waiting-response size/XS labels Apr 18, 2024

Fix location value being written to state...

3c442cf

github-actions bot removed the waiting-response label Apr 18, 2024

WodansSon added this to the v3.100.0 milestone Apr 18, 2024

Deprecated location field for 4.0...

08c8486

github-actions bot added size/L waiting-response and removed size/M labels Apr 19, 2024

terrafmt test cases...

70224a6

github-actions bot removed the waiting-response label Apr 19, 2024

Update documentation...

abeba77

github-actions bot added the documentation label Apr 19, 2024

WodansSon added 2 commits April 18, 2024 21:15

Update deprecated message...

afee769

Update cluster model var name for clarity...

04bb5a6

katbyte approved these changes Apr 19, 2024

View reviewed changes

Update website/docs/r/machine_learning_compute_instance.html.markdown

50f7de7

katbyte mentioned this pull request Apr 19, 2024

Set AML cluster location to workspace location #25670

Closed

14 tasks

WodansSon added a commit that referenced this pull request Apr 19, 2024

Update to include #25643

41aecb6

WodansSon merged commit cf95271 into hashicorp:main Apr 19, 2024
32 checks passed

github-actions bot locked as resolved and limited conversation to collaborators May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_machine_learning_compute_cluster`/`azurerm_machine_learning_compute_instance` - Fix location to point to parent resource for computes #25643

`azurerm_machine_learning_compute_cluster`/`azurerm_machine_learning_compute_instance` - Fix location to point to parent resource for computes #25643

nikhil-pandey commented Apr 17, 2024 •

edited

Loading

WodansSon left a comment

WodansSon commented Apr 19, 2024

katbyte left a comment

katbyte Apr 19, 2024

katbyte Apr 19, 2024

katbyte Apr 19, 2024

katbyte left a comment

github-actions bot commented May 20, 2024

		// NOTE: The 'AmlCompute' 'ComputeLocation' field should always point
		// to configuration files 'location' field...

		// NOTE: The 'ComputeResource' 'Location' field should always point
		// to the workspace's 'location'...

azurerm_machine_learning_compute_cluster/azurerm_machine_learning_compute_instance - Fix location to point to parent resource for computes #25643

azurerm_machine_learning_compute_cluster/azurerm_machine_learning_compute_instance - Fix location to point to parent resource for computes #25643

Conversation

nikhil-pandey commented Apr 17, 2024 • edited Loading

WodansSon left a comment

Choose a reason for hiding this comment

WodansSon commented Apr 19, 2024

katbyte left a comment

Choose a reason for hiding this comment

katbyte Apr 19, 2024

Choose a reason for hiding this comment

katbyte Apr 19, 2024

Choose a reason for hiding this comment

katbyte Apr 19, 2024

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

github-actions bot commented May 20, 2024

`azurerm_machine_learning_compute_cluster`/`azurerm_machine_learning_compute_instance` - Fix location to point to parent resource for computes #25643

`azurerm_machine_learning_compute_cluster`/`azurerm_machine_learning_compute_instance` - Fix location to point to parent resource for computes #25643

nikhil-pandey commented Apr 17, 2024 •

edited

Loading