`azurerm_service_plan` - support for `premium_plan_auto_scale_enabled` by xiaxyi · Pull Request #28524 · hashicorp/terraform-provider-azurerm

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


+			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 !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

 	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


+			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

 ~> **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.

mbfrahry

Hey @xiaxyi , looks like one of my comments was missed. Once that is addressed, this should be good to go

mbfrahry · 2025-02-11T23:59:43Z

internal/services/appservice/service_plan_resource.go

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


It looks like this comment was missed that we shouldn't need this check as it's in the CustomizeDiff

mbfrahry

LGTM!

* CHANGELOG.md for v4.19.0 * Update CHANGELOG.md #28523 * Update CHANGELOG.md #28691 * Updated to include #28717 * Update for #26680 * Update CHANGELOG.md #28633 * Update CHANGELOG.md for #28703 * Update CHANGELOG.md for #28391 * Update CHANGELOG.md #28725 * Update #28733 * Update CHANGELOG.md #28659 * Update for #28741 * Update CHANGELOG.md #28712 * Update CHANGELOG.md #28441 * Update CHANGELOG.md #28441 * Update CHANGELOG.md #28441 * Update CHANGELOG.md for #28602 * Update for #27424 * Update CHANGELOG.md for #28524 * Update CHANGELOG.md #28726 * Update for #28767 * Update for #28195 * prep for release v4.19.0 --------- Co-authored-by: sreallymatt <106555974+sreallymatt@users.noreply.github.com> Co-authored-by: Wodans Son <20408400+WodansSon@users.noreply.github.com> Co-authored-by: stephybun <steph@hashicorp.com> Co-authored-by: Wyatt Fry <wyattfry@gmail.com> Co-authored-by: Matthew Frahry <mbfrahry@gmail.com> Co-authored-by: jackofallops <ste@hashicorp.com>

hashicorp#28524)

* CHANGELOG.md for v4.19.0 * Update CHANGELOG.md hashicorp#28523 * Update CHANGELOG.md hashicorp#28691 * Updated to include hashicorp#28717 * Update for hashicorp#26680 * Update CHANGELOG.md hashicorp#28633 * Update CHANGELOG.md for hashicorp#28703 * Update CHANGELOG.md for hashicorp#28391 * Update CHANGELOG.md hashicorp#28725 * Update hashicorp#28733 * Update CHANGELOG.md hashicorp#28659 * Update for hashicorp#28741 * Update CHANGELOG.md hashicorp#28712 * Update CHANGELOG.md hashicorp#28441 * Update CHANGELOG.md hashicorp#28441 * Update CHANGELOG.md hashicorp#28441 * Update CHANGELOG.md for hashicorp#28602 * Update for hashicorp#27424 * Update CHANGELOG.md for hashicorp#28524 * Update CHANGELOG.md hashicorp#28726 * Update for hashicorp#28767 * Update for hashicorp#28195 * prep for release v4.19.0 --------- Co-authored-by: sreallymatt <106555974+sreallymatt@users.noreply.github.com> Co-authored-by: Wodans Son <20408400+WodansSon@users.noreply.github.com> Co-authored-by: stephybun <steph@hashicorp.com> Co-authored-by: Wyatt Fry <wyattfry@gmail.com> Co-authored-by: Matthew Frahry <mbfrahry@gmail.com> Co-authored-by: jackofallops <ste@hashicorp.com>

github-actions · 2025-03-15T02:14:50Z

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.

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

mbfrahry requested changes Feb 12, 2025

View reviewed changes

github-actions bot added the waiting-response label Feb 12, 2025

xiaxyi added 2 commits February 12, 2025 14:35

Merge remote-tracking branch 'upstream/main' into servicePlan/autoscale

1bfb468

remove redundant check in update

b4c186f

github-actions bot removed the waiting-response label Feb 12, 2025

mbfrahry approved these changes Feb 12, 2025

View reviewed changes

mbfrahry changed the title ~~azurerm_service_plan - support premium service plan auto scale feature~~ azurerm_service_plan - support for premium_plan_auto_scale_enabled Feb 12, 2025

mbfrahry added this to the v4.19.0 milestone Feb 12, 2025

mbfrahry merged commit 0996bec into hashicorp:main Feb 12, 2025
34 checks passed

mbfrahry added a commit that referenced this pull request Feb 12, 2025

Update CHANGELOG.md for #28524

7635f62

hqhqhqhqhqhqhqhqhqhqhq pushed a commit to hqhqhqhqhqhqhqhqhqhqhq/terraform-provider-azurerm that referenced this pull request Feb 26, 2025

azurerm_service_plan - support for premium_plan_auto_scale_enabled (

720e65a

hashicorp#28524)

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_service_plan` - support for `premium_plan_auto_scale_enabled`#28524

`azurerm_service_plan` - support for `premium_plan_auto_scale_enabled`#28524
mbfrahry merged 4 commits intohashicorp:mainfrom
xiaxyi:servicePlan/autoscale

xiaxyi commented Jan 16, 2025 •

edited

Loading

Uh oh!

mbfrahry left a comment

Uh oh!

mbfrahry Jan 16, 2025

Uh oh!

mbfrahry Jan 16, 2025

Uh oh!

mbfrahry Jan 16, 2025

Uh oh!

mbfrahry Jan 16, 2025

Uh oh!

mbfrahry Jan 16, 2025

Uh oh!

xiaxyi commented Jan 17, 2025

Uh oh!

mbfrahry left a comment

Uh oh!

mbfrahry Feb 11, 2025

Uh oh!

mbfrahry left a comment

Uh oh!

Uh oh!

github-actions bot commented Mar 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	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.

Conversation

xiaxyi commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

Uh oh!

mbfrahry left a comment

Choose a reason for hiding this comment

Uh oh!

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

mbfrahry Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

xiaxyi commented Jan 17, 2025

Uh oh!

mbfrahry left a comment

Choose a reason for hiding this comment

Uh oh!

mbfrahry Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

mbfrahry left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Mar 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xiaxyi commented Jan 16, 2025 •

edited

Loading