provider/azurerm: Add resource azurerm_sql_elasticpool #14099

dominik-lekse · 2017-04-30T09:52:20Z

This pull request adds the resource azurerm_sql_elasticpool to create and manage Azure SQL Elastic Pools.

Notes:

In contrast to the existing resources to manage Azure SQL related functionality, azurerm_sql_elasticpool uses the azure-sdk-for-go. For this purpose, I have added it to the vendor dependencies.

Related issues and pull requests:

This pull request will allow [WIP] provider/azurerm: Add support for elastic_pool_name to sql_database #12678 to be completed

Acceptance tests:

TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMSqlElasticPool_ -timeout 120m
=== RUN   TestAccAzureRMSqlElasticPool_importBasic
--- PASS: TestAccAzureRMSqlElasticPool_importBasic (146.96s)
=== RUN   TestAccAzureRMSqlElasticPool_basic
--- PASS: TestAccAzureRMSqlElasticPool_basic (145.20s)
=== RUN   TestAccAzureRMSqlElasticPool_resizeDtu
--- PASS: TestAccAzureRMSqlElasticPool_resizeDtu (187.22s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	479.408s

Tasks:

Resource azurerm_sql_elasticpool
Acceptance test TestAccAzureRMSqlElasticPool_basic (verifies creating a SQL Elasticpool)
Acceptance test TestAccAzureRMSqlElasticPool_resizeDtu (verifies resizing the DTU limit of a SQL Elasticpool)
Import acceptance test TestAccAzureRMSqlElasticPool_importBasic (verifies a SQL Elasticpool can be imported)
Documentation
Review

Fixes: hashicorp#12673 Unfortunately, this ElasticPool has to actually be created so we need to implement the ElasticPool resource

tombuildsstuff

Hi @dominik-lekse

Thanks for this PR :)

I've reviewed and left some comments inline, but it otherwise looks good to me :)

Tests Pass:

