`azurerm_mssql_managed_instance` - support `storage_iops` by ziyeqf · Pull Request #32250 · hashicorp/terraform-provider-azurerm

ziyeqf · 2026-04-27T01:22:49Z

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

add storageIOps property for azurerm_mssql_managed_instance.
swagger: https://github.com/Azure/azure-rest-api-specs/blob/157952a94783054c539123d508f7e122ee784017/specification/sql/resource-manager/Microsoft.Sql/SQL/preview/2023-08-01-preview/ManagedInstances.json#L1107
limitation document: https://learn.microsoft.com/en-us/azure/azure-sql/managed-instance/resource-limits?view=azuresql#iops-and-throughput-in-the-next-gen-general-purpose-service-tier

per communication with svc team current min value is 300, max value is 8000

The storage_iops property is an O+C property, the service side behavior:

Non-GPv2 instances: storageIOps is not applicable. The backend will reject a create/update request that includes it for a non-GPv2 edition.
GPv2, user doesn't specify storageIOps: omit it from the request. Our backend automatically defaults to the included IOPS for the selected storage size The resolved value will appear in the GET response after creation.
GPv2, user specifies storageIOps: pass it through. Backend validates the range (300-80000, depends on vCores and hardware family).

so the behavior of terraform side, mainly for the update logic:

When general_purpose_v2_enabled == true
 
- changed - read from config
- unchanged - read from exisiting response
 
when general_purpose_v2_enabled == false
 
- never carried in payload
 
When changing general_purpose_v2_enabled  from true to false
 
- set it to nil
 
when changing general_purpose_v2_enabled  from false to true
 
- changed(specified in config): read from state
- unchanged (not specified in config): leave it as nil and let the service set default value

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 relevant 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)

Change Log

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

