-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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_container_app_environment
: Add support for workload_profile
#23478
Changes from 5 commits
94c1f72
1d852b1
55d92a9
225d57c
549f695
77c8260
16b6acf
aab804e
2408bb4
7309795
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 |
---|---|---|
|
@@ -64,6 +64,21 @@ func TestAccContainerAppEnvironment_complete(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestAccContainerAppEnvironment_withWorkloadProfile(t *testing.T) { | ||
data := acceptance.BuildTestData(t, "azurerm_container_app_environment", "test") | ||
r := ContainerAppEnvironmentResource{} | ||
|
||
data.ResourceTest(t, r, []acceptance.TestStep{ | ||
{ | ||
Config: r.completeWithWorkloadProfile(data), | ||
Check: acceptance.ComposeTestCheckFunc( | ||
check.That(data.ResourceName).ExistsInAzure(r), | ||
), | ||
}, | ||
data.ImportStep("log_analytics_workspace_id"), | ||
}) | ||
} | ||
|
||
func TestAccContainerAppEnvironment_completeUpdate(t *testing.T) { | ||
data := acceptance.BuildTestData(t, "azurerm_container_app_environment", "test") | ||
r := ContainerAppEnvironmentResource{} | ||
|
@@ -189,6 +204,68 @@ resource "azurerm_container_app_environment" "test" { | |
`, r.templateVNet(data), data.RandomInteger) | ||
} | ||
|
||
func (r ContainerAppEnvironmentResource) completeWithWorkloadProfile(data acceptance.TestData) string { | ||
return fmt.Sprintf(` | ||
provider "azurerm" { | ||
features { | ||
resource_group { | ||
prevent_deletion_if_contains_resources = 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. This indicates a bug in the Terraform Resource or Configuration being used, so this should (almost) never be used in test cases - why's this needed? 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. Hi @tombuildsstuff , when creating this container app environment resource, a NSG will be automatically created along with it. We won't be able to delete the RG without this line since there will be a hanging NSG. 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. In which case, the API should be clearing that up during the deletion of the Container App Environment, since the API is creating it the API manages it's lifecycle (as other Resource Providers do) - would you mind opening an issue on 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 NSG was created to comply an Azure policy internally. It should not affect external users. Will remove this line. |
||
} | ||
} | ||
|
||
%[1]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" "control" { | ||
name = "control-plane" | ||
resource_group_name = azurerm_resource_group.test.name | ||
virtual_network_name = azurerm_virtual_network.test.name | ||
address_prefixes = ["10.0.0.0/23"] | ||
delegation { | ||
name = "acctestdelegation%[2]d" | ||
service_delegation { | ||
actions = ["Microsoft.Network/virtualNetworks/subnets/join/action"] | ||
name = "Microsoft.App/environments" | ||
} | ||
} | ||
} | ||
|
||
resource "azurerm_container_app_environment" "test" { | ||
name = "acctest-CAEnv%[2]d" | ||
resource_group_name = azurerm_resource_group.test.name | ||
location = azurerm_resource_group.test.location | ||
infrastructure_subnet_id = azurerm_subnet.control.id | ||
|
||
workload_profile { | ||
maximum_count = 2 | ||
minimum_count = 0 | ||
name = "My-GP-01" | ||
workload_profile_type = "D4" | ||
} | ||
|
||
workload_profile { | ||
name = "Consumption" | ||
workload_profile_type = "Consumption" | ||
} | ||
|
||
zone_redundancy_enabled = true | ||
|
||
tags = { | ||
Foo = "Bar" | ||
secret = "sauce" | ||
} | ||
} | ||
`, r.template(data), data.RandomInteger) | ||
|
||
} | ||
|
||
func (r ContainerAppEnvironmentResource) completeUpdate(data acceptance.TestData) string { | ||
return fmt.Sprintf(` | ||
provider "azurerm" { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ import ( | |
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" | ||
) | ||
|
||
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 have this new code in a new file? 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. Sure. Reorganized this part of code. |
||
const consumption = "Consumption" | ||
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 feels like there are missing Enums in the Spec, could you check with the service team? 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. According to the service team, this |
||
|
||
type Registry struct { | ||
PasswordSecretRef string `tfschema:"password_secret_name"` | ||
Server string `tfschema:"server"` | ||
|
@@ -3014,3 +3016,84 @@ func (c *ContainerTemplate) flattenContainerAppScaleRules(input *[]containerapps | |
c.TCPScaleRules = tcpScaleRules | ||
} | ||
} | ||
|
||
type WorkloadProfileModel struct { | ||
MaximumCount int64 `tfschema:"maximum_count"` | ||
MinimumCount int64 `tfschema:"minimum_count"` | ||
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 set these 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. Changed. |
||
Name string `tfschema:"name"` | ||
WorkloadProfileType string `tfschema:"workload_profile_type"` | ||
} | ||
|
||
func WorkloadProfileSchema() *pluginsdk.Schema { | ||
return &pluginsdk.Schema{ | ||
Type: pluginsdk.TypeSet, | ||
Optional: true, | ||
Elem: &pluginsdk.Resource{ | ||
Schema: map[string]*pluginsdk.Schema{ | ||
"name": { | ||
Type: pluginsdk.TypeString, | ||
Required: true, | ||
ValidateFunc: validation.StringIsNotEmpty, | ||
}, | ||
|
||
"workload_profile_type": { | ||
Type: pluginsdk.TypeString, | ||
Required: true, | ||
ValidateFunc: validation.StringIsNotEmpty, | ||
}, | ||
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 have specific validation here? These I believe need to be specific values and casing based on the values that the API will accept (i.e. the allowed SKU names) 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. According to the service team, |
||
|
||
"maximum_count": { | ||
Type: pluginsdk.TypeInt, | ||
Optional: true, | ||
}, | ||
|
||
"minimum_count": { | ||
Type: pluginsdk.TypeInt, | ||
Optional: true, | ||
}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func ExpandWorkloadProfiles(input []WorkloadProfileModel) *[]managedenvironments.WorkloadProfile { | ||
if len(input) == 0 { | ||
return nil | ||
} | ||
|
||
result := make([]managedenvironments.WorkloadProfile, 0) | ||
|
||
for _, v := range input { | ||
r := managedenvironments.WorkloadProfile{ | ||
Name: v.Name, | ||
WorkloadProfileType: v.WorkloadProfileType, | ||
} | ||
|
||
if v.Name != consumption { | ||
r.MaximumCount = pointer.To(v.MaximumCount) | ||
r.MinimumCount = pointer.To(v.MinimumCount) | ||
} | ||
|
||
result = append(result, r) | ||
} | ||
|
||
return &result | ||
} | ||
|
||
func FlattenWorkloadProfiles(input *[]managedenvironments.WorkloadProfile) []WorkloadProfileModel { | ||
if input == nil || len(*input) == 0 { | ||
return []WorkloadProfileModel{} | ||
} | ||
result := make([]WorkloadProfileModel, 0) | ||
|
||
for _, v := range *input { | ||
result = append(result, WorkloadProfileModel{ | ||
Name: v.Name, | ||
MaximumCount: pointer.From(v.MaximumCount), | ||
MinimumCount: pointer.From(v.MinimumCount), | ||
WorkloadProfileType: v.WorkloadProfileType, | ||
}) | ||
} | ||
|
||
return result | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -62,8 +62,22 @@ The following arguments are supported: | |||||||||||||
|
||||||||||||||
* `log_analytics_workspace_id` - (Optional) The ID for the Log Analytics Workspace to link this Container Apps Managed Environment to. Changing this forces a new resource to be created. | ||||||||||||||
|
||||||||||||||
* `workload_profile` - (Optional) The profile of the workload to scope the container app execution. A `workload_profile` block as defined below. | ||||||||||||||
|
||||||||||||||
* `tags` - (Optional) A mapping of tags to assign to the resource. | ||||||||||||||
|
||||||||||||||
--- | ||||||||||||||
|
||||||||||||||
A `workload_profile` block supports the following: | ||||||||||||||
|
||||||||||||||
* `name` - (Required) The name of the workload profile. | ||||||||||||||
|
||||||||||||||
* `workload_profile_type` - (Required) Workload profile type for the workloads to run on. | ||||||||||||||
jackofallops marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
|
||||||||||||||
* `maximum_count` - (Optional) The maximum number of containers that can be deployed in the Container App Environment. | ||||||||||||||
|
||||||||||||||
* `minimum_count` - (Optional) The minimum number of containers that can be deployed in the Container App Environment. | ||||||||||||||
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. would this be more clear/explicit if
Suggested change
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. Sure. Will rename. |
||||||||||||||
|
||||||||||||||
## Attributes Reference | ||||||||||||||
|
||||||||||||||
In addition to the Arguments listed above - the following Attributes are exported: | ||||||||||||||
|
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.
Can we add some update testing?
We'll want coverage on adding and removing a profile to an environment, as well as removing all user specified profiles.
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.
Will update the test.