$ envchain azurerm make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMSqlElasticPool'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/02 08:40:45 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMSqlElasticPool -timeout 120m
=== RUN   TestAccAzureRMSqlElasticPool_importBasic
--- PASS: TestAccAzureRMSqlElasticPool_importBasic (132.54s)
=== RUN   TestAccAzureRMSqlElasticPool_basic
--- PASS: TestAccAzureRMSqlElasticPool_basic (144.69s)
=== RUN   TestAccAzureRMSqlElasticPool_resizeDtu
--- PASS: TestAccAzureRMSqlElasticPool_resizeDtu (257.79s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	535.042s

Thanks!

tombuildsstuff · 2017-05-02T07:28:01Z

builtin/providers/azurerm/config.go

@@ -99,6 +100,11 @@ type ArmClient struct {
 	serviceBusSubscriptionsClient servicebus.SubscriptionsClient

 	keyVaultClient keyvault.VaultsClient
+
+	sqlDatabasesClient               sql.DatabasesClient


Given we're currently using the Riviera SDK for this functionality, this client isn't used. As such - is it possible to remove it for the moment?

tombuildsstuff · 2017-05-02T07:28:08Z

builtin/providers/azurerm/config.go

+
+	sqlDatabasesClient               sql.DatabasesClient
+	sqlElasticPoolsClient            sql.ElasticPoolsClient
+	sqlRecommendedElasticPoolsClient sql.RecommendedElasticPoolsClient


Given we're currently using the Riviera SDK for this functionality, this client isn't used. As such - is it possible to remove it for the moment?

tombuildsstuff · 2017-05-02T07:28:09Z

builtin/providers/azurerm/config.go

+	sqlDatabasesClient               sql.DatabasesClient
+	sqlElasticPoolsClient            sql.ElasticPoolsClient
+	sqlRecommendedElasticPoolsClient sql.RecommendedElasticPoolsClient
+	sqlServersClient                 sql.ServersClient


Given we're currently using the Riviera SDK for this functionality, this client isn't used. As such - is it possible to remove it for the moment?

I have removed the unused clients except for sqlElasticPoolsClient which is used by the resource. I assume, Terraform has deprecated the Riviera SDK and will prefer the Azure RM Go SDK, right?

tombuildsstuff · 2017-05-02T07:40:18Z

builtin/providers/azurerm/resource_arm_sql_elasticpool.go

+	d.Set("server_name", serverName)
+	d.Set("edition", string(elasticPool.Edition))
+	d.Set("dtu", int(*elasticPool.Dtu))
+	d.Set("db_dtu_min", int(*elasticPool.DatabaseDtuMin))


is it worth checking that resp.ElasticPoolProperties isn't nil before assigning these, to prevent a possible crash?

Safe guard is now included.

…eature/azurerm-sql-elasticpool

… to verify creating a SQL database in an SQL elastic pool

…o SDK clients; Safe guard ElasticPoolProperties in resourceArmSqlElasticPoolRead

dominik-lekse · 2017-05-02T12:57:06Z

Thanks for the feedback @tombuildsstuff. With the recent commits, I addressed your comments. In addition, I have included the changes proposed in #12678 in this pull request to allow managing databases in elastic pools. Also, the test TestAccAzureRMSqlDatabase_elasticPool is now completed, i.e. it verifies creating a database in a new elastic pool using the new resource.

=== RUN   TestAccAzureRMSqlDatabase_elasticPool
--- PASS: TestAccAzureRMSqlDatabase_elasticPool (186.53s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	186.553s

tombuildsstuff

Hi @dominik-lekse

Thanks for pushing those fixes and integrating #12678

I've taken another look and pushed a small fix for a typo in the documentation - but this is looking great :)

Thanks for this contribution - it's great to see this merged :)

tombuildsstuff · 2017-05-03T05:28:17Z

Tests pass:

ghost · 2020-04-13T02:01:46Z

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

stack72 and others added 3 commits March 14, 2017 15:18

provider/azurerm: Add support for elastic_pool_name to sql_database

a5908cd

Fixes: hashicorp#12673 Unfortunately, this ElasticPool has to actually be created so we need to implement the ElasticPool resource

Added vendor dependency to github.com/Azure/azure-sdk-for-go/arm/sql

f17c5ee

Added resource azurerm_sql_elasticpool

720ae3d

radeksimko added new-resource provider/azurerm labels Apr 30, 2017

stack72 mentioned this pull request Apr 30, 2017

[WIP] provider/azurerm: Add support for elastic_pool_name to sql_database #12678

Closed

dominik-lekse mentioned this pull request May 1, 2017

provider/azurerm Migrate resources using Riviera SDK to Azure ARM SDK #14113

Closed

tombuildsstuff reviewed May 2, 2017

View reviewed changes

dominik-lekse added 3 commits May 2, 2017 14:27

Merge commit 'a5908cd288d5ab3754d47376ce162369e569bba2' into public-f…

f415949

…eature/azurerm-sql-elasticpool

provider/azurerm Completed test TestAccAzureRMSqlDatabase_elasticPool…

8491fb9

… to verify creating a SQL database in an SQL elastic pool

provider/azurerm: Pull request hashicorp#14099 Removed unused Azure G…

bd51a4d

…o SDK clients; Safe guard ElasticPoolProperties in resourceArmSqlElasticPoolRead

tombuildsstuff self-assigned this May 2, 2017

Fixing a typo creation_data -> creation_date

01ad95c

tombuildsstuff approved these changes May 3, 2017

View reviewed changes

tombuildsstuff merged commit 5a2aecd into hashicorp:master May 3, 2017

tombuildsstuff added a commit that referenced this pull request May 3, 2017

Updating to include #14099

05b63f9

hashibot mentioned this pull request Jun 13, 2017

provider/azurerm Migrate resources using Riviera SDK to Azure ARM SDK hashicorp/terraform-provider-azurerm#78

Closed

hashicorp locked and limited conversation to collaborators Apr 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/azurerm: Add resource azurerm_sql_elasticpool #14099

provider/azurerm: Add resource azurerm_sql_elasticpool #14099

dominik-lekse commented Apr 30, 2017

tombuildsstuff left a comment

tombuildsstuff May 2, 2017

tombuildsstuff May 2, 2017

tombuildsstuff May 2, 2017

dominik-lekse May 2, 2017

tombuildsstuff May 2, 2017

dominik-lekse May 2, 2017

dominik-lekse commented May 2, 2017

tombuildsstuff left a comment

tombuildsstuff commented May 3, 2017

ghost commented Apr 13, 2020

provider/azurerm: Add resource azurerm_sql_elasticpool #14099

provider/azurerm: Add resource azurerm_sql_elasticpool #14099

Conversation

dominik-lekse commented Apr 30, 2017

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff May 2, 2017

Choose a reason for hiding this comment

tombuildsstuff May 2, 2017

Choose a reason for hiding this comment

tombuildsstuff May 2, 2017

Choose a reason for hiding this comment

dominik-lekse May 2, 2017

Choose a reason for hiding this comment

tombuildsstuff May 2, 2017

Choose a reason for hiding this comment

dominik-lekse May 2, 2017

Choose a reason for hiding this comment

dominik-lekse commented May 2, 2017

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff commented May 3, 2017

ghost commented Apr 13, 2020