Created postgresql deployment modules based on field experience#22
Created postgresql deployment modules based on field experience#22mateofloreza wants to merge 1 commit into
Conversation
|
Thanks for this. I'll work with the SQL team to look at getting this merged. One thing I'd suggest in the mean time is a simple README.md in each of the directories with a synopsis of what is being deployed. The PR description is a little light, and a description of what each module does in the repo will also help people who come across it in the future :) |
There was a problem hiding this comment.
Thank you for making us better!
Can you please add README.md to each module, showing the right way to deploy/apply it.
Great example is https://github.com/canonical/observability-stack/blob/main/terraform/cos-lite/README.md
I didn't manage to deploy it locally as some options have no meaning to me and descriptions was confusing (see inline).
Some mistakes available: e.g. no relations to tls operator at all OR reusing the same subordinate grafana-agent for different principals. Otherwise, I believe it is a good start! Let's make it deployable, document it and merge.
Thank you!
| @@ -0,0 +1,85 @@ | |||
| resource "juju_machine" "postgresql" { | |||
| model = var.juju_model_name | |||
| base = "ubuntu@22.04" | |||
There was a problem hiding this comment.
Please move hardcoded base to the variable.
| variable "oam_space" { | ||
| description = "OAM space" | ||
| } |
There was a problem hiding this comment.
Mandatory option I do not see in code. Leftover?
| resource "juju_application" "data_integrator" { | ||
| name = "data-integrator-${var.postgresql_database_name}" | ||
| model = var.juju_model_name | ||
| trust = true |
There was a problem hiding this comment.
AFAIK, trust is applicable for K8s only. Is it necessary here?
| variable "postgresql_endpoints" { | ||
| description = "Postgresql endpoint bindings" | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you explain what is postgresql_endpoints? I am not a newbie in Charmed PostgreSQL/Juju, but I do not know what to provide here. Tnx!
Juju spaces?
I see it has been copy&pasted via all applications, please add details here.
| resource "juju_application" "backups_s3_integrator" { | ||
| name = "backups-s3-integrator" | ||
| model = var.juju_model_name | ||
| trust = true |
| resource "juju_integration" "grafana-postgresql" { | ||
| model = var.juju_model_name | ||
| application { | ||
| name = juju_application.machine_postgresql.name | ||
| endpoint = "cos-agent" | ||
| } | ||
| application { | ||
| name = juju_application.grafana_agent.name | ||
| endpoint = "cos-agent" | ||
| } | ||
| } | ||
|
|
||
| resource "juju_integration" "grafana-pgbouncer" { | ||
| model = var.juju_model_name | ||
| application { | ||
| name = juju_application.pgbouncer.name | ||
| endpoint = "cos-agent" | ||
| } | ||
| application { | ||
| name = juju_application.grafana_agent.name | ||
| endpoint = "cos-agent" | ||
| } | ||
| } |
There was a problem hiding this comment.
The grafana-agents for PostgreSQL and PgBouncers must be a different Juju applications. See: https://charmhub.io/pgbouncer/docs/h-enable-monitoring
There was a problem hiding this comment.
I see self_signed_certificates application defined/deployed but not related to any charms. Expected?
IMHO it should be related to PG, PGB at least.
| resource "juju_application" "machine_postgresql" { | ||
| name = var.postgresql_application_name | ||
| model = var.juju_model_name | ||
| trust = true |
There was a problem hiding this comment.
Here and below, is trust=true necessary for K8s only?
| variable "total_units" { | ||
| description = "Total postgresql units" | ||
| } |
There was a problem hiding this comment.
Please use common practice: postgresql_charm_units
| variable "oam_space" { | ||
| description = "OAM space" | ||
| } |
|
@mateofloreza after the checking this PR in details, IMHO, we should go the proper direction instead of copy&pasting everything into one module. Please check our improvements in the official PG module: canonical/postgresql-operator#1052 (and README.md) This will allow you to use PG module in your TF plan, working example: #23 Can you please update your PR accordingly? Maybe it worth to start with one module first, e.g. your P.S. I am on the vacation the next week, but I will monitor this thread. |
3 participants
Description
This PR adds a new Terraform module for deploying PostgreSQL, based on real-world field experience. The module is designed for flexibility and integrates with existing infrastructure modules.
Type
How to test
terraform init
terraform apply