From 9ef639cacbf6c44983f0de94a3e2db37bdbac1ba Mon Sep 17 00:00:00 2001 From: "Hamade, Marcel SF/HZA-ZC3P" Date: Wed, 22 May 2024 14:14:47 +0200 Subject: [PATCH 1/6] Respect ManagedVnet constraints when creating compute clusters --- ...chine_learning_compute_cluster_resource.go | 57 +++++++++---------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go index 5193ca81381e..9591a92f40e8 100644 --- a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go +++ b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/go-azure-sdk/resource-manager/machinelearningservices/2023-10-01/machinelearningcomputes" "github.com/hashicorp/go-azure-sdk/resource-manager/machinelearningservices/2023-10-01/workspaces" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" - "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" @@ -171,21 +170,41 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e if err != nil { return err } - + // Get the Machine Learning Workspace... id := machinelearningcomputes.NewComputeID(workspaceID.SubscriptionId, workspaceID.ResourceGroupName, workspaceID.WorkspaceName, d.Get("name").(string)) + workspace, err := mlWorkspacesClient.Get(ctx, *workspaceID) + if err != nil { + return err + } + + workspaceModel := workspace.Model + if workspaceModel == nil { + return fmt.Errorf("machine learning %s Workspace: model was nil", id) + } + + if workspaceModel.Sku == nil || workspaceModel.Sku.Tier == nil || workspaceModel.Sku.Name == "" { + return fmt.Errorf("machine learning %s Workspace: `SKU` was nil or empty", id) + } + + if workspaceModel.Location == nil { + return fmt.Errorf("machine learning %s Workspace: `Location` was nil", id) + } + + identity, err := expandIdentity(d.Get("identity").([]interface{})) + if err != nil { + return fmt.Errorf("expanding `identity`: %+v", err) + } + existing, err := client.ComputeGet(ctx, id) if err != nil { if !response.WasNotFound(existing.HttpResponse) { return fmt.Errorf("checking for presence of existing %s: %+v", id, err) } } - if !response.WasNotFound(existing.HttpResponse) { - return tf.ImportAsExistsError("azurerm_machine_learning_compute_cluster", id.ID()) - } - if !d.Get("node_public_ip_enabled").(bool) && d.Get("subnet_resource_id").(string) == "" { - return fmt.Errorf("`subnet_resource_id` must be set if `node_public_ip_enabled` is set to `false`") + if !d.Get("node_public_ip_enabled").(bool) && d.Get("subnet_resource_id").(string) == "" && *workspaceModel.Properties.ManagedNetwork.Status.Status != workspaces.ManagedNetworkStatusActive { + return fmt.Errorf("`subnet_resource_id` must be set if `node_public_ip_enabled` is set to `false` if the workspace is not in a managed network") } vmPriority := machinelearningcomputes.VMPriority(d.Get("vm_priority").(string)) @@ -215,30 +234,6 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e DisableLocalAuth: utils.Bool(!d.Get("local_auth_enabled").(bool)), } - // Get the Machine Learning Workspace... - workspace, err := mlWorkspacesClient.Get(ctx, *workspaceID) - if err != nil { - return err - } - - workspaceModel := workspace.Model - if workspaceModel == nil { - return fmt.Errorf("machine learning %s Workspace: model was nil", id) - } - - if workspaceModel.Sku == nil || workspaceModel.Sku.Tier == nil || workspaceModel.Sku.Name == "" { - return fmt.Errorf("machine learning %s Workspace: `SKU` was nil or empty", id) - } - - if workspaceModel.Location == nil { - return fmt.Errorf("machine learning %s Workspace: `Location` was nil", id) - } - - identity, err := expandIdentity(d.Get("identity").([]interface{})) - if err != nil { - return fmt.Errorf("expanding `identity`: %+v", err) - } - // NOTE: The 'ComputeResource' 'Location' field should always point // to the workspace's 'location'... computeClusterParameters := machinelearningcomputes.ComputeResource{ From ac2d44ea48b7625dcaa8523393f8c1a341bbced2 Mon Sep 17 00:00:00 2001 From: Marcel Hamade Date: Thu, 23 May 2024 13:46:06 +0200 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: stephybun --- .../machine_learning_compute_cluster_resource.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go index 9591a92f40e8..c7ebecf6cdd5 100644 --- a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go +++ b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go @@ -175,20 +175,20 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e workspace, err := mlWorkspacesClient.Get(ctx, *workspaceID) if err != nil { - return err + return fmt.Errorf("retrieving %s: %+v, workspaceID, err) } workspaceModel := workspace.Model if workspaceModel == nil { - return fmt.Errorf("machine learning %s Workspace: model was nil", id) + return fmt.Errorf("retrieving %s: `model` was nil", workspaceID) } if workspaceModel.Sku == nil || workspaceModel.Sku.Tier == nil || workspaceModel.Sku.Name == "" { - return fmt.Errorf("machine learning %s Workspace: `SKU` was nil or empty", id) + return fmt.Errorf("retrieving %s: `sku` was nil or empty", workspaceID) } if workspaceModel.Location == nil { - return fmt.Errorf("machine learning %s Workspace: `Location` was nil", id) + return fmt.Errorf("retrieving %s: `location` was nil", workspaceID) } identity, err := expandIdentity(d.Get("identity").([]interface{})) @@ -204,7 +204,7 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e } if !d.Get("node_public_ip_enabled").(bool) && d.Get("subnet_resource_id").(string) == "" && *workspaceModel.Properties.ManagedNetwork.Status.Status != workspaces.ManagedNetworkStatusActive { - return fmt.Errorf("`subnet_resource_id` must be set if `node_public_ip_enabled` is set to `false` if the workspace is not in a managed network") + return fmt.Errorf("`subnet_resource_id` must be set if `node_public_ip_enabled` is set to `false` or the workspace is not in a managed network") } vmPriority := machinelearningcomputes.VMPriority(d.Get("vm_priority").(string)) From 8174490b07046b653ad397ce1f12dd4bee1af128 Mon Sep 17 00:00:00 2001 From: "Hamade, Marcel SF/HZA-ZC3P" Date: Thu, 23 May 2024 14:00:44 +0200 Subject: [PATCH 3/6] extend nil checks --- ...chine_learning_compute_cluster_resource.go | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go index c7ebecf6cdd5..869217934c88 100644 --- a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go +++ b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go @@ -175,7 +175,7 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e workspace, err := mlWorkspacesClient.Get(ctx, *workspaceID) if err != nil { - return fmt.Errorf("retrieving %s: %+v, workspaceID, err) + return fmt.Errorf("retrieving %s: %+v, workspaceID", err) } workspaceModel := workspace.Model @@ -203,7 +203,26 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e } } - if !d.Get("node_public_ip_enabled").(bool) && d.Get("subnet_resource_id").(string) == "" && *workspaceModel.Properties.ManagedNetwork.Status.Status != workspaces.ManagedNetworkStatusActive { + nodePublicIPEnabled, ok := d.Get("node_public_ip_enabled").(bool) + if !ok { + return fmt.Errorf("unable to assert type for `node_public_ip_enabled`") + } + + subnetResourceID, ok := d.Get("subnet_resource_id").(string) + if !ok { + return fmt.Errorf("unable to assert type for `subnet_resource_id`") + } + + workspaceInManagedVnet := false + + if workspaceModel.Properties != nil && + workspaceModel.Properties.ManagedNetwork != nil && + workspaceModel.Properties.ManagedNetwork.Status != nil && + workspaceModel.Properties.ManagedNetwork.Status.Status != nil { + workspaceInManagedVnet = *workspaceModel.Properties.ManagedNetwork.Status.Status == workspaces.ManagedNetworkStatusActive + } + + if !nodePublicIPEnabled && subnetResourceID == "" && !workspaceInManagedVnet { return fmt.Errorf("`subnet_resource_id` must be set if `node_public_ip_enabled` is set to `false` or the workspace is not in a managed network") } From 004893404e27ae619507bd3aa5a41461b3f6c102 Mon Sep 17 00:00:00 2001 From: "Hamade, Marcel SF/HZA-ZC3P" Date: Thu, 23 May 2024 14:12:20 +0200 Subject: [PATCH 4/6] fix error statement --- .../machine_learning_compute_cluster_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go index 869217934c88..6917cb996edd 100644 --- a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go +++ b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go @@ -175,7 +175,7 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e workspace, err := mlWorkspacesClient.Get(ctx, *workspaceID) if err != nil { - return fmt.Errorf("retrieving %s: %+v, workspaceID", err) + return fmt.Errorf("retrieving %s: %+v", workspaceID, err) } workspaceModel := workspace.Model From 7da22374e88219af947291149f17599ff9603c51 Mon Sep 17 00:00:00 2001 From: "Hamade, Marcel SF/HZA-ZC3P" Date: Thu, 23 May 2024 15:05:38 +0200 Subject: [PATCH 5/6] add accidently removed ImportAsExistsErrore --- .../machine_learning_compute_cluster_resource.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go index 6917cb996edd..0ddd5b6ff71a 100644 --- a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go +++ b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go @@ -202,6 +202,9 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e return fmt.Errorf("checking for presence of existing %s: %+v", id, err) } } + + if !response.WasNotFound(existing.HttpResponse) { + return tf.ImportAsExistsError("azurerm_machine_learning_compute_cluster", id.ID()) nodePublicIPEnabled, ok := d.Get("node_public_ip_enabled").(bool) if !ok { From de11ea3cc4dded132bb752ae02666516aba28f57 Mon Sep 17 00:00:00 2001 From: "Hamade, Marcel SF/HZA-ZC3P" Date: Thu, 23 May 2024 16:03:01 +0200 Subject: [PATCH 6/6] fix linting and test issues --- .../machine_learning_compute_cluster_resource.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go index 0ddd5b6ff71a..eeb55e0e5458 100644 --- a/internal/services/machinelearning/machine_learning_compute_cluster_resource.go +++ b/internal/services/machinelearning/machine_learning_compute_cluster_resource.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/go-azure-sdk/resource-manager/machinelearningservices/2023-10-01/machinelearningcomputes" "github.com/hashicorp/go-azure-sdk/resource-manager/machinelearningservices/2023-10-01/workspaces" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" + "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" @@ -202,10 +203,10 @@ func resourceComputeClusterCreate(d *pluginsdk.ResourceData, meta interface{}) e return fmt.Errorf("checking for presence of existing %s: %+v", id, err) } } - + if !response.WasNotFound(existing.HttpResponse) { return tf.ImportAsExistsError("azurerm_machine_learning_compute_cluster", id.ID()) - + } nodePublicIPEnabled, ok := d.Get("node_public_ip_enabled").(bool) if !ok { return fmt.Errorf("unable to assert type for `node_public_ip_enabled`")