Skip to content

Commit

Permalink
Omit capacity when creating basic plans
Browse files Browse the repository at this point in the history
The new [Basic SKU][0] does not support any scaling, and it is an error to
provide `Capacity` or `AutoScaleSettings`. The resource is implemented
in such a way that it was not possible to call
`DeploymentsCreateOrUpdateThenPoll` without one of these in the
`ScalingProperties`, which is illegal for the Basic SKUs.

Currently the backend has a hack to silently drop `ScalingProperties`
for Basic SKUs, but this is a confusing user experience:

- initial `terraform apply` submits a `PUT` with `"scalingProperties":
{"capacity": 20}` - this validation error is currently suppressed and
the server reports back `"scalingProperties": null`

- next `terraform plan` detects a `capacity 0 -> 20`

- applying that plan requests a `PATCH` with `"scalingProperties":
{"capacity": 20}`, that fails with a `UnsupportedOnBasicPlan` error

It gets more misleading if users specify a capacity explicity;
`capacity = 500` or `auto_scale_profile` will apply successfully, but
get silently ignored server-side.

This patch checks for basic plans and omits the `scalingProperties`
when the user hasn't changed from the default value. We already
instruct users to ignore `capacity`, but the check is needed to
prevent the implicit default value of 20 from getting into the
`DeploymentsCreateOrUpdateThenPoll` request.

[0]: https://docs.nginx.com/nginxaas/azure/billing/overview/#basic-plan
  • Loading branch information
ryepup committed Jun 4, 2024
1 parent 6850136 commit c651b0f
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
13 changes: 11 additions & 2 deletions internal/services/nginx/nginx_deployment_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package nginx
import (
"context"
"fmt"
"strings"
"time"

"github.com/hashicorp/go-azure-helpers/lang/pointer"
Expand All @@ -20,6 +21,8 @@ import (
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
)

const defaultCapacity = 20

type FrontendPrivate struct {
IpAddress string `tfschema:"ip_address"`
AllocationMethod string `tfschema:"allocation_method"`
Expand Down Expand Up @@ -82,6 +85,9 @@ type DeploymentModel struct {
Tags map[string]string `tfschema:"tags"`
}

func (m *DeploymentModel) isBasic() bool { return strings.HasPrefix(m.Sku, "basic") }
func (m *DeploymentModel) hasDefaultCapacity() bool { return m.Capacity == defaultCapacity }

type DeploymentResource struct{}

var _ sdk.ResourceWithUpdate = (*DeploymentResource)(nil)
Expand Down Expand Up @@ -123,7 +129,7 @@ func (m DeploymentResource) Arguments() map[string]*pluginsdk.Schema {
Type: pluginsdk.TypeInt,
Optional: true,
ConflictsWith: []string{"auto_scale_profile"},
Default: 20,
Default: defaultCapacity,
ValidateFunc: validation.IntPositive,
},

Expand Down Expand Up @@ -430,7 +436,10 @@ func (m DeploymentResource) Create() sdk.ResourceFunc {
prop.NetworkProfile.NetworkInterfaceConfiguration.SubnetId = pointer.FromString(model.NetworkInterface[0].SubnetId)
}

if model.Capacity > 0 {
// scaling is not supported for basic plans. Ignore default capacity
// for basic plans requests, which happen if a user does not
// configure capacity in their tf
if model.Capacity > 0 && !(model.hasDefaultCapacity() && model.isBasic()) {
prop.ScalingProperties = &nginxdeployment.NginxDeploymentScalingProperties{
Capacity: pointer.FromInt64(model.Capacity),
}
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/nginx_deployment.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ The following arguments are supported:

* `sku` - (Required) Specifies the NGINX Deployment SKU. Possible values are `standard_Monthly` and `basic_Monthly`. Changing this forces a new resource to be created.

-> **NOTE:** If you are setting the `sku` to `basic_Monthly`, you should use [Terraform's `ignore_changes` functionality](https://www.terraform.io/language/meta-arguments/lifecycle#ignore_changes) to ignore changes to the `capacity` field.
-> **NOTE:** If you are setting the `sku` to `basic_Monthly`, setting a non-default `capacity` or `auto_scale_profile` will fail. You should use [Terraform's `ignore_changes` functionality](https://www.terraform.io/language/meta-arguments/lifecycle#ignore_changes) to ignore changes to the `capacity` field.

* `managed_resource_group` - (Optional) Specify the managed resource group to deploy VNet injection related network resources. Changing this forces a new NGINX Deployment to be created.

Expand Down

0 comments on commit c651b0f

Please sign in to comment.