update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds #1449

gabinante · 2018-06-26T21:17:22Z

Fixes issue 1442 by updating the upper bound of PostgreSQL and MySQL database sizes to match the current maximum of 4 TB / 4194304 MB.
#1442

relevant azure documentation:
PostgreSQL - https://docs.microsoft.com/en-us/azure/postgresql/concepts-pricing-tiers
MySQL - https://docs.microsoft.com/en-us/azure/mysql/concepts-pricing-tiers

(fixes #1442)

update

katbyte

Hi @gabinante,

Thank you for this contribution. Could we update the documentation to reflect the new limits? It'll LGTM once that is done.

WodansSon · 2018-06-26T22:35:38Z

azurerm/resource_arm_mysql_server.go

@@ -135,7 +135,7 @@ func resourceArmMySqlServer() *schema.Resource {
 							Type:         schema.TypeInt,
 							Required:     true,
 							ForceNew:     true,
-							ValidateFunc: validateIntBetweenDivisibleBy(5120, 1048576, 1024),
+							ValidateFunc: validateIntBetweenDivisibleBy(5120, 4194304, 1024),


Shouldn't this also validate the SKU since only general purpose and memory optimized support this larger upper limit?

Good catch, i agree

@jeffreyCline I'm not super familiar with your conventions / golang style best practices. would you use a case/switch or an if here? Or is there a better option? Is there an example of another resource in the provider that does this?

@gabinante,

Here is an example of using the a CustomizeDiff to ensure 1 of 2 properties is set.

In your case you can use this to get the storage_mb property is within the bounds of what the current sku allows.

@gabinante I went ahead and added the validation code directly to your branch, hope that was ok. 😃

WodansSon

Hi @gabinante

Thanks for the PR, it mostly looks good to go, but I left a comment. Can you please address it at your earliest convenience.

VaijanathB

The changes mostly look good. The test change requires you to change it for MemoryOptimized test. I have added the line link. Please update this.

VaijanathB · 2018-06-26T22:37:27Z

azurerm/resource_arm_postgresql_server_test.go

@@ -386,7 +386,7 @@ resource "azurerm_postgresql_server" "test" {
  }

  storage_profile {
-    storage_mb = 947200
+    storage_mb = 4194304


This test will fail. You might want to change the memoryOptimized test https://github.com/gabinante/terraform-provider-azurerm/blob/35a13bb4a1ffff476bd91f99391a31d42e60a48d/azurerm/resource_arm_postgresql_server_test.go#L455

WodansSon · 2018-06-27T19:56:10Z

website/docs/r/mysql_server.html.markdown

@@ -84,7 +84,7 @@ The following arguments are supported:

 `storage_profile` supports the following:

-* `storage_mb` - (Required) Max storage allowed for a server, possible values are between `5120 MB` (5GB) and `1048576 MB` (1TB). The step for this value must be in `1024 MB` (1GB) increments. For more information see the [product documentation](https://docs.microsoft.com/en-us/rest/api/mysql/servers/create#StorageProfile).
+* `storage_mb` - (Required) Max storage allowed for a server, possible values are between `5120 MB` (5GB) and `4194304 MB` (4TB). The step for this value must be in `1024 MB` (1GB) increments. For more information see the [product documentation](https://docs.microsoft.com/en-us/rest/api/mysql/servers/create#StorageProfile).


Can we change this to be: ...possible values are between 5120 MB(5GB) and1048576 MB(1TB) for the Basic SKU and between5120 MB(5GB) and4194304 MB (4TB) for General Purpose/Memory Optimized SKUs.?

sure, will add that to my commit with the conditional

WodansSon · 2018-06-27T19:56:26Z

website/docs/r/postgresql_server.html.markdown

@@ -83,7 +83,7 @@ The following arguments are supported:

 `storage_profile` supports the following:

-* `storage_mb` - (Required) Max storage allowed for a server, possible values are between `5120 MB` (5GB) and `1048576 MB` (1TB). The step for this value must be in `1024 MB` (1GB) increments. For more information see the [product documentation](https://docs.microsoft.com/en-us/rest/api/postgresql/servers/create#StorageProfile).
+* `storage_mb` - (Required) Max storage allowed for a server, possible values are between `5120 MB` (5GB) and `4194304 MB` (4TB). The step for this value must be in `1024 MB` (1GB) increments. For more information see the [product documentation](https://docs.microsoft.com/en-us/rest/api/postgresql/servers/create#StorageProfile).


Same as above.

Validated configuration since basic sku only supports up to 1 TB of storage.

Since the basic sku only allows up to 1 TB of storage

katbyte

Thanks for the contributions @gabinante & @jeffreyCline ,

This LGTM 🙂

ghost · 2020-03-30T15:29:03Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

gabinante and others added 3 commits June 26, 2018 10:09

Merge pull request #1 from terraform-providers/master

444236a

update

add increased database size and increase maximum size unit test to match

99768a3

add mysql fix as well

35a13bb

katbyte requested changes Jun 26, 2018

View reviewed changes

katbyte changed the title ~~Bugfix/database size update~~ update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds Jun 26, 2018

katbyte added this to the 1.8.0 milestone Jun 26, 2018

katbyte added bug service/postgresql service/mysql labels Jun 26, 2018

WodansSon reviewed Jun 26, 2018

View reviewed changes

WodansSon requested changes Jun 26, 2018

View reviewed changes

WodansSon added the waiting-response label Jun 26, 2018

VaijanathB suggested changes Jun 26, 2018

View reviewed changes

address test failure and feedback regarding documentation

21e9207

WodansSon reviewed Jun 27, 2018

View reviewed changes

WodansSon and others added 6 commits June 27, 2018 17:20

Added sku validation

6c2b539

Validated configuration since basic sku only supports up to 1 TB of storage.

Added sku validation

66801fc

Since the basic sku only allows up to 1 TB of storage

fixed indentation

0669fd7

fixed intendtation

7b84b55

add a bit of documentation

6d5473e

postgtes & mysql - make fmt to fix formatting

47fa45a

katbyte approved these changes Jun 28, 2018

View reviewed changes

katbyte merged commit d3cdd17 into hashicorp:master Jun 28, 2018

katbyte added a commit that referenced this pull request Jun 28, 2018

Update CHANGELOG.md to include #1449

bc60d9e

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds #1449

update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds #1449

gabinante commented Jun 26, 2018 •

edited by katbyte

Loading

katbyte left a comment

WodansSon Jun 26, 2018

katbyte Jun 26, 2018

gabinante Jun 27, 2018

katbyte Jun 27, 2018

WodansSon Jun 28, 2018

WodansSon left a comment

VaijanathB left a comment

VaijanathB Jun 26, 2018

WodansSon Jun 27, 2018

gabinante Jun 27, 2018

WodansSon Jun 27, 2018

katbyte left a comment

ghost commented Mar 30, 2020

update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds #1449

update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds #1449

Conversation

gabinante commented Jun 26, 2018 • edited by katbyte Loading

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WodansSon left a comment

Choose a reason for hiding this comment

VaijanathB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Mar 30, 2020

gabinante commented Jun 26, 2018 •

edited by katbyte

Loading