Add ability to import `google_sql_database` #12

gh-mlfowler · 2017-06-12T20:22:33Z

Previously raised as hashicorp/terraform#15237

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccGoogleSqlDatabase_ -timeout 120m
?       github.com/terraform-providers/terraform-provider-google        [no test files]
=== RUN   TestAccGoogleSqlDatabase_importBasic
--- PASS: TestAccGoogleSqlDatabase_importBasic (78.63s)
=== RUN   TestAccGoogleSqlDatabase_basic
--- PASS: TestAccGoogleSqlDatabase_basic (67.22s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 145.863s

rileykarson

Thanks for the awesome work! I left a few comments if you don't mind going over them.

rileykarson · 2017-06-16T20:59:36Z

google/resource_sql_database.go

@@ -86,6 +90,13 @@ func resourceSqlDatabaseRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}

+	// In the import case this won't be set
+	if _, ok := d.GetOk("instance"); !ok {


Instead of adding a conditional block here where we set the database name and instance name on refresh, we can just get them from the ID every time. That way, both terraform import and the refreshes that happen (such as part of terraform plan) will follow the same code paths.

To make sure we update the state file, then, we should also d.Set name and instance at the end of the function; we can use the variables we make here, or (preferably imo) pull them off the db object.

Makes sense however for it to work I've had to add a d.setId in the Create method as it's currently set as part of the Read.

rileykarson · 2017-06-16T21:18:29Z

website/docs/r/sql_database.html.markdown

@@ -48,3 +48,12 @@ In addition to the arguments listed above, the following computed attributes are
 exported:

 * `self_link` - The URI of the created resource.
+


Would you mind subbing "users-db" for "image-store-bucket" above in the example? (Github doesn't like comments on unchanged lines)

rileykarson · 2017-06-16T21:18:31Z

website/docs/r/sql_database.html.markdown

+
+## Import
+
+Database resources can be imported using the `name` of the database instance


Let's make this text look more like the router family of resources' import docs:

SQL databases can be imported using the instance and name, e.g.

$ terraform import google_sql_database.users master-instance:users-db

gh-mlfowler · 2017-06-20T17:30:37Z

Thanks for the review @rileykarson. I've added all your suggestions however note that I now conflict as someone has already revised the documentation to make roughly the same change. Do you wish me to rebase?

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccGoogleSqlDatabase_ -timeout 120m
?       github.com/terraform-providers/terraform-provider-google        [no test files]
=== RUN   TestAccGoogleSqlDatabase_importBasic
--- PASS: TestAccGoogleSqlDatabase_importBasic (67.64s)
=== RUN   TestAccGoogleSqlDatabase_basic
--- PASS: TestAccGoogleSqlDatabase_basic (66.62s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 134.264s

rileykarson · 2017-06-20T18:29:41Z

google/resource_sql_database.go

-	database_name := d.Get("name").(string)
-	instance_name := d.Get("instance").(string)
+	s := strings.Split(d.Id(), ":")
+	instance_name := s[0]


We should validate that there are exactly 2 elements in this array, and return an error specifying the format otherwise. That way, users that discover they can import the resource through the Importability page and try just a name will find that they also need to specify instance.

rileykarson · 2017-06-20T18:34:58Z

Let's use "users-db" over "users", so that we don't have "users" as both the name and part of the Terraform address.

Also, I just noticed something I missed initially - sorry about that! I added a comment for it if you don't mind looking at it as well.

gh-mlfowler · 2017-06-20T22:57:07Z

I have added the array check and provided a suitable error message.

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccGoogleSqlDatabase_ -timeout 120m
?       github.com/terraform-providers/terraform-provider-google        [no test files]
=== RUN   TestAccGoogleSqlDatabase_importBasic
--- PASS: TestAccGoogleSqlDatabase_importBasic (79.35s)
=== RUN   TestAccGoogleSqlDatabase_basic
--- PASS: TestAccGoogleSqlDatabase_basic (66.78s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 146.149s```

rileykarson · 2017-06-21T00:33:52Z

Thanks! Would you mind resolving the merge by taking your changes?

gh-mlfowler · 2017-06-21T08:37:28Z

Rebase completed @rileykarson

rileykarson · 2017-06-22T16:10:07Z

LGTM, thanks @gh-mlfowler!

TF_ACC=1 go test ./google -v -run=TestAccGoogleSqlDatabase_ -timeout 120m
=== RUN   TestAccGoogleSqlDatabase_importBasic
--- PASS: TestAccGoogleSqlDatabase_importBasic (72.03s)
=== RUN   TestAccGoogleSqlDatabase_basic
--- PASS: TestAccGoogleSqlDatabase_basic (73.23s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	145.453s

@danawillow

Add `google_sql_database` and `google_sql_database_instance` to Importability page. @danawillow hashicorp/terraform-provider-google#11 hashicorp/terraform-provider-google#12

@danawillow

Add `google_sql_database` and `google_sql_database_instance` to Importability page. @danawillow hashicorp/terraform-provider-google#11 hashicorp/terraform-provider-google#12

* Add ability to import `google_sql_database` * Update from code review * Ensure split id length and report error otherwise

Output from magician generating into ga and beta providers

ghost · 2020-03-31T14:16:09Z

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!

gh-mlfowler mentioned this pull request Jun 12, 2017

Add ability to import google_sql_database hashicorp/terraform#15237

Closed

rileykarson added the enhancement label Jun 14, 2017

rileykarson requested changes Jun 16, 2017

View reviewed changes

gh-mlfowler force-pushed the google_import_database branch from 6235f4e to 6274602 Compare June 20, 2017 17:19

rileykarson reviewed Jun 20, 2017

View reviewed changes

gh-mlfowler force-pushed the google_import_database branch from 4e10b77 to 6274602 Compare June 20, 2017 22:54

gh-mlfowler force-pushed the google_import_database branch from ae6ab0b to 4a8b337 Compare June 21, 2017 08:23

gh-mlfowler added 3 commits June 21, 2017 09:23

Add ability to import google_sql_database

fdec6f3

Update from code review

88d5232

Ensure split id length and report error otherwise

2605a17

gh-mlfowler force-pushed the google_import_database branch from 4a8b337 to 2605a17 Compare June 21, 2017 08:25

rileykarson approved these changes Jun 22, 2017

View reviewed changes

rileykarson merged commit 719c556 into hashicorp:master Jun 22, 2017

rileykarson mentioned this pull request Jun 22, 2017

provider/google: SQL Database Importability hashicorp/terraform#15372

Merged

chrisst referenced this pull request in chrisst/terraform-provider-google Nov 9, 2018

Add ability to import google_sql_database (#12)

18ada1c

* Add ability to import `google_sql_database` * Update from code review * Ensure split id length and report error otherwise

luis-silva pushed a commit to luis-silva/terraform-provider-google that referenced this pull request May 21, 2019

Merge pull request hashicorp#12 from modular-magician/codegen-pr-502

be82687

Output from magician generating into ga and beta providers

luis-silva pushed a commit to luis-silva/terraform-provider-google that referenced this pull request May 21, 2019

Merge pull request hashicorp#12 from modular-magician/codegen-pr-502

82684fc

Output from magician generating into ga and beta providers

ghost locked and limited conversation to collaborators Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to import `google_sql_database` #12

Add ability to import `google_sql_database` #12

gh-mlfowler commented Jun 12, 2017

rileykarson left a comment

rileykarson Jun 16, 2017

gh-mlfowler Jun 20, 2017

rileykarson Jun 16, 2017

rileykarson Jun 16, 2017

gh-mlfowler commented Jun 20, 2017

rileykarson Jun 20, 2017

rileykarson commented Jun 20, 2017

gh-mlfowler commented Jun 20, 2017

rileykarson commented Jun 21, 2017

gh-mlfowler commented Jun 21, 2017

rileykarson commented Jun 22, 2017

ghost commented Mar 31, 2020

		@@ -48,3 +48,12 @@ In addition to the arguments listed above, the following computed attributes are
		exported:

		* `self_link` - The URI of the created resource.


		## Import

		Database resources can be imported using the `name` of the database instance

Add ability to import google_sql_database #12

Add ability to import google_sql_database #12

Conversation

gh-mlfowler commented Jun 12, 2017

rileykarson left a comment

Choose a reason for hiding this comment

rileykarson Jun 16, 2017

Choose a reason for hiding this comment

gh-mlfowler Jun 20, 2017

Choose a reason for hiding this comment

rileykarson Jun 16, 2017

Choose a reason for hiding this comment

rileykarson Jun 16, 2017

Choose a reason for hiding this comment

gh-mlfowler commented Jun 20, 2017

rileykarson Jun 20, 2017

Choose a reason for hiding this comment

rileykarson commented Jun 20, 2017

gh-mlfowler commented Jun 20, 2017

rileykarson commented Jun 21, 2017

gh-mlfowler commented Jun 21, 2017

rileykarson commented Jun 22, 2017

ghost commented Mar 31, 2020

Add ability to import `google_sql_database` #12

Add ability to import `google_sql_database` #12