Skip to content
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.

Sign up for GitHub

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

update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds #1449

Merged
merged 10 commits into from
Jun 28, 2018
Merged

update azurerm_postgresql_server/azurerm_mysql_server storage_mb validation bounds #1449

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
444236a
Merge pull request #1 from terraform-providers/master
gabinante Jun 26, 2018
99768a3
add increased database size and increase maximum size unit test to match
Jun 26, 2018
35a13bb
add mysql fix as well
Jun 26, 2018
21e9207
address test failure and feedback regarding documentation
Jun 27, 2018
6c2b539
Added sku validation
WodansSon Jun 28, 2018
66801fc
Added sku validation
WodansSon Jun 28, 2018
0669fd7
fixed indentation
WodansSon Jun 28, 2018
7b84b55
fixed intendtation
WodansSon Jun 28, 2018
6d5473e
add a bit of documentation
Jun 28, 2018
47fa45a
postgtes & mysql - make fmt to fix formatting
katbyte Jun 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion azurerm/resource_arm_mysql_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, i agree

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

},
"backup_retention_days": {
Type: schema.TypeInt,
Expand Down
2 changes: 1 addition & 1 deletion azurerm/resource_arm_mysql_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ resource "azurerm_mysql_server" "test" {
}

storage_profile {
storage_mb = 1048576,
storage_mb = 4194304,
backup_retention_days = 7
geo_redundant_backup = "Enabled"
}
Expand Down
2 changes: 1 addition & 1 deletion azurerm/resource_arm_postgresql_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func resourceArmPostgreSQLServer() *schema.Resource {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
ValidateFunc: validateIntBetweenDivisibleBy(5120, 1048576, 1024),
ValidateFunc: validateIntBetweenDivisibleBy(5120, 4194304, 1024),
},

"backup_retention_days": {
Expand Down
2 changes: 1 addition & 1 deletion azurerm/resource_arm_postgresql_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ resource "azurerm_postgresql_server" "test" {
}

storage_profile {
storage_mb = 1048576
storage_mb = 4194304
backup_retention_days = 7
geo_redundant_backup = "Enabled"
}
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/mysql_server.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will add that to my commit with the conditional


* `backup_retention_days` - (Optional) Backup retention days for the server, supported values are between `7` and `35` days.

Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/postgresql_server.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.


* `backup_retention_days` - (Optional) Backup retention days for the server, supported values are between `7` and `35` days.

Expand Down