terraform-google-modules / terraform-google-sql-db

Creates a Cloud SQL database instance
https://registry.terraform.io/modules/terraform-google-modules/sql-db/google
Apache License 2.0
263 stars 422 forks source link

feat: enable data_cache_config for postgresql read replicas #619

Closed sofuture closed 1 month ago

sofuture commented 1 month ago

Read replicas for Enterprise Plus Postgres instances can have data caching enabled. This change should allow it to be configured correctly.

imrannayer commented 1 month ago

/gcbrun

imrannayer commented 1 month ago

/gcbrun

imrannayer commented 1 month ago

Fied issues in #620. Will test this PR after merging #620.

ChristopheMaingard commented 1 month ago

I think you should add this within variables.tf

variable "read_replicas" {
  description = "List of read replicas to create. Encryption key is required for replica in different region. For replica in same region as master set encryption_key_name = null"
  type = list(object({
    name                  = string
    name_override         = optional(string)
     ....
    data_cache_enabled = optional(bool) # < otherwise it won't work
  }))
  default = []
}
imrannayer commented 1 month ago

/gcbrun

imrannayer commented 1 month ago

@sofuture integration test failing:

TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185: Error: Operation failed
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185: 
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185:   on ../../modules/postgresql/read_replica.tf line 108, in resource "google_sql_database_instance" "replicas":
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185:  108:       for_each = lookup(each.value, "edition", var.edition) == "ENTERPRISE_PLUS" && lookup(each.value, "data_cache_enabled", var.data_cache_enabled) ? ["cache_enabled"] : []
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185:     ├────────────────
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185:     │ each.value is object with 16 attributes
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185:     │ var.data_cache_enabled is false
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185:     │ var.edition is null
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185: 
TestPostgreSqlPscModule 2024-07-23T08:57:52Z command.go:185: Error during operation: argument must not be null.
TestPostgreSqlPscModule 2024-07-23T08:57:52Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; 
Error: Operation failed

  on ../../modules/postgresql/read_replica.tf line 108, in resource "google_sql_database_instance" "replicas":
 108:       for_each = lookup(each.value, "edition", var.edition) == "ENTERPRISE_PLUS" && lookup(each.value, "data_cache_enabled", var.data_cache_enabled) ? ["cache_enabled"] : []
    ├────────────────
    │ each.value is object with 16 attributes
    │ var.data_cache_enabled is false
    │ var.edition is null

Error during operation: argument must not be null.}
sofuture commented 1 month ago

@imrannayer can the edition variable be set to default to ENTERPRISE instead of null? As I understand it is a binary value in practice, and not actually nullable.

imrannayer commented 1 month ago

/gcbrun

imrannayer commented 1 month ago

/gcbrun

imrannayer commented 1 month ago

/gcbrun