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.

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 - Add scale rules #21274

Closed
wants to merge 1 commit into from

Conversation

jsok
Copy link
Contributor

@jsok jsok commented Apr 4, 2023

Adds initial support for http and custom scale rules.
Note: Authentication support on the triggers is not supported in this PR.

Fixes #20629
Fixes #21207

@jsok jsok changed the title azurerm_container_apps - Add scale rules azurerm_container_app - Add scale rules Apr 4, 2023
@jsok jsok force-pushed the container-app-scale branch from 2e26b8c to a01753b Compare April 4, 2023 07:06
@stewartbeck
Copy link

Is there a possibility of adding Azure Queue support at the same time? That would get to parity with what's available in the azure portal.

@jsok
Copy link
Contributor Author

jsok commented Apr 14, 2023

Is there a possibility of adding Azure Queue support at the same time? That would get to parity with what's available in the azure portal.

Probably not by myself, but this PR should set it up for an easy addition.

Any progress on getting this reviewed @jackofallops ?

@Dyhr

This comment was marked as off-topic.

@stfnzl

This comment was marked as off-topic.

@jsok

This comment was marked as off-topic.

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Hey @jsok - looks like you have some test failures:

Test ended in panic.

------- Stdout: -------
=== RUN   TestAccContainerAppResource_basic
=== PAUSE TestAccContainerAppResource_basic
=== CONT  TestAccContainerAppResource_basic

------- Stderr: -------
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x4a3dbb1]

goroutine 528 [running]:
github.com/hashicorp/terraform-provider-azurerm/internal/services/containerapps/helpers.flattenContainerScale(0xc00283a438)
	/opt/teamcity-agent/work/5e6516bb4d10eb66/internal/services/containerapps/helpers/container_apps.go:1174 +0x51
github.com/hashicorp/terraform-provider-azurerm/internal/services/containerapps/helpers.FlattenContainerAppTemplate(0xc004257500)
	/opt/teamcity-agent/work/5e6516bb4d10eb66/internal/services/containerapps/helpers/container_apps.go:810 +0x68
github.com/hashicorp/terraform-provider-azurerm/internal/services/containerapps.ContainerAppResource.Read.func1({0x79ef9a8, 0xc0040fe900}, {0xc00475e400, {0x79f0230, 0xc00403a9a8}, 0xc004752880, 0x0, {0x79f1420, 0xc14d860}})
	/opt/teamcity-agent/work/5e6516bb4d10eb66/internal/services/containerapps/container_app_resource.go:263 +0xabf
github.com/hashicorp/terraform-provider-azurerm/internal/sdk.(*ResourceWrapper).Resource.func2({0x79ef9a8, 0xc0040fe900}, 0x1a3185c441e?, {0x64ab6a0?, 0xc00475e400?})
	/opt/teamcity-agent/work/5e6516bb4d10eb66/internal/sdk/wrapper_resource.go:59 +0x1e5
github.com/hashicorp/terraform-provider-azurerm/internal/sdk.diagnosticsWrapper.func1({0x79ef9a8, 0xc0040fe900}, 0x0?, {0x64ab6a0, 0xc00475e400})
	/opt/teamcity-agent/work/5e6516bb4d10eb66/internal/sdk/wrapper_resource.go:185 +0xbe
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0xc0019737a0, {0x79ef9e0, 0xc0051ad080}, 0xd?, {0x64ab6a0, 0xc00475e400})
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:707 +0x12e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0xc0019737a0, {0x79ef9e0, 0xc0051ad080}, 0xc000e3aa90, 0xc004752700, {0x64ab6a0, 0xc00475e400})
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:837 +0xa85
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ApplyResourceChange(0xc000cacf90, {0x79ef9e0?, 0xc0051acfc0?}, 0xc0051a60f0)
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/grpc_provider.go:1021 +0xe8d
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ApplyResourceChange(0xc003eda1e0, {0x79ef9e0?, 0xc0051ac600?}, 0xc002f14070)
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server/server.go:818 +0x574
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ApplyResourceChange_Handler({0x6ca6ec0?, 0xc003eda1e0}, {0x79ef9e0, 0xc0051ac600}, 0xc002f14000, 0x0)
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:385 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0021c05a0, {0x7a000e0, 0xc003d4e340}, 0xc002bcc000, 0xc002b3a240, 0xc10c720, 0x0)
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/google.golang.org/grpc/server.go:1340 +0xd23
google.golang.org/grpc.(*Server).handleStream(0xc0021c05a0, {0x7a000e0, 0xc003d4e340}, 0xc002bcc000, 0x0)
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/google.golang.org/grpc/server.go:1713 +0xa2f
google.golang.org/grpc.(*Server).serveStreams.func1.2()
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/google.golang.org/grpc/server.go:965 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/opt/teamcity-agent/work/5e6516bb4d10eb66/vendor/google.golang.org/grpc/server.go:963 +0x28a