azurerm_mssql_managed_instance - support storage_iops [azurerm_mssql_managed_instance - support storage_iops #32250]

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 #0000

AI Assistance Disclosure

AI Assisted - This contribution was made by, or with the assistance of, AI/LLMs

helped on test case composing and code review

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the provider.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

Note

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

Co-authored-by: Copilot <copilot@github.com>

ziyeqf · 2026-04-27T01:24:00Z

/test

github-actions · 2026-04-27T01:24:19Z

⚠️ You are not authorized to run tests. Only members of the designated team can trigger test runs.

Co-authored-by: Copilot <copilot@github.com>

ziyeqf · 2026-05-01T04:33:29Z

Sry I'm keeping push my testing code since my testing sub has limited quota.

gerrytan

Thank you @ziyeqf, there's some concerns and questions I have as per below:

gerrytan · 2026-05-06T00:33:08Z

+		"storage_iops": {
+			Type:     schema.TypeInt,
+			Optional: true,
+			// NOTE: O+C - Azure returns a calculated IOPS value for GPv2 instances when `storage_iops` is omitted.


I have to agree with this O+C despite our guidance in best-practices.md -> Setting Properties to Optional + Computed. I tested this iops setting in portal and the rule is quite complex, it's coupled to vcores and storage in gb.

Ideally there is a validation API we can call, or at least the error message is descriptive if user supplies invalid value?

See below my portal experiment:

mssql_managed_instance_iops.mp4

I don't think there is a validation API, and the current sdk does not support that form.. possibly I can confirm with the svc team what's the rule, e.g.: 6400*vcore? but I'm a bit worried it may change with time

After further tinkering on the update logic, we've decided to not do O+C in here because it over-complicates the meaning of empty / nil value in relation to SKU and other coupled field.

Instead we agree on making this an optional field with default value of 400 or whatever minimum is in portal, then do our best effort validating in CustomizeDiff, and let backend API return with error on other invalid inputs outside coverage.

The final O+C logic after discussion:

When GPv2_enabled == true - changed - read from config - unchanged - read from exisiting when GPv2_enabled == false - never carried in payload When changing from enabled from true to false - set it to nil when changing from false to true - changed(specified in config): read from state - unchanged (not specified in config): leave it as nil and let the service set default value

👍 after further discussion, we have to go back to O+C design because storage_iops is only valid when general_purpose_v2_enabled is on. Cannot use optional + default.

gerrytan · 2026-05-06T00:56:54Z

+
+			// when the MI is not GPv2, the storageIOps is not returned and will be 0 in state. That's not a valid value for the service.
+			if metadata.ResourceData.HasChange("storage_iops") && state.StorageIOps != 0 {
+				props.StorageIOps = pointer.To(state.StorageIOps)


This logic is wrong. If there is no changes, and API returns positive props.StorageIOps value, it will get nil-ed by line 753 above.

I don't get the point. make it nil is on-purpose.
In this scenario: when change a GPv2 MI to non GPv2, the service might return a >0 value, but it's not a valid value in the update payload

Discussed in DM, this scenario is a bug:

resource has storage_iops = 400, user updated other field (eg: tag)

Update method set props.StorageIOps = nil in line 753, but HasChange("storage_iops") evaluates to false

The PUT request for update has props.StorageIOps set to nil instead of 400

as discussed offline, the update loghic of storage_iops is as below, and it's only controled by general_purpose_v2_enabled not sku_name

When GPv2_enabled == true

changed - read from config

unchanged - read from exisiting

when GPv2_enabled == false

never carried in payload

When changing from enabled from true to false

set it to nil

when changing from false to true

changed(specified in config): read from state

unchanged (not specified in config): leave it as nil and let the service set default value

teowa

Hi @ziyeqf , I have left some comments inline.

teowa

Thanks @ziyeqf , LGTM

gerrytan

Thx @ziyeqf , I still found issue, but it's pretty close. Can you also please drop a link to the latest acctest run. In progress run is fine.

gerrytan

Thx @ziyeqf this PR looks good to me now.

Please also run acctest against latest commit and check all the GH workflow passes. They're still in-progress 👍

ziyeqf · 2026-05-11T01:14:09Z

latest test result

azurerm_mssql_managed_instance - support storage_iops

4bf93fc

Co-authored-by: Copilot <copilot@github.com>

github-actions Bot added enhancement documentation service/mssqlmanagedinstance size/L labels Apr 27, 2026

update test case and the logic for zone_redundant_enabled

36aacaf

Co-authored-by: Copilot <copilot@github.com>

github-actions Bot added size/XL and removed size/L labels Apr 27, 2026

ziyeqf added 2 commits April 30, 2026 17:08

handling the storage_iops property

28df690

update code

7f007aa

github-actions Bot added the waiting-response label May 1, 2026

build failure

434f352

github-actions Bot removed the waiting-response label May 1, 2026

add custom poller for network intent policy

35157e6

ziyeqf added 6 commits May 1, 2026 05:35

update poller ctx

62828fb

fix acctest

a72ee73

azurerm-linter

20f0f82

fix acctest

1fd13a5

update test case

15cf54d

cleanup

809524e

github-actions Bot added the ai-assisted label May 5, 2026

ziyeqf requested review from gerrytan and teowa May 5, 2026 01:48

gerrytan suggested changes May 6, 2026

View reviewed changes

github-actions Bot assigned gerrytan May 6, 2026

github-actions Bot added the waiting-response label May 6, 2026

update per comments

35cb768

azurerm-linter

eff5a3e

github-actions Bot removed the waiting-response label May 6, 2026

teowa reviewed May 6, 2026

View reviewed changes

Comment thread internal/services/mssqlmanagedinstance/mssql_managed_instance_resource.go

Comment thread website/docs/r/mssql_managed_instance.html.markdown Outdated

Comment thread ...s/mssqlmanagedinstance/custompollers/managed_instance_subnet_network_intent_policy_poller.go

github-actions Bot added the waiting-response label May 6, 2026

update per comments

6f0b21f

github-actions Bot removed the waiting-response label May 6, 2026

ziyeqf requested review from gerrytan and teowa May 6, 2026 04:36

teowa reviewed May 6, 2026

View reviewed changes

Comment thread internal/services/mssqlmanagedinstance/mssql_managed_instance_resource.go

github-actions Bot added the waiting-response label May 6, 2026

add lock on subnet, during creation

5272b3d

github-actions Bot removed the waiting-response label May 6, 2026

teowa approved these changes May 6, 2026

View reviewed changes

ziyeqf marked this pull request as ready for review May 7, 2026 00:46

ziyeqf requested review from a team, WodansSon and magodo as code owners May 7, 2026 00:46

github-actions Bot added the waiting-response label May 7, 2026

gerrytan suggested changes May 7, 2026

View reviewed changes

Comment thread internal/services/mssqlmanagedinstance/mssql_managed_instance_resource.go Outdated

npe fix

7b336f4

github-actions Bot removed the waiting-response label May 7, 2026

gerrytan approved these changes May 7, 2026

View reviewed changes

github-actions Bot added waiting-response and removed waiting-response labels May 7, 2026

revert mistakes..

14472d8

github-actions Bot added the waiting-response label May 7, 2026

add continues validation for custom poller

9b11d71

github-actions Bot removed the waiting-response label May 8, 2026

mybayern1974 unassigned gerrytan May 15, 2026

Conversation

ziyeqf commented Apr 27, 2026 • 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)

AI Assistance Disclosure

Rollback Plan

Changes to Security Controls

Uh oh!

ziyeqf commented Apr 27, 2026

Uh oh!

github-actions Bot commented Apr 27, 2026

Uh oh!

ziyeqf commented May 1, 2026

Uh oh!

gerrytan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teowa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

teowa left a comment

Choose a reason for hiding this comment

Uh oh!

gerrytan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gerrytan left a comment

Choose a reason for hiding this comment

Uh oh!

ziyeqf commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ziyeqf commented Apr 27, 2026 •

edited

Loading