Conversation
Signed-off-by: Mitchell Murphy <mitchell.murphy@defenseunicorns.com>
|
Any chance of this PR and #3105 getting a review soon? It would be a big help for my use case. |
|
The PR is too big for the next release which I plan to drop this week, but I like to review and include it after that. |
| connection_pooler_config_path: | ||
| type: string | ||
| default: "/etc/pgbouncer/pgbouncer.ini" | ||
| patroni: |
There was a problem hiding this comment.
this diff does not look correct. You only need to add new fields to the pkg.api. package and the CRDs will be generated when running make for example
| @@ -0,0 +1,133 @@ | |||
| <h1>Operator-generated PgBouncer config (Helm)</h1> | |||
There was a problem hiding this comment.
this isn't specific to helm chart, right? Not sure about a new single docs file. Can you move this section to the administrator.md (global config) and add one for users.md, too (local config)
| @@ -0,0 +1,986 @@ | |||
| --- | |||
There was a problem hiding this comment.
pls remove this file from the PR
| @@ -0,0 +1,84 @@ | |||
| --- | |||
There was a problem hiding this comment.
pls remove this file from the PR
| connection_pooler_config_path: | ||
| type: string | ||
| default: "/etc/pgbouncer/pgbouncer.ini" | ||
| patroni: |
There was a problem hiding this comment.
again, looks like the new fields got inserted incorrectly
| not: | ||
| required: | ||
| - s3_wal_path | ||
| - gs_wal_path |
There was a problem hiding this comment.
do not remove fields in postgresql CRD
| @@ -0,0 +1,4824 @@ | |||
| --- | |||
There was a problem hiding this comment.
pls remove this file from the PR
| // +kubebuilder:default="scram-sha-256" | ||
| AuthType string `json:"connection_pooler_auth_type,omitempty"` | ||
| // +kubebuilder:default="/etc/pgbouncer/pgbouncer.ini" | ||
| ConfigPath string `json:"connection_pooler_config_path,omitempty"` |
There was a problem hiding this comment.
I think, it makes sense to also add these fields to the postgresql_type, too. At least we would need this for some teams to still use pgbouncer with md5 while globally we want scram to be the default
| }) | ||
| volumeMounts = append(volumeMounts, v1.VolumeMount{ | ||
| Name: configVolumeName, | ||
| MountPath: c.OpConfig.ConnectionPooler.ConfigPath, |
There was a problem hiding this comment.
not 100% sure that this variable is always set at this point
| if len(c.OpConfig.ConnectionPooler.Command) > 0 { | ||
| poolerContainer.Command = c.OpConfig.ConnectionPooler.Command | ||
| } | ||
| poolerContainer.Args = c.OpConfig.ConnectionPooler.Args |
There was a problem hiding this comment.
same here. Can Args be empty? Should we always set it or use an if like with Command
| checksum, err := c.connectionPoolerConfigChecksum(role) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
when we maybe just raise a warning when the checksum annotation is missing? I would not break the cluster creation just because an annotation of a pooler could not get generated
| - Postgres Operator UI: 'operator-ui.md' | ||
| - Admin guide: 'administrator.md' | ||
| - User guide: 'user.md' | ||
| - PgBouncer generated config: 'pgbouncer-generated-config.md' |
There was a problem hiding this comment.
like I wrote above I would not create a dedicated docs file for this config option now but rather include it in admin and user docs
feat(pooler): operator-generated PgBouncer config
Summary
Adds an opt-in mode where the operator renders
pgbouncer.iniitself and manages it as an operator-owned ConfigMap, instead of relying on the pooler image to generate its own config from environment variables.Motivation
The default Spilo/PgBouncer image renders
pgbouncer.iniat startup, but some images (e.g. the Chainguard PgBouncer distroless image) ship no such entrypoint. Supporting them previously required a static ConfigMap plus a mutating webhook. This PR moves that responsibility into the operator and removes the workaround.What changes
When
connection_pooler_generate_config: true, for each pooler role the operator:pgbouncer.ini(pool mode, auth type/query, TLS, sizes) and writes it to a<pooler>-configConfigMap (create/sync/delete in the pooler lifecycle);config_pathviasubPath, and overrides the containercommand/argsto point PgBouncer at it;Default is
false— no behavior change for existing clusters.New configuration (ConfigMap +
OperatorConfigurationCRD)connection_pooler_generate_configfalseconnection_pooler_commandconnection_pooler_args["/etc/pgbouncer/pgbouncer.ini"]connection_pooler_auth_typescram-sha-256pgbouncer.iniconnection_pooler_config_path/etc/pgbouncer/pgbouncer.iniTests
New/extended coverage in
config_test.go,connection_pooler_test.go, andpgbouncer_config_test.go(ini rendering incl. TLS, checksum stability, ConfigMap generation, pod-template on/off, and sync create + update-on-drift). Verify by running the tests:Also updated
Helm
values.yaml,operator_parameters.md, bothOperatorConfigurationCRDs, and the default operator-configuration manifest.