-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Mysql flexible server iops scaling #23329
Mysql flexible server iops scaling #23329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @TheisFerre.
I left some questions/comments in-line but once those are resolved we can take another look through. Thanks!
@@ -93,6 +93,8 @@ A `storage` block exports the following: | |||
|
|||
* `auto_grow_enabled` - Is Storage Auto Grow enabled? | |||
|
|||
* `io_scaling_enabled` - Should iops be scaled automatically? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `io_scaling_enabled` - Should iops be scaled automatically? | |
* `io_scaling_enabled` - Should IOPS be scaled automatically? |
@@ -189,6 +189,8 @@ A `storage` block supports the following: | |||
|
|||
* `auto_grow_enabled` - (Optional) Should Storage Auto Grow be enabled? Defaults to `true`. | |||
|
|||
* `io_scaling_enabled` - (Optional) Should iops be scaled automatically? If `true`, `iops` can not be set. Defaults to `true`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `io_scaling_enabled` - (Optional) Should iops be scaled automatically? If `true`, `iops` can not be set. Defaults to `true`. | |
* `io_scaling_enabled` - (Optional) Should IOPS be scaled automatically? If `true`, `iops` can not be set. Defaults to `true`. |
Thanks for the quick response @stephybun I have looked at your comments now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @TheisFerre. There's a panic in the tests which needs to be fixed, I left a comment on the offending line.
You also want to run
terrafmt fmt -f internal/services/mysql/mysql_flexible_server_resource_test.go
To fix the config formatting in the test which the failing CI is flagging at the moment.
@@ -355,6 +360,11 @@ func resourceMysqlFlexibleServerCreate(d *pluginsdk.ResourceData, meta interface | |||
} | |||
} | |||
|
|||
storageSettings := expandArmServerStorage(d.Get("storage").([]interface{})) | |||
if storageSettings.Iops != nil && *storageSettings.AutoIoScaling == servers.EnableStatusEnumEnabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are failing at this point, this probably wants to be checked differently
Test ended in panic.
------- Stdout: -------
=== RUN TestAccDataSourceMySqlFlexibleServer_basic
=== PAUSE TestAccDataSourceMySqlFlexibleServer_basic
=== CONT TestAccDataSourceMySqlFlexibleServer_basic
------- Stderr: -------
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4a64c28]
goroutine 391 [running]:
github.com/hashicorp/terraform-provider-azurerm/internal/services/mysql.resourceMysqlFlexibleServerCreate(0x0?, {0x66bcc80?, 0xc001d7c000?})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/mysql/mysql_flexible_server_resource.go:364 +0x6e8
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0x7d06a00?, {0x7d06a00?, 0xc000fe5ef0?}, 0xd?, {0x66bcc80?, 0xc001d7c000?})
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:766 +0x178
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0xc00225bce0, {0x7d06a00, 0xc000fe5ef0}, 0xc001f8b450, 0xc000dc9800, {0x66bcc80, 0xc001d7c000})
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:909 +0xa7e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ApplyResourceChange(0xc0044a00d8, {0x7d06a00?, 0xc000fe5e30?}, 0xc0003b3db0)
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/grpc_provider.go:1060 +0xe8d
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ApplyResourceChange(0xc003a09c20, {0x7d06a00?, 0xc000fe5620?}, 0xc000fe9810)
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server/server.go:859 +0x574
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ApplyResourceChange_Handler({0x710d960?, 0xc003a09c20}, {0x7d06a00, 0xc000fe5620}, 0xc000fe97a0, 0x0)
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:467 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0052e25a0, {0x7d17cc0, 0xc005d804e0}, 0xc000fe3c20, 0xc002ee5650, 0xc8723f8, 0x0)
/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:1376 +0xdd2
google.golang.org/grpc.(*Server).handleStream(0xc0052e25a0, {0x7d17cc0, 0xc005d804e0}, 0xc000fe3c20, 0x0)
/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:1753 +0xa36
google.golang.org/grpc.(*Server).serveStreams.func1.1()
/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:998 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:996 +0x18c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed - forgot to check whether the Storage
block was even set.
storageSettings := expandArmServerStorage(d.Get("storage").([]interface{})) | ||
if storageSettings != nil { | ||
if storageSettings.Iops != nil && *storageSettings.AutoIoScaling == servers.EnableStatusEnumEnabled { | ||
return fmt.Errorf("`iops` can not be set if `io_scaling_enabled` is set to true") | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about this check that i have added in the resourceMysqlFlexibleServerCreate
function. Should i add it to the resourceMysqlFlexibleServerUpdate
function as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this check to the update will be problematic because iops
is a Computed
property.
This means it will always have a value and cannot be cleared. Adding this check to the update would prevent users from being able to switch over to io_scaling_enabled
so I think we should omit it here for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @TheisFerre! Tests are looking good. LGTM 🌭
<Actions> <action id="4a39167e811ac038e4a588362092472c27cfbe9e4929ae61d035f708a093a669"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>"hashicorp/azurerm" updated from "3.74.0" to "3.75.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.75.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.75.0
FEATURES:

* New Resource: `azurerm_application_load_balancer` ([#22517](hashicorp/terraform-provider-azurerm#22517 New Resource: `azurerm_resource_management_private_link` ([#23098](https://github.com/hashicorp/terraform-provider-azurerm/issues/23098))

ENHANCEMENTS:

* dependencies: `firewall` migrated to `hashicorp/go-azure-sdk` ([#22863](hashicorp/terraform-provider-azurerm#22863 `azurerm_bot_service_azure_bot` - add support for the `icon_url` property ([#23114](hashicorp/terraform-provider-azurerm#23114 `azurerm_cognitive_deployment` - `capacity` property is now updateable ([#23251](hashicorp/terraform-provider-azurerm#23251 `azurerm_container_group` - added support for `key_vault_user_identity_id` ([#23332](hashicorp/terraform-provider-azurerm#23332 `azurerm_data_factory` - added support for the `publish_enabled` property ([#2334](hashicorp/terraform-provider-azurerm#2334 `azurerm_firewall_policy_rule_collection_group` - add support for the `description` property ([#23354](hashicorp/terraform-provider-azurerm#23354 `azurerm_kubernetes_cluster` - `network_profile.network_policy` can be migrated to `cilium` ([#23342](hashicorp/terraform-provider-azurerm#23342 `azurerm_log_analytics_workspace` - add support for the `data_collection_rule_id` property ([#23347](hashicorp/terraform-provider-azurerm#23347 `azurerm_mysql_flexible_server` - add support for the `io_scaling_enabled` property ([#23329](https://github.com/hashicorp/terraform-provider-azurerm/issues/23329))

BUG FIXES:

* `azurerm_api_management_api` - fix importing `openapi` format content file issue ([#23348](hashicorp/terraform-provider-azurerm#23348 `azurerm_cdn_frontdoor_rule` - allow a `cache_duration` of `00:00:00` ([#23384](hashicorp/terraform-provider-azurerm#23384 `azurerm_cosmosdb_cassandra_datacenter` - `sku_name` is now updatable ([#23419](hashicorp/terraform-provider-azurerm#23419 `azurerm_key_vault_certificate` - fix a bug that prevented soft-deleted certificates from being recovered ([#23204](hashicorp/terraform-provider-azurerm#23204 `azurerm_log_analytics_solution` - fix create and update lifecycle of resource by splitting methods ([#23333](hashicorp/terraform-provider-azurerm#23333 `azurerm_management_group_subscription_association` - mark resource as gone correctly if not found when retrieving ([#23335](hashicorp/terraform-provider-azurerm#23335 `azurerm_management_lock` - add polling after create and delete to check for RP propagation ([#23345](hashicorp/terraform-provider-azurerm#23345 `azurerm_monitor_diagnostic_setting` - added validation to ensure at least one of `category` or `category_group` is supplied ([#23308](hashicorp/terraform-provider-azurerm#23308 `azurerm_palo_alto_local_rulestack_prefix_list` - fix rulestack not being committed on delete ([#23362](hashicorp/terraform-provider-azurerm#23362 `azurerm_palo_alto_local_rulestack_fqdn_list` - fix rulestack not being committed on delete ([#23362](hashicorp/terraform-provider-azurerm#23362 `security_center_subscription_pricing_resource` - disabled extensions logic now works as expected ([#22997](https://github.com/hashicorp/terraform-provider-azurerm/issues/22997))



</pre> </details> <details> <summary>3.76.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.76.0
FEATURES:

* New Resource: `azurerm_security_center_storage_defender` ([#23242](hashicorp/terraform-provider-azurerm#23242 New Resource: `azurerm_spring_cloud_application_insights_application_performance_monitoring` ([#23107](https://github.com/hashicorp/terraform-provider-azurerm/issues/23107))

ENHANCEMENTS:

* provider: updating to build using Go `1.21.3` ([#23514](hashicorp/terraform-provider-azurerm#23514 provider: the `roll_instances_when_required` provider feature in the `virtual_machine_scale_set` block is now optional ([#22976](hashicorp/terraform-provider-azurerm#22976 dependencies: updating to `v0.20231012.1141427` of `github.com/hashicorp/go-azure-sdk` ([#23534](hashicorp/terraform-provider-azurerm#23534 Data Source: `azurerm_application_gateway` - support for `backend_http_settings`, `global`, `gateway_ip_configuration` and additional attributes ([#23318](hashicorp/terraform-provider-azurerm#23318 Data Source: `azurerm_network_service_tags` - export the `name` attribute ([#23382](hashicorp/terraform-provider-azurerm#23382 `azurerm_cosmosdb_postgresql_cluster` - add support for `sql_version` of `16` and `citus_version` of `12.1` ([#23476](hashicorp/terraform-provider-azurerm#23476 `azurerm_palo_alto_local_rulestack` - correctly normalize the `location` property ([#23483](hashicorp/terraform-provider-azurerm#23483 `azurerm_static_site` - add support for `app_settings` ([#23421](https://github.com/hashicorp/terraform-provider-azurerm/issues/23421))

BUG FIXES:

* `azurerm_automation_schedule` - fix a bug when updating `start_time` ([#23494](hashicorp/terraform-provider-azurerm#23494 `azurerm_eventhub` - remove ForceNew and check `partition_count` is not decreased ([#23499](hashicorp/terraform-provider-azurerm#23499 `azurerm_managed_lustre_file_system` - update validation for `storage_capacity_in_tb` according to `sku_name` in use ([#23428](hashicorp/terraform-provider-azurerm#23428 `azurerm_virtual_machine` - fix a crash when the API response for the `os_profile` block contains nil properties ([#23535](https://github.com/hashicorp/terraform-provider-azurerm/issues/23535))


</pre> </details> </details> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> --------- Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com> Co-authored-by: Damien Duportal <damien.duportal@gmail.com>
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. |
Related to #19125
Adds support for the
autoIoScaling
field in thestorage
block of themysql flexible-server
resource. This is a new field introduced in the2022-01-01
version of the APII have been a bit unsure about the following:
resourceMysqlFlexibleServerCreate
function, whereiops
can not be set, ifio_scaling_enabled
istrue
. Not sure if we want this?io_scaling_enabled
, but not quite sure i like that name. Maybe you have some other suggestion?