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.

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

PostgreSQL high availability #1001

Merged
merged 5 commits into from
Jan 24, 2018

Conversation

lawrencejones
Copy link
Contributor

Hey all,

This change continues on from #695 to address the review comments there and finish off the high availability support.

The only changes from #695 are the addition of an integration test and some fixing up of the sqladmin package. Also added some comments to clarify that the 1st Gen instances will required diff suppressing, and a default is provided server side when availability zone isn't given.

@katzj, I hope you don't mind but we really needed this at GoCardless a few months back!


Integration test output is:

λ ~> make testacc TEST=./google TESTARGS='-run=TestAccGoogleSqlDatabaseInstance_highAvailability' GOOGLE_USE_DEFAULT_CREDENTIALS=true
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccGoogleSqlDatabaseInstance_highAvailability -timeout 120m
=== RUN   TestAccGoogleSqlDatabaseInstance_highAvailability
--- PASS: TestAccGoogleSqlDatabaseInstance_highAvailability (561.98s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 562.009s

katzj and others added 3 commits January 23, 2018 11:36
Pull in updates to the generated sqladmin api and update callers for
the change in the StorageAutoResize setting
Allow specifying ZONAL or REGIONAL to allow for PostgreSQL HA
setup.
@lawrencejones
Copy link
Contributor Author

Hey @danawillow, this PR should cover the comments you made on #695. Can you give this a second pass please?

@danawillow danawillow self-requested a review January 24, 2018 00:01
@danawillow danawillow self-assigned this Jan 24, 2018
@@ -568,7 +606,7 @@ func testAccCheckGoogleSqlDatabaseInstanceEquals(n string,
return fmt.Errorf("Error settings.crash_safe_replication mismatch, (%s, %s)", server, local)
}

server = strconv.FormatBool(instance.Settings.StorageAutoResize)
server = strconv.FormatBool(*instance.Settings.StorageAutoResize)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing panics at this line when running tests, can you check it out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having a look at this now- do you have the test that caused the fatal? Would speed up things on my end, as the few I've been running do not!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think I've got a fix for this, will update the PR once I've confirmed tests pass.

Add tests that cover the creation of a Postgres database with
AvailabilityType set to REGIONAL, and correct some small issues that
were preventing compilation.
@lawrencejones lawrencejones force-pushed the lawrence-postgresql-ha branch from 24e27cc to 9c895d3 Compare January 24, 2018 14:28
googleapis/google-api-go-client@95e5582

The cloudsql admin client changed the way it handles StorageAutoResize
as a parameter, in order to be more explicit about when the server has
ommitted the field. This changed the type from being bool to *bool, and
we need to modify provider code so that we supply the right value to the
api client.
@lawrencejones lawrencejones force-pushed the lawrence-postgresql-ha branch from 9c895d3 to 3faf55a Compare January 24, 2018 16:18
@lawrencejones
Copy link
Contributor Author

Looking a lot healthier this time around:

λ ~> make testacc TEST=./google TESTARGS='-run=TestAccGoogleSqlDatabaseInstance_.*' GOOGLE_USE_DEFAULT_CREDENTIALS=true==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccGoogleSqlDatabaseInstance_.* -timeout 120m
=== RUN   TestAccGoogleSqlDatabaseInstance_importBasic
=== RUN   TestAccGoogleSqlDatabaseInstance_importBasic3
=== RUN   TestAccGoogleSqlDatabaseInstance_basic
=== RUN   TestAccGoogleSqlDatabaseInstance_basic2
=== RUN   TestAccGoogleSqlDatabaseInstance_basic3
=== RUN   TestAccGoogleSqlDatabaseInstance_dontDeleteDefaultUserOnReplica
=== RUN   TestAccGoogleSqlDatabaseInstance_settings_basic
=== RUN   TestAccGoogleSqlDatabaseInstance_slave
=== RUN   TestAccGoogleSqlDatabaseInstance_highAvailability
=== RUN   TestAccGoogleSqlDatabaseInstance_diskspecs
=== RUN   TestAccGoogleSqlDatabaseInstance_maintenance
=== RUN   TestAccGoogleSqlDatabaseInstance_settings_upgrade
=== RUN   TestAccGoogleSqlDatabaseInstance_settingsDowngrade
=== RUN   TestAccGoogleSqlDatabaseInstance_authNets
=== RUN   TestAccGoogleSqlDatabaseInstance_multipleOperations
--- PASS: TestAccGoogleSqlDatabaseInstance_settingsDowngrade (58.60s)
--- PASS: TestAccGoogleSqlDatabaseInstance_importBasic (60.88s)
--- PASS: TestAccGoogleSqlDatabaseInstance_settings_upgrade (109.66s)
--- PASS: TestAccGoogleSqlDatabaseInstance_authNets (73.10s)
--- PASS: TestAccGoogleSqlDatabaseInstance_multipleOperations (130.19s)
--- PASS: TestAccGoogleSqlDatabaseInstance_highAvailability (519.86s)
--- PASS: TestAccGoogleSqlDatabaseInstance_maintenance (422.80s)
--- PASS: TestAccGoogleSqlDatabaseInstance_settings_basic (50.60s)
--- PASS: TestAccGoogleSqlDatabaseInstance_diskspecs (453.66s)
--- PASS: TestAccGoogleSqlDatabaseInstance_basic3 (456.09s)
--- PASS: TestAccGoogleSqlDatabaseInstance_basic (81.38s)
--- PASS: TestAccGoogleSqlDatabaseInstance_basic2 (61.35s)
--- PASS: TestAccGoogleSqlDatabaseInstance_importBasic3 (535.08s)
--- PASS: TestAccGoogleSqlDatabaseInstance_dontDeleteDefaultUserOnReplica (714.24s)
--- PASS: TestAccGoogleSqlDatabaseInstance_slave (898.62s)
PASS

@danawillow
Copy link
Contributor

Looks good, thanks!

@danawillow danawillow merged commit 9f42aeb into hashicorp:master Jan 24, 2018
@lawrencejones lawrencejones deleted the lawrence-postgresql-ha branch January 24, 2018 17:50
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
* Update sqladmin api

Pull in updates to the generated sqladmin api and update callers for
the change in the StorageAutoResize setting

* Add support for availability_type setting

Allow specifying ZONAL or REGIONAL to allow for PostgreSQL HA
setup.

* vendor: update sqladmin/v1beta4

* Test setting AvailabilityType for PostgreSQL

Add tests that cover the creation of a Postgres database with
AvailabilityType set to REGIONAL, and correct some small issues that
were preventing compilation.

* Fix breaking change w/ disk_autoresize in cloudsql

googleapis/google-api-go-client@95e5582

The cloudsql admin client changed the way it handles StorageAutoResize
as a parameter, in order to be more explicit about when the server has
ommitted the field. This changed the type from being bool to *bool, and
we need to modify provider code so that we supply the right value to the
api client.
@ghost
Copy link

ghost commented Mar 29, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants