-
Notifications
You must be signed in to change notification settings - Fork 263
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
Support cloud sql private ip (incorporating previous PR feedback) #145
Conversation
For context, a link to the previous PR: #46 |
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.
Looks good, some very small comments
reserved_peering_ranges = ["${google_compute_global_address.foobar.name}"] | ||
} | ||
|
||
# TODO figure out a way to specify the dependency to the connection resource |
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.
do you still need this TODO?
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.
Removed.
@@ -714,6 +738,42 @@ resource "google_sql_database_instance" "instance-failover" { | |||
`, instanceName, failoverName) | |||
} | |||
|
|||
func testAccSqlDatabaseInstance_with_private_network(databaseName, networkName, addressRangeName string) string { | |||
return fmt.Sprintf(` | |||
resource "google_compute_network" "foobar" { |
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.
FYI I think the test will run faster if you do auto_create_subnetworks = false
(unless you need them for the test, in which case carry on)
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.
Done.
@@ -594,6 +594,30 @@ func TestAccSqlDatabaseInstance_basic_with_user_labels(t *testing.T) { | |||
}) | |||
} | |||
|
|||
func TestAccSqlDatabaseInstance_with_private_network(t *testing.T) { |
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.
nit: could it be called TestAccSqlDatabaseInstance_withPrivateNetwork
?
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.
Done.
Thanks for taking a look :) |
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.
oh wait, can you also add documentation for the new fields?
Documentation added. |
Closing this out. Thanks again for the help. |
Support cloud sql private ip (incorporating previous PR feedback)
Support cloud sql private ip (incorporating previous PR feedback)
This PR picks up on the work done by @yhuang incorporating the feedback given by @danawillow.