From 9ab60ecc7b40db7747ba7a03c202991da2af2cb4 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Tue, 27 Dec 2016 15:28:27 -0800 Subject: [PATCH] Add an Exists function for each resource type. Also add a provider RWMutex. Some of the checks didn't support concurrent updates. This should improve the reliability of the provider. --- builtin/providers/postgresql/GNUmakefile | 2 +- builtin/providers/postgresql/config.go | 8 +++ .../resource_postgresql_database.go | 46 +++++++++++++++- .../resource_postgresql_extension.go | 46 +++++++++++++++- .../postgresql/resource_postgresql_role.go | 49 +++++++++++++++-- .../postgresql/resource_postgresql_schema.go | 52 +++++++++++++++++-- 6 files changed, 189 insertions(+), 14 deletions(-) diff --git a/builtin/providers/postgresql/GNUmakefile b/builtin/providers/postgresql/GNUmakefile index e93005de98..f8f2da5c43 100644 --- a/builtin/providers/postgresql/GNUmakefile +++ b/builtin/providers/postgresql/GNUmakefile @@ -1,4 +1,4 @@ -# env TESTARGS='-test.parallel=1 -run TestAccPostgresqlSchema_AddPolicy' TF_LOG=warn make test +# env TESTARGS='-run TestAccPostgresqlSchema_AddPolicy' TF_LOG=warn make test # # NOTE: As of PostgreSQL 9.6.1 the -test.parallel=1 is required when # performing `DROP ROLE`-related actions. This behavior and requirement diff --git a/builtin/providers/postgresql/config.go b/builtin/providers/postgresql/config.go index 66c91a8702..cef6f27106 100644 --- a/builtin/providers/postgresql/config.go +++ b/builtin/providers/postgresql/config.go @@ -5,6 +5,7 @@ import ( "database/sql" "fmt" "log" + "sync" "unicode" _ "github.com/lib/pq" //PostgreSQL db @@ -27,6 +28,13 @@ type Config struct { type Client struct { username string connStr string + + // PostgreSQL lock on pg_catalog. Many of the operations that Terraform + // performs are not permitted to be concurrent. Unlike traditional + // PostgreSQL tables that use MVCC, many of the PostgreSQL system + // catalogs look like tables, but are not in-fact able to be + // concurrently updated. + catalogLock sync.RWMutex } // NewClient returns new client config diff --git a/builtin/providers/postgresql/resource_postgresql_database.go b/builtin/providers/postgresql/resource_postgresql_database.go index 29539ed746..d04fcba9d8 100644 --- a/builtin/providers/postgresql/resource_postgresql_database.go +++ b/builtin/providers/postgresql/resource_postgresql_database.go @@ -32,6 +32,7 @@ func resourcePostgreSQLDatabase() *schema.Resource { Read: resourcePostgreSQLDatabaseRead, Update: resourcePostgreSQLDatabaseUpdate, Delete: resourcePostgreSQLDatabaseDelete, + Exists: resourcePostgreSQLDatabaseExists, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -107,6 +108,10 @@ func resourcePostgreSQLDatabase() *schema.Resource { func resourcePostgreSQLDatabaseCreate(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return errwrap.Wrapf("Error connecting to PostgreSQL: {{err}}", err) @@ -184,11 +189,14 @@ func resourcePostgreSQLDatabaseCreate(d *schema.ResourceData, meta interface{}) d.SetId(dbName) - return resourcePostgreSQLDatabaseRead(d, meta) + return resourcePostgreSQLDatabaseReadImpl(d, meta) } func resourcePostgreSQLDatabaseDelete(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return errwrap.Wrapf("Error connecting to PostgreSQL: {{err}}", err) @@ -220,7 +228,38 @@ func resourcePostgreSQLDatabaseDelete(d *schema.ResourceData, meta interface{}) return nil } +func resourcePostgreSQLDatabaseExists(d *schema.ResourceData, meta interface{}) (bool, error) { + c := meta.(*Client) + c.catalogLock.RLock() + defer c.catalogLock.RUnlock() + + conn, err := c.Connect() + if err != nil { + return false, err + } + defer conn.Close() + + var dbName string + err = conn.QueryRow("SELECT d.datname from pg_database d WHERE datname=$1", d.Id()).Scan(&dbName) + switch { + case err == sql.ErrNoRows: + return false, nil + case err != nil: + return false, err + } + + return true, nil +} + func resourcePostgreSQLDatabaseRead(d *schema.ResourceData, meta interface{}) error { + c := meta.(*Client) + c.catalogLock.RLock() + defer c.catalogLock.RUnlock() + + return resourcePostgreSQLDatabaseReadImpl(d, meta) +} + +func resourcePostgreSQLDatabaseReadImpl(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) conn, err := c.Connect() if err != nil { @@ -276,6 +315,9 @@ func resourcePostgreSQLDatabaseRead(d *schema.ResourceData, meta interface{}) er func resourcePostgreSQLDatabaseUpdate(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return err @@ -308,7 +350,7 @@ func resourcePostgreSQLDatabaseUpdate(d *schema.ResourceData, meta interface{}) // Empty values: ALTER DATABASE name RESET configuration_parameter; - return resourcePostgreSQLDatabaseRead(d, meta) + return resourcePostgreSQLDatabaseReadImpl(d, meta) } func setDBName(conn *sql.DB, d *schema.ResourceData) error { diff --git a/builtin/providers/postgresql/resource_postgresql_extension.go b/builtin/providers/postgresql/resource_postgresql_extension.go index 240de3eb30..3b2158053a 100644 --- a/builtin/providers/postgresql/resource_postgresql_extension.go +++ b/builtin/providers/postgresql/resource_postgresql_extension.go @@ -24,6 +24,7 @@ func resourcePostgreSQLExtension() *schema.Resource { Read: resourcePostgreSQLExtensionRead, Update: resourcePostgreSQLExtensionUpdate, Delete: resourcePostgreSQLExtensionDelete, + Exists: resourcePostgreSQLExtensionExists, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -52,6 +53,9 @@ func resourcePostgreSQLExtension() *schema.Resource { func resourcePostgreSQLExtensionCreate(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return err @@ -79,11 +83,43 @@ func resourcePostgreSQLExtensionCreate(d *schema.ResourceData, meta interface{}) d.SetId(extName) - return resourcePostgreSQLExtensionRead(d, meta) + return resourcePostgreSQLExtensionReadImpl(d, meta) +} + +func resourcePostgreSQLExtensionExists(d *schema.ResourceData, meta interface{}) (bool, error) { + c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + + conn, err := c.Connect() + if err != nil { + return false, err + } + defer conn.Close() + + var extName string + err = conn.QueryRow("SELECT extname FROM pg_catalog.pg_extension WHERE extname = $1", d.Id()).Scan(&extName) + switch { + case err == sql.ErrNoRows: + return false, nil + case err != nil: + return false, err + } + + return true, nil } func resourcePostgreSQLExtensionRead(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.RLock() + defer c.catalogLock.RUnlock() + + return resourcePostgreSQLExtensionReadImpl(d, meta) +} + +func resourcePostgreSQLExtensionReadImpl(d *schema.ResourceData, meta interface{}) error { + c := meta.(*Client) + conn, err := c.Connect() if err != nil { return err @@ -111,6 +147,9 @@ func resourcePostgreSQLExtensionRead(d *schema.ResourceData, meta interface{}) e func resourcePostgreSQLExtensionDelete(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return err @@ -132,6 +171,9 @@ func resourcePostgreSQLExtensionDelete(d *schema.ResourceData, meta interface{}) func resourcePostgreSQLExtensionUpdate(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return err @@ -148,7 +190,7 @@ func resourcePostgreSQLExtensionUpdate(d *schema.ResourceData, meta interface{}) return err } - return resourcePostgreSQLExtensionRead(d, meta) + return resourcePostgreSQLExtensionReadImpl(d, meta) } func setExtSchema(conn *sql.DB, d *schema.ResourceData) error { diff --git a/builtin/providers/postgresql/resource_postgresql_role.go b/builtin/providers/postgresql/resource_postgresql_role.go index 724445734f..382ed6f081 100644 --- a/builtin/providers/postgresql/resource_postgresql_role.go +++ b/builtin/providers/postgresql/resource_postgresql_role.go @@ -38,6 +38,7 @@ func resourcePostgreSQLRole() *schema.Resource { Read: resourcePostgreSQLRoleRead, Update: resourcePostgreSQLRoleUpdate, Delete: resourcePostgreSQLRoleDelete, + Exists: resourcePostgreSQLRoleExists, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -140,6 +141,9 @@ func resourcePostgreSQLRole() *schema.Resource { func resourcePostgreSQLRoleCreate(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return errwrap.Wrapf("Error connecting to PostgreSQL: {{err}}", err) @@ -243,12 +247,15 @@ func resourcePostgreSQLRoleCreate(d *schema.ResourceData, meta interface{}) erro d.SetId(roleName) - return resourcePostgreSQLRoleRead(d, meta) + return resourcePostgreSQLRoleReadImpl(d, meta) } func resourcePostgreSQLRoleDelete(d *schema.ResourceData, meta interface{}) error { - client := meta.(*Client) - conn, err := client.Connect() + c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + + conn, err := c.Connect() if err != nil { return err } @@ -290,7 +297,38 @@ func resourcePostgreSQLRoleDelete(d *schema.ResourceData, meta interface{}) erro return nil } +func resourcePostgreSQLRoleExists(d *schema.ResourceData, meta interface{}) (bool, error) { + c := meta.(*Client) + c.catalogLock.RLock() + defer c.catalogLock.RUnlock() + + conn, err := c.Connect() + if err != nil { + return false, err + } + defer conn.Close() + + var roleName string + err = conn.QueryRow("SELECT rolname FROM pg_catalog.pg_roles WHERE rolname=$1", d.Id()).Scan(&roleName) + switch { + case err == sql.ErrNoRows: + return false, nil + case err != nil: + return false, err + } + + return true, nil +} + func resourcePostgreSQLRoleRead(d *schema.ResourceData, meta interface{}) error { + c := meta.(*Client) + c.catalogLock.RLock() + defer c.catalogLock.RUnlock() + + return resourcePostgreSQLRoleReadImpl(d, meta) +} + +func resourcePostgreSQLRoleReadImpl(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) conn, err := c.Connect() if err != nil { @@ -347,6 +385,9 @@ func resourcePostgreSQLRoleRead(d *schema.ResourceData, meta interface{}) error func resourcePostgreSQLRoleUpdate(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return err @@ -393,7 +434,7 @@ func resourcePostgreSQLRoleUpdate(d *schema.ResourceData, meta interface{}) erro return err } - return resourcePostgreSQLRoleRead(d, meta) + return resourcePostgreSQLRoleReadImpl(d, meta) } func setRoleName(conn *sql.DB, d *schema.ResourceData) error { diff --git a/builtin/providers/postgresql/resource_postgresql_schema.go b/builtin/providers/postgresql/resource_postgresql_schema.go index 4d3edad30f..12f650971f 100644 --- a/builtin/providers/postgresql/resource_postgresql_schema.go +++ b/builtin/providers/postgresql/resource_postgresql_schema.go @@ -34,6 +34,7 @@ func resourcePostgreSQLSchema() *schema.Resource { Read: resourcePostgreSQLSchemaRead, Update: resourcePostgreSQLSchemaUpdate, Delete: resourcePostgreSQLSchemaDelete, + Exists: resourcePostgreSQLSchemaExists, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -105,6 +106,8 @@ func resourcePostgreSQLSchema() *schema.Resource { } func resourcePostgreSQLSchemaCreate(d *schema.ResourceData, meta interface{}) error { + c := meta.(*Client) + queries := []string{} schemaName := d.Get(schemaNameAttr).(string) @@ -150,7 +153,9 @@ func resourcePostgreSQLSchemaCreate(d *schema.ResourceData, meta interface{}) er queries = append(queries, policy.Grants(schemaName)...) } - c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return errwrap.Wrapf("Error connecting to PostgreSQL: {{err}}", err) @@ -176,12 +181,15 @@ func resourcePostgreSQLSchemaCreate(d *schema.ResourceData, meta interface{}) er d.SetId(schemaName) - return resourcePostgreSQLSchemaRead(d, meta) + return resourcePostgreSQLSchemaReadImpl(d, meta) } func resourcePostgreSQLSchemaDelete(d *schema.ResourceData, meta interface{}) error { - client := meta.(*Client) - conn, err := client.Connect() + c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + + conn, err := c.Connect() if err != nil { return err } @@ -211,7 +219,38 @@ func resourcePostgreSQLSchemaDelete(d *schema.ResourceData, meta interface{}) er return nil } +func resourcePostgreSQLSchemaExists(d *schema.ResourceData, meta interface{}) (bool, error) { + c := meta.(*Client) + c.catalogLock.RLock() + defer c.catalogLock.RUnlock() + + conn, err := c.Connect() + if err != nil { + return false, err + } + defer conn.Close() + + var schemaName string + err = conn.QueryRow("SELECT n.nspname FROM pg_catalog.pg_namespace n WHERE n.nspname=$1", d.Id()).Scan(&schemaName) + switch { + case err == sql.ErrNoRows: + return false, nil + case err != nil: + return false, errwrap.Wrapf("Error reading schema: {{err}}", err) + } + + return true, nil +} + func resourcePostgreSQLSchemaRead(d *schema.ResourceData, meta interface{}) error { + c := meta.(*Client) + c.catalogLock.RLock() + defer c.catalogLock.RUnlock() + + return resourcePostgreSQLSchemaReadImpl(d, meta) +} + +func resourcePostgreSQLSchemaReadImpl(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) conn, err := c.Connect() if err != nil { @@ -263,6 +302,9 @@ func resourcePostgreSQLSchemaRead(d *schema.ResourceData, meta interface{}) erro func resourcePostgreSQLSchemaUpdate(d *schema.ResourceData, meta interface{}) error { c := meta.(*Client) + c.catalogLock.Lock() + defer c.catalogLock.Unlock() + conn, err := c.Connect() if err != nil { return err @@ -291,7 +333,7 @@ func resourcePostgreSQLSchemaUpdate(d *schema.ResourceData, meta interface{}) er return errwrap.Wrapf("Error committing schema: {{err}}", err) } - return resourcePostgreSQLSchemaRead(d, meta) + return resourcePostgreSQLSchemaReadImpl(d, meta) } func setSchemaName(txn *sql.Tx, d *schema.ResourceData) error {