`azurerm_service_plan` - support premium service plan auto scale feature #28524

xiaxyi · 2025-01-16T08:17:09Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevent documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

--- PASS: TestAccServicePlan_completeUpdate (223.57s)
--- PASS: TestAccServicePlan_completeUpdate (223.57s)
--- PASS: TestAccServicePlan_completeUpdate (223.57s)
--- PASS: TestAccServicePlan_maxElasticWorkerCount (230.74s)
--- PASS: TestAccServicePlan_basic (128.02s)
--- PASS: TestAccServicePlan_requiresImport (108.08s)
--- PASS: TestAccServicePlan_linuxFlexConsumption (111.20s)
--- PASS: TestAccServicePlan_linuxConsumption (119.28s)
--- PASS: TestAccServicePlan_linuxPremiumAutoScaleFeatureUpdate (311.50s)
--- PASS: TestAccServicePlanIsolated_appServiceEnvironmentV3 (4023.99s)
FAIL

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

azurerm_service_plan - support for the premium_plan_auto_scale_enabled property [[azurerm_service_plan] Support for the new Automatic Scaling (currently in preview) #22313]

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Fixes #22313
Fixes #28358

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

mbfrahry

Hey @xiaxyi, this overall looks good but there are a few things I've called out. Mainly that there is duplicate checks curing Create/Update that should be caught by the CustomizeDiff. Did you see something that required those checks to be run at plan and apply time?

Also I'm seeing a test failure

=== CONT  TestAccServicePlan_linuxPremiumAutoScaleFeatureUpdate
    testcase.go:173: Step 4/9 error: Error running apply: exit status 1
        Error: `maximum_elastic_worker_count` can only be specified with Elastic Premium Skus
          with azurerm_service_plan.test,
          on terraform_plugin_test.tf line 40, in resource "azurerm_service_plan" "test":
          40: resource "azurerm_service_plan" "test" {
        `maximum_elastic_worker_count` can only be specified with Elastic Premium
        Skus
--- FAIL: TestAccServicePlan_linuxPremiumAutoScaleFeatureUpdate (235.71s)```

mbfrahry · 2025-01-16T16:18:05Z

internal/services/appservice/service_plan_resource.go

@@ -191,9 +200,19 @@ func (r ServicePlanResource) Create() sdk.ResourceFunc {
 				}
 			}

+			if servicePlan.PremiumPlanAutoScaleEnabled {
+				if !helpers.PlanIsPremium(&servicePlan.Sku) {
+					return fmt.Errorf("`premium_plan_auto_scale_enabled` can only be set for premium app service plan")


This is a duplicate of what is in the CustomizeDiff

mbfrahry · 2025-01-16T16:18:35Z

internal/services/appservice/service_plan_resource.go

 			if servicePlan.MaximumElasticWorkerCount > 0 {
 				if !isServicePlanSupportScaleOut(servicePlan.Sku) {
-					return fmt.Errorf("`maximum_elastic_worker_count` can only be specified with Elastic Premium Skus")
+					if helpers.PlanIsPremium(&servicePlan.Sku) && !servicePlan.PremiumPlanAutoScaleEnabled {
+						return fmt.Errorf("`maximum_elastic_worker_count` can only be specified with Elastic Premium Skus or Premium Skus that has `premium_plan_auto_scale_enabled` set to `true`")


This seems to be a duplicate of what is already in the CustomizeDiff. Do we need this check twice?

mbfrahry · 2025-01-16T16:23:00Z

internal/services/appservice/helpers/service_plan.go

@@ -88,6 +94,19 @@ func PlanIsConsumption(input *string) bool {
 	return false
 }

+func PlanIsPremium(input *string) bool {


Do we have to pass a pointer to a string for this function? It looks like every time we call this function, you're passing in the address of a string when we could just pass the string

Suggested change

func PlanIsPremium(input *string) bool {

func PlanIsPremium(input string) bool {

mbfrahry · 2025-01-16T16:25:09Z

internal/services/appservice/service_plan_resource.go

@@ -341,6 +364,14 @@ func (r ServicePlanResource) Update() sdk.ResourceFunc {
 				model.Sku.Capacity = pointer.To(state.WorkerCount)
 			}

+			if metadata.ResourceData.HasChange("premium_plan_auto_scale_enabled") {
+				if !helpers.PlanIsPremium(&state.Sku) {
+					return fmt.Errorf("`premium_plan_auto_scale_enabled` can only be set for premium app service plan")


This also seems like a duplicate of the CustomizeDiff

mbfrahry · 2025-01-16T16:43:59Z

website/docs/r/service_plan.html.markdown

@@ -55,7 +55,9 @@ The following arguments are supported:

 ~> **NOTE:** Requires an Isolated SKU. Use one of `I1`, `I2`, `I3` for `azurerm_app_service_environment`, or `I1v2`, `I2v2`, `I3v2` for `azurerm_app_service_environment_v3`

-* `maximum_elastic_worker_count` - (Optional) The maximum number of workers to use in an Elastic SKU Plan. Cannot be set unless using an Elastic SKU.
+* `premium_plan_auto_scale_enabled` - (Optional) Should automatic scaling be enabled for the Premium SKU Plan. Defaults to `false`.Cannot be set unless using a Premium SKU.


Suggested change

* `premium_plan_auto_scale_enabled` - (Optional) Should automatic scaling be enabled for the Premium SKU Plan. Defaults to `false`.Cannot be set unless using a Premium SKU.

* `premium_plan_auto_scale_enabled` - (Optional) Should automatic scaling be enabled for the Premium SKU Plan. Defaults to `false`. Cannot be set unless using a Premium SKU.

xiaxyi · 2025-01-17T09:14:18Z

@mbfrahry Thanks for the comments, code is updated and tests are passed. The failed tests are irrelevant to the code change.

enabling premium service plan auto scale feature

dd68955

xiaxyi requested a review from a team as a code owner January 16, 2025 08:17

github-actions bot added documentation service/app-service size/L labels Jan 16, 2025

mbfrahry requested changes Jan 16, 2025

View reviewed changes

github-actions bot assigned mbfrahry Jan 16, 2025

github-actions bot added the waiting-response label Jan 16, 2025

fix code per comment

c8805da

github-actions bot removed the waiting-response label Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_service_plan` - support premium service plan auto scale feature #28524

`azurerm_service_plan` - support premium service plan auto scale feature #28524

xiaxyi commented Jan 16, 2025 •

edited

Loading

mbfrahry left a comment

mbfrahry Jan 16, 2025

mbfrahry Jan 16, 2025

mbfrahry Jan 16, 2025

mbfrahry Jan 16, 2025

mbfrahry Jan 16, 2025

xiaxyi commented Jan 17, 2025

	func PlanIsPremium(input *string) bool {
	func PlanIsPremium(input string) bool {

	* `premium_plan_auto_scale_enabled` - (Optional) Should automatic scaling be enabled for the Premium SKU Plan. Defaults to `false`.Cannot be set unless using a Premium SKU.
	* `premium_plan_auto_scale_enabled` - (Optional) Should automatic scaling be enabled for the Premium SKU Plan. Defaults to `false`. Cannot be set unless using a Premium SKU.

azurerm_service_plan - support premium service plan auto scale feature #28524

Are you sure you want to change the base?

azurerm_service_plan - support premium service plan auto scale feature #28524

Conversation

xiaxyi commented Jan 16, 2025 • edited Loading

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

mbfrahry left a comment

Choose a reason for hiding this comment

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

xiaxyi commented Jan 17, 2025

`azurerm_service_plan` - support premium service plan auto scale feature #28524

`azurerm_service_plan` - support premium service plan auto scale feature #28524

xiaxyi commented Jan 16, 2025 •

edited

Loading