@jsok jsok force-pushed the container-app-scale branch from a01753b to a0d8dd3 Compare June 12, 2023 23:58
@jsok jsok requested a review from katbyte June 12, 2023 23:59
@jsok
Copy link
Contributor Author

jsok commented Jun 13, 2023

@katbyte thanks for picking up on that, I've pushed a fix and run the acceptance tests locally to verify it works:

TF_ACC=1 go test -v ./internal/services/containerapps -run=TestAccContainerAppResource_basic -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccContainerAppResource_basic
=== PAUSE TestAccContainerAppResource_basic
=== CONT  TestAccContainerAppResource_basic
--- PASS: TestAccContainerAppResource_basic (806.35s)

Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

Hi @jsok
Thanks for this contribution and apologies for the delayed review.

It's off to a good start, but I think we can take a look at the rules schema there to provide a better plan-time experience and provide pre-apply validation and avoid users getting into apply-time errors? If you can take a look at the suggestion below (which will change the expands / flattens quite a bit) I think that'll get us to a better place.

Thanks!

Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the volume.",
Copy link
Member

Choose a reason for hiding this comment

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

Copy paste error here?

Suggested change
Description: "The name of the volume.",
Description: "The custom rule type.",


func flattenContainerCustomScaleRule(input *containerapps.CustomScaleRule) []ContainerCustomScaleRule {
if input == nil {
return nil
Copy link
Member

Choose a reason for hiding this comment

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

This should return an empty list

Suggested change
return nil
return []ContainerCustomScaleRule{}


func flattenContainerHTTPScaleRule(input *containerapps.HTTPScaleRule) []ContainerHTTPScaleRule {
if input == nil {
return nil
Copy link
Member

Choose a reason for hiding this comment

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

This should return an empty list

Suggested change
return nil
return []ContainerHTTPScaleRule{}


func flattenContainerScale(input *containerapps.Scale) []ContainerScale {
if input == nil || input.Rules == nil {
return nil
Copy link
Member

Choose a reason for hiding this comment

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

This should return an empty list

Suggested change
return nil
return []ContainerScale{}

Comment on lines +1068 to +1131
func ContainerScaleSchema() *pluginsdk.Schema {
return &pluginsdk.Schema{
Type: pluginsdk.TypeList,
MaxItems: 1,
Optional: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"rule": {
Type: pluginsdk.TypeList,
Optional: true,
MinItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the rule.",
},
"custom": {
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the volume.",
},
"metadata": {
Type: pluginsdk.TypeMap,
Required: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
"http": {
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"metadata": {
Type: pluginsdk.TypeMap,
Optional: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
},
},
},
},
},
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think if we flatten this schema to containing rule types, we can provide better schema validation here? (Which will allow us to remove the apply-time error messaging in the expand functions later?
e.g.

Suggested change
func ContainerScaleSchema() *pluginsdk.Schema {
return &pluginsdk.Schema{
Type: pluginsdk.TypeList,
MaxItems: 1,
Optional: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"rule": {
Type: pluginsdk.TypeList,
Optional: true,
MinItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the rule.",
},
"custom": {
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the volume.",
},
"metadata": {
Type: pluginsdk.TypeMap,
Required: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
"http": {
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"metadata": {
Type: pluginsdk.TypeMap,
Optional: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
},
},
},
},
},
}
}
func ContainerScaleSchema() *pluginsdk.Schema {
return &pluginsdk.Schema{
Type: pluginsdk.TypeList,
MaxItems: 1,
Optional: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"http_rule": {
Type: pluginsdk.TypeList,
Optional: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the rule.",
},
"metadata": {
Type: pluginsdk.TypeMap,
Required: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
"custom_rule": {
Type: pluginsdk.TypeList,
Optional: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the rule.",
},
"type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
},
"metadata": {
Type: pluginsdk.TypeMap,
Required: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
"azure_queue_rule": {
Type: pluginsdk.TypeList,
Optional: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
Description: "The name of the rule.",
},
"queue_name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
},
"queue_length": {
Type: pluginsdk.TypeInt,
Required: true,
},
},
},
},
},
},
}
}

@ivanl-out
Copy link

We'd also need an auth section for most scale rules as outlined here:
https://learn.microsoft.com/en-us/azure/templates/microsoft.app/containerapps?pivots=deployment-language-terraform#customscalerule-2

@ivanl-out
Copy link

ivanl-out commented Jul 3, 2023

I see there was a similar PR #22219 which was closed in favour of this one. That PR did have the auth schema implemented. Perhaps those bits be cherry picked into this PR?

@jsok
Copy link
Contributor Author

jsok commented Sep 17, 2023

Sorry all, I'm not in a position to follow up on these requested changes. My team has since decided to not use Container Apps after an initial PoC (Cloud Run is more mature and has better TF support).
I hope someone from the ACA team can pick this up and get it merged!

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for custom scaling rules in container apps Setting scale rules
7 participants