-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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_workspace
- Add support for serverless_compute
#25660
Changes from 3 commits
66324eb
9378f15
e2fa392
df90c80
8efd38c
d92a211
fd9afde
58578c3
3b93028
110fbcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -22,6 +22,7 @@ import ( | |||||
"github.com/hashicorp/terraform-provider-azurerm/internal/clients" | ||||||
"github.com/hashicorp/terraform-provider-azurerm/internal/features" | ||||||
"github.com/hashicorp/terraform-provider-azurerm/internal/services/machinelearning/validate" | ||||||
networkValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/network/validate" | ||||||
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" | ||||||
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/suppress" | ||||||
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" | ||||||
|
@@ -230,6 +231,26 @@ func resourceMachineLearningWorkspace() *pluginsdk.Resource { | |||||
Default: false, | ||||||
}, | ||||||
|
||||||
"serverless_compute": { | ||||||
Type: pluginsdk.TypeList, | ||||||
Optional: true, | ||||||
MaxItems: 1, | ||||||
Elem: &pluginsdk.Resource{ | ||||||
Schema: map[string]*pluginsdk.Schema{ | ||||||
"subnet_id": { | ||||||
Type: pluginsdk.TypeString, | ||||||
Optional: true, | ||||||
ValidateFunc: networkValidate.SubnetID, | ||||||
}, | ||||||
"public_ip_enabled": { | ||||||
Type: pluginsdk.TypeBool, | ||||||
Optional: true, | ||||||
Default: true, | ||||||
}, | ||||||
}, | ||||||
}, | ||||||
}, | ||||||
|
||||||
"discovery_url": { | ||||||
Type: pluginsdk.TypeString, | ||||||
Computed: true, | ||||||
|
@@ -315,6 +336,22 @@ func resourceMachineLearningWorkspaceCreateOrUpdate(d *pluginsdk.ResourceData, m | |||||
}, | ||||||
} | ||||||
|
||||||
serverlessCompute := expandMachineLearningWorkspaceServerlessCompute(d.Get("serverless_compute").([]interface{})) | ||||||
if serverlessCompute != nil { | ||||||
if !networkAccessBehindVnetEnabled && !*serverlessCompute.ServerlessComputeNoPublicIP { | ||||||
return fmt.Errorf(" Not supported to set `public_ip_enabled` to `false` without `subnet_id` when `public_network_access_enabled` is `false`") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we re-phrase this to something more actionable for the user
Suggested change
|
||||||
} | ||||||
|
||||||
if serverlessCompute.ServerlessComputeCustomSubnet == nil { | ||||||
oldVal, newVal := d.GetChange("serverless_compute.0.public_ip_enabled") | ||||||
if oldVal.(bool) && !newVal.(bool) { | ||||||
return fmt.Errorf(" Not supported to update `public_ip_enabled` from `true` to `false` when `subnet_id` is null or empty") | ||||||
} | ||||||
} | ||||||
Comment on lines
+344
to
+349
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic looks like it belongs in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried several ways to update this property, customers can still update it but it takes several steps. We can omit `CustomizeDiff``, cx can update the settings according to the error message.
Error message cx may get:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the explanation @xuzhang3. Is this something we can do in the resource by calling the e.g. First There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, the operation needs to be split into several steps There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that because of
Is this an external thing that they need to configure? What is meant by may are there situations where they don't need to? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example, I create a new MLW with following config and I want to set the resource "azurerm_machine_learning_workspace" "example" {
name = "mymlwexample"
location = azurerm_resource_group.example.location
resource_group_name = azurerm_resource_group.example.name
application_insights_id = azurerm_application_insights.example.id
key_vault_id = azurerm_key_vault.example.id
storage_account_id = azurerm_storage_account.example.id
# public_network_access_enabled = true
image_build_compute_name = "terraformComputeUpdate11"
serverless_compute {
# subnet_id = azurerm_subnet.test.id
public_ip_enabled = true
}
identity {
type = "SystemAssigned"
}
} |
||||||
} | ||||||
|
||||||
workspace.Properties.ServerlessComputeSettings = serverlessCompute | ||||||
|
||||||
if networkAccessBehindVnetEnabled { | ||||||
workspace.Properties.PublicNetworkAccess = pointer.To(workspaces.PublicNetworkAccessEnabled) | ||||||
} | ||||||
|
@@ -414,6 +451,7 @@ func resourceMachineLearningWorkspaceRead(d *pluginsdk.ResourceData, meta interf | |||||
d.Set("v1_legacy_mode_enabled", props.V1LegacyMode) | ||||||
d.Set("workspace_id", props.WorkspaceId) | ||||||
d.Set("managed_network", flattenMachineLearningWorkspaceManagedNetwork(props.ManagedNetwork)) | ||||||
d.Set("serverless_compute", flattenMachineLearningWorkspaceServerlessCompute(props.ServerlessComputeSettings)) | ||||||
|
||||||
kvId, err := commonids.ParseKeyVaultIDInsensitively(*props.KeyVault) | ||||||
if err != nil { | ||||||
|
@@ -670,3 +708,39 @@ func flattenMachineLearningWorkspaceManagedNetwork(i *workspaces.ManagedNetworkS | |||||
|
||||||
return &[]interface{}{out} | ||||||
} | ||||||
|
||||||
func expandMachineLearningWorkspaceServerlessCompute(i []interface{}) *workspaces.ServerlessComputeSettings { | ||||||
if len(i) == 0 || i[0] == nil { | ||||||
return nil | ||||||
} | ||||||
|
||||||
v := i[0].(map[string]interface{}) | ||||||
|
||||||
serverlessCompute := workspaces.ServerlessComputeSettings{ | ||||||
ServerlessComputeNoPublicIP: pointer.To(!v["public_ip_enabled"].(bool)), | ||||||
} | ||||||
|
||||||
if subnetId, ok := v["subnet_id"].(string); ok && subnetId != "" { | ||||||
serverlessCompute.ServerlessComputeCustomSubnet = pointer.To(subnetId) | ||||||
} | ||||||
|
||||||
return &serverlessCompute | ||||||
} | ||||||
|
||||||
func flattenMachineLearningWorkspaceServerlessCompute(i *workspaces.ServerlessComputeSettings) *[]interface{} { | ||||||
if i == nil { | ||||||
return &[]interface{}{} | ||||||
} | ||||||
|
||||||
out := map[string]interface{}{} | ||||||
|
||||||
if i.ServerlessComputeCustomSubnet != nil { | ||||||
out["subnet_id"] = *i.ServerlessComputeCustomSubnet | ||||||
} | ||||||
|
||||||
if i.ServerlessComputeNoPublicIP != nil { | ||||||
out["public_ip_enabled"] = !*i.ServerlessComputeNoPublicIP | ||||||
} | ||||||
|
||||||
return &[]interface{}{out} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -353,6 +353,24 @@ func TestAccMachineLearningWorkspace_purgeSoftDelete(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestAccMachineLearningWorkspace_serverlessCompute(t *testing.T) { | ||
data := acceptance.BuildTestData(t, "azurerm_machine_learning_workspace", "test") | ||
r := WorkspaceResource{} | ||
|
||
data.ResourceTest(t, r, []acceptance.TestStep{ | ||
{ | ||
Config: r.serverlessCompute(data), | ||
Check: acceptance.ComposeTestCheckFunc( | ||
check.That(data.ResourceName).ExistsInAzure(r), | ||
check.That(data.ResourceName).Key("serverless_compute.#").HasValue("1"), | ||
check.That(data.ResourceName).Key("serverless_compute.0.subnet_id").Exists(), | ||
check.That(data.ResourceName).Key("serverless_compute.0.public_ip_enabled").HasValue("false"), | ||
), | ||
}, | ||
data.ImportStep(), | ||
}) | ||
} | ||
|
||
func (r WorkspaceResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { | ||
workspacesClient := client.MachineLearning.Workspaces | ||
id, err := workspaces.ParseWorkspaceID(state.ID) | ||
|
@@ -1166,3 +1184,40 @@ resource "azurerm_machine_learning_workspace" "test" { | |
} | ||
`, template, data.RandomInteger) | ||
} | ||
|
||
func (r WorkspaceResource) serverlessCompute(data acceptance.TestData) string { | ||
template := r.template(data) | ||
return fmt.Sprintf(` | ||
%s | ||
|
||
resource "azurerm_virtual_network" "test" { | ||
name = "acctestvirtnet%[2]d" | ||
address_space = ["10.0.0.0/16"] | ||
location = azurerm_resource_group.test.location | ||
resource_group_name = azurerm_resource_group.test.name | ||
} | ||
resource "azurerm_subnet" "test" { | ||
name = "internal" | ||
resource_group_name = azurerm_resource_group.test.name | ||
virtual_network_name = azurerm_virtual_network.test.name | ||
address_prefixes = ["10.0.2.0/24"] | ||
} | ||
|
||
resource "azurerm_machine_learning_workspace" "test" { | ||
name = "acctest-MLW-%[2]d" | ||
location = azurerm_resource_group.test.location | ||
resource_group_name = azurerm_resource_group.test.name | ||
application_insights_id = azurerm_application_insights.test.id | ||
key_vault_id = azurerm_key_vault.test.id | ||
storage_account_id = azurerm_storage_account.test.id | ||
|
||
serverless_compute { | ||
subnet_id = azurerm_subnet.test.id | ||
} | ||
Comment on lines
+1231
to
+1233
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also add a test for the case where we don't supply |
||
|
||
identity { | ||
type = "SystemAssigned" | ||
} | ||
} | ||
`, template, data.RandomInteger) | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -390,6 +390,8 @@ The following arguments are supported: | |||||||||
|
||||||||||
* `sku_name` - (Optional) SKU/edition of the Machine Learning Workspace, possible values are `Free`, `Basic`, `Standard` and `Premium`. Defaults to `Basic`. | ||||||||||
|
||||||||||
* `serverless_compute` - (Optional) A `serverless_compute` block as defined below. | ||||||||||
|
||||||||||
* `tags` - (Optional) A mapping of tags to assign to the resource. | ||||||||||
|
||||||||||
--- | ||||||||||
|
@@ -422,6 +424,18 @@ An `managed_network` block supports the following: | |||||||||
|
||||||||||
--- | ||||||||||
|
||||||||||
An `serverless_compute` block supports the following: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
||||||||||
* `custom_subnet_id` - (Optional) The ID of an existing virtual network subnet in which serverless compute nodes should be deployed. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just called
Suggested change
|
||||||||||
|
||||||||||
* `no_public_ip_enabled` - (Optional) Is serverless compute nodes deployed in custom vNet would have no public IP addresses enabled for a workspace with private endpoint? Defaults to `false`. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
||||||||||
~> **Note:** Not supported to set `public_ip_enabled` to `false` without `subnet_id` when `public_network_access_enabled` is `false`. | ||||||||||
|
||||||||||
~> **Note:** Not supported to update `public_ip_enabled` from `true` to `false` when `subnet_id` is null or empty | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this needs to be updated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. delete the notes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to name the property There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can consolidate these
Suggested change
|
||||||||||
|
||||||||||
--- | ||||||||||
|
||||||||||
An `feature_store` block supports the following: | ||||||||||
|
||||||||||
* `computer_spark_runtime_version` - (Optional) The version of Spark runtime. | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use the Validation function from the
commonids
package for subnets